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 missing error after test #8005

Merged
merged 4 commits into from Feb 28, 2019
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 @@ -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

Expand Down
13 changes: 11 additions & 2 deletions e2e/__tests__/failures.test.ts
Expand Up @@ -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, '')
Expand Down Expand Up @@ -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"/);
});
14 changes: 14 additions & 0 deletions 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();
});
137 changes: 73 additions & 64 deletions packages/jest-circus/src/utils.ts
Expand Up @@ -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 => {
Expand Down
15 changes: 13 additions & 2 deletions packages/jest-jasmine2/src/jasmine/Env.js
Expand Up @@ -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,
});
};
}
Expand Down