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

fix(jest-circus): improve test.concurrent #12748

Merged
merged 2 commits into from Apr 26, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -6,6 +6,8 @@

### Fixes

- `[jest-circus]` Improve `test.concurrent` ([#12748](https://github.com/facebook/jest/pull/12748))

### Chore & Maintenance

- `[jest-serializer]` Remove deprecated module from source tree ([#12735](https://github.com/facebook/jest/pull/12735))
Expand Down
16 changes: 14 additions & 2 deletions e2e/__tests__/jasmineAsync.test.ts
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
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
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
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
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 @@ -173,6 +184,7 @@ const test: Global.It = (() => {

return dispatchSync({
asyncError,
concurrent,
fn,
mode,
name: 'add_test',
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
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
Expand Up @@ -5,6 +5,7 @@
* LICENSE file in the root directory of this source tree.
*/

import throat from 'throat';
import type {Circus} from '@jest/types';
import {dispatch, getState} from './state';
import {RETRY_TIMES} from './types';
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,
): Array<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
Expand Up @@ -27,6 +27,7 @@ const createState = (): Circus.State => {
hasFocusedTests: false,
hasStarted: false,
includeTestLocationInResult: false,
maxConcurrency: 5,
parentProcess: null,
rootDescribeBlock: ROOT_DESCRIBE_BLOCK,
testNamePattern: null,
Expand Down
7 changes: 7 additions & 0 deletions packages/jest-circus/src/utils.ts
Expand Up @@ -56,13 +56,15 @@ export const makeDescribe = (
export const makeTest = (
fn: Circus.TestFn,
mode: Circus.TestMode,
concurrent: boolean,
name: Circus.TestName,
parent: Circus.DescribeBlock,
timeout: number | undefined,
asyncError: Circus.Exception,
): Circus.TestEntry => ({
type: 'test', // eslint-disable-next-line sort-keys
asyncError,
concurrent,
duration: null,
errors: [],
fn,
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