Skip to content

Commit 33b8df1

Browse files
authored
feat: fail tests when multiple done() calls are made (#10624)
1 parent e32e274 commit 33b8df1

File tree

13 files changed

+264
-22
lines changed

13 files changed

+264
-22
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
### Features
44

5+
- `[jest-circus]` [**BREAKING**] Fail tests when multiple `done()` calls are made ([#10624](https://github.com/facebook/jest/pull/10624))
56
- `[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))
67
- `[jest-config]` [**BREAKING**] Default to Node testing environment instead of browser (JSDOM) ([#9874](https://github.com/facebook/jest/pull/9874))
78
- `[jest-config]` [**BREAKING**] Use `jest-circus` as default test runner ([#10686](https://github.com/facebook/jest/pull/10686))
Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
// Jest Snapshot v1, https://goo.gl/fbAQLP
2+
3+
exports[`\`done()\` should not be called more than once 1`] = `
4+
FAIL __tests__/index.test.js
5+
\`done()\` called more than once › should fail
6+
7+
Expected done to be called once, but it was called multiple times.
8+
9+
8 | it('should fail', done => {
10+
9 | done();
11+
> 10 | done();
12+
| ^
13+
11 | });
14+
12 |
15+
13 | it('should fail inside a promise', done => {
16+
17+
at Object.done (__tests__/index.test.js:10:5)
18+
19+
● \`done()\` called more than once › should fail inside a promise
20+
21+
Expected done to be called once, but it was called multiple times.
22+
23+
15 | .then(() => {
24+
16 | done();
25+
> 17 | done();
26+
| ^
27+
18 | })
28+
19 | .catch(err => err);
29+
20 | });
30+
31+
at done (__tests__/index.test.js:17:9)
32+
33+
● multiple \`done()\` inside beforeEach › should fail
34+
35+
Expected done to be called once, but it was called multiple times.
36+
37+
24 | beforeEach(done => {
38+
25 | done();
39+
> 26 | done();
40+
| ^
41+
27 | });
42+
28 |
43+
29 | it('should fail', () => {
44+
45+
at Object.done (__tests__/index.test.js:26:5)
46+
47+
multiple \`done()\` inside afterEach › should fail
48+
49+
Expected done to be called once, but it was called multiple times.
50+
51+
35 | afterEach(done => {
52+
36 | done();
53+
> 37 | done();
54+
| ^
55+
38 | });
56+
39 |
57+
40 | it('should fail', () => {
58+
59+
at Object.done (__tests__/index.test.js:37:5)
60+
61+
multiple \`done()\` inside beforeAll › should fail
62+
63+
Expected done to be called once, but it was called multiple times.
64+
65+
46 | beforeAll(done => {
66+
47 | done();
67+
> 48 | done();
68+
| ^
69+
49 | });
70+
50 |
71+
51 | it('should fail', () => {
72+
73+
at done (__tests__/index.test.js:48:5)
74+
75+
76+
Test suite failed to run
77+
78+
Expected done to be called once, but it was called multiple times.
79+
80+
57 | afterAll(done => {
81+
58 | done();
82+
> 59 | done();
83+
| ^
84+
60 | });
85+
61 |
86+
62 | it('should fail', () => {
87+
88+
at done (__tests__/index.test.js:59:5)
89+
`;

e2e/__tests__/__snapshots__/circusDeclarationErrors.test.ts.snap

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ FAIL __tests__/asyncDefinition.test.js
1616
14 | });
1717
15 | });
1818
19-
at eventHandler (../../packages/jest-circus/build/eventHandler.js:144:11)
19+
at eventHandler (../../packages/jest-circus/build/eventHandler.js:146:11)
2020
at test (__tests__/asyncDefinition.test.js:12:5)
2121
2222
● Test suite failed to run
@@ -31,7 +31,7 @@ FAIL __tests__/asyncDefinition.test.js
3131
15 | });
3232
16 |
3333
34-
at eventHandler (../../packages/jest-circus/build/eventHandler.js:113:11)
34+
at eventHandler (../../packages/jest-circus/build/eventHandler.js:114:11)
3535
at afterAll (__tests__/asyncDefinition.test.js:13:5)
3636
3737
● Test suite failed to run
@@ -46,7 +46,7 @@ FAIL __tests__/asyncDefinition.test.js
4646
20 | });
4747
21 |
4848
49-
at eventHandler (../../packages/jest-circus/build/eventHandler.js:144:11)
49+
at eventHandler (../../packages/jest-circus/build/eventHandler.js:146:11)
5050
at test (__tests__/asyncDefinition.test.js:18:3)
5151
5252
● Test suite failed to run
@@ -60,6 +60,6 @@ FAIL __tests__/asyncDefinition.test.js
6060
20 | });
6161
21 |
6262
63-
at eventHandler (../../packages/jest-circus/build/eventHandler.js:113:11)
63+
at eventHandler (../../packages/jest-circus/build/eventHandler.js:114:11)
6464
at afterAll (__tests__/asyncDefinition.test.js:19:3)
6565
`;
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
/**
2+
* Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
import wrap from 'jest-snapshot-serializer-raw';
8+
import {skipSuiteOnJasmine} from '@jest/test-utils';
9+
import {extractSummary} from '../Utils';
10+
import runJest from '../runJest';
11+
12+
skipSuiteOnJasmine();
13+
test('`done()` should not be called more than once', () => {
14+
const {exitCode, stderr} = runJest('call-done-twice');
15+
const {rest} = extractSummary(stderr);
16+
expect(wrap(rest)).toMatchSnapshot();
17+
expect(exitCode).toBe(1);
18+
});

e2e/__tests__/doneInHooks.test.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
/**
2+
* Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
import {skipSuiteOnJasmine} from '@jest/test-utils';
9+
import runJest from '../runJest';
10+
11+
skipSuiteOnJasmine();
12+
test('`done()` works properly in hooks', () => {
13+
const {exitCode} = runJest('done-in-hooks');
14+
expect(exitCode).toEqual(0);
15+
});
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
/**
2+
* Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
describe('`done()` called more than once', () => {
8+
it('should fail', done => {
9+
done();
10+
done();
11+
});
12+
13+
it('should fail inside a promise', done => {
14+
Promise.resolve()
15+
.then(() => {
16+
done();
17+
done();
18+
})
19+
.catch(err => err);
20+
});
21+
});
22+
23+
describe('multiple `done()` inside beforeEach', () => {
24+
beforeEach(done => {
25+
done();
26+
done();
27+
});
28+
29+
it('should fail', () => {
30+
expect('foo').toMatch('foo');
31+
});
32+
});
33+
34+
describe('multiple `done()` inside afterEach', () => {
35+
afterEach(done => {
36+
done();
37+
done();
38+
});
39+
40+
it('should fail', () => {
41+
expect('foo').toMatch('foo');
42+
});
43+
});
44+
45+
describe('multiple `done()` inside beforeAll', () => {
46+
beforeAll(done => {
47+
done();
48+
done();
49+
});
50+
51+
it('should fail', () => {
52+
expect('foo').toMatch('foo');
53+
});
54+
});
55+
56+
describe('multiple `done()` inside afterAll', () => {
57+
afterAll(done => {
58+
done();
59+
done();
60+
});
61+
62+
it('should fail', () => {
63+
expect('foo').toMatch('foo');
64+
});
65+
});

e2e/call-done-twice/package.json

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"jest": {
3+
"testEnvironment": "node"
4+
}
5+
}
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
/**
2+
* Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*
7+
*/
8+
describe('`done()` should work with hooks', done => {
9+
beforeEach(done => done());
10+
it('foo', () => {
11+
expect('foo').toMatch('foo');
12+
});
13+
it('bar', () => {
14+
expect('bar').toMatch('bar');
15+
});
16+
});

e2e/done-in-hooks/package.json

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"jest": {
3+
"testEnvironment": "node"
4+
}
5+
}

packages/jest-circus/src/eventHandler.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ const eventHandler: Circus.EventHandler = (
3131
break;
3232
}
3333
case 'hook_start': {
34+
event.hook.seenDone = false;
3435
break;
3536
}
3637
case 'start_describe_definition': {
@@ -113,7 +114,14 @@ const eventHandler: Circus.EventHandler = (
113114
}
114115
const parent = currentDescribeBlock;
115116

116-
currentDescribeBlock.hooks.push({asyncError, fn, parent, timeout, type});
117+
currentDescribeBlock.hooks.push({
118+
asyncError,
119+
fn,
120+
parent,
121+
seenDone: false,
122+
timeout,
123+
type,
124+
});
117125
break;
118126
}
119127
case 'add_test': {
@@ -188,6 +196,10 @@ const eventHandler: Circus.EventHandler = (
188196
event.test.invocations += 1;
189197
break;
190198
}
199+
case 'test_fn_start': {
200+
event.test.seenDone = false;
201+
break;
202+
}
191203
case 'test_fn_failure': {
192204
const {
193205
error,

0 commit comments

Comments
 (0)