Skip to content

Commit e8e8146

Browse files
authored
align circus with jasmine's top-to-bottom execution order (#9965)
1 parent 968a301 commit e8e8146

File tree

7 files changed

+167
-90
lines changed

7 files changed

+167
-90
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
- `[jest-circus, jest-console, jest-jasmine2, jest-reporters, jest-util, pretty-format]` Fix time durating formatting and consolidate time formatting code ([#9765](https://github.com/facebook/jest/pull/9765))
1313
- `[jest-circus]` [**BREAKING**] Fail tests if a test takes a done callback and have return values ([#9129](https://github.com/facebook/jest/pull/9129))
1414
- `[jest-circus]` [**BREAKING**] Throw a proper error if a test / hook is defined asynchronously ([#8096](https://github.com/facebook/jest/pull/8096))
15+
- `[jest-circus]` [**BREAKING**] Align execution order of tests to match `jasmine`'s top to bottom order ([#9965](https://github.com/facebook/jest/pull/9965))
1516
- `[jest-config, jest-resolve]` [**BREAKING**] Remove support for `browser` field ([#9943](https://github.com/facebook/jest/pull/9943))
1617
- `[jest-haste-map]` Stop reporting files as changed when they are only accessed ([#7347](https://github.com/facebook/jest/pull/7347))
1718
- `[jest-resolve]` Show relative path from root dir for `module not found` errors ([#9963](https://github.com/facebook/jest/pull/9963))

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,22 @@ Time: <<REPLACED>>
9595
Ran all test suites.
9696
`;
9797
98+
exports[`interleaved describe and test children order 1`] = `
99+
PASS __tests__/interleaved.test.js
100+
✓ above
101+
✓ below
102+
describe
103+
✓ inside
104+
`;
105+
106+
exports[`interleaved describe and test children order 2`] = `
107+
Test Suites: 1 passed, 1 total
108+
Tests: 3 passed, 3 total
109+
Snapshots: 0 total
110+
Time: <<REPLACED>>
111+
Ran all test suites.
112+
`;
113+
98114
exports[`only 1`] = `
99115
PASS __tests__/onlyConstructs.test.js
100116
✓ test.only

e2e/__tests__/globals.test.ts

Lines changed: 45 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -45,11 +45,49 @@ test('basic test constructs', () => {
4545

4646
writeFiles(TEST_DIR, {[filename]: content});
4747
const {stderr, exitCode} = runJest(DIR);
48+
49+
const {summary, rest} = extractSummary(stderr);
50+
expect(wrap(rest)).toMatchSnapshot();
51+
expect(wrap(summary)).toMatchSnapshot();
4852
expect(exitCode).toBe(0);
53+
});
54+
55+
test('interleaved describe and test children order', () => {
56+
const filename = 'interleaved.test.js';
57+
const content = `
58+
let lastTest;
59+
test('above', () => {
60+
try {
61+
expect(lastTest).toBe(undefined);
62+
} finally {
63+
lastTest = 'above';
64+
}
65+
});
66+
describe('describe', () => {
67+
test('inside', () => {
68+
try {
69+
expect(lastTest).toBe('above');
70+
} finally {
71+
lastTest = 'inside';
72+
}
73+
});
74+
});
75+
test('below', () => {
76+
try {
77+
expect(lastTest).toBe('inside');
78+
} finally {
79+
lastTest = 'below';
80+
}
81+
});
82+
`;
83+
84+
writeFiles(TEST_DIR, {[filename]: content});
85+
const {stderr, exitCode} = runJest(DIR);
4986

5087
const {summary, rest} = extractSummary(stderr);
5188
expect(wrap(rest)).toMatchSnapshot();
5289
expect(wrap(summary)).toMatchSnapshot();
90+
expect(exitCode).toBe(0);
5391
});
5492

5593
test('skips', () => {
@@ -106,11 +144,11 @@ test('only', () => {
106144

107145
writeFiles(TEST_DIR, {[filename]: content});
108146
const {stderr, exitCode} = runJest(DIR);
109-
expect(exitCode).toBe(0);
110147

111148
const {summary, rest} = extractSummary(stderr);
112149
expect(wrap(rest)).toMatchSnapshot();
113150
expect(wrap(summary)).toMatchSnapshot();
151+
expect(exitCode).toBe(0);
114152
});
115153

116154
test('cannot have describe with no implementation', () => {
@@ -121,13 +159,13 @@ test('cannot have describe with no implementation', () => {
121159

122160
writeFiles(TEST_DIR, {[filename]: content});
123161
const {stderr, exitCode} = runJest(DIR);
124-
expect(exitCode).toBe(1);
125162

126163
const rest = cleanStderr(stderr);
127164
const {summary} = extractSummary(stderr);
128165

129166
expect(wrap(rest)).toMatchSnapshot();
130167
expect(wrap(summary)).toMatchSnapshot();
168+
expect(exitCode).toBe(1);
131169
});
132170

133171
test('cannot test with no implementation', () => {
@@ -140,11 +178,11 @@ test('cannot test with no implementation', () => {
140178

141179
writeFiles(TEST_DIR, {[filename]: content});
142180
const {stderr, exitCode} = runJest(DIR);
143-
expect(exitCode).toBe(1);
144181

145182
const {summary} = extractSummary(stderr);
146183
expect(wrap(cleanStderr(stderr))).toMatchSnapshot();
147184
expect(wrap(summary)).toMatchSnapshot();
185+
expect(exitCode).toBe(1);
148186
});
149187

150188
test('skips with expand arg', () => {
@@ -171,11 +209,11 @@ test('skips with expand arg', () => {
171209

172210
writeFiles(TEST_DIR, {[filename]: content});
173211
const {stderr, exitCode} = runJest(DIR, ['--expand']);
174-
expect(exitCode).toBe(0);
175212

176213
const {summary, rest} = extractSummary(stderr);
177214
expect(wrap(rest)).toMatchSnapshot();
178215
expect(wrap(summary)).toMatchSnapshot();
216+
expect(exitCode).toBe(0);
179217
});
180218

181219
test('only with expand arg', () => {
@@ -201,11 +239,11 @@ test('only with expand arg', () => {
201239

202240
writeFiles(TEST_DIR, {[filename]: content});
203241
const {stderr, exitCode} = runJest(DIR, ['--expand']);
204-
expect(exitCode).toBe(0);
205242

206243
const {summary, rest} = extractSummary(stderr);
207244
expect(wrap(rest)).toMatchSnapshot();
208245
expect(wrap(summary)).toMatchSnapshot();
246+
expect(exitCode).toBe(0);
209247
});
210248

211249
test('cannot test with no implementation with expand arg', () => {
@@ -218,11 +256,11 @@ test('cannot test with no implementation with expand arg', () => {
218256

219257
writeFiles(TEST_DIR, {[filename]: content});
220258
const {stderr, exitCode} = runJest(DIR, ['--expand']);
221-
expect(exitCode).toBe(1);
222259

223260
const {summary} = extractSummary(stderr);
224261
expect(wrap(cleanStderr(stderr))).toMatchSnapshot();
225262
expect(wrap(summary)).toMatchSnapshot();
263+
expect(exitCode).toBe(1);
226264
});
227265

228266
test('function as descriptor', () => {
@@ -236,9 +274,9 @@ test('function as descriptor', () => {
236274

237275
writeFiles(TEST_DIR, {[filename]: content});
238276
const {stderr, exitCode} = runJest(DIR);
239-
expect(exitCode).toBe(0);
240277

241278
const {summary, rest} = extractSummary(stderr);
242279
expect(wrap(rest)).toMatchSnapshot();
243280
expect(wrap(summary)).toMatchSnapshot();
281+
expect(exitCode).toBe(0);
244282
});

packages/jest-circus/src/eventHandler.ts

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -60,24 +60,26 @@ const eventHandler: Circus.EventHandler = (
6060
});
6161
}
6262

63-
// inherit mode from its parent describe but
64-
// do not inherit "only" mode when there is already tests with "only" mode
65-
const shouldInheritMode = !(
63+
// pass mode of currentDescribeBlock to tests
64+
// but do not when there is already a single test with "only" mode
65+
const shouldPassMode = !(
6666
currentDescribeBlock.mode === 'only' &&
67-
currentDescribeBlock.tests.find(test => test.mode === 'only')
67+
currentDescribeBlock.children.some(
68+
child => child.type === 'test' && child.mode === 'only',
69+
)
6870
);
69-
70-
if (shouldInheritMode) {
71-
currentDescribeBlock.tests.forEach(test => {
72-
if (!test.mode) {
73-
test.mode = currentDescribeBlock.mode;
71+
if (shouldPassMode) {
72+
currentDescribeBlock.children.forEach(child => {
73+
if (child.type === 'test' && !child.mode) {
74+
child.mode = currentDescribeBlock.mode;
7475
}
7576
});
7677
}
77-
7878
if (
7979
!state.hasFocusedTests &&
80-
currentDescribeBlock.tests.some(test => test.mode === 'only')
80+
currentDescribeBlock.children.some(
81+
child => child.type === 'test' && child.mode === 'only',
82+
)
8183
) {
8284
state.hasFocusedTests = true;
8385
}
@@ -129,7 +131,7 @@ const eventHandler: Circus.EventHandler = (
129131
if (test.mode === 'only') {
130132
state.hasFocusedTests = true;
131133
}
132-
currentDescribeBlock.tests.push(test);
134+
currentDescribeBlock.children.push(test);
133135
break;
134136
}
135137
case 'hook_failure': {

packages/jest-circus/src/run.ts

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -43,16 +43,25 @@ const _runTestsForDescribeBlock = async (
4343
const retryTimes = parseInt(global[RETRY_TIMES], 10) || 0;
4444
const deferredRetryTests = [];
4545

46-
for (const test of describeBlock.tests) {
47-
const hasErrorsBeforeTestRun = test.errors.length > 0;
48-
await _runTest(test);
49-
50-
if (
51-
hasErrorsBeforeTestRun === false &&
52-
retryTimes > 0 &&
53-
test.errors.length > 0
54-
) {
55-
deferredRetryTests.push(test);
46+
for (const child of describeBlock.children) {
47+
switch (child.type) {
48+
case 'describeBlock': {
49+
await _runTestsForDescribeBlock(child);
50+
break;
51+
}
52+
case 'test': {
53+
const hasErrorsBeforeTestRun = child.errors.length > 0;
54+
await _runTest(child);
55+
56+
if (
57+
hasErrorsBeforeTestRun === false &&
58+
retryTimes > 0 &&
59+
child.errors.length > 0
60+
) {
61+
deferredRetryTests.push(child);
62+
}
63+
break;
64+
}
5665
}
5766
}
5867

@@ -69,10 +78,6 @@ const _runTestsForDescribeBlock = async (
6978
}
7079
}
7180

72-
for (const child of describeBlock.children) {
73-
await _runTestsForDescribeBlock(child);
74-
}
75-
7681
for (const hook of afterAll) {
7782
await _callCircusHook({describeBlock, hook});
7883
}

0 commit comments

Comments
 (0)