diff --git a/CHANGELOG.md b/CHANGELOG.md index d26f7237dbde..b83b64e30170 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,8 @@ - `[jest-worker]` Fix `jest-worker` when using pre-allocated jobs ([#7934](https://github.com/facebook/jest/pull/7934)) - `[jest-changed-files]` Fix `getChangedFilesFromRoots` to not return parts of the commit messages as if they were files, when the commit messages contained multiple paragraphs ([#7961](https://github.com/facebook/jest/pull/7961)) - `[static]` Remove console log '-' on the front page +- `[jest-jasmine2]`: Throw explicit error when errors happen after test is considered complete ([#8005](https://github.com/facebook/jest/pull/8005)) +- `[jest-circus]`: Throw explicit error when errors happen after test is considered complete ([#8005](https://github.com/facebook/jest/pull/8005)) ### Chore & Maintenance diff --git a/e2e/__tests__/failures.test.ts b/e2e/__tests__/failures.test.ts index f99bed6a4938..851bb3d1a39b 100644 --- a/e2e/__tests__/failures.test.ts +++ b/e2e/__tests__/failures.test.ts @@ -12,9 +12,9 @@ import runJest from '../runJest'; const dir = path.resolve(__dirname, '../failures'); -const normalizeDots = text => text.replace(/\.{1,}$/gm, '.'); +const normalizeDots = (text: string) => text.replace(/\.{1,}$/gm, '.'); -function cleanStderr(stderr) { +function cleanStderr(stderr: string) { const {rest} = extractSummary(stderr); return rest .replace(/.*(jest-jasmine2|jest-circus).*\n/g, '') @@ -182,3 +182,12 @@ test('works with named snapshot failures', () => { wrap(result.substring(0, result.indexOf('Snapshot Summary'))), ).toMatchSnapshot(); }); + +test('errors after test has completed', () => { + const {stderr} = runJest(dir, ['errorAfterTestComplete.test.js']); + + expect(stderr).toMatch( + /Error: Caught error after test environment was torn down/, + ); + expect(stderr).toMatch(/Failed: "fail async"/); +}); diff --git a/e2e/failures/__tests__/errorAfterTestComplete.test.js b/e2e/failures/__tests__/errorAfterTestComplete.test.js new file mode 100644 index 000000000000..6bfb7dc3ae79 --- /dev/null +++ b/e2e/failures/__tests__/errorAfterTestComplete.test.js @@ -0,0 +1,14 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @emails oncall+jsinfra + */ +'use strict'; + +test('a failing test', done => { + setTimeout(() => done('fail async'), 5); + done(); +}); diff --git a/packages/jest-circus/src/utils.ts b/packages/jest-circus/src/utils.ts index 7917d1356e9d..19e115baec86 100644 --- a/packages/jest-circus/src/utils.ts +++ b/packages/jest-circus/src/utils.ts @@ -153,84 +153,93 @@ const _makeTimeoutMessage = (timeout: number, isHook: boolean) => // the original values in the variables before we require any files. const {setTimeout, clearTimeout} = global; -export const callAsyncCircusFn = ( +function checkIsError(error: any): error is Error { + return !!(error && (error as Error).message && (error as Error).stack); +} + +export const callAsyncCircusFn = async ( fn: AsyncFn, testContext: TestContext | undefined, {isHook, timeout}: {isHook?: boolean | null; timeout: number}, ): Promise => { - let timeoutID: NodeJS.Timeout; - - return new Promise((resolve, reject) => { - timeoutID = setTimeout( - () => reject(_makeTimeoutMessage(timeout, !!isHook)), - timeout, - ); - - // If this fn accepts `done` callback we return a promise that fulfills as - // soon as `done` called. - if (fn.length) { - const done = (reason?: Error | string): void => { - const isError = - reason && (reason as Error).message && (reason as Error).stack; - return reason - ? reject( - isError - ? reason - : new Error(`Failed: ${prettyFormat(reason, {maxDepth: 3})}`), - ) - : resolve(); - }; - - return fn.call(testContext, done); - } + let timeoutID: NodeJS.Timeout | undefined; + let completed = false; + + try { + return await new Promise((resolve, reject) => { + timeoutID = setTimeout( + () => reject(_makeTimeoutMessage(timeout, !!isHook)), + timeout, + ); + + // If this fn accepts `done` callback we return a promise that fulfills as + // soon as `done` called. + if (fn.length) { + const done = (reason?: Error | string): void => { + const errorAsErrorObject = checkIsError(reason) + ? reason + : new Error(`Failed: ${prettyFormat(reason, {maxDepth: 3})}`); + + // Consider always throwing, regardless if `reason` is set or not + if (completed && reason) { + errorAsErrorObject.message = + 'Caught error after test environment was torn down\n\n' + + errorAsErrorObject.message; + + throw errorAsErrorObject; + } - let returnedValue; - if (isGeneratorFn(fn)) { - returnedValue = co.wrap(fn).call({}); - } else { - try { - returnedValue = fn.call(testContext); - } catch (error) { - return reject(error); + return reason ? reject(errorAsErrorObject) : resolve(); + }; + + return fn.call(testContext, done); } - } - // If it's a Promise, return it. Test for an object with a `then` function - // to support custom Promise implementations. - if ( - typeof returnedValue === 'object' && - returnedValue !== null && - typeof returnedValue.then === 'function' - ) { - return returnedValue.then(resolve, reject); - } + let returnedValue; + if (isGeneratorFn(fn)) { + returnedValue = co.wrap(fn).call({}); + } else { + try { + returnedValue = fn.call(testContext); + } catch (error) { + return reject(error); + } + } + + // If it's a Promise, return it. Test for an object with a `then` function + // to support custom Promise implementations. + if ( + typeof returnedValue === 'object' && + returnedValue !== null && + typeof returnedValue.then === 'function' + ) { + return returnedValue.then(resolve, reject); + } - if (!isHook && returnedValue !== void 0) { - return reject( - new Error( - ` + if (!isHook && returnedValue !== void 0) { + return reject( + new Error( + ` test functions can only return Promise or undefined. Returned value: ${String(returnedValue)} `, - ), - ); - } + ), + ); + } - // Otherwise this test is synchronous, and if it didn't throw it means - // it passed. - return resolve(); - }) - .then(() => { - // If timeout is not cleared/unrefed the node process won't exit until - // it's resolved. - timeoutID.unref && timeoutID.unref(); - clearTimeout(timeoutID); - }) - .catch(error => { + // Otherwise this test is synchronous, and if it didn't throw it means + // it passed. + return resolve(); + }); + } finally { + completed = true; + // If timeout is not cleared/unrefed the node process won't exit until + // it's resolved. + if (timeoutID) { timeoutID.unref && timeoutID.unref(); clearTimeout(timeoutID); - throw error; - }); + } + } }; export const getTestDuration = (test: TestEntry): number | null => { diff --git a/packages/jest-jasmine2/src/jasmine/Env.js b/packages/jest-jasmine2/src/jasmine/Env.js index 49e8963980b7..c1e6b70992b0 100644 --- a/packages/jest-jasmine2/src/jasmine/Env.js +++ b/packages/jest-jasmine2/src/jasmine/Env.js @@ -586,13 +586,24 @@ export default function(j$) { message = check.message; } - currentRunnable().addExpectationResult(false, { + const errorAsErrorObject = checkIsError ? error : new Error(message); + const runnable = currentRunnable(); + + if (!runnable) { + errorAsErrorObject.message = + 'Caught error after test environment was torn down\n\n' + + errorAsErrorObject.message; + + throw errorAsErrorObject; + } + + runnable.addExpectationResult(false, { matcherName: '', passed: false, expected: '', actual: '', message, - error: checkIsError ? error : new Error(message), + error: errorAsErrorObject, }); }; }