Skip to content

Commit

Permalink
fix(jest-circus): improve test.concurrent
Browse files Browse the repository at this point in the history
  • Loading branch information
dmitri-gb committed Apr 25, 2022
1 parent b30f908 commit 2df3e93
Show file tree
Hide file tree
Showing 10 changed files with 127 additions and 57 deletions.
16 changes: 14 additions & 2 deletions e2e/__tests__/jasmineAsync.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* LICENSE file in the root directory of this source tree.
*/

import {isJestJasmineRun} from '@jest/test-utils';
import runJest, {json as runWithJson} from '../runJest';

describe('async jasmine', () => {
Expand Down Expand Up @@ -107,16 +108,25 @@ describe('async jasmine', () => {
});

it('works with concurrent', () => {
const {json} = runWithJson('jasmine-async', ['concurrent.test.js']);
const {json, stderr} = runWithJson('jasmine-async', ['concurrent.test.js']);
expect(json.numTotalTests).toBe(4);
expect(json.numPassedTests).toBe(2);
expect(json.numFailedTests).toBe(1);
expect(json.numPendingTests).toBe(1);
expect(json.testResults[0].message).toMatch(/concurrent test fails/);
if (!isJestJasmineRun()) {
expect(stderr.match(/\[\[\w+\]\]/g)).toEqual([
'[[beforeAll]]',
'[[test]]',
'[[test]]',
'[[test]]',
'[[afterAll]]',
]);
}
});

it('works with concurrent within a describe block when invoked with testNamePattern', () => {
const {json} = runWithJson('jasmine-async', [
const {json, stderr} = runWithJson('jasmine-async', [
'--testNamePattern',
'one concurrent test fails',
'concurrentWithinDescribe.test.js',
Expand All @@ -126,6 +136,8 @@ describe('async jasmine', () => {
expect(json.numFailedTests).toBe(1);
expect(json.numPendingTests).toBe(1);
expect(json.testResults[0].message).toMatch(/concurrent test fails/);
expect(stderr).toMatch(/this is logged \d/);
expect(stderr).not.toMatch(/this is not logged \d/);
});

it('works with concurrent.each', () => {
Expand Down
28 changes: 24 additions & 4 deletions e2e/jasmine-async/__tests__/concurrent.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,27 @@

'use strict';

it.concurrent('one', () => Promise.resolve());
it.concurrent.skip('two', () => Promise.resolve());
it.concurrent('three', () => Promise.resolve());
it.concurrent('concurrent test fails', () => Promise.reject());
const marker = s => console.log('[[' + s + ']]');

beforeAll(() => marker('beforeAll'));
afterAll(() => marker('afterAll'));

beforeEach(() => marker('beforeEach'));
afterEach(() => marker('afterEach'));

it.concurrent('one', () => {
marker('test');
return Promise.resolve();
});
it.concurrent.skip('two', () => {
marker('test');
return Promise.resolve();
});
it.concurrent('three', () => {
marker('test');
return Promise.resolve();
});
it.concurrent('concurrent test fails', () => {
marker('test');
return Promise.reject();
});
10 changes: 8 additions & 2 deletions e2e/jasmine-async/__tests__/concurrentWithinDescribe.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@
'use strict';

describe('one', () => {
it.concurrent('concurrent test gets skipped', () => Promise.resolve());
it.concurrent('concurrent test fails', () => Promise.reject());
it.concurrent('concurrent test gets skipped', () => {
console.log('this is not logged ' + Math.random());
return Promise.resolve();
});
it.concurrent('concurrent test fails', () => {
console.log('this is logged ' + Math.random());
return Promise.reject(new Error());
});
});
3 changes: 2 additions & 1 deletion packages/jest-circus/src/eventHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ const eventHandler: Circus.EventHandler = (event, state) => {
}
case 'add_test': {
const {currentDescribeBlock, currentlyRunningTest, hasStarted} = state;
const {asyncError, fn, mode, testName: name, timeout} = event;
const {asyncError, fn, mode, testName: name, timeout, concurrent} = event;

if (currentlyRunningTest) {
currentlyRunningTest.errors.push(
Expand All @@ -143,6 +143,7 @@ const eventHandler: Circus.EventHandler = (event, state) => {
const test = makeTest(
fn,
mode,
concurrent,
name,
currentDescribeBlock,
timeout,
Expand Down
25 changes: 21 additions & 4 deletions packages/jest-circus/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,17 +117,27 @@ const test: Global.It = (() => {
testName: Circus.TestNameLike,
fn: Circus.TestFn,
timeout?: number,
): void => _addTest(testName, undefined, fn, test, timeout);
): void => _addTest(testName, undefined, false, fn, test, timeout);
const skip = (
testName: Circus.TestNameLike,
fn?: Circus.TestFn,
timeout?: number,
): void => _addTest(testName, 'skip', fn, skip, timeout);
): void => _addTest(testName, 'skip', false, fn, skip, timeout);
const only = (
testName: Circus.TestNameLike,
fn: Circus.TestFn,
timeout?: number,
): void => _addTest(testName, 'only', fn, test.only, timeout);
): void => _addTest(testName, 'only', false, fn, test.only, timeout);
const concurrentTest = (
testName: Circus.TestNameLike,
fn: Circus.TestFn,
timeout?: number,
): void => _addTest(testName, undefined, true, fn, concurrentTest, timeout);
const concurrentOnly = (
testName: Circus.TestNameLike,
fn: Circus.TestFn,
timeout?: number,
): void => _addTest(testName, 'only', true, fn, concurrentOnly, timeout);

test.todo = (testName: Circus.TestNameLike, ...rest: Array<any>): void => {
if (rest.length > 0 || typeof testName !== 'string') {
Expand All @@ -136,12 +146,13 @@ const test: Global.It = (() => {
test.todo,
);
}
return _addTest(testName, 'todo', () => {}, test.todo);
return _addTest(testName, 'todo', false, () => {}, test.todo);
};

const _addTest = (
testName: Circus.TestNameLike,
mode: Circus.TestMode,
concurrent: boolean,
fn: Circus.TestFn | undefined,
testFn: (
testName: Circus.TestNameLike,
Expand Down Expand Up @@ -175,6 +186,7 @@ const test: Global.It = (() => {
asyncError,
fn,
mode,
concurrent,
name: 'add_test',
testName,
timeout,
Expand All @@ -184,9 +196,14 @@ const test: Global.It = (() => {
test.each = bindEach(test);
only.each = bindEach(only);
skip.each = bindEach(skip);
concurrentTest.each = bindEach(concurrentTest, false);
concurrentOnly.each = bindEach(concurrentOnly, false);

test.only = only;
test.skip = skip;
test.concurrent = concurrentTest;
concurrentTest.only = concurrentOnly;
concurrentTest.skip = skip;

return test;
})();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
* LICENSE file in the root directory of this source tree.
*/

import throat from 'throat';
import type {JestEnvironment} from '@jest/environment';
import {JestExpect, jestExpect} from '@jest/expect';
import {
Expand All @@ -16,7 +15,6 @@ import {
createEmptyTestResult,
} from '@jest/test-result';
import type {Circus, Config, Global} from '@jest/types';
import {bind} from 'jest-each';
import {formatExecError, formatResultsErrors} from 'jest-message-util';
import {
SnapshotState,
Expand Down Expand Up @@ -63,8 +61,7 @@ export const initialize = async ({
if (globalConfig.testTimeout) {
getRunnerState().testTimeout = globalConfig.testTimeout;
}

const mutex = throat(globalConfig.maxConcurrency);
getRunnerState().maxConcurrency = globalConfig.maxConcurrency;

// @ts-expect-error
const globalsObject: Global.TestFrameworkGlobals = {
Expand All @@ -76,45 +73,6 @@ export const initialize = async ({
xtest: globals.it.skip,
};

globalsObject.test.concurrent = (test => {
const concurrent = (
testName: Global.TestNameLike,
testFn: Global.ConcurrentTestFn,
timeout?: number,
) => {
// For concurrent tests we first run the function that returns promise, and then register a
// normal test that will be waiting on the returned promise (when we start the test, the promise
// will already be in the process of execution).
// Unfortunately at this stage there's no way to know if there are any `.only` tests in the suite
// that will result in this test to be skipped, so we'll be executing the promise function anyway,
// even if it ends up being skipped.
const promise = mutex(() => testFn());
// Avoid triggering the uncaught promise rejection handler in case the test errors before
// being awaited on.
promise.catch(() => {});
globalsObject.test(testName, () => promise, timeout);
};

const only = (
testName: Global.TestNameLike,
testFn: Global.ConcurrentTestFn,
timeout?: number,
) => {
const promise = mutex(() => testFn());
// eslint-disable-next-line jest/no-focused-tests
test.only(testName, () => promise, timeout);
};

concurrent.only = only;
concurrent.skip = test.skip;

concurrent.each = bind(test, false);
concurrent.skip.each = bind(test.skip, false);
only.each = bind(test.only, false);

return concurrent;
})(globalsObject.test);

addEventHandler(eventHandler);

if (environment.handleTestEvent) {
Expand Down
46 changes: 45 additions & 1 deletion packages/jest-circus/src/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import type {Circus} from '@jest/types';
import throat from 'throat';
import {dispatch, getState} from './state';
import {RETRY_TIMES} from './types';
import {
Expand All @@ -20,7 +21,7 @@ import {
const run = async (): Promise<Circus.RunResult> => {
const {rootDescribeBlock} = getState();
await dispatch({name: 'run_start'});
await _runTestsForDescribeBlock(rootDescribeBlock);
await _runTestsForDescribeBlock(rootDescribeBlock, true);
await dispatch({name: 'run_finish'});
return makeRunResult(
getState().rootDescribeBlock,
Expand All @@ -30,6 +31,7 @@ const run = async (): Promise<Circus.RunResult> => {

const _runTestsForDescribeBlock = async (
describeBlock: Circus.DescribeBlock,
isRootBlock = false,
) => {
await dispatch({describeBlock, name: 'run_describe_start'});
const {beforeAll, afterAll} = getAllHooksForDescribe(describeBlock);
Expand All @@ -42,6 +44,24 @@ const _runTestsForDescribeBlock = async (
}
}

if (isRootBlock) {
const concurrentTests = collectConcurrentTests(describeBlock);
const mutex = throat(getState().maxConcurrency);
for (const test of concurrentTests) {
try {
const promise = mutex(test.fn);
// Avoid triggering the uncaught promise rejection handler in case the
// test errors before being awaited on.
promise.catch(() => {});
test.fn = () => promise;
} catch (err) {
test.fn = () => {
throw err;
};
}
}
}

// Tests that fail and are retried we run after other tests
// eslint-disable-next-line no-restricted-globals
const retryTimes = parseInt(global[RETRY_TIMES], 10) || 0;
Expand Down Expand Up @@ -91,6 +111,30 @@ const _runTestsForDescribeBlock = async (
await dispatch({describeBlock, name: 'run_describe_finish'});
};

function collectConcurrentTests(
describeBlock: Circus.DescribeBlock,
): (Omit<Circus.TestEntry, 'fn'> & {fn: Circus.ConcurrentTestFn})[] {
if (describeBlock.mode === 'skip') {
return [];
}
const {hasFocusedTests, testNamePattern} = getState();
return describeBlock.children.flatMap(child => {
switch (child.type) {
case 'describeBlock':
return collectConcurrentTests(child);
case 'test':
const skip =
!child.concurrent ||
child.mode === 'skip' ||
(hasFocusedTests && child.mode !== 'only') ||
(testNamePattern && !testNamePattern.test(getTestID(child)));
return skip
? []
: [child as Circus.TestEntry & {fn: Circus.ConcurrentTestFn}];
}
});
}

const _runTest = async (
test: Circus.TestEntry,
parentSkipped: boolean,
Expand Down
1 change: 1 addition & 0 deletions packages/jest-circus/src/state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ const createState = (): Circus.State => {
testNamePattern: null,
testTimeout: 5000,
unhandledErrors: [],
maxConcurrency: 5,
};
};

Expand Down
7 changes: 7 additions & 0 deletions packages/jest-circus/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ export const makeDescribe = (
export const makeTest = (
fn: Circus.TestFn,
mode: Circus.TestMode,
concurrent: boolean,
name: Circus.TestName,
parent: Circus.DescribeBlock,
timeout: number | undefined,
Expand All @@ -68,6 +69,7 @@ export const makeTest = (
fn,
invocations: 0,
mode,
concurrent,
name: convertDescriptorToString(name),
parent,
retryReasons: [],
Expand Down Expand Up @@ -128,6 +130,11 @@ type TestHooks = {

export const getEachHooksForTest = (test: Circus.TestEntry): TestHooks => {
const result: TestHooks = {afterEach: [], beforeEach: []};
if (test.concurrent) {
// *Each hooks are not run for concurrent tests
return result;
}

let block: Circus.DescribeBlock | undefined | null = test.parent;

do {
Expand Down
4 changes: 4 additions & 0 deletions packages/jest-types/src/Circus.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export type TestMode = BlockMode;
export type TestName = Global.TestName;
export type TestNameLike = Global.TestNameLike;
export type TestFn = Global.TestFn;
export type ConcurrentTestFn = Global.ConcurrentTestFn;
export type HookFn = Global.HookFn;
export type AsyncFn = TestFn | HookFn;
export type SharedHookType = 'afterAll' | 'beforeAll';
Expand Down Expand Up @@ -71,6 +72,7 @@ export type SyncEvent =
testName: TestName;
fn: TestFn;
mode?: TestMode;
concurrent: boolean;
timeout: number | undefined;
}
| {
Expand Down Expand Up @@ -216,6 +218,7 @@ export type State = {
testTimeout: number;
unhandledErrors: Array<Exception>;
includeTestLocationInResult: boolean;
maxConcurrency: number;
};

export type DescribeBlock = {
Expand All @@ -239,6 +242,7 @@ export type TestEntry = {
fn: TestFn;
invocations: number;
mode: TestMode;
concurrent: boolean;
name: TestName;
parent: DescribeBlock;
startedAt?: number | null;
Expand Down

0 comments on commit 2df3e93

Please sign in to comment.