diff --git a/CHANGELOG.md b/CHANGELOG.md index 57c0f6692aa5..9704759fe201 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ### Features +- `[jest-circus]` [**BREAKING**] Fail tests when multiple `done()` calls are made ([#10624](https://github.com/facebook/jest/pull/10624)) - `[jest-circus, jest-jasmine2]` [**BREAKING**] Fail the test instead of just warning when describe returns a value ([#10947](https://github.com/facebook/jest/pull/10947)) - `[jest-config]` [**BREAKING**] Default to Node testing environment instead of browser (JSDOM) ([#9874](https://github.com/facebook/jest/pull/9874)) - `[jest-config]` [**BREAKING**] Use `jest-circus` as default test runner ([#10686](https://github.com/facebook/jest/pull/10686)) diff --git a/e2e/__tests__/__snapshots__/callDoneTwice.test.ts.snap b/e2e/__tests__/__snapshots__/callDoneTwice.test.ts.snap new file mode 100644 index 000000000000..ef03864dc154 --- /dev/null +++ b/e2e/__tests__/__snapshots__/callDoneTwice.test.ts.snap @@ -0,0 +1,89 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`\`done()\` should not be called more than once 1`] = ` +FAIL __tests__/index.test.js + ● \`done()\` called more than once › should fail + + Expected done to be called once, but it was called multiple times. + + 8 | it('should fail', done => { + 9 | done(); + > 10 | done(); + | ^ + 11 | }); + 12 | + 13 | it('should fail inside a promise', done => { + + at Object.done (__tests__/index.test.js:10:5) + + ● \`done()\` called more than once › should fail inside a promise + + Expected done to be called once, but it was called multiple times. + + 15 | .then(() => { + 16 | done(); + > 17 | done(); + | ^ + 18 | }) + 19 | .catch(err => err); + 20 | }); + + at done (__tests__/index.test.js:17:9) + + ● multiple \`done()\` inside beforeEach › should fail + + Expected done to be called once, but it was called multiple times. + + 24 | beforeEach(done => { + 25 | done(); + > 26 | done(); + | ^ + 27 | }); + 28 | + 29 | it('should fail', () => { + + at Object.done (__tests__/index.test.js:26:5) + + ● multiple \`done()\` inside afterEach › should fail + + Expected done to be called once, but it was called multiple times. + + 35 | afterEach(done => { + 36 | done(); + > 37 | done(); + | ^ + 38 | }); + 39 | + 40 | it('should fail', () => { + + at Object.done (__tests__/index.test.js:37:5) + + ● multiple \`done()\` inside beforeAll › should fail + + Expected done to be called once, but it was called multiple times. + + 46 | beforeAll(done => { + 47 | done(); + > 48 | done(); + | ^ + 49 | }); + 50 | + 51 | it('should fail', () => { + + at done (__tests__/index.test.js:48:5) + + + ● Test suite failed to run + + Expected done to be called once, but it was called multiple times. + + 57 | afterAll(done => { + 58 | done(); + > 59 | done(); + | ^ + 60 | }); + 61 | + 62 | it('should fail', () => { + + at done (__tests__/index.test.js:59:5) +`; diff --git a/e2e/__tests__/__snapshots__/circusDeclarationErrors.test.ts.snap b/e2e/__tests__/__snapshots__/circusDeclarationErrors.test.ts.snap index 1d1d382ab232..f58244619e04 100644 --- a/e2e/__tests__/__snapshots__/circusDeclarationErrors.test.ts.snap +++ b/e2e/__tests__/__snapshots__/circusDeclarationErrors.test.ts.snap @@ -16,7 +16,7 @@ FAIL __tests__/asyncDefinition.test.js 14 | }); 15 | }); - at eventHandler (../../packages/jest-circus/build/eventHandler.js:144:11) + at eventHandler (../../packages/jest-circus/build/eventHandler.js:146:11) at test (__tests__/asyncDefinition.test.js:12:5) ● Test suite failed to run @@ -31,7 +31,7 @@ FAIL __tests__/asyncDefinition.test.js 15 | }); 16 | - at eventHandler (../../packages/jest-circus/build/eventHandler.js:113:11) + at eventHandler (../../packages/jest-circus/build/eventHandler.js:114:11) at afterAll (__tests__/asyncDefinition.test.js:13:5) ● Test suite failed to run @@ -46,7 +46,7 @@ FAIL __tests__/asyncDefinition.test.js 20 | }); 21 | - at eventHandler (../../packages/jest-circus/build/eventHandler.js:144:11) + at eventHandler (../../packages/jest-circus/build/eventHandler.js:146:11) at test (__tests__/asyncDefinition.test.js:18:3) ● Test suite failed to run @@ -60,6 +60,6 @@ FAIL __tests__/asyncDefinition.test.js 20 | }); 21 | - at eventHandler (../../packages/jest-circus/build/eventHandler.js:113:11) + at eventHandler (../../packages/jest-circus/build/eventHandler.js:114:11) at afterAll (__tests__/asyncDefinition.test.js:19:3) `; diff --git a/e2e/__tests__/callDoneTwice.test.ts b/e2e/__tests__/callDoneTwice.test.ts new file mode 100644 index 000000000000..10d84ffc1ddf --- /dev/null +++ b/e2e/__tests__/callDoneTwice.test.ts @@ -0,0 +1,18 @@ +/** + * 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. + */ +import wrap from 'jest-snapshot-serializer-raw'; +import {skipSuiteOnJasmine} from '@jest/test-utils'; +import {extractSummary} from '../Utils'; +import runJest from '../runJest'; + +skipSuiteOnJasmine(); +test('`done()` should not be called more than once', () => { + const {exitCode, stderr} = runJest('call-done-twice'); + const {rest} = extractSummary(stderr); + expect(wrap(rest)).toMatchSnapshot(); + expect(exitCode).toBe(1); +}); diff --git a/e2e/__tests__/doneInHooks.test.ts b/e2e/__tests__/doneInHooks.test.ts new file mode 100644 index 000000000000..7f43632e4ad1 --- /dev/null +++ b/e2e/__tests__/doneInHooks.test.ts @@ -0,0 +1,15 @@ +/** + * 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. + */ + +import {skipSuiteOnJasmine} from '@jest/test-utils'; +import runJest from '../runJest'; + +skipSuiteOnJasmine(); +test('`done()` works properly in hooks', () => { + const {exitCode} = runJest('done-in-hooks'); + expect(exitCode).toEqual(0); +}); diff --git a/e2e/call-done-twice/__tests__/index.test.js b/e2e/call-done-twice/__tests__/index.test.js new file mode 100644 index 000000000000..5f0e25005ff0 --- /dev/null +++ b/e2e/call-done-twice/__tests__/index.test.js @@ -0,0 +1,65 @@ +/** + * 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. + */ +describe('`done()` called more than once', () => { + it('should fail', done => { + done(); + done(); + }); + + it('should fail inside a promise', done => { + Promise.resolve() + .then(() => { + done(); + done(); + }) + .catch(err => err); + }); +}); + +describe('multiple `done()` inside beforeEach', () => { + beforeEach(done => { + done(); + done(); + }); + + it('should fail', () => { + expect('foo').toMatch('foo'); + }); +}); + +describe('multiple `done()` inside afterEach', () => { + afterEach(done => { + done(); + done(); + }); + + it('should fail', () => { + expect('foo').toMatch('foo'); + }); +}); + +describe('multiple `done()` inside beforeAll', () => { + beforeAll(done => { + done(); + done(); + }); + + it('should fail', () => { + expect('foo').toMatch('foo'); + }); +}); + +describe('multiple `done()` inside afterAll', () => { + afterAll(done => { + done(); + done(); + }); + + it('should fail', () => { + expect('foo').toMatch('foo'); + }); +}); diff --git a/e2e/call-done-twice/package.json b/e2e/call-done-twice/package.json new file mode 100644 index 000000000000..e22eaf20573d --- /dev/null +++ b/e2e/call-done-twice/package.json @@ -0,0 +1,5 @@ +{ + "jest": { + "testEnvironment": "node" + } +} diff --git a/e2e/done-in-hooks/__tests__/index.test.js b/e2e/done-in-hooks/__tests__/index.test.js new file mode 100644 index 000000000000..b042dec1bf83 --- /dev/null +++ b/e2e/done-in-hooks/__tests__/index.test.js @@ -0,0 +1,16 @@ +/** + * 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. + * + */ +describe('`done()` should work with hooks', done => { + beforeEach(done => done()); + it('foo', () => { + expect('foo').toMatch('foo'); + }); + it('bar', () => { + expect('bar').toMatch('bar'); + }); +}); diff --git a/e2e/done-in-hooks/package.json b/e2e/done-in-hooks/package.json new file mode 100644 index 000000000000..148788b25446 --- /dev/null +++ b/e2e/done-in-hooks/package.json @@ -0,0 +1,5 @@ +{ + "jest": { + "testEnvironment": "node" + } +} diff --git a/packages/jest-circus/src/eventHandler.ts b/packages/jest-circus/src/eventHandler.ts index af46770bcc62..017c72599888 100644 --- a/packages/jest-circus/src/eventHandler.ts +++ b/packages/jest-circus/src/eventHandler.ts @@ -31,6 +31,7 @@ const eventHandler: Circus.EventHandler = ( break; } case 'hook_start': { + event.hook.seenDone = false; break; } case 'start_describe_definition': { @@ -113,7 +114,14 @@ const eventHandler: Circus.EventHandler = ( } const parent = currentDescribeBlock; - currentDescribeBlock.hooks.push({asyncError, fn, parent, timeout, type}); + currentDescribeBlock.hooks.push({ + asyncError, + fn, + parent, + seenDone: false, + timeout, + type, + }); break; } case 'add_test': { @@ -188,6 +196,10 @@ const eventHandler: Circus.EventHandler = ( event.test.invocations += 1; break; } + case 'test_fn_start': { + event.test.seenDone = false; + break; + } case 'test_fn_failure': { const { error, diff --git a/packages/jest-circus/src/utils.ts b/packages/jest-circus/src/utils.ts index ae8a6695eead..ea0f1858fc60 100644 --- a/packages/jest-circus/src/utils.ts +++ b/packages/jest-circus/src/utils.ts @@ -70,6 +70,7 @@ export const makeTest = ( mode, name: convertDescriptorToString(name), parent, + seenDone: false, startedAt: null, status: null, timeout, @@ -189,9 +190,25 @@ export const callAsyncCircusFn = ( // soon as `done` called. if (takesDoneCallback(fn)) { let returnedValue: unknown = undefined; + const done = (reason?: Error | string): void => { // We need to keep a stack here before the promise tick const errorAtDone = new ErrorWithStack(undefined, done); + + if (!completed && testOrHook.seenDone) { + errorAtDone.message = + 'Expected done to be called once, but it was called multiple times.'; + + if (reason) { + errorAtDone.message += + ' Reason: ' + prettyFormat(reason, {maxDepth: 3}); + } + reject(errorAtDone); + throw errorAtDone; + } else { + testOrHook.seenDone = true; + } + // Use `Promise.resolve` to allow the event loop to go a single tick in case `done` is called synchronously Promise.resolve().then(() => { if (returnedValue !== undefined) { @@ -203,7 +220,6 @@ export const callAsyncCircusFn = ( } let errorAsErrorObject: Error; - if (checkIsError(reason)) { errorAsErrorObject = reason; } else { @@ -274,12 +290,12 @@ export const callAsyncCircusFn = ( completed = true; // If timeout is not cleared/unrefed the node process won't exit until // it's resolved. - timeoutID.unref && timeoutID.unref(); + timeoutID.unref?.(); clearTimeout(timeoutID); }) .catch(error => { completed = true; - timeoutID.unref && timeoutID.unref(); + timeoutID.unref?.(); clearTimeout(timeoutID); throw error; }); diff --git a/packages/jest-core/src/__tests__/SearchSource.test.ts b/packages/jest-core/src/__tests__/SearchSource.test.ts index af43fd464335..eccd32d78c45 100644 --- a/packages/jest-core/src/__tests__/SearchSource.test.ts +++ b/packages/jest-core/src/__tests__/SearchSource.test.ts @@ -415,7 +415,7 @@ describe('SearchSource', () => { ); const rootPath = path.join(rootDir, 'root.js'); - beforeEach(done => { + beforeEach(async () => { const {options: config} = normalize( { haste: { @@ -435,12 +435,11 @@ describe('SearchSource', () => { }, {} as Config.Argv, ); - Runtime.createContext(config, {maxWorkers, watchman: false}).then( - context => { - searchSource = new SearchSource(context); - done(); - }, - ); + const context = await Runtime.createContext(config, { + maxWorkers, + watchman: false, + }); + searchSource = new SearchSource(context); }); it('makes sure a file is related to itself', () => { @@ -477,7 +476,7 @@ describe('SearchSource', () => { }); describe('findRelatedTestsFromPattern', () => { - beforeEach(done => { + beforeEach(async () => { const {options: config} = normalize( { moduleFileExtensions: ['js', 'jsx', 'foobar'], @@ -487,12 +486,11 @@ describe('SearchSource', () => { }, {} as Config.Argv, ); - Runtime.createContext(config, {maxWorkers, watchman: false}).then( - context => { - searchSource = new SearchSource(context); - done(); - }, - ); + const context = await Runtime.createContext(config, { + maxWorkers, + watchman: false, + }); + searchSource = new SearchSource(context); }); it('returns empty search result for empty input', () => { diff --git a/packages/jest-types/src/Circus.ts b/packages/jest-types/src/Circus.ts index 3dab1d8d7abe..d93e653d7822 100644 --- a/packages/jest-types/src/Circus.ts +++ b/packages/jest-types/src/Circus.ts @@ -28,6 +28,7 @@ export type Hook = { fn: HookFn; type: HookType; parent: DescribeBlock; + seenDone: boolean; timeout: number | undefined | null; }; @@ -238,6 +239,7 @@ export type TestEntry = { parent: DescribeBlock; startedAt?: number | null; duration?: number | null; + seenDone: boolean; status?: TestStatus | null; // whether the test has been skipped or run already timeout?: number; };