From 39e18542f6b27bf36f0ccf67f496928cf2267e98 Mon Sep 17 00:00:00 2001 From: connectdotz Date: Wed, 31 Aug 2022 16:34:53 -0400 Subject: [PATCH 1/6] fix TestScheduler fail to dispatch onRunComplete upon exec error --- packages/jest-core/src/TestScheduler.ts | 168 ++++++++++-------- .../src/__tests__/TestScheduler.test.js | 138 +++++++++++++- packages/jest-test-result/src/types.ts | 1 + 3 files changed, 235 insertions(+), 72 deletions(-) diff --git a/packages/jest-core/src/TestScheduler.ts b/packages/jest-core/src/TestScheduler.ts index 7c088051334b..6088a46768d9 100644 --- a/packages/jest-core/src/TestScheduler.ts +++ b/packages/jest-core/src/TestScheduler.ts @@ -31,13 +31,13 @@ import { } from '@jest/test-result'; import {createScriptTransformer} from '@jest/transform'; import type {Config} from '@jest/types'; -import {formatExecError} from 'jest-message-util'; +import {formatExecError, separateMessageFromStack} from 'jest-message-util'; import type {JestTestRunner, TestRunnerContext} from 'jest-runner'; import { buildSnapshotResolver, cleanup as cleanupSnapshots, } from 'jest-snapshot'; -import {requireOrImportModule} from 'jest-util'; +import {ErrorWithStack, requireOrImportModule} from 'jest-util'; import type {TestWatcher} from 'jest-watcher'; import ReporterDispatcher from './ReporterDispatcher'; import {shouldRunInBand} from './testSchedulerHelper'; @@ -201,6 +201,27 @@ class TestScheduler { aggregatedResults.snapshot.filesRemoved) ); }; + const buildExecError = (err: unknown): SerializableError => { + const fromString = (errString: string): SerializableError => { + const {message, stack} = separateMessageFromStack(errString); + if (stack.length > 0) { + return {message, stack}; + } + const error = new ErrorWithStack(message, buildExecError); + return {message, stack: error.stack || ''}; + }; + if (typeof err === 'string' || err == null) { + return fromString(err || 'Error'); + } + const anyErr: any = err as any; + if (typeof anyErr.message === 'string') { + if (typeof anyErr.stack === 'string' && anyErr.stack.length > 0) { + return anyErr; + } + return fromString(anyErr.message); + } + return fromString(JSON.stringify(err)); + }; await this._dispatcher.onRunStart(aggregatedResults, { estimatedTime, @@ -209,79 +230,86 @@ class TestScheduler { const testRunners: Record = Object.create(null); const contextsByTestRunner = new WeakMap(); - await Promise.all( - Array.from(testContexts).map(async context => { - const {config} = context; - if (!testRunners[config.runner]) { - const transformer = await createScriptTransformer(config); - const Runner: TestRunnerConstructor = - await transformer.requireAndTranspileModule(config.runner); - const runner = new Runner(this._globalConfig, { - changedFiles: this._context.changedFiles, - sourcesRelatedToTestsInChangedFiles: - this._context.sourcesRelatedToTestsInChangedFiles, - }); - testRunners[config.runner] = runner; - contextsByTestRunner.set(runner, context); - } - }), - ); - const testsByRunner = this._partitionTests(testRunners, tests); + try{ + await Promise.all( + Array.from(testContexts).map(async context => { + const {config} = context; + if (!testRunners[config.runner]) { + const transformer = await createScriptTransformer(config); + const Runner: TestRunnerConstructor = + await transformer.requireAndTranspileModule(config.runner); + const runner = new Runner(this._globalConfig, { + changedFiles: this._context.changedFiles, + sourcesRelatedToTestsInChangedFiles: + this._context.sourcesRelatedToTestsInChangedFiles, + }); + testRunners[config.runner] = runner; + contextsByTestRunner.set(runner, context); + } + }), + ); - if (testsByRunner) { - try { - for (const runner of Object.keys(testRunners)) { - const testRunner = testRunners[runner]; - const context = contextsByTestRunner.get(testRunner); - - invariant(context); - - const tests = testsByRunner[runner]; - - const testRunnerOptions = { - serial: runInBand || Boolean(testRunner.isSerial), - }; - - if (testRunner.supportsEventEmitters) { - const unsubscribes = [ - testRunner.on('test-file-start', ([test]) => - onTestFileStart(test), - ), - testRunner.on('test-file-success', ([test, testResult]) => - onResult(test, testResult), - ), - testRunner.on('test-file-failure', ([test, error]) => - onFailure(test, error), - ), - testRunner.on( - 'test-case-result', - ([testPath, testCaseResult]) => { - const test: Test = {context, path: testPath}; - this._dispatcher.onTestCaseResult(test, testCaseResult); - }, - ), - ]; - - await testRunner.runTests(tests, watcher, testRunnerOptions); - - unsubscribes.forEach(sub => sub()); - } else { - await testRunner.runTests( - tests, - watcher, - onTestFileStart, - onResult, - onFailure, - testRunnerOptions, - ); + const testsByRunner = this._partitionTests(testRunners, tests); + + if (testsByRunner) { + try { + for (const runner of Object.keys(testRunners)) { + const testRunner = testRunners[runner]; + const context = contextsByTestRunner.get(testRunner); + + invariant(context); + + const tests = testsByRunner[runner]; + + const testRunnerOptions = { + serial: runInBand || Boolean(testRunner.isSerial), + }; + + if (testRunner.supportsEventEmitters) { + const unsubscribes = [ + testRunner.on('test-file-start', ([test]) => + onTestFileStart(test), + ), + testRunner.on('test-file-success', ([test, testResult]) => + onResult(test, testResult), + ), + testRunner.on('test-file-failure', ([test, error]) => + onFailure(test, error), + ), + testRunner.on( + 'test-case-result', + ([testPath, testCaseResult]) => { + const test: Test = {context, path: testPath}; + this._dispatcher.onTestCaseResult(test, testCaseResult); + }, + ), + ]; + + await testRunner.runTests(tests, watcher, testRunnerOptions); + + unsubscribes.forEach(sub => sub()); + } else { + await testRunner.runTests( + tests, + watcher, + onTestFileStart, + onResult, + onFailure, + testRunnerOptions, + ); + } + } + } catch (error) { + if (!watcher.isInterrupted()) { + throw error; } - } - } catch (error) { - if (!watcher.isInterrupted()) { - throw error; } } + } catch(error){ + aggregatedResults.runExecError = buildExecError(error); + await this._dispatcher.onRunComplete(testContexts, aggregatedResults); + throw error; } await updateSnapshotState(); diff --git a/packages/jest-core/src/__tests__/TestScheduler.test.js b/packages/jest-core/src/__tests__/TestScheduler.test.js index 66aba2262bb0..9f1d402b7301 100644 --- a/packages/jest-core/src/__tests__/TestScheduler.test.js +++ b/packages/jest-core/src/__tests__/TestScheduler.test.js @@ -17,6 +17,7 @@ import { import {makeGlobalConfig, makeProjectConfig} from '@jest/test-utils'; import {createTestScheduler} from '../TestScheduler'; import * as testSchedulerHelper from '../testSchedulerHelper'; +import * as transform from '@jest/transform'; jest .mock('ci-info', () => ({GITHUB_ACTIONS: true})) @@ -28,8 +29,13 @@ jest onTestStart() {}, })), {virtual: true}, - ); - + ) + .mock('@jest/transform', () => { + return { + __esModule: true, + ...jest.requireActual('@jest/transform'), + }; + }) const mockSerialRunner = { isSerial: true, runTests: jest.fn(), @@ -233,6 +239,134 @@ test('.addReporter() .removeReporter()', async () => { expect(scheduler._dispatcher._reporters).not.toContain(reporter); }); +describe('scheduleTests should always dispatch runStart and runComplete events', () => { + const mockReporter = { + onRunStart: jest.fn(), + onRunComplete: jest.fn(), + }; + + const errorMsg = 'runtime-error'; + let scheduler, t; + + beforeEach(async () => { + mockReporter.onRunStart.mockClear(); + mockReporter.onRunComplete.mockClear(); + + t = { + context: { + config: makeProjectConfig({ + moduleFileExtensions: ['.js'], + rootDir: './', + runner: 'jest-runner-serial', + transform: [], + }), + hasteFS: { + matchFiles: jest.fn(() => []), + }, + }, + path: './test/path.js', + }; + + scheduler = await createTestScheduler(makeGlobalConfig(), {}, {}); + scheduler.addReporter(mockReporter); + }); + + test('during normal run', async () => { + expect.hasAssertions(); + const result = await scheduler.scheduleTests([t], { + isInterrupted: jest.fn(), + isWatchMode: () => true, + setState: jest.fn(), + }); + + expect(result.numTotalTestSuites).toEqual(1); + + expect(mockReporter.onRunStart).toBeCalledTimes(1); + expect(mockReporter.onRunComplete).toBeCalledTimes(1); + const aggregatedResult = mockReporter.onRunComplete.mock.calls[0][1]; + expect(aggregatedResult.runExecError).toBeUndefined(); + + expect(aggregatedResult).toEqual(result); + }); + test.each` + runtimeError | message + ${errorMsg} | ${errorMsg} + ${123} | ${'123'} + ${new Error(errorMsg)} | ${errorMsg} + ${{message: errorMsg}} | ${errorMsg} + ${{message: errorMsg, stack: 'stack-string'}} | ${errorMsg} + ${`${errorMsg}\n Require stack:xxxx`} | ${errorMsg} + `('with runtime error: $runtimeError', async ({runtimeError, message}) => { + expect.hasAssertions(); + + const spyCreateScriptTransformer = jest.spyOn( + transform, + 'createScriptTransformer', + ); + spyCreateScriptTransformer.mockImplementation(async () => { + throw runtimeError; + }); + + await expect( + scheduler.scheduleTests([t], { + isInterrupted: jest.fn(), + isWatchMode: () => true, + setState: jest.fn(), + }), + ).rejects.toEqual(runtimeError); + + expect(mockReporter.onRunStart).toBeCalledTimes(1); + expect(mockReporter.onRunComplete).toBeCalledTimes(1); + const aggregatedResult = mockReporter.onRunComplete.mock.calls[0][1]; + expect(aggregatedResult.runExecError.message).toEqual(message); + expect(aggregatedResult.runExecError.stack.length).toBeGreaterThan(0); + + spyCreateScriptTransformer.mockRestore(); + }); + test.each` + watchMode | isInterrupted | hasExecError + ${false} | ${false} | ${true} + ${true} | ${false} | ${true} + ${true} | ${true} | ${false} + `( + 'with runner exception: watchMode=$watchMode, isInterrupted=$isInterrupted', + async ({watchMode, isInterrupted, hasExecError}) => { + expect.hasAssertions(); + + mockSerialRunner.runTests.mockImplementation(() => { + throw errorMsg; + }); + + try { + const result = await scheduler.scheduleTests([t], { + isInterrupted: () => isInterrupted, + isWatchMode: () => watchMode, + setState: jest.fn(), + }); + if (hasExecError) { + throw new Error('should throw exception'); + } + expect(result.runExecError).toBeUndefined(); + } catch (e) { + expect(e).toEqual(errorMsg); + } + + expect(mockReporter.onRunStart).toBeCalledTimes(1); + expect(mockReporter.onRunComplete).toBeCalledTimes(1); + + const aggregatedResult = mockReporter.onRunComplete.mock.calls[0][1]; + if (hasExecError) { + expect(aggregatedResult.runExecError.message).toEqual(errorMsg); + expect(aggregatedResult.runExecError.stack.length).toBeGreaterThan(0); + } else { + expect(aggregatedResult.runExecError).toBeUndefined(); + } + + mockSerialRunner.runTests.mockReset(); + }, + ); +}); + test('schedule tests run in parallel per default', async () => { const scheduler = await createTestScheduler(makeGlobalConfig(), {}, {}); const test = { diff --git a/packages/jest-test-result/src/types.ts b/packages/jest-test-result/src/types.ts index ff112858f8a1..201552a8cb3a 100644 --- a/packages/jest-test-result/src/types.ts +++ b/packages/jest-test-result/src/types.ts @@ -68,6 +68,7 @@ export type AggregatedResultWithoutCoverage = { success: boolean; testResults: Array; wasInterrupted: boolean; + runExecError?: SerializableError; }; export type AggregatedResult = AggregatedResultWithoutCoverage & { From 520ea8e75ad9135194b3e70bffa9c02fa970d4cb Mon Sep 17 00:00:00 2001 From: connectdotz Date: Wed, 31 Aug 2022 17:06:53 -0400 Subject: [PATCH 2/6] fix lint and changelog --- CHANGELOG.md | 1 + packages/jest-core/src/TestScheduler.ts | 4 ++-- packages/jest-core/src/__tests__/TestScheduler.test.js | 6 +++--- packages/jest-test-result/src/types.ts | 2 +- 4 files changed, 7 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 80867e863a56..8334b31b5eab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ ### Fixes - `[babel-plugin-jest-hoist]` Support imported `jest` in mock factory ([#13188](https://github.com/facebook/jest/pull/13188)) +- `[jest-core]` Capture execError during `TestScheduler.scheduleTests` and dispatch to reporters. ([#13203](https://github.com/facebook/jest/pull/13203)) ### Chore & Maintenance diff --git a/packages/jest-core/src/TestScheduler.ts b/packages/jest-core/src/TestScheduler.ts index 6088a46768d9..a90bdeb714ef 100644 --- a/packages/jest-core/src/TestScheduler.ts +++ b/packages/jest-core/src/TestScheduler.ts @@ -231,7 +231,7 @@ class TestScheduler { const testRunners: Record = Object.create(null); const contextsByTestRunner = new WeakMap(); - try{ + try { await Promise.all( Array.from(testContexts).map(async context => { const {config} = context; @@ -306,7 +306,7 @@ class TestScheduler { } } } - } catch(error){ + } catch (error) { aggregatedResults.runExecError = buildExecError(error); await this._dispatcher.onRunComplete(testContexts, aggregatedResults); throw error; diff --git a/packages/jest-core/src/__tests__/TestScheduler.test.js b/packages/jest-core/src/__tests__/TestScheduler.test.js index 9f1d402b7301..d9c14e49cadf 100644 --- a/packages/jest-core/src/__tests__/TestScheduler.test.js +++ b/packages/jest-core/src/__tests__/TestScheduler.test.js @@ -15,9 +15,9 @@ import { VerboseReporter, } from '@jest/reporters'; import {makeGlobalConfig, makeProjectConfig} from '@jest/test-utils'; +import * as transform from '@jest/transform'; import {createTestScheduler} from '../TestScheduler'; import * as testSchedulerHelper from '../testSchedulerHelper'; -import * as transform from '@jest/transform'; jest .mock('ci-info', () => ({GITHUB_ACTIONS: true})) @@ -35,7 +35,7 @@ jest __esModule: true, ...jest.requireActual('@jest/transform'), }; - }) + }); const mockSerialRunner = { isSerial: true, runTests: jest.fn(), @@ -241,8 +241,8 @@ test('.addReporter() .removeReporter()', async () => { describe('scheduleTests should always dispatch runStart and runComplete events', () => { const mockReporter = { - onRunStart: jest.fn(), onRunComplete: jest.fn(), + onRunStart: jest.fn(), }; const errorMsg = 'runtime-error'; diff --git a/packages/jest-test-result/src/types.ts b/packages/jest-test-result/src/types.ts index 201552a8cb3a..752ee750c730 100644 --- a/packages/jest-test-result/src/types.ts +++ b/packages/jest-test-result/src/types.ts @@ -68,7 +68,7 @@ export type AggregatedResultWithoutCoverage = { success: boolean; testResults: Array; wasInterrupted: boolean; - runExecError?: SerializableError; + runExecError?: SerializableError; }; export type AggregatedResult = AggregatedResultWithoutCoverage & { From b5971c806bcbb49b1a99a8b703743ac12f5144b9 Mon Sep 17 00:00:00 2001 From: connectdotz Date: Fri, 2 Sep 2022 12:05:34 -0400 Subject: [PATCH 3/6] update vscode setting change autoRun setting to avoid the long startup full-test run. --- .vscode/settings.json | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index c38b6ef3b307..cc28863cb0ea 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -5,5 +5,9 @@ }, "javascript.validate.enable": false, "jest.jestCommandLine": "yarn jest", - "typescript.tsdk": "node_modules/typescript/lib" + "typescript.tsdk": "node_modules/typescript/lib", + "jest.autoRun": { + "watch": false, + "onSave": "test-src-file", + } } From 5d62031bafed82f8b5c2c6cf095fa8f6a56fc789 Mon Sep 17 00:00:00 2001 From: connectdotz Date: Sat, 3 Sep 2022 14:56:40 -0400 Subject: [PATCH 4/6] address review comments --- .vscode/settings.json | 6 +----- packages/jest-core/src/TestScheduler.ts | 24 ++++++++++++------------ 2 files changed, 13 insertions(+), 17 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index cc28863cb0ea..c38b6ef3b307 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -5,9 +5,5 @@ }, "javascript.validate.enable": false, "jest.jestCommandLine": "yarn jest", - "typescript.tsdk": "node_modules/typescript/lib", - "jest.autoRun": { - "watch": false, - "onSave": "test-src-file", - } + "typescript.tsdk": "node_modules/typescript/lib" } diff --git a/packages/jest-core/src/TestScheduler.ts b/packages/jest-core/src/TestScheduler.ts index a90bdeb714ef..047806af611c 100644 --- a/packages/jest-core/src/TestScheduler.ts +++ b/packages/jest-core/src/TestScheduler.ts @@ -201,26 +201,26 @@ class TestScheduler { aggregatedResults.snapshot.filesRemoved) ); }; + const strToError = (errString: string): SerializableError => { + const {message, stack} = separateMessageFromStack(errString); + if (stack.length > 0) { + return {message, stack}; + } + const error = new ErrorWithStack(message, buildExecError); + return {message, stack: error.stack || ''}; + }; const buildExecError = (err: unknown): SerializableError => { - const fromString = (errString: string): SerializableError => { - const {message, stack} = separateMessageFromStack(errString); - if (stack.length > 0) { - return {message, stack}; - } - const error = new ErrorWithStack(message, buildExecError); - return {message, stack: error.stack || ''}; - }; if (typeof err === 'string' || err == null) { - return fromString(err || 'Error'); + return strToError(err || 'Error'); } - const anyErr: any = err as any; + const anyErr = err as any; if (typeof anyErr.message === 'string') { if (typeof anyErr.stack === 'string' && anyErr.stack.length > 0) { return anyErr; } - return fromString(anyErr.message); + return strToError(anyErr.message); } - return fromString(JSON.stringify(err)); + return strToError(JSON.stringify(err)); }; await this._dispatcher.onRunStart(aggregatedResults, { From 0762ae2ffed0f06c4412386f9204ee5b79839366 Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Sun, 4 Sep 2022 11:08:40 +0200 Subject: [PATCH 5/6] move changelog entry --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 136af2c97582..a99c2fb23f35 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ ### Fixes +- `[jest-core]` Capture execError during `TestScheduler.scheduleTests` and dispatch to reporters ([#13203](https://github.com/facebook/jest/pull/13203)) + ### Chore & Maintenance ### Performance @@ -19,7 +21,6 @@ ### Fixes - `[babel-plugin-jest-hoist]` Support imported `jest` in mock factory ([#13188](https://github.com/facebook/jest/pull/13188)) -- `[jest-core]` Capture execError during `TestScheduler.scheduleTests` and dispatch to reporters ([#13203](https://github.com/facebook/jest/pull/13203)) - `[jest-mock]` Align the behavior and return type of `generateFromMetadata` method ([#13207](https://github.com/facebook/jest/pull/13207)) - `[jest-runtime]` Support `jest.resetModules()` with ESM ([#13211](https://github.com/facebook/jest/pull/13211)) From c8977296ed37d6c2ac2898b178b34ebda4066f47 Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Sun, 4 Sep 2022 11:09:39 +0200 Subject: [PATCH 6/6] no inline functions --- packages/jest-core/src/TestScheduler.ts | 44 +++++++++++++------------ 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/packages/jest-core/src/TestScheduler.ts b/packages/jest-core/src/TestScheduler.ts index 047806af611c..ad96154c2fbb 100644 --- a/packages/jest-core/src/TestScheduler.ts +++ b/packages/jest-core/src/TestScheduler.ts @@ -201,27 +201,6 @@ class TestScheduler { aggregatedResults.snapshot.filesRemoved) ); }; - const strToError = (errString: string): SerializableError => { - const {message, stack} = separateMessageFromStack(errString); - if (stack.length > 0) { - return {message, stack}; - } - const error = new ErrorWithStack(message, buildExecError); - return {message, stack: error.stack || ''}; - }; - const buildExecError = (err: unknown): SerializableError => { - if (typeof err === 'string' || err == null) { - return strToError(err || 'Error'); - } - const anyErr = err as any; - if (typeof anyErr.message === 'string') { - if (typeof anyErr.stack === 'string' && anyErr.stack.length > 0) { - return anyErr; - } - return strToError(anyErr.message); - } - return strToError(JSON.stringify(err)); - }; await this._dispatcher.onRunStart(aggregatedResults, { estimatedTime, @@ -459,3 +438,26 @@ const getEstimatedTime = (timings: Array, workers: number) => { ? max : Math.max(timings.reduce((sum, time) => sum + time) / workers, max); }; + +const strToError = (errString: string): SerializableError => { + const {message, stack} = separateMessageFromStack(errString); + if (stack.length > 0) { + return {message, stack}; + } + const error = new ErrorWithStack(message, buildExecError); + return {message, stack: error.stack || ''}; +}; + +const buildExecError = (err: unknown): SerializableError => { + if (typeof err === 'string' || err == null) { + return strToError(err || 'Error'); + } + const anyErr = err as any; + if (typeof anyErr.message === 'string') { + if (typeof anyErr.stack === 'string' && anyErr.stack.length > 0) { + return anyErr; + } + return strToError(anyErr.message); + } + return strToError(JSON.stringify(err)); +};