fix(options): resolve false positives in unknown options warning (#2208)

* Don't give a warning for "packageManager" config option
* Use `commander.opts` to retrieve options to make sure we don't get unknown properties.

e2e/test/vue-cli-javascript-jest/stryker.conf.json

@@ -15,9 +15,5 @@
     "progress",
     "clear-text",
     "html"
-  ],
-  "spec": [
-    "dist/js/chunk-vendors.js",
-    "dist/js/tests.js"
   ]
 }

packages/api/schema/stryker-core.json

@@ -273,6 +273,10 @@
       "default": "javascript",
       "errorMessage": "should be an \"object\" describing the mutator or a \"string\". See https://github.com/stryker-mutator/stryker/tree/master/packages/core#mutator."
     },
+    "packageManager": {
+      "enum": ["npm", "yarn"],
+      "description": "The package manager Stryker can use to install missing dependencies."
+    },
     "plugins": {
       "description": "With 'plugins', you can add additional Node modules for Stryker to load (or require). By default, all node_modules starting with @stryker-mutator/* will be loaded, so you would normally not need to specify this option. These modules should be installed right next to stryker. For a current list of plugins, you can consult 'npm' or 'stryker-mutator.io.'",
       "type": "array",

packages/api/src/core/PartialStrykerOptions.ts

@@ -8,5 +8,5 @@ import { StrykerOptions } from '../../core';
 export type PartialStrykerOptions = DeepPartial<StrykerOptions>;
 
 type DeepPartial<T> = {
-  [P in keyof T]?: T[P] extends object ? DeepPartial<T[P]> : T[P];
+  [P in keyof T]?: T[P] extends object ? DeepPartial<T[P]> | undefined : T[P];
 };

packages/core/src/Stryker.ts

@@ -1,4 +1,4 @@
-import { StrykerOptions } from '@stryker-mutator/api/core';
+import { StrykerOptions, PartialStrykerOptions } from '@stryker-mutator/api/core';
 import { Logger } from '@stryker-mutator/api/logging';
 import { commonTokens, PluginKind } from '@stryker-mutator/api/plugin';
 import { MutantResult } from '@stryker-mutator/api/report';
@@ -43,7 +43,7 @@ export default class Stryker {
    * @constructor
    * @param {Object} [cliOptions] - Optional options.
    */
-  constructor(cliOptions: Partial<StrykerOptions>) {
+  constructor(cliOptions: PartialStrykerOptions) {
     LogConfigurator.configureMainProcess(cliOptions.logLevel, cliOptions.fileLogLevel, cliOptions.allowConsoleColors);
     this.injector = buildMainInjector(cliOptions);
     this.log = this.injector.resolve(commonTokens.getLogger)(Stryker.name);

packages/core/src/StrykerCli.ts

@@ -1,6 +1,6 @@
 import * as commander from 'commander';
 import { getLogger } from 'log4js';
-import { DashboardOptions, StrykerOptions, ALL_REPORT_TYPES } from '@stryker-mutator/api/core';
+import { DashboardOptions, ALL_REPORT_TYPES, PartialStrykerOptions } from '@stryker-mutator/api/core';
 import { Logger } from '@stryker-mutator/api/logging';
 
 import { initializerFactory } from './initializer';
@@ -37,7 +37,7 @@ export default class StrykerCli {
   constructor(
     private readonly argv: string[],
     private readonly program: commander.Command = new commander.Command(),
-    private readonly runMutationTest = async (options: Partial<StrykerOptions>) => new Stryker(options).runMutationTest(),
+    private readonly runMutationTest = async (options: PartialStrykerOptions) => new Stryker(options).runMutationTest(),
     private readonly log: Logger = getLogger(StrykerCli.name)
   ) {}
 
@@ -92,7 +92,7 @@ export default class StrykerCli {
         '--fileLogLevel <level>',
         `Set the log4js log level for the "stryker.log" file. Possible values: fatal, error, warn, info, debug, trace, all and off. Default is "${defaultValues.fileLogLevel}"`
       )
-      .option('--allowConsoleColors <true/false>', 'Indicates whether or not Stryker should use colors in console.', parseBoolean, true)
+      .option('--allowConsoleColors <true/false>', 'Indicates whether or not Stryker should use colors in console.', parseBoolean)
       .option(
         '--dashboard.project <name>',
         'Indicates which project name to use if the "dashboard" reporter is enabled. Defaults to the git url configured in the environment of your CI server.',
@@ -125,29 +125,25 @@ export default class StrykerCli {
       .parse(this.argv);
 
     // Earliest opportunity to configure the log level based on the logLevel argument
-    LogConfigurator.configureMainProcess(this.program.logLevel);
+    const options: PartialStrykerOptions = this.program.opts();
+    LogConfigurator.configureMainProcess(options.logLevel);
 
     // Cleanup commander state
-    delete this.program.options;
-    delete this.program.rawArgs;
-    delete this.program.args;
-    delete this.program.Command;
-    delete this.program.Option;
-    delete this.program.commands;
-    for (const i in this.program) {
-      if (i.startsWith('_') || i.startsWith('dashboard.')) {
-        delete this.program[i];
-      }
-    }
+    delete options.version;
+    Object.keys(options)
+      .filter((key) => key.startsWith('dashboard.'))
+      .forEach((key) => delete options[key]);
 
     if (this.strykerConfig) {
-      this.program.configFile = this.strykerConfig;
+      options.configFile = this.strykerConfig;
+    }
+    if (Object.keys(dashboard).length > 0) {
+      options.dashboard = dashboard;
     }
-    this.program.dashboard = dashboard;
 
     const commands: { [cmd: string]: () => Promise<any> } = {
       init: () => initializerFactory().initialize(),
-      run: () => this.runMutationTest(this.program),
+      run: () => this.runMutationTest(options),
     };
 
     if (Object.keys(commands).includes(this.command)) {

packages/core/src/config/ConfigReader.ts

@@ -41,7 +41,6 @@ export default class ConfigReader {
     if (this.log.isDebugEnabled()) {
       this.log.debug(`Loaded config: ${JSON.stringify(options, null, 2)}`);
     }
-
     return options;
   }
 

packages/core/src/di/buildMainInjector.ts

@@ -1,4 +1,4 @@
-import { StrykerOptions, strykerCoreSchema } from '@stryker-mutator/api/core';
+import { StrykerOptions, strykerCoreSchema, PartialStrykerOptions } from '@stryker-mutator/api/core';
 import { commonTokens, Injector, OptionsContext, PluginKind, Scope, tokens } from '@stryker-mutator/api/plugin';
 import { Reporter } from '@stryker-mutator/api/report';
 import { TestFramework } from '@stryker-mutator/api/test_framework';
@@ -36,7 +36,7 @@ export interface MainContext extends OptionsContext {
 type BasicInjector = Injector<Pick<MainContext, 'logger' | 'getLogger'>>;
 type PluginResolverInjector = Injector<Pick<MainContext, 'logger' | 'getLogger' | 'options' | 'pluginResolver'>>;
 
-export function buildMainInjector(cliOptions: Partial<StrykerOptions>): Injector<MainContext> {
+export function buildMainInjector(cliOptions: PartialStrykerOptions): Injector<MainContext> {
   const basicInjector = createBasicInjector();
   const pluginResolverInjector = createPluginResolverInjector(cliOptions, basicInjector);
   return pluginResolverInjector
@@ -54,7 +54,7 @@ function createBasicInjector(): BasicInjector {
   return rootInjector.provideValue(commonTokens.getLogger, getLogger).provideFactory(commonTokens.logger, loggerFactory, Scope.Transient);
 }
 
-export function createPluginResolverInjector(cliOptions: Partial<StrykerOptions>, parent: BasicInjector): PluginResolverInjector {
+export function createPluginResolverInjector(cliOptions: PartialStrykerOptions, parent: BasicInjector): PluginResolverInjector {
   return parent
     .provideValue(coreTokens.cliOptions, cliOptions)
     .provideValue(coreTokens.validationSchema, strykerCoreSchema)

packages/core/src/initializer/StrykerConfigWriter.ts

@@ -46,7 +46,7 @@ export default class StrykerConfigWriter {
   ): Promise<string> {
     const configObject: Partial<StrykerOptions> = {
       mutator: selectedMutator ? selectedMutator.name : '',
-      packageManager: selectedPackageManager.name,
+      packageManager: selectedPackageManager.name as 'npm' | 'yarn',
       reporters: selectedReporters.map((rep) => rep.name),
       testRunner: selectedTestRunner ? selectedTestRunner.name : '',
       transpilers: selectedTranspilers ? selectedTranspilers.map((t) => t.name) : [],

packages/core/test/unit/StrykerCli.spec.ts

@@ -3,7 +3,7 @@ import * as sinon from 'sinon';
 import { Logger } from '@stryker-mutator/api/logging';
 import { logger } from '@stryker-mutator/test-helpers/src/factory';
 import { expect } from 'chai';
-import { DashboardOptions, StrykerOptions, ReportType } from '@stryker-mutator/api/core';
+import { DashboardOptions, StrykerOptions, ReportType, PartialStrykerOptions } from '@stryker-mutator/api/core';
 
 import LogConfigurator from '../../src/logging/LogConfigurator';
 import StrykerCli from '../../src/StrykerCli';
@@ -25,6 +25,33 @@ describe(StrykerCli.name, () => {
     expect(configureLoggerStub).calledWith('error');
   });
 
+  it('should accept a config file as last argument', () => {
+    arrangeActAssertConfigOption(['stryker.foo.js'], { configFile: 'stryker.foo.js' });
+  });
+
+  describe('flat options', () => {
+    const testCases: Array<[string[], PartialStrykerOptions]> = [
+      [['--files', 'foo.js,bar.js'], { files: ['foo.js', 'bar.js'] }],
+      [['--mutate', 'foo.js,bar.js'], { mutate: ['foo.js', 'bar.js'] }],
+      [['--transpilers', 'foo,bar'], { transpilers: ['foo', 'bar'] }],
+      [['--reporters', 'foo,bar'], { reporters: ['foo', 'bar'] }],
+      [['--plugins', 'foo,bar'], { plugins: ['foo', 'bar'] }],
+      [['--mutator', 'foo'], { mutator: 'foo' }],
+      [['--timeoutMS', '42'], { timeoutMS: 42 }],
+      [['--timeoutFactor', '42'], { timeoutFactor: 42 }],
+      [['--maxConcurrentTestRunners', '42'], { maxConcurrentTestRunners: 42 }],
+      [['--tempDirName', 'foo-tmp'], { tempDirName: 'foo-tmp' }],
+      [['--testFramework', 'foo-framework'], { testFramework: 'foo-framework' }],
+      [['--testRunner', 'foo-running'], { testRunner: 'foo-running' }],
+      [['--coverageAnalysis', 'all'], { coverageAnalysis: 'all' }],
+    ];
+    testCases.forEach(([args, expected]) => {
+      it(`should expect option "${args.join(' ')}"`, () => {
+        arrangeActAssertConfigOption(args, expected);
+      });
+    });
+  });
+
   describe('dashboard options', () => {
     beforeEach(() => {
       runMutationTestingStub.resolves();
@@ -63,7 +90,22 @@ describe(StrykerCli.name, () => {
     });
   });
 
-  function actRun(args: string[]) {
+  function actRun(args: string[]): void {
     new StrykerCli(['node', 'stryker', 'run', ...args], new Command(), runMutationTestingStub, logMock).run();
   }
+
+  function arrangeActAssertConfigOption(args: string[], expectedOptions: PartialStrykerOptions): void {
+    runMutationTestingStub.resolves();
+    actRun(args);
+    expect(runMutationTestingStub).called;
+    const actualOptions: PartialStrykerOptions = runMutationTestingStub.getCall(0).args[0];
+    for (const option in actualOptions) {
+      // Unfortunately, commander leaves all unspecified options as `undefined` on the object.
+      // This is not a problem for stryker, so let's clean them for this test.
+      if (actualOptions[option] === undefined) {
+        delete actualOptions[option];
+      }
+    }
+    expect(runMutationTestingStub).calledWith(expectedOptions);
+  }
 });