Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 3e72065

Browse filesBrowse files
MoLowaduh95
MoLow
authored and
aduh95
committed
test_runner: fix suite rerun edge case
Signed-off-by: Moshe Atlow <moshe@atlow.co.il> PR-URL: #62860 Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Jacob Smith <jacob@frende.me>
1 parent f4ea495 commit 3e72065
Copy full SHA for 3e72065

3 files changed

+67-23Lines changed: 67 additions & 23 deletions

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎lib/internal/test_runner/test.js‎

Copy file name to clipboardExpand all lines: lib/internal/test_runner/test.js
+7-1Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -765,7 +765,13 @@ class Test extends AsyncResource {
765765
loc: [child.line, child.column, child.file],
766766
}, noop);
767767
t.endTime = t.startTime = hrtime();
768-
t.start();
768+
// For suites, Suite.run() starts the subtests via SafePromiseAll.
769+
// Starting them here as well would run them twice, re-invoking the
770+
// synthetic children-creator against a now-incremented disambiguator
771+
// and producing spurious failures.
772+
if (this.reportedType !== 'suite') {
773+
t.start();
774+
}
769775
}
770776
};
771777
}
Collapse file

‎test/fixtures/test-runner/rerun.js‎

Copy file name to clipboardExpand all lines: test/fixtures/test-runner/rerun.js
+19Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,3 +46,22 @@ describe('describe rerun', { timeout: 1000, concurrency: 1000 }, () => {
4646
});
4747
test('a');
4848
});
49+
50+
51+
// Shared helper creates subtests at the same source location each time it's
52+
// called, producing ambiguous test identifiers (disambiguated with ":(N)"
53+
// suffixes in the rerun state file). Regression coverage for a bug where the
54+
// suite's synthetic rerun fn double-started its direct children, which then
55+
// re-invoked the synthetic descendant-creator against an already-incremented
56+
// disambiguator map and emitted spurious failures.
57+
function ambiguousHelper(t) {
58+
return Promise.all([
59+
t.test('shared sub A', () => {}),
60+
t.test('shared sub B', () => {}),
61+
]);
62+
}
63+
64+
describe('rerun with ambiguous shared helper', { timeout: 1000, concurrency: 1000 }, () => {
65+
test('first caller', (t) => ambiguousHelper(t));
66+
test('second caller', (t) => ambiguousHelper(t));
67+
});
Collapse file

‎test/parallel/test-runner-test-rerun-failures.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-runner-test-rerun-failures.js
+41-22Lines changed: 41 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,13 @@ const expectedStateFile = [
2727
'test/fixtures/test-runner/rerun.js:44:3': { passed_on_attempt: 0, name: 'passed on first attempt' },
2828
'test/fixtures/test-runner/rerun.js:47:3': { passed_on_attempt: 0, name: 'a' },
2929
'test/fixtures/test-runner/rerun.js:43:1': { passed_on_attempt: 0, name: 'describe rerun' },
30+
'test/fixtures/test-runner/rerun.js:59:7': { passed_on_attempt: 0, name: 'shared sub A' },
31+
'test/fixtures/test-runner/rerun.js:60:7': { passed_on_attempt: 0, name: 'shared sub B' },
32+
'test/fixtures/test-runner/rerun.js:65:3': { passed_on_attempt: 0, name: 'first caller' },
33+
'test/fixtures/test-runner/rerun.js:59:7:(1)': { passed_on_attempt: 0, name: 'shared sub A' },
34+
'test/fixtures/test-runner/rerun.js:60:7:(1)': { passed_on_attempt: 0, name: 'shared sub B' },
35+
'test/fixtures/test-runner/rerun.js:66:3': { passed_on_attempt: 0, name: 'second caller' },
36+
'test/fixtures/test-runner/rerun.js:64:1': { passed_on_attempt: 0, name: 'rerun with ambiguous shared helper' },
3037
},
3138
{
3239
'test/fixtures/test-runner/rerun.js:9:1': { passed_on_attempt: 0, name: 'ok' },
@@ -38,11 +45,17 @@ const expectedStateFile = [
3845
'test/fixtures/test-runner/rerun.js:39:1': { passed_on_attempt: 0, name: 'nested ambiguous (expectedAttempts=0)' },
3946
'test/fixtures/test-runner/rerun.js:30:16:(1)': { passed_on_attempt: 0, name: '2 levels deep' },
4047
'test/fixtures/test-runner/rerun.js:35:13:(1)': { passed_on_attempt: 0, name: 'ok' },
41-
'test/fixtures/test-runner/rerun.js:43:1': { passed_on_attempt: 0, name: 'describe rerun' },
42-
'test/fixtures/test-runner/rerun.js:44:3': { passed_on_attempt: 0, name: 'passed on first attempt' },
4348
'test/fixtures/test-runner/rerun.js:45:13': { passed_on_attempt: 0, name: 'nested' },
44-
'test/fixtures/test-runner/rerun.js:45:13:(1)': { passed_on_attempt: 1, name: 'nested' },
49+
'test/fixtures/test-runner/rerun.js:44:3': { passed_on_attempt: 0, name: 'passed on first attempt' },
4550
'test/fixtures/test-runner/rerun.js:47:3': { passed_on_attempt: 0, name: 'a' },
51+
'test/fixtures/test-runner/rerun.js:43:1': { passed_on_attempt: 0, name: 'describe rerun' },
52+
'test/fixtures/test-runner/rerun.js:59:7': { passed_on_attempt: 0, name: 'shared sub A' },
53+
'test/fixtures/test-runner/rerun.js:60:7': { passed_on_attempt: 0, name: 'shared sub B' },
54+
'test/fixtures/test-runner/rerun.js:65:3': { passed_on_attempt: 0, name: 'first caller' },
55+
'test/fixtures/test-runner/rerun.js:59:7:(1)': { passed_on_attempt: 0, name: 'shared sub A' },
56+
'test/fixtures/test-runner/rerun.js:60:7:(1)': { passed_on_attempt: 0, name: 'shared sub B' },
57+
'test/fixtures/test-runner/rerun.js:66:3': { passed_on_attempt: 0, name: 'second caller' },
58+
'test/fixtures/test-runner/rerun.js:64:1': { passed_on_attempt: 0, name: 'rerun with ambiguous shared helper' },
4659
},
4760
{
4861
'test/fixtures/test-runner/rerun.js:3:1': { passed_on_attempt: 2, name: 'should fail on first two attempts' },
@@ -53,15 +66,21 @@ const expectedStateFile = [
5366
'test/fixtures/test-runner/rerun.js:29:13': { passed_on_attempt: 0, name: 'nested' },
5467
'test/fixtures/test-runner/rerun.js:35:13': { passed_on_attempt: 0, name: 'ok' },
5568
'test/fixtures/test-runner/rerun.js:39:1': { passed_on_attempt: 0, name: 'nested ambiguous (expectedAttempts=0)' },
56-
'test/fixtures/test-runner/rerun.js:29:13:(1)': { passed_on_attempt: 2, name: 'nested' },
5769
'test/fixtures/test-runner/rerun.js:30:16:(1)': { passed_on_attempt: 0, name: '2 levels deep' },
70+
'test/fixtures/test-runner/rerun.js:29:13:(1)': { passed_on_attempt: 2, name: 'nested' },
5871
'test/fixtures/test-runner/rerun.js:35:13:(1)': { passed_on_attempt: 0, name: 'ok' },
5972
'test/fixtures/test-runner/rerun.js:40:1': { passed_on_attempt: 2, name: 'nested ambiguous (expectedAttempts=1)' },
60-
'test/fixtures/test-runner/rerun.js:43:1': { passed_on_attempt: 0, name: 'describe rerun' },
61-
'test/fixtures/test-runner/rerun.js:44:3': { passed_on_attempt: 0, name: 'passed on first attempt' },
6273
'test/fixtures/test-runner/rerun.js:45:13': { passed_on_attempt: 0, name: 'nested' },
63-
'test/fixtures/test-runner/rerun.js:45:13:(1)': { passed_on_attempt: 1, name: 'nested' },
74+
'test/fixtures/test-runner/rerun.js:44:3': { passed_on_attempt: 0, name: 'passed on first attempt' },
6475
'test/fixtures/test-runner/rerun.js:47:3': { passed_on_attempt: 0, name: 'a' },
76+
'test/fixtures/test-runner/rerun.js:43:1': { passed_on_attempt: 0, name: 'describe rerun' },
77+
'test/fixtures/test-runner/rerun.js:59:7': { passed_on_attempt: 0, name: 'shared sub A' },
78+
'test/fixtures/test-runner/rerun.js:60:7': { passed_on_attempt: 0, name: 'shared sub B' },
79+
'test/fixtures/test-runner/rerun.js:65:3': { passed_on_attempt: 0, name: 'first caller' },
80+
'test/fixtures/test-runner/rerun.js:59:7:(1)': { passed_on_attempt: 0, name: 'shared sub A' },
81+
'test/fixtures/test-runner/rerun.js:60:7:(1)': { passed_on_attempt: 0, name: 'shared sub B' },
82+
'test/fixtures/test-runner/rerun.js:66:3': { passed_on_attempt: 0, name: 'second caller' },
83+
'test/fixtures/test-runner/rerun.js:64:1': { passed_on_attempt: 0, name: 'rerun with ambiguous shared helper' },
6584
},
6685
];
6786

@@ -81,26 +100,26 @@ test('test should pass on third rerun', async () => {
81100
let { code, stdout, signal } = await common.spawnPromisified(process.execPath, args);
82101
assert.strictEqual(code, 1);
83102
assert.strictEqual(signal, null);
84-
assert.match(stdout, /pass 11/);
103+
assert.match(stdout, /pass 17/);
85104
assert.match(stdout, /fail 4/);
86-
assert.match(stdout, /suites 1/);
105+
assert.match(stdout, /suites 2/);
87106
assert.deepStrictEqual(await getStateFile(), expectedStateFile.slice(0, 1));
88107

89108
({ code, stdout, signal } = await common.spawnPromisified(process.execPath, args));
90109
assert.strictEqual(code, 1);
91110
assert.strictEqual(signal, null);
92-
assert.match(stdout, /pass 13/);
111+
assert.match(stdout, /pass 18/);
93112
assert.match(stdout, /fail 3/);
94-
assert.match(stdout, /suites 1/);
113+
assert.match(stdout, /suites 2/);
95114
assert.deepStrictEqual(await getStateFile(), expectedStateFile.slice(0, 2));
96115

97116

98117
({ code, stdout, signal } = await common.spawnPromisified(process.execPath, args));
99118
assert.strictEqual(code, 0);
100119
assert.strictEqual(signal, null);
101-
assert.match(stdout, /pass 18/);
120+
assert.match(stdout, /pass 21/);
102121
assert.match(stdout, /fail 0/);
103-
assert.match(stdout, /suites 1/);
122+
assert.match(stdout, /suites 2/);
104123
assert.deepStrictEqual(await getStateFile(), expectedStateFile);
105124
});
106125

@@ -110,32 +129,32 @@ test('test should pass on third rerun with `--test`', async () => {
110129
let { code, stdout, signal } = await common.spawnPromisified(process.execPath, args);
111130
assert.strictEqual(code, 1);
112131
assert.strictEqual(signal, null);
113-
assert.match(stdout, /pass 11/);
132+
assert.match(stdout, /pass 17/);
114133
assert.match(stdout, /fail 4/);
115-
assert.match(stdout, /suites 1/);
134+
assert.match(stdout, /suites 2/);
116135
assert.deepStrictEqual(await getStateFile(), expectedStateFile.slice(0, 1));
117136

118137
({ code, stdout, signal } = await common.spawnPromisified(process.execPath, args));
119138
assert.strictEqual(code, 1);
120139
assert.strictEqual(signal, null);
121-
assert.match(stdout, /pass 13/);
140+
assert.match(stdout, /pass 18/);
122141
assert.match(stdout, /fail 3/);
123-
assert.match(stdout, /suites 1/);
142+
assert.match(stdout, /suites 2/);
124143
assert.deepStrictEqual(await getStateFile(), expectedStateFile.slice(0, 2));
125144

126145

127146
({ code, stdout, signal } = await common.spawnPromisified(process.execPath, args));
128147
assert.strictEqual(code, 0);
129148
assert.strictEqual(signal, null);
130-
assert.match(stdout, /pass 18/);
149+
assert.match(stdout, /pass 21/);
131150
assert.match(stdout, /fail 0/);
132-
assert.match(stdout, /suites 1/);
151+
assert.match(stdout, /suites 2/);
133152
assert.deepStrictEqual(await getStateFile(), expectedStateFile);
134153
});
135154

136155
test('using `run` api', async () => {
137156
let stream = run({ files: [fixture], rerunFailuresFilePath: stateFile });
138-
stream.on('test:pass', common.mustCall(12));
157+
stream.on('test:pass', common.mustCall(19));
139158
stream.on('test:fail', common.mustCall(4));
140159

141160
// eslint-disable-next-line no-unused-vars
@@ -145,7 +164,7 @@ test('using `run` api', async () => {
145164

146165

147166
stream = run({ files: [fixture], rerunFailuresFilePath: stateFile });
148-
stream.on('test:pass', common.mustCall(14));
167+
stream.on('test:pass', common.mustCall(20));
149168
stream.on('test:fail', common.mustCall(3));
150169

151170
// eslint-disable-next-line no-unused-vars
@@ -155,7 +174,7 @@ test('using `run` api', async () => {
155174

156175

157176
stream = run({ files: [fixture], rerunFailuresFilePath: stateFile });
158-
stream.on('test:pass', common.mustCall(19));
177+
stream.on('test:pass', common.mustCall(23));
159178
stream.on('test:fail', common.mustNotCall());
160179

161180
// eslint-disable-next-line no-unused-vars

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.