refactor: use zx for jest tests

jfmengels · Dec 9, 2024 · 73cc247 · 73cc247
1 parent 653a00a
commit 73cc247
Show file tree

Hide file tree

Showing 7 changed files with 136 additions and 108 deletions.
diff --git a/test/compiler-flag.test.js b/test/compiler-flag.test.js
@@ -12,23 +12,35 @@ function testName(name) {
 
 test('should retrieve `elm` binary from PATH', async () => {
   const output = await TestCli.run(
-    '--config ../config-that-triggers-no-errors --force-build --compiler elm',
+    [
+      '--config=../config-that-triggers-no-errors',
+      '--force-build',
+      '--compiler=elm'
+    ],
     {project: 'project-with-errors/'}
   );
   expect(output).toEqual('I found no errors!\n');
 });
 
 test('should retrieve `elm` binary locally when path is relative', async () => {
   const output = await TestCli.run(
-    '--config ../config-that-triggers-no-errors --force-build --compiler ../../node_modules/.bin/elm',
+    [
+      '--config=../config-that-triggers-no-errors',
+      '--force-build',
+      '--compiler=../../node_modules/.bin/elm'
+    ],
     {project: 'project-with-errors/'}
   );
   expect(output).toEqual('I found no errors!\n');
 });
 
 test('should report an error when compiler could not be found', async () => {
   const output = await TestCli.runAndExpectError(
-    '--config ../config-that-triggers-no-errors --force-build --compiler notfound',
+    [
+      '--config=../config-that-triggers-no-errors',
+      '--force-build',
+      '--compiler=notfound'
+    ],
     {project: 'project-using-es2015-module'}
   );
   expect(output).toMatchFile(testName('compiler-not-found'));

diff --git a/test/flags.test.js b/test/flags.test.js
@@ -11,132 +11,141 @@ function testName(name) {
 }
 
 test('Running with an unknown flag', async () => {
-  const output = await TestCli.runAndExpectError('--watc');
+  const output = await TestCli.runAndExpectError(['--watc']);
   expect(output).toMatchFile(testName('unknown-flag'));
 });
 
 test('Running with an unknown shorthand flag', async () => {
-  const output = await TestCli.runAndExpectError('-u');
+  const output = await TestCli.runAndExpectError(['-u']);
   expect(output).toMatchFile(testName('unknown-shorthand-flag'));
 });
 
 test('Running with unknown flag --suppress', async () => {
-  const output = await TestCli.runAndExpectError('--suppress');
+  const output = await TestCli.runAndExpectError(['--suppress']);
   expect(output).toMatchFile(testName('unknown-suppress-flag'));
 });
 
 test('Running --compiler without an argument', async () => {
-  const output = await TestCli.runAndExpectError('--compiler');
+  const output = await TestCli.runAndExpectError(['--compiler']);
   expect(output).toMatchFile(testName('missing-argument-compiler'));
 });
 
 test('Running --config without an argument', async () => {
-  const output = await TestCli.runAndExpectError('--config');
+  const output = await TestCli.runAndExpectError(['--config']);
   expect(output).toMatchFile(testName('missing-argument-config'));
 });
 
 test('Running --template without an argument', async () => {
-  const output = await TestCli.runAndExpectError('--template');
+  const output = await TestCli.runAndExpectError(['--template']);
   expect(output).toMatchFile(testName('missing-argument-template'));
 });
 
 test('Running --elmjson without an argument', async () => {
-  const output = await TestCli.runAndExpectError('--elmjson');
+  const output = await TestCli.runAndExpectError(['--elmjson']);
   expect(output).toMatchFile(testName('missing-argument-elmjson'));
 });
 
 test('Running --report without an argument', async () => {
-  const output = await TestCli.runAndExpectError('--report');
+  const output = await TestCli.runAndExpectError(['--report']);
   expect(output).toMatchFile(testName('missing-argument-report'));
 });
 
 test('Running --elm-format-path without an argument', async () => {
-  const output = await TestCli.runAndExpectError('--elm-format-path');
+  const output = await TestCli.runAndExpectError(['--elm-format-path']);
   expect(output).toMatchFile(testName('missing-argument-elm-format-path'));
 });
 
 test('Running --rules without an argument', async () => {
-  const output = await TestCli.runAndExpectError('--rules');
+  const output = await TestCli.runAndExpectError(['--rules']);
   expect(output).toMatchFile(testName('missing-argument-rules'));
 });
 
 test('Running --suppressed-rules without an argument', async () => {
-  const output = await TestCli.runAndExpectError('--suppressed-rules');
+  const output = await TestCli.runAndExpectError(['--suppressed-rules']);
   expect(output).toMatchFile(testName('missing-argument-suppressed-rules'));
 });
 
 test('Running --ignore-dirs without an argument', async () => {
-  const output = await TestCli.runAndExpectError('--ignore-dirs');
+  const output = await TestCli.runAndExpectError(['--ignore-dirs']);
   expect(output).toMatchFile(testName('missing-argument-ignore-dirs'));
 });
 
 test('Running --ignore-files without an argument', async () => {
-  const output = await TestCli.runAndExpectError('--ignore-files');
+  const output = await TestCli.runAndExpectError(['--ignore-files']);
   expect(output).toMatchFile(testName('missing-argument-ignore-files'));
 });
 
 test('Running init --compiler without an argument', async () => {
-  const output = await TestCli.runAndExpectError('init --compiler');
+  const output = await TestCli.runAndExpectError(['init', '--compiler']);
   expect(output).toMatchFile(testName('missing-argument-init-compiler'));
 });
 
 test('Running init --config without an argument', async () => {
-  const output = await TestCli.runAndExpectError('init --config');
+  const output = await TestCli.runAndExpectError(['init', '--config']);
   expect(output).toMatchFile(testName('missing-argument-init-config'));
 });
 
 test('Running init --template without an argument', async () => {
-  const output = await TestCli.runAndExpectError('init --template');
+  const output = await TestCli.runAndExpectError(['init', '--template']);
   expect(output).toMatchFile(testName('missing-argument-init-template'));
 });
 
 test('Running new-package --compiler without an argument', async () => {
-  const output = await TestCli.runAndExpectError('new-package --compiler');
+  const output = await TestCli.runAndExpectError(['new-package', '--compiler']);
   expect(output).toMatchFile(testName('missing-argument-new-package-compiler'));
 });
 
 test('Running --github-auth with a bad value', async () => {
-  const output = await TestCli.runAndExpectError('--github-auth=bad');
+  const output = await TestCli.runAndExpectError(['--github-auth=bad']);
   expect(output).toMatchFile(testName('github-auth-bad-argument'));
 });
 
 test('Running --report with an unknown value', async () => {
-  const output = await TestCli.runAndExpectError('--report=unknown');
+  const output = await TestCli.runAndExpectError(['--report=unknown']);
   expect(output).toMatchFile(testName('report-unknown-argument'));
 });
 
 test('Running --template with a bad value', async () => {
-  const output = await TestCli.runAndExpectError('--template=not-github-repo');
+  const output = await TestCli.runAndExpectError([
+    '--template=not-github-repo'
+  ]);
   expect(output).toMatchFile(testName('template-bad-argument'));
 });
 
 test('Running init --template with a bad value', async () => {
-  const output = await TestCli.runAndExpectError(
-    'init --template=not-github-repo'
-  );
+  const output = await TestCli.runAndExpectError([
+    'init',
+    '--template=not-github-repo'
+  ]);
   expect(output).toMatchFile(testName('init-template-bad-argument'));
 });
 
 test('Using the same flag twice', async () => {
-  const output = await TestCli.runAndExpectError('--config a/ --config b/');
+  const output = await TestCli.runAndExpectError([
+    '--config=a/',
+    '--config=b/'
+  ]);
   expect(output).toMatchFile(testName('duplicate-flags'));
 });
 
 test('Using both --template and --offline (regular run)', async () => {
-  const output = await TestCli.runAndExpectError(
-    '--template jfmengels/elm-review-unused/example --offline'
-  );
+  const output = await TestCli.runAndExpectError([
+    '--template=jfmengels/elm-review-unused/example',
+    '--offline'
+  ]);
   expect(output).toMatchFile(testName('template-and-offline-run'));
 });
 
 test('Using both --template and --offline (init)', async () => {
-  const output = await TestCli.runAndExpectError(
-    'init --template jfmengels/elm-review-unused/example --offline'
-  );
+  const output = await TestCli.runAndExpectError([
+    'init',
+    '--template=jfmengels/elm-review-unused/example',
+    '--offline'
+  ]);
   expect(output).toMatchFile(testName('template-and-offline-init'));
 });
 
 test('Using both new-package and --offline', async () => {
-  const output = await TestCli.runAndExpectError('new-package --offline');
+  const output = await TestCli.runAndExpectError(['new-package', '--offline']);
   expect(output).toMatchFile(testName('offline-new-package'));
 });
diff --git a/test/help.test.js b/test/help.test.js
@@ -11,31 +11,31 @@ function testName(name) {
 }
 
 test('--help', async () => {
-  const output = await TestCli.run('--help');
+  const output = await TestCli.run(['--help']);
   expect(output).toMatchFile(testName('default'));
 });
 
 test('init --help', async () => {
-  const output = await TestCli.run('init --help');
+  const output = await TestCli.run(['init', '--help']);
   expect(output).toMatchFile(testName('init'));
 });
 
 test('suppress --help', async () => {
-  const output = await TestCli.run('suppress --help');
+  const output = await TestCli.run(['suppress', '--help']);
   expect(output).toMatchFile(testName('suppress'));
 });
 
 test('new-package --help', async () => {
-  const output = await TestCli.run('new-package --help');
+  const output = await TestCli.run(['new-package', '--help']);
   expect(output).toMatchFile(testName('new-package'));
 });
 
 test('new-rule --help', async () => {
-  const output = await TestCli.run('new-rule --help');
+  const output = await TestCli.run(['new-rule', '--help']);
   expect(output).toMatchFile(testName('new-rule'));
 });
 
 test('prepare-offline --help', async () => {
-  const output = await TestCli.run('prepare-offline --help');
+  const output = await TestCli.run(['prepare-offline', '--help']);
   expect(output).toMatchFile(testName('prepare-offline'));
 });
diff --git a/test/jest-helpers/cli.js b/test/jest-helpers/cli.js
@@ -1,72 +1,76 @@
+/**
+ * @import {ProcessOutput} from 'zx' with {'resolution-mode': 'import'};
+ * @import {Options} from './types/cli';
+ */
+
 const path = require('node:path');
-const {promisify} = require('node:util');
 const {toMatchFile} = require('jest-file-snapshot');
-const exec = promisify(require('node:child_process').exec);
+// @ts-expect-error(TS1479): zx doesn't ship CJS types.
+const {$} = require('zx');
 
 const cli = path.resolve(__dirname, '../../bin/elm-review');
 expect.extend({toMatchFile});
 
 /**
- * @import {Options} from './types/cli';
- */
-
-/**
- * @param {string} args
+ * @param {string[]} args
  * @param {Options} [options]
  * @returns {Promise<string>}
  */
 async function run(args, options) {
-  return await internalExec(`--FOR-TESTS ${args}`, options);
+  const output = await internalExec(['--FOR-TESTS', ...args], options);
+
+  if (output.exitCode !== 0) throw new Error(output.text());
+
+  return output.stdout;
 }
 
 /**
- * @param {string} args
+ * @param {string[]} args
  * @param {Options | undefined} [options]
  * @returns {Promise<unknown>}
  */
 async function runAndExpectError(args, options) {
-  try {
-    const output = await internalExec(`--FOR-TESTS ${args}`, options);
-    throw new Error(
-      `CLI did not exit with an exit code as expected. Here is its output:\n\n${output}`
-    );
-  } catch (/** @type {unknown} */ error) {
-    return error;
+  const output = await internalExec(['--FOR-TESTS', ...args], options);
+  if (output.exitCode !== 0) {
+    return output.stdout; // Should this be stderr?
   }
+
+  throw new Error(
+    `CLI did not exit with an exit code as expected. Here is its output:
+
+  ${output.text()}`
+  );
 }
 
 /**
- * @param {string} args
+ * @param {string[]} args
  * @param {Options | undefined} [options]
  * @returns {Promise<string>}
  */
 async function runWithoutTestMode(args, options) {
-  return await internalExec(args, options);
+  const output = await internalExec(args, options);
+
+  if (output.exitCode !== 0) throw new Error(output.text());
+
+  return output.stdout;
 }
 
 /**
- * @param {string} args
+ * @param {string[]} args
  * @param {Options} [options]
- * @returns {Promise<string>}
+ * @returns {Promise<ProcessOutput>}
  */
 async function internalExec(args, options = {}) {
-  // Overriding FORCE_COLOR because Jest forcefully adds it as well,
-  // which otherwise enables colors when we don't want it.
-  const colorFlag = 'FORCE_COLOR=' + (options.colors ? '1' : '0');
-
-  try {
-    // If this just uses child_process.exec, the shell scripts are pointless, and should be all migrated to Jest tests.
-    const result = await exec(
-      [colorFlag, cli, reportMode(options), colors(options), args].join(' '),
-      {
-        ...options,
-        cwd: cwdFromOptions(options)
-      }
-    );
-    return result.stdout;
-  } catch (error) {
-    throw error.stdout;
-  }
+  const result = await $({
+    cwd: cwdFromOptions(options),
+    env: {
+      ...process.env,
+      // Overriding `FORCE_COLOR` because Jest forcefully adds it as well,
+      // which otherwise enables colors when we don't want it.
+      FORCE_COLOR: options.colors ? '1' : '0'
+    }
+  })`${cli} ${reportMode(options)} ${colors(options)} ${args}`.nothrow();
+  return result;
 }
 
 /**
@@ -83,26 +87,26 @@ function cwdFromOptions(options) {
 
 /**
  * @param {Options} options
- * @returns {string}
+ * @returns {string[]}
  */
 function reportMode(options) {
   if (!options.report) {
-    return '';
+    return [];
   }
 
-  return `--report=${options.report}`;
+  return [`--report=${options.report}`];
 }
 
 /**
  * @param {Options} options
- * @returns {"" | "--no-color"}
+ * @returns {string[]}
  */
 function colors(options) {
   if (options.colors) {
-    return '';
+    return [];
   }
 
-  return `--no-color`;
+  return ['--no-color'];
 }
 
 module.exports = {