improve schema commands, testing (#373)

this commit:
  - adds several tests for schema commands
  - adds mocha root hooks to clean up sinon
  - adds sinon-chai for better assertion syntax
  - swaps @cloudcmd/stub for sinon in most places
  - rewrites schema pull helpers to parallelize work

package.json

@@ -33,6 +33,7 @@
     "prettier": "^2.3.0",
     "rate-limiter-flexible": "^2.3.6",
     "sinon-called-with-diff": "^3.1.1",
+    "sinon-chai": "^4.0.0",
     "stream-json": "^1.7.3",
     "supports-color": "^8",
     "yargs": "^17.7.2"
@@ -109,6 +110,7 @@
     "local": "export $(cat .env | xargs); node bin/run",
     "local-test": "export $(cat .env | xargs); mocha \"test/**/*.test.{js,ts}\"",
     "test": "c8 -r html mocha --forbid-only \"test/**/*.test.{js,ts}\"",
+    "test-yargs": "mocha --recursive ./yargs-test --require ./yargs-test/mocha-root-hooks.mjs",
     "version": "oclif-dev readme && git add README.md",
     "fmt": "prettier -w src"
   },

src/cli.mjs

@@ -86,11 +86,9 @@ function buildYargs(argvInput) {
       const exit = container.resolve("exit")
       const message = `${chalk.reset(yargs.help())}\n\n${chalk.red(msg || err?.message)}`
       logger.stderr(message)
-      // for some reason, this causes 2 promise rejections to be printed?
-      // debug by using `fauna reject`
-      // if (err && err.stack) {
-      //   logger.fatal(err.stack)
-      // }
+      if (err && err.stack) {
+        logger.fatal(err.stack)
+      }
       exit(1)
     })
     .exitProcess(false)

src/config/setup-container.mjs

@@ -8,15 +8,20 @@ import { getSimpleClient } from "../lib/command-helpers.mjs";
 import {
   gatherFSL,
   gatherRelativeFSLFilePaths,
+  getAllSchemaFileContents,
   getStagedSchemaStatus,
+  getSchemaFile,
   getSchemaFiles,
+  deleteUnusedSchemaFiles,
+  writeSchemaFiles,
 } from "../lib/schema.mjs";
 import { confirm } from "@inquirer/prompts";
 import fetchWrapper from "../lib/fetch-wrapper.mjs";
 import { FaunaAccountClient } from "../lib/fauna-account-client.mjs";
 import open from "open";
 import OAuthClient from "../lib/auth/oauth-client.mjs";
 import { Lifetime } from "awilix";
+import fs from 'node:fs'
 
 // import { findUpSync } from 'find-up'
 // import fs from 'node:fs'
@@ -35,8 +40,9 @@ export function setupCommonContainer() {
 
 export const injectables = {
   // node libraries
-  fetch: awilix.asValue(fetchWrapper),
   exit: awilix.asValue(exit),
+  fetch: awilix.asValue(fetchWrapper),
+  fs: awilix.asValue(fs),
 
   // third-party libraries
   confirm: awilix.asValue(confirm),
@@ -54,8 +60,12 @@ export const injectables = {
   // feature-specific lib (homemade utilities)
   gatherFSL: awilix.asValue(gatherFSL),
   gatherRelativeFSLFilePaths: awilix.asValue(gatherRelativeFSLFilePaths),
+  getSchemaFile: awilix.asValue(getSchemaFile),
   getSchemaFiles: awilix.asValue(getSchemaFiles),
+  writeSchemaFiles: awilix.asValue(writeSchemaFiles),
+  getAllSchemaFileContents: awilix.asValue(getAllSchemaFileContents),
   getStagedSchemaStatus: awilix.asValue(getStagedSchemaStatus),
+  deleteUnusedSchemaFiles: awilix.asValue(deleteUnusedSchemaFiles),
 };
 
 export function setupRealContainer() {

src/config/setup-test-container.mjs

@@ -1,6 +1,10 @@
+import fs from 'node:fs'
+
 import * as awilix from 'awilix/lib/awilix.module.mjs'
 import { setupCommonContainer, injectables } from './setup-container.mjs'
-import stub from '@cloudcmd/stub'
+
+import sinon, { stub } from 'sinon'
+
 import logger from '../lib/logger.mjs'
 
 // Mocks all _functions_ declared on the injectables export from setup-container.mjs
@@ -31,6 +35,7 @@ export function setupTestContainer() {
   const thingsToManuallyMock = automock(container)
 
   const manualMocks = {
+    fs: awilix.asValue(sinon.stub(fs)),
     logger: awilix.asValue({
       // use these for making dev, support tickets easier.
       // they're not mocked because we shouldn't test them

src/lib/db.mjs

@@ -1,15 +1,22 @@
 import { container } from '../cli.mjs'
 
-export async function makeFaunaRequest({ argv, url, params, method, shouldThrow = true }) {
+export async function makeFaunaRequest({ secret, baseUrl, path, params, method, shouldThrow = true }) {
   const fetch = container.resolve("fetch")
   const paramsString = params
     ? `?${new URLSearchParams(params)}`
     : ''
+  let fullUrl
+
+  try {
+    fullUrl = new URL(`${path}${paramsString}`, baseUrl).href
+  } catch (e) {
+    e.message = `Could not build valid URL out of base url (${baseUrl}), path (${path}), and params string (${paramsString}) built from params (${JSON.stringify(params)}).`
+    throw e
+  }
 
-  const fullUrl = new URL(`${url}${paramsString}`, argv.url)
   const response = await fetch(fullUrl, {
     method,
-    headers: { AUTHORIZATION: `Bearer ${argv.secret}` },
+    headers: { AUTHORIZATION: `Bearer ${secret}` },
   })
 
   const obj = await response.json()

src/lib/logger.mjs

@@ -6,7 +6,7 @@ export function log(text, verbosity, stream, component="unknown", formatter, arg
     argv = builtYargs.argv
   }
 
-  if (argv.verbosity >= verbosity || argv.verboseComponent?.includes(component)) {
+  if (!argv.then && (argv.verbosity >= verbosity || argv.verboseComponent.includes(component))) {
     // fails on intentional multi-line output
     // demo with `--verbose-component argv`
     // const prefix = /^(\n*)(.*)$/gm.exec(text)[1]

src/lib/schema.mjs

@@ -1,8 +1,8 @@
-import * as fs from "fs";
 import * as path from "path";
 import { dirExists, dirIsWriteable } from "./file-util.mjs"
 import { container } from '../cli.mjs'
 import { makeFaunaRequest } from '../lib/db.mjs'
+import { builtYargs } from '../cli.mjs'
 
 function checkDirUsability(dir) {
   if (!dirExists(dir)) {
@@ -40,7 +40,8 @@ function read(dir, relpaths) {
 // Fails if there are too many files.
 // returns string[]
 export async function gatherRelativeFSLFilePaths(dir) {
-  const logger = (await container.resolve("logger"))
+  const logger = container.resolve("logger")
+  const fs = container.resolve("fs")
 
   const FILE_LIMIT = 32000;
   // rel: string, curr: string[]
@@ -70,6 +71,16 @@ export async function gatherRelativeFSLFilePaths(dir) {
   return files;
 }
 
+export async function deleteUnusedSchemaFiles(dir, filesToDelete) {
+  const fs = container.resolve("fs")
+  const promises = []
+  for (const fileName of filesToDelete) {
+    promises.push(fs.unlink(path.join(dir, fileName)))
+  }
+
+  return Promise.all(promises)
+}
+
 export async function gatherFSL(dir) {
   const gatherRelativeFSLFilePaths = container.resolve("gatherRelativeFSLFilePaths")
 
@@ -79,20 +90,61 @@ export async function gatherFSL(dir) {
   return JSON.stringify(files)
 }
 
-export async function getSchemaFiles({ argv, ...overrides }) {
+export async function writeSchemaFiles(filenameToContentsHash) {
+  const fs = container.resolve("fs")
+  const argv = builtYargs.argv
+  fs.mkdirSync(path.dirname(argv.dir), { recursive: true });
+
+  const promises = []
+  for (const [filename, fileContents] of Object.entries(filenameToContentsHash)) {
+    const fp = path.join(argv.dir, filename);
+    promises.push(fs.writeFile(fp, fileContents))
+  }
+
+  return Promise.all(promises)
+}
+
+export async function getAllSchemaFileContents(filenames) {
+  const promises = []
+  const fileContents = {}
+  for (const filename of filenames) {
+    promises.push(getSchemaFile(filename).then((fileContent) => {
+      fileContents[filename] = fileContent
+    }))
+  }
+
+  return Promise.all(promises)
+}
+
+export async function getSchemaFiles({ ...overrides } = {}) {
+  const argv = builtYargs.argv
+  const args = {
+    baseUrl: argv.url,
+    path: "/schema/1/files",
+    method: "GET",
+    ...overrides
+  }
+  return makeFaunaRequest({ secret: argv.secret, ...args })
+}
+
+export async function getSchemaFile(filename, { ...overrides } = {}) {
+  const argv = builtYargs.argv
   const args = {
-    url: "/schema/1/files",
+    baseUrl: argv.url,
+    path: `/schema/1/files/${encodeURIComponent(filename)}`,
     method: "GET",
     ...overrides
   }
-  return makeFaunaRequest({ argv, ...args})
+  return makeFaunaRequest({ secret: argv.secret, ...args })
 }
 
-export async function getStagedSchemaStatus({ argv, ...overrides }) {
+export async function getStagedSchemaStatus({ ...overrides } = {}) {
+  const argv = builtYargs.argv
   const args = {
-    url: "/schema/1/staged/status",
+    baseUrl: argv.url,
+    path: "/schema/1/staged/status",
     method: "GET",
     ...overrides
   }
-  return makeFaunaRequest({ argv, ...args })
+  return makeFaunaRequest({ secret: argv.secret, ...args })
 }

src/yargs-commands/eval.mjs

@@ -183,7 +183,7 @@ async function doEval(argv) {
       version: argv.version,
       argv
     })).client
-    : await (container.resolve("getSimpleClient")(argv))
+    : container.resolve("getSimpleClient")(argv)
 
   const readQuery = argv.stdin || argv.file !== undefined;
   let queryFromFile;
@@ -212,7 +212,7 @@ async function doEval(argv) {
   );
 
   if (result) {
-    (await container.resolve("logger")).stdout(result);
+    container.resolve("logger").stdout(result);
   }
 
   // required to make the process not hang

src/yargs-commands/schema/pull.mjs

@@ -2,37 +2,40 @@ import { container } from '../../cli.mjs'
 import { commonQueryOptions } from '../../lib/command-helpers.mjs'
 
 async function doPull(argv) {
-  const logger = (await container.resolve("logger"))
-  const gatherRelativeFSLFilePaths = container.resolve("gatherRelativeFSLFilePaths")
-  const fetch = container.resolve("fetch")
+  const logger = container.resolve("logger")
+  const gatherFSL = container.resolve("gatherFSL")
   const confirm = container.resolve("confirm")
   const getSchemaFiles = container.resolve("getSchemaFiles")
   const getStagedSchemaStatus = container.resolve("getStagedSchemaStatus")
+  const exit = container.resolve("exit")
 
-  const filesResponse = await getSchemaFiles({ argv })
+  // fetch the list of remote FSL files
+  const filesResponse = await getSchemaFiles()
 
-  // Check if there's a staged schema, and require `--staged` if there is one.
+  // check if there's a staged schema
   const statusResponse = await getStagedSchemaStatus({
-    argv,
-    params: { version: filesResponse.version },
-    shouldThrow: false
+    params: { version: filesResponse.version }
   })
 
+  // if there's a staged schema, require the --staged flag.
+  // getting unstaged FSL while staged FSL exists is not yet
+  // implemented at the service level.
   if (statusResponse.status !== "none" && !argv.staged) {
-    logger.stdout("There is a staged schema change. Use --staged to pull it.");
+    logger.stderr("There is a staged schema change. Use --staged to pull it.")
+    exit(1)
   } else if (statusResponse.status === "none" && argv.staged) {
-    logger.stdout("There are no staged schema changes to pull.");
+    logger.stderr("There are no staged schema changes to pull.")
+    exit(1)
   }
 
-  console.log(filesResponse.files)
-  // Sort for consistent order. It's nice for tests.
+  // sort for consistent order (it's nice for tests)
   const filenames = filesResponse.files
   .map((file) => file.filename)
   .filter((name) => name.endsWith(".fsl"))
   .sort();
 
   // Gather local .fsl files to overwrite or delete.
-  const existing = await gatherRelativeFSLFilePaths(argv.dir);
+  const existing = await gatherFSL(argv.dir);
 
   // Summarize file changes.
   const adds = [];
@@ -53,48 +56,41 @@ async function doPull(argv) {
   }
   deletes.sort();
 
-  console.log("Pull makes the following changes:");
+  logger.stdout("Pull makes the following changes:");
   if (argv.delete) {
     for (const deleteme of deletes) {
-      console.log(`delete:    ${deleteme}`);
+      logger.stdout(`delete:    ${deleteme}`);
     }
   }
   for (const add of adds) {
-    console.log(`add:       ${add}`);
+    logger.stdout(`add:       ${add}`);
   }
   for (const overwrite of overwrites) {
-    console.log(`overwrite: ${overwrite}`);
-  }
-
-  if (argv.delete) {
-    // Delete extra .fsl files.
-      for (const deleteme of deletes) {
-        fs.unlinkSync(path.join(argv.dir, deleteme));
-      }
+    logger.stdout(`overwrite: ${overwrite}`);
   }
 
   const confirmed = await confirm({
     message: "Accept the changes?",
     default: false,
-  });
+  })
 
   if (confirmed) {
-    for (const filename of filenames) {
-      const fileres = await fetch(
-        new URL(`/schema/1/files/${encodeURIComponent(filename)}`, argv.url),
-        {
-          method: "GET",
-          headers: { AUTHORIZATION: `Bearer ${argv.secret}` },
-        }
-      );
-      const filejson = await fileres.json();
-      if (filejson.error) {
-        logger.stderr(filejson.error.message);
-      }
-      const fp = path.join(argv.dir, filename);
-      fs.mkdirSync(path.dirname(fp), { recursive: true });
-      fs.writeFileSync(fp, filejson.content);
+    const writeSchemaFiles = container.resolve("writeSchemaFiles")
+    const getAllSchemaFileContents = container.resolve("getAllSchemaFileContents")
+    const contents = await getAllSchemaFileContents(filenames)
+
+    // don't start writing or deleting files until we've successfully fetched all
+    // the remote schema files
+    const promises = []
+    promises.push(writeSchemaFiles(contents))
+    if (argv.delete) {
+      const deleteUnusedSchemaFiles = container.resolve("deleteUnusedSchemaFiles")
+      promises.push(deleteUnusedSchemaFiles(argv.dir, deletes))
     }
+
+    // process writes and deletes together async - it'll be fastest
+    await Promise.all(promises)
+
   } else {
     logger.stdout("Change cancelled");
   }
@@ -126,7 +122,7 @@ function buildPullCommand(yargs) {
 
 export default {
   command: 'pull',
-  describe: 'Pull a database schema\'s .fsl files into the current project',
+  describe: "Pull a database schema's .fsl files into the current project",
   builder: buildPullCommand,
-  handler: async (argv) => { await doPull(argv) }
+  handler: doPull
 }