From fbb6f2a0d2c6d026fdde3cf2974f42061ca40fe2 Mon Sep 17 00:00:00 2001 From: Simon Lydell Date: Sat, 16 Jan 2021 00:31:18 +0100 Subject: [PATCH] Upgrade to commander 7 (#486) This allows getting rid of all workarounds we had for commander. --- lib/Report.js | 13 +--- lib/elm-test.js | 164 ++++++++++++++++------------------------------ package-lock.json | 6 +- package.json | 2 +- tests/ci.js | 10 +++ 5 files changed, 72 insertions(+), 123 deletions(-) diff --git a/lib/Report.js b/lib/Report.js index e8fbc33c..09582148 100644 --- a/lib/Report.js +++ b/lib/Report.js @@ -4,16 +4,7 @@ // https://github.com/prettier/prettier/issues/2597 const Report /*: 'console' | 'json' | 'junit' */ = 'console'; -function parse(string /*: string */) /*: typeof Report */ { - switch (string) { - case 'console': - case 'json': - case 'junit': - return string; - default: - throw new Error(`unknown reporter: ${string}`); - } -} +const all = ['json', 'junit', 'console']; function isMachineReadable(report /*: typeof Report */) /*: boolean */ { switch (report) { @@ -27,6 +18,6 @@ function isMachineReadable(report /*: typeof Report */) /*: boolean */ { module.exports = { Report, - parse, + all, isMachineReadable, }; diff --git a/lib/elm-test.js b/lib/elm-test.js index 8f9d8e68..44b59c90 100644 --- a/lib/elm-test.js +++ b/lib/elm-test.js @@ -1,6 +1,6 @@ // @flow -const { program } = require('commander'); +const { InvalidOptionArgumentError, Option, program } = require('commander'); const fs = require('fs'); const os = require('os'); const path = require('path'); @@ -17,42 +17,17 @@ const RunTests = require('./RunTests'); void Report; -// TODO(https://github.com/rtfeldman/node-test-runner/pull/465): replace this -// function with commander's custom error messages once -// https://github.com/tj/commander.js/pull/1392 lands and is released. -const parsePositiveInteger = (flag /*: string */) => ( - string /*: string */ -) /*: number */ => { +const parsePositiveInteger = (string /*: string */) /*: number */ => { const number = Number(string); if (!/^\d+$/.test(string)) { - console.error( - `error: option '${flag}' expected one or more digits, but got: ${string}` - ); - throw process.exit(1); + throw new InvalidOptionArgumentError('Expected one or more digits.'); } else if (!Number.isFinite(number)) { - console.error( - `error: option '${flag}' expected a finite number, but got: ${number}` - ); - throw process.exit(1); + throw new InvalidOptionArgumentError('Expected a finite number.'); } else { return number; } }; -// TODO(https://github.com/rtfeldman/node-test-runner/pull/465): replace this -// function with commander's validating sets of strings once -// https://github.com/tj/commander.js/issues/518 is released (probably v7). -const parseReport = (flag /*: string */) => ( - string /*: string */ -) /*: typeof Report.Report */ => { - try { - return Report.parse(string); - } catch (error) { - console.error(`error: option '${flag}' ${error.message}`); - throw process.exit(1); - } -}; - function findClosestElmJson(dir /*: string */) /*: string | void */ { const entry = ElmJson.getPath(dir); return fs.existsSync(entry) @@ -99,33 +74,6 @@ function getPathToElmBinary(compiler /*: string | void */) /*: string */ { } } -// Unfortunately commander is very permissive about extra arguments. Therefore, -// we manually check for excessive arguments. -// See: https://github.com/tj/commander.js/issues/1268 -function handleTooManyArgs(action) { - return (...args) => { - if (args.length < 2) { - action(...args); - } else { - // The arguments to Commander actions are: - // expectedCliArg1, expectedCliArg2, expectedCliArgN, Cmd, restCliArgs - const rest = args[args.length - 1]; - if (rest.length > 0) { - const expected = args.length - 2; - const s = expected === 1 ? '' : 's'; - console.error( - `Expected ${expected} argument${s}, but got ${ - expected + rest.length - }.` - ); - process.exit(1); - } else { - action(...args); - } - } - }; -} - const examples = ` elm-test Run tests in the tests/ folder @@ -138,7 +86,7 @@ function main() { process.title = 'elm-test'; program - .storeOptionsAsProperties(false) + .allowExcessArguments(false) .name('elm-test') .usage('[options] [globs...]') .description(examples) @@ -150,23 +98,24 @@ function main() { 'Use a custom path to an Elm executable (default: elm)', undefined ) - .option( - '--seed ', - 'Run with a previous fuzzer seed', - parsePositiveInteger('--seed '), - Math.floor(Math.random() * 407199254740991) + 1000 + .addOption( + new Option('--seed ', 'Run with a previous fuzzer seed') + .default(Math.floor(Math.random() * 407199254740991) + 1000, 'random') + .argParser(parsePositiveInteger) ) .option( '--fuzz ', 'Run with each fuzz test performing this many iterations', - parsePositiveInteger('--fuzz '), + parsePositiveInteger, 100 ) - .option( - '--report ', - 'Print results to stdout in the given format', - parseReport('--report '), - 'console' + .addOption( + new Option( + '--report ', + 'Print results to stdout in the given format' + ) + .default('console') + .choices(Report.all) ) .option('--watch', 'Run tests on file changes', false) .version(packageInfo.version, '--version', 'Print version and exit') @@ -176,53 +125,49 @@ function main() { program .command('init') .description('Create example tests') - .action( - handleTooManyArgs(() => { - const options = program.opts(); - const pathToElmBinary = getPathToElmBinary(options.compiler); - const project = getProject('init'); - try { - Install.install(project, pathToElmBinary, 'elm-explorations/test'); - fs.mkdirSync(project.testsDir, { recursive: true }); - fs.copyFileSync( - path.join(__dirname, '..', 'templates', 'tests', 'Example.elm'), - path.join(project.testsDir, 'Example.elm') - ); - } catch (error) { - console.error(error.message); - throw process.exit(1); - } - console.log( - '\nCheck out the documentation for getting started at https://package.elm-lang.org/packages/elm-explorations/test/latest' + .action(() => { + const options = program.opts(); + const pathToElmBinary = getPathToElmBinary(options.compiler); + const project = getProject('init'); + try { + Install.install(project, pathToElmBinary, 'elm-explorations/test'); + fs.mkdirSync(project.testsDir, { recursive: true }); + fs.copyFileSync( + path.join(__dirname, '..', 'templates', 'tests', 'Example.elm'), + path.join(project.testsDir, 'Example.elm') ); - process.exit(0); - }) - ); + } catch (error) { + console.error(error.message); + throw process.exit(1); + } + console.log( + '\nCheck out the documentation for getting started at https://package.elm-lang.org/packages/elm-explorations/test/latest' + ); + process.exit(0); + }); program .command('install ') .description( 'Like `elm install package`, except it installs to "test-dependencies" in your elm.json' ) - .action( - handleTooManyArgs((packageName) => { - const options = program.opts(); - const pathToElmBinary = getPathToElmBinary(options.compiler); - const project = getProject('install'); - try { - const result = Install.install(project, pathToElmBinary, packageName); - // This mirrors the behavior of `elm install` passing a package that is - // already installed. Say it's already installed, then exit 0. - if (result === 'AlreadyInstalled') { - console.log('It is already installed!'); - } - process.exit(0); - } catch (error) { - console.error(error.message); - process.exit(1); + .action((packageName) => { + const options = program.opts(); + const pathToElmBinary = getPathToElmBinary(options.compiler); + const project = getProject('install'); + try { + const result = Install.install(project, pathToElmBinary, packageName); + // This mirrors the behavior of `elm install` passing a package that is + // already installed. Say it's already installed, then exit 0. + if (result === 'AlreadyInstalled') { + console.log('It is already installed!'); } - }) - ); + process.exit(0); + } catch (error) { + console.error(error.message); + process.exit(1); + } + }); program .command('make [globs...]') @@ -234,7 +179,10 @@ function main() { const make = async () => { await Generate.generateElmJson(project, () => null); await Compile.compileSources( - FindTests.resolveGlobs(testFileGlobs, project.rootDir), + FindTests.resolveGlobs( + testFileGlobs.length === 0 ? [project.testsDir] : testFileGlobs, + project.rootDir + ), project.generatedCodeDir, pathToElmBinary, options.report diff --git a/package-lock.json b/package-lock.json index eb02b06c..733e7778 100644 --- a/package-lock.json +++ b/package-lock.json @@ -449,9 +449,9 @@ "dev": true }, "commander": { - "version": "6.2.1", - "resolved": "https://registry.npmjs.org/commander/-/commander-6.2.1.tgz", - "integrity": "sha512-U7VdrJFnJgo4xjrHpTzu0yrHPGImdsmD95ZlgYSEajAn2JKzDhDTPG9kBTefmObL2w/ngeZnilk+OV9CG3d7UA==" + "version": "7.0.0", + "resolved": "https://registry.npmjs.org/commander/-/commander-7.0.0.tgz", + "integrity": "sha512-ovx/7NkTrnPuIV8sqk/GjUIIM1+iUQeqA3ye2VNpq9sVoiZsooObWlQy+OPWGI17GDaEoybuAGJm6U8yC077BA==" }, "concat-map": { "version": "0.0.1", diff --git a/package.json b/package.json index 92cccb81..fa4d1485 100644 --- a/package.json +++ b/package.json @@ -46,7 +46,7 @@ "dependencies": { "chalk": "^4.1.0", "chokidar": "^3.4.2", - "commander": "^6.2.0", + "commander": "^7.0.0", "cross-spawn": "^7.0.3", "elm-tooling": "^1.0.0", "glob": "^7.1.6", diff --git a/tests/ci.js b/tests/ci.js index f5383b23..39c6c3b5 100755 --- a/tests/ci.js +++ b/tests/ci.js @@ -115,6 +115,11 @@ describe('Testing elm-test on an example application', () => { const runResult = execElmTest([args], cwd); assertTestFailure(runResult); }).timeout(60000); + + it('Should successfully run `elm-test make`', () => { + const runResult = execElmTest(['make'], cwd); + assertTestSuccess(runResult); + }).timeout(60000); }); describe('Testing elm-test on an example package', () => { @@ -131,6 +136,11 @@ describe('Testing elm-test on an example package', () => { const runResult = execElmTest([args], cwd); assertTestFailure(runResult); }).timeout(60000); + + it('Should successfully run `elm-test make`', () => { + const runResult = execElmTest(['make'], cwd); + assertTestSuccess(runResult); + }).timeout(60000); }); describe('Testing elm-test on example-application-src', () => {