Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(stryker): #438 Extensive config validation #549

Merged
merged 5 commits into from
Dec 19, 2017
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 0 additions & 15 deletions packages/stryker/src/ConfigReader.ts
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);
}
}
}
54 changes: 54 additions & 0 deletions packages/stryker/src/ConfigValidator.ts
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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love this, very much readable!

this.validateCoverageAnalysis();
this.downgradeCoverageAnalysisIfNeeded();
this.crashIfNeeded();
}
Expand Down Expand Up @@ -48,6 +57,27 @@ export default class ConfigValidator {
}
}

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('\`logLevel\` is invalid, expected one of \`fatal\`, \`error\`, \`warn\`, \`info\`, \`debug\`, \`trace\`, \`all\` and \`off\`');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could use VALID_LOG_LEVEL_VALUES .join(',') here to reduce duplication.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually done in validateCoverageAnalysis. I'm a fan on using the join here as well.

}
}

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 folowing: ${VALID_COVERAGE_ANALYSIS_VALUES.map(v => `"${v}"`).join(', ')}`);
}
}

private downgradeCoverageAnalysisIfNeeded() {
if (this.strykerConfig.transpilers.length && this.strykerConfig.coverageAnalysis !== 'off') {
this.log.info('Disabled coverage analysis for this run (off). Coverage analysis using transpilers is not supported yet.');
Expand All @@ -61,6 +91,30 @@ export default class ConfigValidator {
}
}

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

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

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

private invalidate(message: string) {
this.log.fatal(message);
this.isValid = false;
Expand Down
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
103 changes: 103 additions & 0 deletions packages/stryker/test/unit/ConfigValidatorSpec.ts
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 @@ -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('`logLevel` is invalid, expected one of `fatal`, `error`, `warn`, `info`, `debug`, `trace`, `all` and `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('timeoutMs is invalid, 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('timeoutFactor is invalid, 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('mutator is invalid, 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('plugins is invalid, 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('plugins is invalid, expected an array of strings');
});
});

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('reporter is invalid, 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('reporter is invalid, expected an array of strings');
});
});

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('transpilers is invalid, 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('transpilers is invalid, expected an array of strings');
});
});

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 folowing: "perTest", "all", "off"');
});
});