Skip to content

Commit

Permalink
fix(mocha-runner): don't allow custom timeout (#2463)
Browse files Browse the repository at this point in the history
Don't allow a custom timeout option. Stryker will decide the timeout to use for a test run.
  • Loading branch information
nicojs committed Sep 9, 2020
1 parent 582e01b commit e90b563
Show file tree
Hide file tree
Showing 13 changed files with 23 additions and 54 deletions.
2 changes: 1 addition & 1 deletion packages/instrumenter/package.json
Expand Up @@ -42,7 +42,7 @@
"devDependencies": {
"@babel/preset-react": "~7.10.4",
"@stryker-mutator/test-helpers": "4.0.0-beta.4",
"@types/babel__core": "~7.1.7",
"@types/babel__core": "~7.1.9",
"@types/babel__generator": "~7.6.1",
"@types/babel__parser": "~7.1.1",
"@types/chai-jest-snapshot": "~1.3.5",
Expand Down
Expand Up @@ -12,7 +12,7 @@ function nameAnonymousClassOrFunctionExpression(path: NodePath<types.Expression>
} else if (
path.parentPath.isObjectProperty() &&
types.isIdentifier(path.parentPath.node.key) &&
path.getStatementParent().isVariableDeclaration()
path.getStatementParent()?.isVariableDeclaration()
) {
path.node.id = path.parentPath.node.key;
}
Expand Down
5 changes: 0 additions & 5 deletions packages/instrumenter/test/unit/parsers/js-parser.spec.ts
Expand Up @@ -23,10 +23,8 @@ describe('js-parser', () => {
describe('with features', () => {
const itShouldSupportAst = createActArrangeAndAssertHelper((name) => `https://babeljs.io/docs/en/babel-plugin-syntax-${name}`);
itShouldSupportAst('async-generators', 'async function* agf() { await 1;}', (t) => t.isFunctionDeclaration() && t.node.generator);
// @ts-expect-error isImport is not (yet) defined in the types
itShouldSupportAst('dynamic-import', 'import("fs").then(console.log)', (t) => t.isImport());
itShouldSupportAst('import-meta', 'console.log(import.meta);', (t) => t.isMetaProperty());
// @ts-expect-error is not (yet) defined in the types
itShouldSupportAst('big-int', 'const theBiggestInt = 9007199254740991n', (t) => t.isBigIntLiteral());
itShouldSupportAst('logical-assignment-operators', 'a &&= b;', (t) => t.isAssignmentExpression() && t.node.operator === '&&=');
});
Expand All @@ -38,9 +36,7 @@ describe('js-parser', () => {
itShouldSupportAst('do-expressions', 'let a = do { if(x > 10) { "big"; } else { "small"; } }', (t) => t.isDoExpression());
itShouldSupportAst('object-rest-spread', 'let { x, y, ...z } = { x: 1, y: 2, a: 3, b: 4 };', (t) => t.isRestElement());
itShouldSupportAst('class-properties', 'class Foo { bar = "baz" }', (t) => t.isClassProperty());
// @ts-expect-error not (yet) defined in the types
itShouldSupportAst('class-properties (private properties)', 'class Foo { #bar = "baz" }', (t) => t.isClassPrivateProperty());
// @ts-expect-error not (yet) defined in the types
itShouldSupportAst('class-properties (private methods)', 'class Foo { #bar(){ return "baz"; } }', (t) => t.isClassPrivateMethod());

itShouldSupportAst('export-default-from', 'export v from "mod";', (t) => t.isExportNamedDeclaration());
Expand All @@ -51,7 +47,6 @@ describe('js-parser', () => {
);
itShouldSupportAst('numeric-separator', 'let budget = 1_000_000_000_000;', (t) => t.isNumericLiteral());
itShouldSupportAst('optional-catch-binding', 'try{ throw 0; } catch { }', (t) => t.isCatchClause() && t.node.param === null);
// @ts-expect-error isOptionalMemberExpression is not (yet) defined in the types
itShouldSupportAst('optional-chaining', 'const baz = obj?.foo?.bar?.baz;', (t) => t.isOptionalMemberExpression());
itShouldSupportAst('pipeline-operator', 'let result = "hello" |> doubleSay |> capitalize |> exclaim;', (t) => t.isBinaryExpression());
itShouldSupportAst(
Expand Down
1 change: 0 additions & 1 deletion packages/instrumenter/test/unit/parsers/ts-parser.spec.ts
Expand Up @@ -15,7 +15,6 @@ describe('ts-parser', () => {
expect(format).eq(expected.format);
expect(rawContent).eq(expected.rawContent);
expect(originFileName).eq(expected.originFileName);
// @ts-expect-error is not added to type definitions yet
expectAst(root, (p) => p.isTSTypeAnnotation());
});

Expand Down
Expand Up @@ -81,7 +81,6 @@ describe('babel-transformer', () => {
it('should skip type annotations', () => {
const ast = createTSAst({ rawContent: 'const foo: string;' });
transformBabel(ast, mutantCollectorMock, context);
// @ts-expect-error
expectMutateNotCalledWith((t) => t.isTSTypeAnnotation());
});

Expand Down
4 changes: 0 additions & 4 deletions packages/mocha-runner/schema/mocha-runner-options.json
Expand Up @@ -56,10 +56,6 @@
"type": "boolean",
"description": "Explicit turn off [mocha config file](https://mochajs.org/#-config-path)"
},
"timeout": {
"description": "Set mocha's [`timeout` option](https://mochajs.org/#-t---timeout-ms)",
"type": "number"
},
"async-only": {
"description": "Set mocha's [`async-only` option](https://mochajs.org/#-async-only-a)",
"type": "boolean"
Expand Down
15 changes: 1 addition & 14 deletions packages/mocha-runner/src/MochaOptionsLoader.ts
Expand Up @@ -22,7 +22,6 @@ export const DEFAULT_MOCHA_OPTIONS: Readonly<MochaOptions> = Object.freeze({
ignore: [],
opts: './test/mocha.opts',
spec: ['test'],
timeout: 2000,
ui: 'bdd',
'no-package': false,
'no-opts': false,
Expand All @@ -34,7 +33,7 @@ export default class MochaOptionsLoader {
public static inject = tokens(commonTokens.logger);
constructor(private readonly log: Logger) {}

public load(strykerOptions: MochaRunnerWithStrykerOptions): MochaOptions {
public load(strykerOptions: MochaRunnerWithStrykerOptions) {
const mochaOptions = { ...strykerOptions.mochaOptions } as MochaOptions;
return { ...DEFAULT_MOCHA_OPTIONS, ...this.loadMochaOptions(mochaOptions), ...mochaOptions };
}
Expand Down Expand Up @@ -111,10 +110,6 @@ export default class MochaOptionsLoader {
}
mochaRunnerOptions.require.push(...args);
break;
case '--timeout':
case '-t':
mochaRunnerOptions.timeout = this.parseNextInt(args, DEFAULT_MOCHA_OPTIONS.timeout!);
break;
case '--async-only':
case '-A':
mochaRunnerOptions['async-only'] = true;
Expand All @@ -140,14 +135,6 @@ export default class MochaOptionsLoader {
return mochaRunnerOptions;
}

private parseNextInt(args: string[], otherwise: number): number {
if (args.length > 1) {
return parseInt(args[1], 10);
} else {
return otherwise;
}
}

private parseNextString(args: string[]): string | undefined {
if (args.length > 1) {
return args[1];
Expand Down
2 changes: 1 addition & 1 deletion packages/mocha-runner/src/MochaTestRunner.ts
Expand Up @@ -88,6 +88,7 @@ export class MochaTestRunner implements TestRunner2 {
const mocha = this.mochaAdapter.create({
reporter: StrykerMochaReporter as any,
bail: true,
timeout: false as any, // Mocha 5 doesn't support `0`
rootHooks: this.rootHooks,
} as Mocha.MochaOptions);
this.configure(mocha);
Expand Down Expand Up @@ -145,7 +146,6 @@ export class MochaTestRunner implements TestRunner2 {
}

setIfDefined(options['async-only'], (asyncOnly) => asyncOnly && mocha.asyncOnly());
setIfDefined(options.timeout, mocha.timeout);
setIfDefined(options.ui, mocha.ui);
setIfDefined(options.grep, mocha.grep);
}
Expand Down
2 changes: 1 addition & 1 deletion packages/mocha-runner/test/helpers/factories.ts
@@ -1,6 +1,6 @@
import { MochaOptions } from '../../src-generated/mocha-runner-options';
import { DEFAULT_MOCHA_OPTIONS } from '../../src/MochaOptionsLoader';

export function createMochaOptions(overrides: Partial<MochaOptions>): MochaOptions {
export function createMochaOptions(overrides?: Partial<MochaOptions>): MochaOptions {
return { ...DEFAULT_MOCHA_OPTIONS, ...overrides };
}
Expand Up @@ -28,7 +28,6 @@ describe(`${MochaOptionsLoader.name} integration`, () => {
opts: './test/mocha.opts',
package: false, // mocha sets package: false after loading it...
extension: ['js'],
timeout: 2000,
ui: 'bdd',
});
});
Expand All @@ -42,7 +41,6 @@ describe(`${MochaOptionsLoader.name} integration`, () => {
package: false, // mocha sets package: false after loading it...
config: configFile,
extension: ['json', 'js'],
timeout: 2000,
ui: 'bdd',
});
});
Expand All @@ -56,7 +54,6 @@ describe(`${MochaOptionsLoader.name} integration`, () => {
opts: './test/mocha.opts',
package: false, // mocha sets package: false after loading it...
extension: ['jsonc', 'js'],
timeout: 2000,
ui: 'bdd',
});
});
Expand All @@ -75,7 +72,6 @@ describe(`${MochaOptionsLoader.name} integration`, () => {
ignore: ['/path/to/some/excluded/file'],
require: ['@babel/register'],
spec: ['test/**/*.spec.js'],
timeout: false,
ui: 'bdd',
});
});
Expand All @@ -86,7 +82,6 @@ describe(`${MochaOptionsLoader.name} integration`, () => {
expect(actualConfig).deep.include({
['async-only']: true,
extension: ['json'],
timeout: 20,
ui: 'tdd',
});
});
Expand All @@ -97,7 +92,6 @@ describe(`${MochaOptionsLoader.name} integration`, () => {
expect(actualConfig).deep.include({
['async-only']: true,
extension: ['js', 'json'],
timeout: 2000,
ui: 'bdd',
});
});
Expand All @@ -109,12 +103,11 @@ describe(`${MochaOptionsLoader.name} integration`, () => {
'no-package': true,
'no-opts': true,
});
const expectedOptions = {
const expectedOptions: MochaOptions = {
extension: ['js', 'cjs', 'mjs'],
['no-config']: true,
['no-opts']: true,
['no-package']: true,
timeout: 2000,
ui: 'bdd',
};
expect(actualConfig).deep.include(expectedOptions);
Expand Down
20 changes: 6 additions & 14 deletions packages/mocha-runner/test/unit/MochaOptionsLoader.spec.ts
Expand Up @@ -66,7 +66,6 @@ describe(MochaOptionsLoader.name, () => {
// Following are valid options
rawOptions.extension = ['foo'];
rawOptions.require = ['bar'];
rawOptions.timeout = 4200;
rawOptions['async-only'] = true;
rawOptions.ui = 'qunit';
rawOptions.grep = 'quuz';
Expand All @@ -76,19 +75,18 @@ describe(MochaOptionsLoader.name, () => {

rawOptions.garply = 'waldo'; // this should be filtered out
const result = sut.load(options);
const expected: MochaOptions = createMochaOptions({
const expected = createMochaOptions({
extension: ['foo'],
file: ['grault'],
grep: 'quuz',
ignore: ['garply'],
opts: './test/mocha.opts',
require: ['bar'],
spec: ['test/**/*.js'],
timeout: 4200,
'async-only': true,
ui: 'qunit',
});
expect(result).deep.eq({ ...expected });
expect(result).deep.contains(expected);
});

it('should trace log the mocha call', () => {
Expand All @@ -104,7 +102,7 @@ describe(MochaOptionsLoader.name, () => {

it("should respect mocha's defaults", () => {
const actualOptions = sut.load(options);
expect(actualOptions).deep.eq(createMochaOptions());
expect(actualOptions).deep.contains(createMochaOptions());
});

it('should not allow to set parallel', () => {
Expand Down Expand Up @@ -177,7 +175,7 @@ describe(MochaOptionsLoader.name, () => {
it('should not load default mocha.opts file if not found', () => {
existsFileStub.returns(false);
const mochaOptions = sut.load(options);
expect(mochaOptions).deep.eq(createMochaOptions());
expect(mochaOptions).deep.contains(createMochaOptions());
expect(testInjector.logger.debug).calledWith(
'No mocha opts file found, not loading additional mocha options (%s was not defined).',
'mochaOptions.opts'
Expand All @@ -204,8 +202,6 @@ describe(MochaOptionsLoader.name, () => {
});
}

itShouldLoadProperty('--timeout', '2000', { timeout: 2000 });
itShouldLoadProperty('-t', '2000', { timeout: 2000 });
itShouldLoadProperty('-A', '', { 'async-only': true });
itShouldLoadProperty('--async-only', '', { 'async-only': true });
itShouldLoadProperty('--ui', 'qunit', { ui: 'qunit' });
Expand All @@ -225,18 +221,16 @@ describe(MochaOptionsLoader.name, () => {
'async-only': false,
opts: 'path/to/opts/file',
require: ['ts-node/register'],
timeout: 4000,
ui: 'exports',
};
const mochaOptions = sut.load(options);
expect(mochaOptions).deep.equal(
expect(mochaOptions).deep.contains(
createMochaOptions({
'async-only': false,
extension: ['js'],
opts: 'path/to/opts/file',
require: ['ts-node/register'],
spec: ['test'],
timeout: 4000,
ui: 'exports',
})
);
Expand Down Expand Up @@ -266,12 +260,11 @@ describe(MochaOptionsLoader.name, () => {
opts: 'some/mocha.opts/file',
};
const mochaOptions = sut.load(options);
expect(mochaOptions).deep.eq(
expect(mochaOptions).deep.contain(
createMochaOptions({
extension: ['js'],
opts: 'some/mocha.opts/file',
spec: ['test'],
timeout: 2000,
ui: 'bdd',
})
);
Expand All @@ -290,7 +283,6 @@ describe(MochaOptionsLoader.name, () => {
ignore: [],
opts: './test/mocha.opts',
spec: ['test'],
timeout: 2000,
ui: 'bdd',
...overrides,
};
Expand Down
14 changes: 11 additions & 3 deletions packages/mocha-runner/test/unit/MochaTestRunner.spec.ts
Expand Up @@ -66,7 +66,7 @@ describe(MochaTestRunner.name, () => {

it('should collect the files', async () => {
const expectedTestFileNames = ['foo.js', 'foo.spec.js'];
const mochaOptions = Object.freeze(createMochaOptions({ timeout: 23 }));
const mochaOptions = Object.freeze(createMochaOptions());
mochaOptionsLoaderMock.load.returns(mochaOptions);
mochaAdapterMock.collectFiles.returns(expectedTestFileNames);

Expand Down Expand Up @@ -121,19 +121,27 @@ describe(MochaTestRunner.name, () => {
sut.mochaOptions.grep = 'grepme';
sut.mochaOptions.opts = 'opts';
sut.mochaOptions.require = [];
sut.mochaOptions.timeout = 2000;
sut.mochaOptions.ui = 'exports';

// Act
await actDryRun();

// Assert
expect(mocha.asyncOnly).called;
expect(mocha.timeout).calledWith(2000);
expect(mocha.ui).calledWith('exports');
expect(mocha.grep).calledWith('grepme');
});

it('should force timeout off', async () => {
await actDryRun();
expect(mochaAdapterMock.create).calledWithMatch({ timeout: false });
});

it('should force bail', async () => {
await actDryRun();
expect(mochaAdapterMock.create).calledWithMatch({ bail: true });
});

it("should don't set asyncOnly if asyncOnly is false", async () => {
sut.mochaOptions['async-only'] = false;
await actDryRun();
Expand Down

0 comments on commit e90b563

Please sign in to comment.