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

CHANGELOG.md

@@ -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))

e2e/__tests__/jasmineAsync.test.ts

@@ -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', () => {
@@ -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',
@@ -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', () => {

e2e/jasmine-async/__tests__/concurrent.test.js

@@ -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();
+});

e2e/jasmine-async/__tests__/concurrentWithinDescribe.test.js

@@ -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());
+  });
 });

packages/jest-circus/src/eventHandler.ts

@@ -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(
@@ -143,6 +143,7 @@ const eventHandler: Circus.EventHandler = (event, state) => {
       const test = makeTest(
         fn,
         mode,
+        concurrent,
         name,
         currentDescribeBlock,
         timeout,

packages/jest-circus/src/index.ts

@@ -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') {
@@ -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,
@@ -173,6 +184,7 @@ const test: Global.It = (() => {
 
     return dispatchSync({
       asyncError,
+      concurrent,
       fn,
       mode,
       name: 'add_test',
@@ -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;
 })();

packages/jest-circus/src/legacy-code-todo-rewrite/jestAdapterInit.ts

@@ -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 {
@@ -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,
@@ -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 = {
@@ -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) {

packages/jest-circus/src/run.ts

@@ -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';
@@ -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,
@@ -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);
@@ -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;
@@ -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,

packages/jest-circus/src/state.ts

@@ -27,6 +27,7 @@ const createState = (): Circus.State => {
     hasFocusedTests: false,
     hasStarted: false,
     includeTestLocationInResult: false,
+    maxConcurrency: 5,
     parentProcess: null,
     rootDescribeBlock: ROOT_DESCRIBE_BLOCK,
     testNamePattern: null,

packages/jest-circus/src/utils.ts

@@ -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,
@@ -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 {