From 5856037368ee8d8a21f11eadbfe93d5f46507f60 Mon Sep 17 00:00:00 2001 From: mgiambalvo Date: Mon, 23 Jan 2017 15:10:44 -0800 Subject: [PATCH] fix(cli): Allow frameworks to specify flags they recognize. (#3994) Fix for #3978. Our initial plan to allow setting --disableChecks with an environment variable is insufficient, since the custom framework isn't even require()'d until after the config is parsed. This moves the unknown flag check into the runner, and gives frameworks a way to specify extra flags they accept. --- lib/cli.ts | 16 ++++------------ lib/config.ts | 9 ++++++++- lib/frameworks/README.md | 3 ++- lib/runner.ts | 14 +++++++++++++- scripts/errorTest.js | 2 +- 5 files changed, 28 insertions(+), 16 deletions(-) diff --git a/lib/cli.ts b/lib/cli.ts index f1d6cee75..4bf0b78d0 100644 --- a/lib/cli.ts +++ b/lib/cli.ts @@ -175,18 +175,10 @@ if (argv.version) { process.exit(0); } -if (!argv.disableChecks) { - // Check to see if additional flags were used. - let unknownKeys: string[] = Object.keys(argv).filter((element: string) => { - return element !== '$0' && element !== '_' && allowedNames.indexOf(element) === -1; - }); - - if (unknownKeys.length > 0) { - throw new Error( - 'Found extra flags: ' + unknownKeys.join(', ') + - ', please use --disableChecks flag to disable the Protractor CLI flag checks.'); - } -} +// Check to see if additional flags were used. +argv.unknownFlags_ = Object.keys(argv).filter((element: string) => { + return element !== '$0' && element !== '_' && allowedNames.indexOf(element) === -1; +}); /** * Helper to resolve comma separated lists of file pattern strings relative to diff --git a/lib/config.ts b/lib/config.ts index d274bdd18..d4dee0c2c 100644 --- a/lib/config.ts +++ b/lib/config.ts @@ -595,7 +595,13 @@ export interface Config { */ ng12Hybrid?: boolean; - seleniumArgs?: Array; + /** + * Protractor will exit with an error if it sees any command line flags it doesn't + * recognize. Set disableChecks true to disable this check. + */ + disableChecks?: boolean; + + seleniumArgs?: any[]; jvmArgs?: string[]; configDir?: string; troubleshoot?: boolean; @@ -607,4 +613,5 @@ export interface Config { frameworkPath?: string; elementExplorer?: any; debug?: boolean; + unknownFlags_?: string[]; } diff --git a/lib/frameworks/README.md b/lib/frameworks/README.md index a0efc6f2f..1cbd8fa5b 100644 --- a/lib/frameworks/README.md +++ b/lib/frameworks/README.md @@ -37,7 +37,8 @@ Requirements - `runner.runTestPreparer` must be called after the framework has been initialized but before any spec files are run. This function returns a - promise which should be waited on before executing tests. + promise which should be waited on before executing tests. The framework should + also pass an array of extra command line flags it accepts, if any. - `runner.getConfig().onComplete` must be called when tests are finished. It might return a promise, in which case `exports.run`'s promise should not diff --git a/lib/runner.ts b/lib/runner.ts index 983e17338..d38715b06 100644 --- a/lib/runner.ts +++ b/lib/runner.ts @@ -6,6 +6,7 @@ import * as util from 'util'; import {ProtractorBrowser} from './browser'; import {Config} from './config'; import {buildDriverProvider, DriverProvider} from './driverProviders'; +import {ConfigError} from './exitCodes'; import {Logger} from './logger'; import {Plugins} from './plugins'; import {protractor} from './ptor'; @@ -79,10 +80,21 @@ export class Runner extends EventEmitter { /** * Executor of testPreparer * @public + * @param {string[]=} An optional list of command line arguments the framework will accept. * @return {q.Promise} A promise that will resolve when the test preparers * are finished. */ - runTestPreparer(): q.Promise { + runTestPreparer(extraFlags?: string[]): q.Promise { + let unknownFlags = this.config_.unknownFlags_ || []; + if (extraFlags) { + unknownFlags = unknownFlags.filter((f) => extraFlags.indexOf(f) === -1); + } + if (unknownFlags.length > 0 && !this.config_.disableChecks) { + throw new ConfigError( + logger, + 'Found extra flags: ' + unknownFlags.join(', ') + + ', please use --disableChecks flag to disable the Protractor CLI flag checks. '); + } return this.plugins_.onPrepare().then(() => { return helper.runFilenameOrFn_(this.config_.configDir, this.preparer_); }); diff --git a/scripts/errorTest.js b/scripts/errorTest.js index 7a86f1bf6..6e55dcad2 100644 --- a/scripts/errorTest.js +++ b/scripts/errorTest.js @@ -21,7 +21,7 @@ var checkLogs = function(output, messages) { runProtractor = spawn('node', ['bin/protractor', 'example/conf.js', '--foobar', 'foobar']); -output = runProtractor.stderr.toString(); +output = runProtractor.stdout.toString(); messages = ['Error: Found extra flags: foobar']; checkLogs(output, messages);