Skip to content

Commit

Permalink
feat(config): #438 Extensive config validation (#549)
Browse files Browse the repository at this point in the history 
Validate known config properties during startup.
  • Loading branch information
@hrb90 @nicojs
hrb90 authored and nicojs committed Dec 19, 2017
1 parent 20cfdc1 commit dc6fdf2
Show file tree
Hide file tree
Showing 4 changed files with 166 additions and 35 deletions.
15 changes: 0 additions & 15 deletions packages/stryker/src/ConfigReader.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ import * as log4js from 'log4js';
import * as path from 'path';
import * as _ from 'lodash';

const VALID_COVERAGE_ANALYSIS_VALUES = ['perTest', 'all', 'off'];

export const CONFIG_SYNTAX_HELP = ' module.exports = function(config) {\n' +
' config.set({\n' +
' // your config\n' +
Expand All @@ -33,7 +31,6 @@ export default class ConfigReader {

// merge the config from config file and cliOptions (precedence)
config.set(this.cliOptions);
this.validate(config);
return config;
}

Expand Down Expand Up @@ -76,16 +73,4 @@ export default class ConfigReader {
return configModule;
}

private validate(options: Config) {

if (VALID_COVERAGE_ANALYSIS_VALUES.indexOf(options.coverageAnalysis) < 0) {
this.log.fatal(`Value "${options.coverageAnalysis}" is invalid for \`coverageAnalysis\`. Expected one of the folowing: ${VALID_COVERAGE_ANALYSIS_VALUES.map(v => `"${v}"`).join(', ')}`);
process.exit(1);
}
if (options.coverageAnalysis === 'perTest' && !options.testFramework) {
const validCoverageAnalysisSettingsExceptPerTest = VALID_COVERAGE_ANALYSIS_VALUES.filter(v => v !== 'perTest').map(v => `"${v}"`).join(', ');
this.log.fatal(`Configured coverage analysis 'perTest' requires a test framework to be configured. Either configure your test framework (for example testFramework: 'jasmine') or set coverageAnalysis setting to one of the following: ${validCoverageAnalysisSettingsExceptPerTest}`);
process.exit(1);
}
}
}
62 changes: 60 additions & 2 deletions packages/stryker/src/ConfigValidator.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,15 @@ export default class ConfigValidator {
validate() {
this.validateTestFramework();
this.validateThresholds();
this.validateLogLevel();
this.validateTimeout();
this.validateIsNumber('port', this.strykerConfig.port);
this.validateIsNumber('maxConcurrentTestRunners', this.strykerConfig.maxConcurrentTestRunners);
this.validateIsString('mutator', this.strykerConfig.mutator);
this.validateIsStringArray('plugins', this.strykerConfig.plugins);
this.validateIsStringArray('reporter', this.strykerConfig.reporter);
this.validateIsStringArray('transpilers', this.strykerConfig.transpilers);
this.validateCoverageAnalysis();
this.downgradeCoverageAnalysisIfNeeded();
this.crashIfNeeded();
}
Expand All @@ -38,13 +47,34 @@ export default class ConfigValidator {

private validateThresholdValue(name: keyof MutationScoreThresholds, value: number | null) {
if (typeof value === 'number' && (value < 0 || value > 100)) {
this.invalidate(`thresholds.${name} should be between 0 and 100 (was ${value})`);
this.invalidate(`Value "${value}" is invalid for \`thresholds.${name}\`. Expected a number between 0 and 100`);
}
}

private validateThresholdsValueExists(name: keyof MutationScoreThresholds, value: number | undefined) {
if (typeof value !== 'number') {
this.invalidate(`thresholds.${name} is invalid, expected a number between 0 and 100 (was ${value}).`);
this.invalidate(`Value "${value}" is invalid for \`thresholds.${name}\`. Expected a number between 0 and 100`);
}
}

private validateLogLevel() {
const logLevel = this.strykerConfig.logLevel;
const VALID_LOG_LEVEL_VALUES = ['fatal', 'error', 'warn', 'info', 'debug', 'trace', 'all', 'off'];
if (VALID_LOG_LEVEL_VALUES.indexOf(logLevel) < 0) {
this.invalidate(`Value "${logLevel}" is invalid for \`logLevel\`. Expected one of the following: ${this.joinQuotedList(VALID_LOG_LEVEL_VALUES)}`);
}
}

private validateTimeout() {
this.validateIsNumber('timeoutMs', this.strykerConfig.timeoutMs);
this.validateIsNumber('timeoutFactor', this.strykerConfig.timeoutFactor);
}

private validateCoverageAnalysis() {
const VALID_COVERAGE_ANALYSIS_VALUES = ['perTest', 'all', 'off'];
const coverageAnalysis = this.strykerConfig.coverageAnalysis;
if (VALID_COVERAGE_ANALYSIS_VALUES.indexOf(coverageAnalysis) < 0) {
this.invalidate(`Value "${coverageAnalysis}" is invalid for \`coverageAnalysis\`. Expected one of the following: ${this.joinQuotedList(VALID_COVERAGE_ANALYSIS_VALUES)}`);
}
}

Expand All @@ -61,8 +91,36 @@ export default class ConfigValidator {
}
}

private validateIsNumber(fieldName: keyof Config, value: any) {
if (typeof value !== 'number') {
this.invalidate(`Value "${value}" is invalid for \`${fieldName}\`. Expected a number`);
}
}

private validateIsString(fieldName: keyof Config, value: any) {
if (typeof value !== 'string') {
this.invalidate(`Value "${value}" is invalid for \`${fieldName}\`. Expected a string`);
}
}

private validateIsStringArray(fieldName: keyof Config, value: any) {
if (!Array.isArray(value)) {
this.invalidate(`Value "${value}" is invalid for \`${fieldName}\`. Expected an array`);
} else {
value.forEach(v => {
if (typeof v !== 'string') {
this.invalidate(`Value "${v}" is an invalid element of \`${fieldName}\`. Expected a string`);
}
});
}
}

private invalidate(message: string) {
this.log.fatal(message);
this.isValid = false;
}

private joinQuotedList(arr: string[]) {
return arr.map(v => `"${v}"`).join(', ');
}
}
15 changes: 0 additions & 15 deletions packages/stryker/test/integration/config-reader/ConfigReaderSpec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,21 +63,6 @@ describe('ConfigReader', () => {
});
});

describe('with invalid coverageAnalysis', () => {
beforeEach(() => {
sut = new ConfigReader({ coverageAnalysis: <any>'invalid' });
result = sut.readConfig();
});

it('should report a fatal error', () => {
expect(log.fatal).to.have.been.calledWith('Value "invalid" is invalid for `coverageAnalysis`. Expected one of the folowing: "perTest", "all", "off"');
});

it('should exit with 1', () => {
expect(process.exit).to.have.been.calledWith(1);
});
});

describe('with config file', () => {
it('should read config file', () => {
sut = new ConfigReader({ configFile: 'testResources/config-reader/valid.conf.js' });
Expand Down
109 changes: 106 additions & 3 deletions packages/stryker/test/unit/ConfigValidatorSpec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ describe('ConfigValidator', () => {
let sut: ConfigValidator;
let log: Mock<Logger>;

function breakConfig(oldConfig: Config, key: keyof Config, value: any): any {
return Object.assign({}, oldConfig, { [key]: value });
}

beforeEach(() => {
log = currentLogMock();
config = new Config();
Expand Down Expand Up @@ -49,8 +53,8 @@ describe('ConfigValidator', () => {
sut.validate();
expect(exitStub).calledWith(1);
expect(log.fatal).calledWith('`thresholds.high` is lower than `thresholds.low` (-1 < 101)');
expect(log.fatal).calledWith('thresholds.high should be between 0 and 100 (was -1)');
expect(log.fatal).calledWith('thresholds.low should be between 0 and 100 (was 101)');
expect(log.fatal).calledWith('Value "-1" is invalid for `thresholds.high`. Expected a number between 0 and 100');
expect(log.fatal).calledWith('Value "101" is invalid for `thresholds.low`. Expected a number between 0 and 100');
});

it('should be invalid with thresholds.high null', () => {
Expand All @@ -59,7 +63,7 @@ describe('ConfigValidator', () => {
sut = new ConfigValidator(config, testFramework());
sut.validate();
expect(exitStub).calledWith(1);
expect(log.fatal).calledWith('thresholds.high is invalid, expected a number between 0 and 100 (was null).');
expect(log.fatal).calledWith('Value "null" is invalid for `thresholds.high`. Expected a number between 0 and 100');
});
});

Expand All @@ -72,4 +76,103 @@ describe('ConfigValidator', () => {
expect(config.coverageAnalysis).eq('off');
});

it('should be invalid with invalid logLevel', () => {
config.logLevel = 'thisTestPasses';
sut = new ConfigValidator(config, testFramework());
sut.validate();
expect(exitStub).calledWith(1);
expect(log.fatal).calledWith('Value "thisTestPasses" is invalid for `logLevel`. Expected one of the following: "fatal", "error", "warn", "info", "debug", "trace", "all", "off"');
});

it('should be invalid with nonnumeric timeoutMs', () => {
let brokenConfig = breakConfig(config, 'timeoutMs', 'break');
sut = new ConfigValidator(brokenConfig, testFramework());
sut.validate();
expect(exitStub).calledWith(1);
expect(log.fatal).calledWith('Value "break" is invalid for `timeoutMs`. Expected a number');
});

it('should be invalid with nonnumeric timeoutFactor', () => {
let brokenConfig = breakConfig(config, 'timeoutFactor', 'break');
sut = new ConfigValidator(brokenConfig, testFramework());
sut.validate();
expect(exitStub).calledWith(1);
expect(log.fatal).calledWith('Value "break" is invalid for `timeoutFactor`. Expected a number');
});

it('should be invalid with non-string mutator', () => {
let brokenConfig = breakConfig(config, 'mutator', 0);
sut = new ConfigValidator(brokenConfig, testFramework());
sut.validate();
expect(exitStub).calledWith(1);
expect(log.fatal).calledWith('Value "0" is invalid for `mutator`. Expected a string');
});

describe('plugins', () => {
it('should be invalid with non-array plugins', () => {
let brokenConfig = breakConfig(config, 'plugins', 'stryker-typescript');
sut = new ConfigValidator(brokenConfig, testFramework());
sut.validate();
expect(exitStub).calledWith(1);
expect(log.fatal).calledWith('Value "stryker-typescript" is invalid for `plugins`. Expected an array');
});

it('should be invalid with non-string array elements', () => {
let brokenConfig = breakConfig(config, 'plugins', ['stryker-jest', 0]);
sut = new ConfigValidator(brokenConfig, testFramework());
sut.validate();
expect(exitStub).calledWith(1);
expect(log.fatal).calledWith('Value "0" is an invalid element of `plugins`. Expected a string');
});
});

describe('reporter', () => {
it('should be invalid with non-array reporter', () => {
let brokenConfig = breakConfig(config, 'reporter', 'stryker-typescript');
sut = new ConfigValidator(brokenConfig, testFramework());
sut.validate();
expect(exitStub).calledWith(1);
expect(log.fatal).calledWith('Value "stryker-typescript" is invalid for `reporter`. Expected an array');
});

it('should be invalid with non-string array elements', () => {
let brokenConfig = breakConfig(config, 'reporter', [
'stryker-jest',
0
]);
sut = new ConfigValidator(brokenConfig, testFramework());
sut.validate();
expect(exitStub).calledWith(1);
expect(log.fatal).calledWith('Value "0" is an invalid element of `reporter`. Expected a string');
});
});

describe('transpilers', () => {
it('should be invalid with non-array transpilers', () => {
let brokenConfig = breakConfig(config, 'transpilers', 'stryker-typescript');
sut = new ConfigValidator(brokenConfig, testFramework());
sut.validate();
expect(exitStub).calledWith(1);
expect(log.fatal).calledWith('Value "stryker-typescript" is invalid for `transpilers`. Expected an array');
});

it('should be invalid with non-string array elements', () => {
let brokenConfig = breakConfig(config, 'transpilers', [
'stryker-jest',
0
]);
sut = new ConfigValidator(brokenConfig, testFramework());
sut.validate();
expect(exitStub).calledWith(1);
expect(log.fatal).calledWith('Value "0" is an invalid element of `transpilers`. Expected a string');
});
});

it('should be invalid with invalid coverageAnalysis', () => {
let brokenConfig = breakConfig(config, 'coverageAnalysis', 'invalid');
sut = new ConfigValidator(brokenConfig, testFramework());
sut.validate();
expect(exitStub).calledWith(1);
expect(log.fatal).calledWith('Value "invalid" is invalid for `coverageAnalysis`. Expected one of the following: "perTest", "all", "off"');
});
});

0 comments on commit dc6fdf2

Please sign in to comment.