Skip to content

Commit dc0d757

Browse 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 873c2bc commit dc0d757

3 files changed

Lines changed: 67 additions & 23 deletions

File tree

‎lib/internal/test_runner/test.js‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -777,7 +777,13 @@ class Test extends AsyncResource {
777777
loc: [child.line,child.column,child.file],
778778
},noop);
779779
t.endTime=t.startTime=hrtime();
780-
t.start();
780+
// For suites, Suite.run() starts the subtests via SafePromiseAll.
781+
// Starting them here as well would run them twice, re-invoking the
782+
// synthetic children-creator against a now-incremented disambiguator
783+
// and producing spurious failures.
784+
if(this.reportedType!=='suite'){
785+
t.start();
786+
}
781787
}
782788
};
783789
}

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

Lines 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+
functionambiguousHelper(t){
58+
returnPromise.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+
});

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

Lines 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 }=awaitcommon.spawnPromisified(process.execPath,args);
82101
assert.strictEqual(code,1);
83102
assert.strictEqual(signal,null);
84-
assert.match(stdout,/pass11/);
103+
assert.match(stdout,/pass17/);
85104
assert.match(stdout,/fail4/);
86-
assert.match(stdout,/suites1/);
105+
assert.match(stdout,/suites2/);
87106
assert.deepStrictEqual(awaitgetStateFile(),expectedStateFile.slice(0,1));
88107

89108
({ code, stdout, signal }=awaitcommon.spawnPromisified(process.execPath,args));
90109
assert.strictEqual(code,1);
91110
assert.strictEqual(signal,null);
92-
assert.match(stdout,/pass13/);
111+
assert.match(stdout,/pass18/);
93112
assert.match(stdout,/fail3/);
94-
assert.match(stdout,/suites1/);
113+
assert.match(stdout,/suites2/);
95114
assert.deepStrictEqual(awaitgetStateFile(),expectedStateFile.slice(0,2));
96115

97116

98117
({ code, stdout, signal }=awaitcommon.spawnPromisified(process.execPath,args));
99118
assert.strictEqual(code,0);
100119
assert.strictEqual(signal,null);
101-
assert.match(stdout,/pass18/);
120+
assert.match(stdout,/pass21/);
102121
assert.match(stdout,/fail0/);
103-
assert.match(stdout,/suites1/);
122+
assert.match(stdout,/suites2/);
104123
assert.deepStrictEqual(awaitgetStateFile(),expectedStateFile);
105124
});
106125

@@ -110,32 +129,32 @@ test('test should pass on third rerun with `--test`', async () => {
110129
let{ code, stdout, signal }=awaitcommon.spawnPromisified(process.execPath,args);
111130
assert.strictEqual(code,1);
112131
assert.strictEqual(signal,null);
113-
assert.match(stdout,/pass11/);
132+
assert.match(stdout,/pass17/);
114133
assert.match(stdout,/fail4/);
115-
assert.match(stdout,/suites1/);
134+
assert.match(stdout,/suites2/);
116135
assert.deepStrictEqual(awaitgetStateFile(),expectedStateFile.slice(0,1));
117136

118137
({ code, stdout, signal }=awaitcommon.spawnPromisified(process.execPath,args));
119138
assert.strictEqual(code,1);
120139
assert.strictEqual(signal,null);
121-
assert.match(stdout,/pass13/);
140+
assert.match(stdout,/pass18/);
122141
assert.match(stdout,/fail3/);
123-
assert.match(stdout,/suites1/);
142+
assert.match(stdout,/suites2/);
124143
assert.deepStrictEqual(awaitgetStateFile(),expectedStateFile.slice(0,2));
125144

126145

127146
({ code, stdout, signal }=awaitcommon.spawnPromisified(process.execPath,args));
128147
assert.strictEqual(code,0);
129148
assert.strictEqual(signal,null);
130-
assert.match(stdout,/pass18/);
149+
assert.match(stdout,/pass21/);
131150
assert.match(stdout,/fail0/);
132-
assert.match(stdout,/suites1/);
151+
assert.match(stdout,/suites2/);
133152
assert.deepStrictEqual(awaitgetStateFile(),expectedStateFile);
134153
});
135154

136155
test('using `run` api',async()=>{
137156
letstream=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)