From 73cc24739153967eecc50eda44240c4fb5ba903a Mon Sep 17 00:00:00 2001 From: Eli <88557639+lishaduck@users.noreply.github.com> Date: Sun, 8 Dec 2024 21:43:54 -0600 Subject: [PATCH] refactor: use zx for jest tests --- test/compiler-flag.test.js | 18 ++++++-- test/flags.test.js | 71 +++++++++++++++++------------- test/help.test.js | 12 ++--- test/jest-helpers/cli.js | 90 ++++++++++++++++++++------------------ test/review.test.js | 37 +++++++++------- test/suppress.test.js | 12 ++--- test/version.test.js | 4 +- 7 files changed, 136 insertions(+), 108 deletions(-) diff --git a/test/compiler-flag.test.js b/test/compiler-flag.test.js index 987f33435..f2dc21762 100644 --- a/test/compiler-flag.test.js +++ b/test/compiler-flag.test.js @@ -12,7 +12,11 @@ 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'); @@ -20,7 +24,11 @@ test('should retrieve `elm` binary from PATH', async () => { 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'); @@ -28,7 +36,11 @@ test('should retrieve `elm` binary locally when path is relative', async () => { 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 index 1af25be5e..becd08510 100644 --- 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 index bae28a342..fe64a8be1 100644 --- 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 index 124cb33d8..4f1f8b3f8 100644 --- 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} */ 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} */ 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} */ 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} + * @returns {Promise} */ 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 = { diff --git a/test/review.test.js b/test/review.test.js index c95d2e74b..db90018c5 100644 --- a/test/review.test.js +++ b/test/review.test.js @@ -12,14 +12,14 @@ function testName(name) { } test('Regular run from inside the project', async () => { - const output = await TestCli.runAndExpectError('', { + const output = await TestCli.runAndExpectError([], { project: 'project-with-errors/' }); expect(output).toMatchFile(testName('review-with-errors')); }); test('Regular run from inside the project (JSON output)', async () => { - const output = await TestCli.runAndExpectError('', { + const output = await TestCli.runAndExpectError([], { project: 'project-with-errors/', report: 'json' }); @@ -27,7 +27,7 @@ test('Regular run from inside the project (JSON output)', async () => { }); test('Regular run from inside the project (ndjson output)', async () => { - const output = await TestCli.runAndExpectError('', { + const output = await TestCli.runAndExpectError([], { project: 'project-with-errors/', report: 'ndjson' }); @@ -36,7 +36,7 @@ test('Regular run from inside the project (ndjson output)', async () => { test('Running using other configuration (without errors)', async () => { const output = await TestCli.run( - '--config ../config-that-triggers-no-errors', + ['--config=../config-that-triggers-no-errors'], {project: 'project-with-errors/'} ); expect(output).toMatchFile(testName('no-errors')); @@ -44,21 +44,24 @@ test('Running using other configuration (without errors)', async () => { test('Regular run using --elmjson and --config', async () => { const output = await TestCli.runAndExpectError( - '--elmjson project-with-errors/elm.json --config project-with-errors/review', + [ + '--elmjson=project-with-errors/elm.json', + '--config=project-with-errors/review' + ], {cwd: path.resolve(__dirname, '.')} ); expect(output).toMatchFile(testName('run-with-elmjson-flag')); }); test('Running in a project using ES2015 modules', async () => { - const output = await TestCli.runAndExpectError('', { + const output = await TestCli.runAndExpectError([], { project: 'project-using-es2015-module' }); expect(output).toMatchFile(testName('config-es2015-modules')); }); test('Using an empty configuration', async () => { - const output = await TestCli.runAndExpectError('--config ../config-empty', { + const output = await TestCli.runAndExpectError(['--config=../config-empty'], { project: 'project-with-errors' }); expect(output).toMatchFile(testName('config-empty')); @@ -66,7 +69,7 @@ test('Using an empty configuration', async () => { test('Using a configuration with a missing direct elm-review dependency', async () => { const output = await TestCli.runAndExpectError( - '--config ../config-without-elm-review', + ['--config=../config-without-elm-review'], {project: 'project-with-errors'} ); expect(output).toMatchFile(testName('without-elm-review')); @@ -74,7 +77,7 @@ test('Using a configuration with a missing direct elm-review dependency', async test('Using a configuration with an outdated elm-review package', async () => { const output = await TestCli.runAndExpectError( - '--config ../config-for-outdated-elm-review-version', + ['--config=../config-for-outdated-elm-review-version'], {project: 'project-with-errors'} ); expect(output).toMatchFile(testName('outdated-version')); @@ -82,7 +85,7 @@ test('Using a configuration with an outdated elm-review package', async () => { test('Using a configuration which fails due to unknown module', async () => { const output = await TestCli.runAndExpectError( - '--config ../config-error-unknown-module', + ['--config=../config-error-unknown-module'], {project: 'project-with-errors'} ); expect(output).toMatchFile(testName('config-error-unknown-module')); @@ -90,7 +93,7 @@ test('Using a configuration which fails due to unknown module', async () => { test('Using a configuration which fails due to syntax error', async () => { const output = await TestCli.runAndExpectError( - '--config ../config-syntax-error', + ['--config=../config-syntax-error'], {project: 'project-with-errors'} ); expect(output).toMatchFile(testName('config-syntax-error')); @@ -98,7 +101,7 @@ test('Using a configuration which fails due to syntax error', async () => { test('Using a configuration which fails due to configuration error', async () => { const output = await TestCli.runAndExpectError( - '--config ../config-configuration-error', + ['--config=../config-configuration-error'], {project: 'project-with-errors'} ); expect(output).toMatchFile(testName('config-configuration-error')); @@ -106,7 +109,7 @@ test('Using a configuration which fails due to configuration error', async () => test('Using a configuration which fails due to debug remnants', async () => { const output = await TestCli.runAndExpectError( - '--config ../config-error-debug', + ['--config=../config-error-debug'], {project: 'project-with-errors'} ); expect(output).toMatchFile(testName('config-error-debug')); @@ -114,7 +117,7 @@ test('Using a configuration which fails due to debug remnants', async () => { test('Running on project with unknown file', async () => { const output = await TestCli.runAndExpectError( - '--config ../config-that-triggers-no-errors unknown-target', + ['--config=../config-that-triggers-no-errors', 'unknown-target'], {project: 'project-with-errors'} ); expect(output).toMatchFile(testName('run-with-unknown-target')); @@ -122,7 +125,7 @@ test('Running on project with unknown file', async () => { test('Running on project with a directory ending in .elm (without arg)', async () => { const output = await TestCli.run( - '--config ../config-that-triggers-no-errors', + ['--config', '../config-that-triggers-no-errors'], {project: 'project-with-dir-ending-in-elm'} ); expect(output).toMatchFile(testName('src.elm-project-without-arg')); @@ -130,14 +133,14 @@ test('Running on project with a directory ending in .elm (without arg)', async ( test('Running on project with a directory ending in .elm (with arg)', async () => { const output = await TestCli.run( - '--config ../config-that-triggers-no-errors src.elm', + ['--config', '../config-that-triggers-no-errors', 'src.elm'], {project: 'project-with-dir-ending-in-elm'} ); expect(output).toMatchFile(testName('src.elm-project-with-arg')); }); test('Includes files from a source-directory if that source-directory is in elm-stuff', async () => { - const output = await TestCli.runAndExpectError('', { + const output = await TestCli.runAndExpectError([], { project: 'project-with-files-in-elm-stuff' }); expect(output).toMatchFile( diff --git a/test/suppress.test.js b/test/suppress.test.js index 85819fddd..cf846cd3a 100644 --- a/test/suppress.test.js +++ b/test/suppress.test.js @@ -14,13 +14,13 @@ function testName(name) { } test('Running on project with only suppressed errors remaining should not exit with failure', async () => { - return await TestCli.run('', { + return await TestCli.run([], { project: 'project-with-suppressed-errors' }); }); test('Running with --unsuppress should report suppressed errors', async () => { - const output = await TestCli.runAndExpectError('--unsuppress', { + const output = await TestCli.runAndExpectError(['--unsuppress'], { project: 'project-with-suppressed-errors' }); expect(output).toMatchFile(testName('suppressed-errors-unsuppress')); @@ -28,14 +28,14 @@ test('Running with --unsuppress should report suppressed errors', async () => { test('Running with --unsuppress-rules should report suppressed errors for that rule', async () => { const output = await TestCli.runAndExpectError( - '--unsuppress-rules NoUnused.Dependencies', + ['--unsuppress-rules', 'NoUnused.Dependencies'], {project: 'project-with-suppressed-errors'} ); expect(output).toMatchFile(testName('suppressed-errors-unsuppress-rules')); }); test('Running with "suppress --check-after-tests" when there are no uncommitted changes should not exit with failure', async () => { - const output = await TestCli.run('suppress --check-after-tests', { + const output = await TestCli.run(['suppress', '--check-after-tests'], { project: 'project-with-suppressed-errors2' }); expect(output).toEqual(''); @@ -50,7 +50,7 @@ test('Running with "suppress --check-after-tests" when there are uncommitted cha await $`rm -r ${filePath}`; const output = await TestCli.runAndExpectError( - 'suppress --check-after-tests', + ['suppress', '--check-after-tests'], {project: 'project-with-suppressed-errors'} ); // Remove uncommitted suppression files @@ -72,6 +72,6 @@ test('Running with unsupported version of suppression files should exit with fai ); await $`chmod -w ${filePath}`; - const output = await TestCli.runAndExpectError('', {project}); + const output = await TestCli.runAndExpectError([], {project}); expect(output).toMatchFile(testName('write-failure')); }); diff --git a/test/version.test.js b/test/version.test.js index c3d3ade24..37f5d9736 100644 --- a/test/version.test.js +++ b/test/version.test.js @@ -2,11 +2,11 @@ const packageJson = require('../package.json'); const TestCli = require('./jest-helpers/cli'); test('Running with --version', async () => { - const output = await TestCli.runWithoutTestMode('--version'); + const output = await TestCli.runWithoutTestMode(['--version']); expect(output.trimEnd()).toEqual(packageJson.version); }); test('Running with the shorthand -v', async () => { - const output = await TestCli.runWithoutTestMode('-v'); + const output = await TestCli.runWithoutTestMode(['-v']); expect(output.trimEnd()).toEqual(packageJson.version); });