diff --git a/packages/api/schema/stryker-core.json b/packages/api/schema/stryker-core.json index caffe47094..7e545e781c 100644 --- a/packages/api/schema/stryker-core.json +++ b/packages/api/schema/stryker-core.json @@ -180,6 +180,18 @@ "required": [ "name" ] + }, + "warningOptions": { + "title": "WarningOptions", + "type": "object", + "default": {}, + "properties": { + "unknownOptions": { + "description": "decide whether or not to log warnings when additional stryker options are configured", + "type": "boolean", + "default": true + } + } } }, "properties": { @@ -328,6 +340,18 @@ "type": "string" }, "default": [] + }, + "warnings": { + "default": true, + "oneOf": [ + { + "type": "boolean" + }, + { + "$ref": "#/definitions/warningOptions" + } + ], + "description": "Enable or disable certain warnings" } } } diff --git a/packages/api/testResources/module/useCore.ts b/packages/api/testResources/module/useCore.ts index 30b15da9b6..29cec5ed31 100644 --- a/packages/api/testResources/module/useCore.ts +++ b/packages/api/testResources/module/useCore.ts @@ -38,6 +38,7 @@ const optionsAllArgs: StrykerOptions = { baseDir: 'mydir' }, tempDirName: '.stryker-tmp', + warnings: true }; const textFile: File = new File('foo/bar.js', Buffer.from('foobar')); diff --git a/packages/core/src/config/OptionsValidator.ts b/packages/core/src/config/OptionsValidator.ts index f77dbf71e6..a8e9e7d063 100644 --- a/packages/core/src/config/OptionsValidator.ts +++ b/packages/core/src/config/OptionsValidator.ts @@ -1,11 +1,12 @@ import Ajv = require('ajv'); -import { StrykerOptions, strykerCoreSchema } from '@stryker-mutator/api/core'; +import { StrykerOptions, strykerCoreSchema, WarningOptions } from '@stryker-mutator/api/core'; import { tokens, commonTokens } from '@stryker-mutator/api/plugin'; -import { noopLogger, normalizeWhitespaces } from '@stryker-mutator/util'; +import { noopLogger, normalizeWhitespaces, propertyPath } from '@stryker-mutator/util'; import { Logger } from '@stryker-mutator/api/logging'; import { coreTokens } from '../di'; import { ConfigError } from '../errors'; +import { isWarningEnabled } from '../utils/objectUtils'; import { describeErrors } from './validationErrors'; @@ -68,8 +69,33 @@ export function defaultOptions(): StrykerOptions { return options; } +validateOptions.inject = tokens(commonTokens.options, coreTokens.optionsValidator); export function validateOptions(options: unknown, optionsValidator: OptionsValidator): StrykerOptions { optionsValidator.validate(options); return options; } -validateOptions.inject = tokens(commonTokens.options, coreTokens.optionsValidator); + +markUnknownOptions.inject = tokens(commonTokens.options, coreTokens.validationSchema, commonTokens.logger); +export function markUnknownOptions(options: StrykerOptions, schema: object, log: Logger): StrykerOptions { + const OPTIONS_ADDED_BY_STRYKER = ['set', 'configFile', '$schema']; + if (isWarningEnabled('unknownOptions', options.warnings)) { + const unknownPropertyNames = Object.keys(options) + .filter((key) => !key.endsWith('_comment')) + .filter((key) => !OPTIONS_ADDED_BY_STRYKER.includes(key)) + .filter((key) => !Object.keys((schema as any).properties).includes(key)); + unknownPropertyNames.forEach((unknownPropertyName) => { + log.warn(`Unknown stryker config option "${unknownPropertyName}".`); + }); + const p = `${propertyPath('warnings')}.${propertyPath('unknownOptions')}`; + if (unknownPropertyNames.length) { + log.warn(` + Possible causes: + * Is it a typo on your end? + * Did you only write this property as a comment? If so, please postfix it with "_comment". + * You might be missing a plugin that is supposed to use it. Stryker loaded plugins from: ${JSON.stringify(options.plugins)} + * The plugin that is using it did not contribute explicit validation. + (disable "${p}" to ignore this warning)`); + } + } + return options; +} diff --git a/packages/core/src/di/buildMainInjector.ts b/packages/core/src/di/buildMainInjector.ts index 0b547742da..bfbd04e855 100644 --- a/packages/core/src/di/buildMainInjector.ts +++ b/packages/core/src/di/buildMainInjector.ts @@ -5,7 +5,14 @@ import { TestFramework } from '@stryker-mutator/api/test_framework'; import { getLogger } from 'log4js'; import { rootInjector } from 'typed-inject'; -import { OptionsEditorApplier, readConfig, buildSchemaWithPluginContributions, OptionsValidator, validateOptions } from '../config'; +import { + OptionsEditorApplier, + readConfig, + buildSchemaWithPluginContributions, + OptionsValidator, + validateOptions, + markUnknownOptions, +} from '../config'; import ConfigReader from '../config/ConfigReader'; import BroadcastReporter from '../reporters/BroadcastReporter'; import { TemporaryDirectory } from '../utils/TemporaryDirectory'; @@ -26,10 +33,29 @@ export interface MainContext extends OptionsContext { [coreTokens.temporaryDirectory]: TemporaryDirectory; } +type BasicInjector = Injector>; +type PluginResolverInjector = Injector>; + export function buildMainInjector(cliOptions: Partial): Injector { - return rootInjector - .provideValue(commonTokens.getLogger, getLogger) - .provideFactory(commonTokens.logger, loggerFactory, Scope.Transient) + const basicInjector = createBasicInjector(); + const pluginResolverInjector = createPluginResolverInjector(cliOptions, basicInjector); + return pluginResolverInjector + .provideFactory(commonTokens.mutatorDescriptor, mutatorDescriptorFactory) + .provideFactory(coreTokens.pluginCreatorReporter, PluginCreator.createFactory(PluginKind.Reporter)) + .provideFactory(coreTokens.pluginCreatorTestFramework, PluginCreator.createFactory(PluginKind.TestFramework)) + .provideFactory(coreTokens.pluginCreatorMutator, PluginCreator.createFactory(PluginKind.Mutator)) + .provideClass(coreTokens.reporter, BroadcastReporter) + .provideFactory(coreTokens.testFramework, testFrameworkFactory) + .provideClass(coreTokens.temporaryDirectory, TemporaryDirectory) + .provideClass(coreTokens.timer, Timer); +} + +function createBasicInjector(): BasicInjector { + return rootInjector.provideValue(commonTokens.getLogger, getLogger).provideFactory(commonTokens.logger, loggerFactory, Scope.Transient); +} + +export function createPluginResolverInjector(cliOptions: Partial, parent: BasicInjector): PluginResolverInjector { + return parent .provideValue(coreTokens.cliOptions, cliOptions) .provideValue(coreTokens.validationSchema, strykerCoreSchema) .provideClass(coreTokens.optionsValidator, OptionsValidator) @@ -40,18 +66,11 @@ export function buildMainInjector(cliOptions: Partial): Injector .provideFactory(coreTokens.validationSchema, buildSchemaWithPluginContributions) .provideClass(coreTokens.optionsValidator, OptionsValidator) .provideFactory(commonTokens.options, validateOptions) + .provideFactory(commonTokens.options, markUnknownOptions) .provideFactory(coreTokens.pluginCreatorConfigEditor, PluginCreator.createFactory(PluginKind.ConfigEditor)) .provideFactory(coreTokens.pluginCreatorOptionsEditor, PluginCreator.createFactory(PluginKind.OptionsEditor)) .provideClass(coreTokens.configOptionsApplier, OptionsEditorApplier) - .provideFactory(commonTokens.options, applyOptionsEditors) - .provideFactory(commonTokens.mutatorDescriptor, mutatorDescriptorFactory) - .provideFactory(coreTokens.pluginCreatorReporter, PluginCreator.createFactory(PluginKind.Reporter)) - .provideFactory(coreTokens.pluginCreatorTestFramework, PluginCreator.createFactory(PluginKind.TestFramework)) - .provideFactory(coreTokens.pluginCreatorMutator, PluginCreator.createFactory(PluginKind.Mutator)) - .provideClass(coreTokens.reporter, BroadcastReporter) - .provideFactory(coreTokens.testFramework, testFrameworkFactory) - .provideClass(coreTokens.temporaryDirectory, TemporaryDirectory) - .provideClass(coreTokens.timer, Timer); + .provideFactory(commonTokens.options, applyOptionsEditors); } function pluginDescriptorsFactory(options: StrykerOptions): readonly string[] { diff --git a/packages/core/src/utils/objectUtils.ts b/packages/core/src/utils/objectUtils.ts index 05415a9426..3d98cc8caf 100644 --- a/packages/core/src/utils/objectUtils.ts +++ b/packages/core/src/utils/objectUtils.ts @@ -1,5 +1,6 @@ import treeKill = require('tree-kill'); -import { StrykerError } from '@stryker-mutator/util'; +import { StrykerError, KnownKeys } from '@stryker-mutator/util'; +import { WarningOptions } from '@stryker-mutator/api/core'; export { serialize, deserialize } from 'surrial'; @@ -26,6 +27,14 @@ export function getEnvironmentVariableOrThrow(name: string): string { } } +export function isWarningEnabled(warningType: KnownKeys, warningOptions: WarningOptions | boolean): boolean { + if (typeof warningOptions === 'boolean') { + return warningOptions; + } else { + return !!warningOptions[warningType]; + } +} + /** * A wrapper around `process.exitCode = n` (for testability) */ diff --git a/packages/core/stryker.conf.js b/packages/core/stryker.conf.js index 5bc45794b3..fa5a53f3c4 100644 --- a/packages/core/stryker.conf.js +++ b/packages/core/stryker.conf.js @@ -2,4 +2,4 @@ const path = require('path'); const settings = require('../../stryker.parent.conf'); const moduleName = __dirname.split(path.sep).pop(); settings.dashboard.module = moduleName; -module.exports = settings; \ No newline at end of file +module.exports = settings; diff --git a/packages/core/test/helpers/testUtils.ts b/packages/core/test/helpers/testUtils.ts index 2d80d77eab..e51ecf0d5d 100644 --- a/packages/core/test/helpers/testUtils.ts +++ b/packages/core/test/helpers/testUtils.ts @@ -1,5 +1,11 @@ +import { resolve } from 'path'; + export function sleep(ms = 0) { return new Promise((res) => { setTimeout(res, ms); }); } + +export function resolveFromRoot(...pathSegments: string[]) { + return resolve(__dirname, '..', '..', ...pathSegments); +} diff --git a/packages/core/test/integration/options-validation/options-validation.it.spec.ts b/packages/core/test/integration/options-validation/options-validation.it.spec.ts new file mode 100644 index 0000000000..f480540c2c --- /dev/null +++ b/packages/core/test/integration/options-validation/options-validation.it.spec.ts @@ -0,0 +1,21 @@ +import { expect } from 'chai'; +import { testInjector } from '@stryker-mutator/test-helpers'; +import { commonTokens } from '@stryker-mutator/api/plugin'; + +import { createPluginResolverInjector } from '../../../src/di'; +import { resolveFromRoot } from '../../helpers/testUtils'; + +import sinon = require('sinon'); + +describe('Options validation integration', () => { + it('should log about unknown properties in log file', () => { + const optionsProvider = createPluginResolverInjector( + { + configFile: resolveFromRoot('testResources', 'options-validation', 'unknown-options.conf.json'), + }, + testInjector.injector + ); + optionsProvider.resolve(commonTokens.options); + expect(testInjector.logger.warn).calledWithMatch(sinon.match('Unknown stryker config option "this is an unknown property"')); + }); +}); diff --git a/packages/core/test/integration/repoters/html/HtmlReporter.it.spec.ts b/packages/core/test/integration/reporters/html/HtmlReporter.it.spec.ts similarity index 100% rename from packages/core/test/integration/repoters/html/HtmlReporter.it.spec.ts rename to packages/core/test/integration/reporters/html/HtmlReporter.it.spec.ts diff --git a/packages/core/test/integration/repoters/html/simpleReport.ts b/packages/core/test/integration/reporters/html/simpleReport.ts similarity index 100% rename from packages/core/test/integration/repoters/html/simpleReport.ts rename to packages/core/test/integration/reporters/html/simpleReport.ts diff --git a/packages/core/test/unit/config/OptionsValidator.spec.ts b/packages/core/test/unit/config/OptionsValidator.spec.ts index cff15bbf79..9fcdb72c94 100644 --- a/packages/core/test/unit/config/OptionsValidator.spec.ts +++ b/packages/core/test/unit/config/OptionsValidator.spec.ts @@ -1,9 +1,10 @@ +import sinon = require('sinon'); import { strykerCoreSchema, StrykerOptions } from '@stryker-mutator/api/core'; -import { testInjector } from '@stryker-mutator/test-helpers'; +import { testInjector, factory } from '@stryker-mutator/test-helpers'; import { expect } from 'chai'; import { normalizeWhitespaces } from '@stryker-mutator/util'; -import { OptionsValidator } from '../../../src/config/OptionsValidator'; +import { OptionsValidator, validateOptions, markUnknownOptions } from '../../../src/config/OptionsValidator'; import { coreTokens } from '../../../src/di'; describe(OptionsValidator.name, () => { @@ -221,3 +222,62 @@ describe(OptionsValidator.name, () => { } } }); + +describe(validateOptions.name, () => { + let optionsValidatorMock: sinon.SinonStubbedInstance; + + beforeEach(() => { + optionsValidatorMock = sinon.createStubInstance(OptionsValidator); + }); + + it('should validate the options using given optionsValidator', () => { + const options = { foo: 'bar' }; + const output = validateOptions(options, (optionsValidatorMock as unknown) as OptionsValidator); + expect(options).eq(output); + expect(optionsValidatorMock.validate).calledWith(options); + }); +}); + +describe(markUnknownOptions.name, () => { + it('should not warn when there are no unknown properties', () => { + testInjector.options.htmlReporter = { + baseDir: 'test', + }; + expect(testInjector.logger.warn).not.called; + }); + + it('should return the options, no matter what', () => { + testInjector.options['this key does not exist'] = 'foo'; + const output = markUnknownOptions(testInjector.options, strykerCoreSchema, testInjector.logger); + expect(output).eq(testInjector.options); + }); + + it('should not warn when unknown properties are postfixed with "_comment"', () => { + testInjector.options['maxConcurrentTestRunners_comment'] = 'Recommended to use half of your cores'; + markUnknownOptions(testInjector.options, strykerCoreSchema, testInjector.logger); + expect(testInjector.logger.warn).not.called; + }); + + it('should warn about unknown properties', () => { + testInjector.options['karma'] = {}; + testInjector.options['jest'] = {}; + markUnknownOptions(testInjector.options, strykerCoreSchema, testInjector.logger); + expect(testInjector.logger.warn).calledThrice; + expect(testInjector.logger.warn).calledWith('Unknown stryker config option "karma".'); + expect(testInjector.logger.warn).calledWith('Unknown stryker config option "jest".'); + expect(testInjector.logger.warn).calledWithMatch('Possible causes'); + }); + it('should not warn about unknown properties when warnings are disabled', () => { + testInjector.options['karma'] = {}; + testInjector.options.warnings = factory.warningOptions({ unknownOptions: false }); + markUnknownOptions(testInjector.options, strykerCoreSchema, testInjector.logger); + expect(testInjector.logger.warn).not.called; + }); + it('should ignore options added by Stryker itself', () => { + testInjector.options['set'] = {}; + testInjector.options['configFile'] = {}; + testInjector.options['$schema'] = ''; + markUnknownOptions(testInjector.options, strykerCoreSchema, testInjector.logger); + expect(testInjector.logger.warn).not.called; + }); +}); diff --git a/packages/core/test/unit/di/buildMainInjector.spec.ts b/packages/core/test/unit/di/buildMainInjector.spec.ts index 9bb18a7aaf..b6f54c3482 100644 --- a/packages/core/test/unit/di/buildMainInjector.spec.ts +++ b/packages/core/test/unit/di/buildMainInjector.spec.ts @@ -21,11 +21,11 @@ describe(buildMainInjector.name, () => { let testFrameworkMock: TestFramework; let configReaderMock: sinon.SinonStubbedInstance; let pluginCreatorMock: sinon.SinonStubbedInstance>; - let buildSchemaWithPluginContributionsStub: sinon.SinonStub; let optionsEditorApplierMock: sinon.SinonStubbedInstance; let broadcastReporterMock: sinon.SinonStubbedInstance; let optionsValidatorStub: sinon.SinonStubbedInstance; let expectedConfig: StrykerOptions; + let validationSchemaContributions: object[]; beforeEach(() => { configReaderMock = sinon.createStubInstance(ConfigReader); @@ -36,14 +36,14 @@ describe(buildMainInjector.name, () => { testFrameworkOrchestratorMock = sinon.createStubInstance(TestFrameworkOrchestrator); testFrameworkOrchestratorMock.determineTestFramework.returns(testFrameworkMock); pluginLoaderMock = sinon.createStubInstance(di.PluginLoader); + validationSchemaContributions = []; + pluginLoaderMock.resolveValidationSchemaContributions.returns(validationSchemaContributions); optionsValidatorStub = sinon.createStubInstance(configModule.OptionsValidator); - buildSchemaWithPluginContributionsStub = sinon.stub(); expectedConfig = factory.strykerOptions(); broadcastReporterMock = factory.reporter('broadcast'); configReaderMock.readConfig.returns(expectedConfig); stubInjectable(PluginCreator, 'createFactory').returns(() => pluginCreatorMock); stubInjectable(configModule, 'OptionsEditorApplier').returns(optionsEditorApplierMock); - stubInjectable(configModule, 'buildSchemaWithPluginContributions').returns(buildSchemaWithPluginContributionsStub); stubInjectable(configModule, 'OptionsValidator').returns(optionsValidatorStub); stubInjectable(di, 'PluginLoader').returns(pluginLoaderMock); stubInjectable(configReaderModule, 'default').returns(configReaderMock); @@ -97,6 +97,12 @@ describe(buildMainInjector.name, () => { buildMainInjector({}).resolve(commonTokens.options); expect(optionsValidatorStub.validate).calledWith(expectedConfig); }); + + it('should warn about unknown properties', () => { + expectedConfig.foo = 'bar'; + buildMainInjector({}).resolve(commonTokens.options); + expect(currentLogMock().warn).calledWithMatch('Unknown stryker config option "foo"'); + }); }); it('should supply mutatorDescriptor', () => { diff --git a/packages/core/testResources/options-validation/unknown-options.conf.json b/packages/core/testResources/options-validation/unknown-options.conf.json new file mode 100644 index 0000000000..e987ee84b4 --- /dev/null +++ b/packages/core/testResources/options-validation/unknown-options.conf.json @@ -0,0 +1,5 @@ +{ + "$schema": "https://raw.githubusercontent.com/stryker-mutator/stryker/master/packages/api/schema/stryker-core.json", + "plugins": [], + "this is an unknown property": "foo" +} diff --git a/packages/test-helpers/src/factory.ts b/packages/test-helpers/src/factory.ts index 95a564c0b4..b96561c52d 100644 --- a/packages/test-helpers/src/factory.ts +++ b/packages/test-helpers/src/factory.ts @@ -1,6 +1,14 @@ import Ajv = require('ajv'); import { ConfigEditor } from '@stryker-mutator/api/config'; -import { File, Location, MutationScoreThresholds, StrykerOptions, MutatorDescriptor, strykerCoreSchema } from '@stryker-mutator/api/core'; +import { + File, + Location, + MutationScoreThresholds, + StrykerOptions, + MutatorDescriptor, + strykerCoreSchema, + WarningOptions, +} from '@stryker-mutator/api/core'; import { Logger } from '@stryker-mutator/api/logging'; import { Mutant } from '@stryker-mutator/api/mutant'; import { MatchedMutant, MutantResult, MutantStatus, mutationTestReportSchema, Reporter } from '@stryker-mutator/api/report'; @@ -49,6 +57,10 @@ export function pluginResolver(): sinon.SinonStubbedInstance { }; } +export const warningOptions = factoryMethod(() => ({ + unknownOptions: true, +})); + export const mutantResult = factoryMethod(() => ({ id: '256', location: location(), diff --git a/packages/util/src/KnownKeys.ts b/packages/util/src/KnownKeys.ts new file mode 100644 index 0000000000..eb567a43e8 --- /dev/null +++ b/packages/util/src/KnownKeys.ts @@ -0,0 +1,9 @@ +/** + * Known keys filters out the index signature from the keys of a type + * @see https://stackoverflow.com/questions/51465182/typescript-remove-index-signature-using-mapped-types + */ +export type KnownKeys = { + [K in keyof T]: string extends K ? never : number extends K ? never : K; +} extends { [_ in keyof T]: infer U } + ? U + : never; diff --git a/packages/util/src/index.ts b/packages/util/src/index.ts index 134d36c1f1..e8939d0a89 100644 --- a/packages/util/src/index.ts +++ b/packages/util/src/index.ts @@ -2,6 +2,7 @@ export { default as childProcessAsPromised } from './childProcessAsPromised'; export { default as StrykerError } from './StrykerError'; export * from './errors'; export * from './Immutable'; +export * from './KnownKeys'; export * from './stringUtils'; export * from './noopLogger'; export * from './notEmpty'; diff --git a/packages/util/src/stringUtils.ts b/packages/util/src/stringUtils.ts index 02ae727d00..9f95ad978c 100644 --- a/packages/util/src/stringUtils.ts +++ b/packages/util/src/stringUtils.ts @@ -1,4 +1,5 @@ import { notEmpty } from './notEmpty'; +import { KnownKeys } from './KnownKeys'; /** * Consolidates multiple consecutive white spaces into a single space. @@ -8,6 +9,11 @@ export function normalizeWhitespaces(str: string) { return str.replace(/\s+/g, ' ').trim(); } -export function propertyPath(prop: keyof T, prop2?: keyof T[typeof prop]): string { +/** + * Given a base type, allows type safe access to the name (or deep path) of a property. + * @param prop The first property key + * @param prop2 The optional second property key. Add prop3, prop4, etc to this method signature when needed. + */ +export function propertyPath(prop: keyof Pick>, prop2?: keyof Pick>[typeof prop]): string { return [prop, prop2].filter(notEmpty).join('.'); } diff --git a/tasks/generate-json-schema-to-ts.js b/tasks/generate-json-schema-to-ts.js index 379b341ea0..3470af85f5 100644 --- a/tasks/generate-json-schema-to-ts.js +++ b/tasks/generate-json-schema-to-ts.js @@ -85,6 +85,12 @@ function preprocessSchema(inputSchema) { $ref: inputSchema.$ref } } + if(inputSchema.oneOf) { + return { + ...inputSchema, + oneOf: inputSchema.oneOf.map(preprocessSchema) + } + } return inputSchema; } } catch (err) {