Skip to content

Commit

Permalink
feat(cli): error out on unexpected options or parameters (#3589)
Browse files Browse the repository at this point in the history 
This should make CLI more helpful as it will error out early and users can see that they have passed a wrong option instead of guessing why it does not have any effect.

Notes:

- units tests use same parser configuration as production code (hence changes to tests)
- logic and test case for _ typos in option names was removed as this is covered by yargs strict mode now
- added documentation for couple of existing options as otherwise they are considered unknown and error out (but they do exist and were found in the unit tests)

BREAKING CHANGE: Karma is more strict and will error out if unknown option or argument is passed to CLI.
  • Loading branch information
@devoto13
devoto13 authored Dec 17, 2020
1 parent 7a3bd55 commit 603bbc0
Show file tree
Hide file tree
Showing 3 changed files with 140 additions and 61 deletions.
52 changes: 38 additions & 14 deletions lib/cli.js
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
'use strict'

const path = require('path')
const assert = require('assert')
const yargs = require('yargs')
const fs = require('graceful-fs')

Expand All @@ -10,12 +9,9 @@ const helper = require('./helper')
const constant = require('./constants')

function processArgs (argv, options, fs, path) {
// TODO(vojta): warn/throw when unknown argument (probably mispelled)
Object.getOwnPropertyNames(argv).forEach(function (name) {
let argumentValue = argv[name]
if (name !== '_' && name !== '$0') {
assert(!name.includes('_'), `Bad argument: ${name} did you mean ${name.replace('_', '-')}`)

if (Array.isArray(argumentValue)) {
argumentValue = argumentValue.pop() // If the same argument is defined multiple times, override.
}
Expand Down Expand Up @@ -99,7 +95,7 @@ function processArgs (argv, options, fs, path) {
options.refresh = options.refresh === 'true'
}

let configFile = argv._.shift()
let configFile = argv.configFile

if (!configFile) {
// default config file (if exists)
Expand Down Expand Up @@ -151,13 +147,13 @@ function describeRoot () {
'Run --help with particular command to see its description and available options.\n\n' +
'Usage:\n' +
' $0 <command>')
.command('init', 'Initialize a config file.', describeInit)
.command('start', 'Start the server / do a single run.', describeStart)
.command('run', 'Trigger a test run.', describeRun)
.command('stop', 'Stop the server.', describeStop)
.command('init [configFile]', 'Initialize a config file.', describeInit)
.command('start [configFile]', 'Start the server / do a single run.', describeStart)
.command('run [configFile]', 'Trigger a test run.', describeRun)
.command('stop [configFile]', 'Stop the server.', describeStop)
.command('completion', 'Shell completion for karma.', describeCompletion)
.demandCommand(1, 'Command not specified.')
.strictCommands()
.strict()
.describe('help', 'Print usage and options.')
.describe('version', 'Print current version.')
}
Expand All @@ -168,8 +164,11 @@ function describeInit (yargs) {
'INIT - Initialize a config file.\n\n' +
'Usage:\n' +
' $0 init [configFile]')
.strictCommands(false)
.version(false)
.positional('configFile', {
describe: 'Name of the generated Karma configuration file',
type: 'string'
})
.describe('log-level', '<disable | error | warn | info | debug> Level of logging.')
.describe('colors', 'Use colors when reporting and printing logs.')
.describe('no-colors', 'Do not use colors when reporting or printing logs.')
Expand All @@ -183,6 +182,10 @@ function describeStart (yargs) {
' $0 start [configFile]')
.strictCommands(false)
.version(false)
.positional('configFile', {
describe: 'Path to the Karma configuration file',
type: 'string'
})
.describe('port', '<integer> Port where the server is running.')
.describe('auto-watch', 'Auto watch source files and run on change.')
.describe('detached', 'Detach the server.')
Expand All @@ -200,6 +203,10 @@ function describeStart (yargs) {
.describe('no-fail-on-empty-test-suite', 'Do not fail on empty test suite.')
.describe('fail-on-failing-test-suite', 'Fail on failing test suite.')
.describe('no-fail-on-failing-test-suite', 'Do not fail on failing test suite.')
.option('format-error', {
describe: 'A path to a file that exports the format function.',
type: 'string'
})
}

function describeRun (yargs) {
Expand All @@ -208,15 +215,30 @@ function describeRun (yargs) {
'RUN - Run the tests (requires running server).\n\n' +
'Usage:\n' +
' $0 run [configFile] [-- <clientArgs>]')
.strictCommands(false)
.version(false)
.positional('configFile', {
describe: 'Path to the Karma configuration file',
type: 'string'
})
.describe('port', '<integer> Port where the server is listening.')
.describe('no-refresh', 'Do not re-glob all the patterns.')
.describe('fail-on-empty-test-suite', 'Fail on empty test suite.')
.describe('no-fail-on-empty-test-suite', 'Do not fail on empty test suite.')
.describe('log-level', '<disable | error | warn | info | debug> Level of logging.')
.describe('colors', 'Use colors when reporting and printing logs.')
.describe('no-colors', 'Do not use colors when reporting or printing logs.')
.option('removed-files', {
describe: 'Comma-separated paths to removed files. Useful when automatic file watching is disabled.',
type: 'string'
})
.option('changed-files', {
describe: 'Comma-separated paths to changed files. Useful when automatic file watching is disabled.',
type: 'string'
})
.option('added-files', {
describe: 'Comma-separated paths to added files. Useful when automatic file watching is disabled.',
type: 'string'
})
}

function describeStop (yargs) {
Expand All @@ -225,8 +247,11 @@ function describeStop (yargs) {
'STOP - Stop the server (requires running server).\n\n' +
'Usage:\n' +
' $0 stop [configFile]')
.strictCommands(false)
.version(false)
.positional('configFile', {
describe: 'Path to the Karma configuration file',
type: 'string'
})
.describe('port', '<integer> Port where the server is listening.')
.describe('log-level', '<disable | error | warn | info | debug> Level of logging.')
}
Expand All @@ -237,7 +262,6 @@ function describeCompletion (yargs) {
'COMPLETION - Bash/ZSH completion for karma.\n\n' +
'Installation:\n' +
' $0 completion >> ~/.bashrc')
.strictCommands(false)
.version(false)
}

Expand Down
90 changes: 79 additions & 11 deletions test/e2e/cli.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@ Feature: CLI
karma <command>
Commands:
karma init Initialize a config file.
karma start Start the server / do a single run.
karma run Trigger a test run.
karma stop Stop the server.
karma completion Shell completion for karma.
karma init [configFile] Initialize a config file.
karma start [configFile] Start the server / do a single run.
karma run [configFile] Trigger a test run.
karma stop [configFile] Stop the server.
karma completion Shell completion for karma.
Options:
--help Print usage and options. [boolean]
Expand All @@ -45,17 +45,62 @@ Feature: CLI
karma <command>
Commands:
karma init Initialize a config file.
karma start Start the server / do a single run.
karma run Trigger a test run.
karma stop Stop the server.
karma completion Shell completion for karma.
karma init [configFile] Initialize a config file.
karma start [configFile] Start the server / do a single run.
karma run [configFile] Trigger a test run.
karma stop [configFile] Stop the server.
karma completion Shell completion for karma.
Options:
--help Print usage and options. [boolean]
--version Print current version. [boolean]
Unknown command: strat
Unknown argument: strat
"""

Scenario: Error when option is unknown
When I execute Karma with arguments: "start --invalid-option"
Then the stderr is exactly:
"""
Karma - Spectacular Test Runner for JavaScript.
START - Start the server / do a single run.
Usage:
karma start [configFile]
Positionals:
configFile Path to the Karma configuration file [string]
Options:
--help Print usage and options. [boolean]
--port <integer> Port where the server is running.
--auto-watch Auto watch source files and run on change.
--detached Detach the server.
--no-auto-watch Do not watch source files.
--log-level <disable | error | warn | info | debug> Level
of logging.
--colors Use colors when reporting and printing logs.
--no-colors Do not use colors when reporting or printing
logs.
--reporters List of reporters (available: dots, progress,
junit, growl, coverage).
--browsers List of browsers to start (eg. --browsers
Chrome,ChromeCanary,Firefox).
--capture-timeout <integer> Kill browser if does not capture in
given time [ms].
--single-run Run the test when browsers captured and exit.
--no-single-run Disable single-run.
--report-slower-than <integer> Report tests that are slower than
given time [ms].
--fail-on-empty-test-suite Fail on empty test suite.
--no-fail-on-empty-test-suite Do not fail on empty test suite.
--fail-on-failing-test-suite Fail on failing test suite.
--no-fail-on-failing-test-suite Do not fail on failing test suite.
--format-error A path to a file that exports the format
function. [string]
Unknown arguments: invalid-option, invalidOption
"""

Scenario: Init command help
Expand All @@ -69,6 +114,9 @@ Feature: CLI
Usage:
karma init [configFile]
Positionals:
configFile Name of the generated Karma configuration file [string]
Options:
--help Print usage and options. [boolean]
--log-level <disable | error | warn | info | debug> Level of logging.
Expand All @@ -87,6 +135,9 @@ Feature: CLI
Usage:
karma start [configFile]
Positionals:
configFile Path to the Karma configuration file [string]
Options:
--help Print usage and options. [boolean]
--port <integer> Port where the server is running.
Expand All @@ -112,6 +163,8 @@ Feature: CLI
--no-fail-on-empty-test-suite Do not fail on empty test suite.
--fail-on-failing-test-suite Fail on failing test suite.
--no-fail-on-failing-test-suite Do not fail on failing test suite.
--format-error A path to a file that exports the format
function. [string]
"""

Scenario: Run command help
Expand All @@ -125,6 +178,9 @@ Feature: CLI
Usage:
karma run [configFile] [-- <clientArgs>]
Positionals:
configFile Path to the Karma configuration file [string]
Options:
--help Print usage and options. [boolean]
--port <integer> Port where the server is listening.
Expand All @@ -136,6 +192,15 @@ Feature: CLI
--colors Use colors when reporting and printing logs.
--no-colors Do not use colors when reporting or printing
logs.
--removed-files Comma-separated paths to removed files. Useful
when automatic file watching is disabled.
[string]
--changed-files Comma-separated paths to changed files. Useful
when automatic file watching is disabled.
[string]
--added-files Comma-separated paths to added files. Useful
when automatic file watching is disabled.
[string]
"""

Scenario: Stop command help
Expand All @@ -149,6 +214,9 @@ Feature: CLI
Usage:
karma stop [configFile]
Positionals:
configFile Path to the Karma configuration file [string]
Options:
--help Print usage and options. [boolean]
--port <integer> Port where the server is listening.
Expand Down
Loading

0 comments on commit 603bbc0

Please sign in to comment.