Fix missing error after test (#8005)

CHANGELOG.md

@@ -26,6 +26,8 @@
 - `[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))
 - `[jest-haste-map]` Enforce uniqueness in names (mocks and haste ids) ([#8002](https://github.com/facebook/jest/pull/8002))
 - `[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
 

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"/);
+});

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

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<any> => {
-  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 => {

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,
       });
     };
   }