Skip to content

Commit d0bda90

Browse files
committed
test: migrate test runner message tests to snapshot
PR-URL: #47392 Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
1 parent b34de64 commit d0bda90

41 files changed

Lines changed: 460 additions & 331 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.github/CODEOWNERS‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,6 @@
135135

136136
# Test runner
137137

138-
/test/message/test_runner_*@nodejs/test_runner
139138
/test/parallel/test-runner-*@nodejs/test_runner
140139
/doc/api/test.md@nodejs/test_runner
141140
/lib/test.js@nodejs/test_runner

‎test/common/README.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -642,6 +642,12 @@ environment variables.
642642
If set, `NODE_COMMON_PORT`'s value overrides the `common.PORT` default value of
643643
12346\.
644644

645+
### `NODE_REGENERATE_SNAPSHOTS`
646+
647+
If set, test snapshots for a the current test are regenerated.
648+
for example `NODE_REGENERATE_SNAPSHOTS=1 out/Release/node test/parallel/test-runner-output.mjs`
649+
will update all the test runner output snapshots.
650+
645651
### `NODE_SKIP_FLAG_CHECK`
646652

647653
If set, command line arguments passed to individual tests are not validated.

‎test/common/assertSnapshot.js‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
'use strict';
2+
constcommon=require('.');
3+
constpath=require('node:path');
4+
constfs=require('node:fs/promises');
5+
constassert=require('node:assert/strict');
6+
7+
8+
conststackFramesRegexp=/(\s+)((.+?)\s+\()?(?:\(?(.+?):(\d+)(?::(\d+))?)\)?(\s+\{)?(\n|$)/g;
9+
constwindowNewlineRegexp=/\r/g;
10+
11+
functionreplaceStackTrace(str){
12+
returnstr.replace(stackFramesRegexp,'$1*$7\n');
13+
}
14+
15+
functionreplaceWindowsLineEndings(str){
16+
returnstr.replace(windowNewlineRegexp,'');
17+
}
18+
19+
functiontransform(...args){
20+
return(str)=>args.reduce((acc,fn)=>fn(acc),str);
21+
}
22+
23+
functiongetSnapshotPath(filename){
24+
const{ name, dir }=path.parse(filename);
25+
returnpath.resolve(dir,`${name}.snapshot`);
26+
}
27+
28+
asyncfunctionassertSnapshot(actual,filename=process.argv[1]){
29+
constsnapshot=getSnapshotPath(filename);
30+
if(process.env.NODE_REGENERATE_SNAPSHOTS){
31+
awaitfs.writeFile(snapshot,actual);
32+
}else{
33+
constexpected=awaitfs.readFile(snapshot,'utf8');
34+
assert.strictEqual(actual,replaceWindowsLineEndings(expected));
35+
}
36+
}
37+
38+
asyncfunctionspawnAndAssert(filename,transform=(x)=>x){
39+
// TODO: Add an option to this function to alternatively or additionally compare stderr.
40+
// For now, tests that want to check stderr or both stdout and stderr can use spawnPromisified.
41+
constflags=common.parseTestFlags(filename);
42+
const{ stdout }=awaitcommon.spawnPromisified(process.execPath,[...flags,filename]);
43+
awaitassertSnapshot(transform(stdout),filename);
44+
}
45+
46+
module.exports={
47+
assertSnapshot,
48+
getSnapshotPath,
49+
replaceStackTrace,
50+
replaceWindowsLineEndings,
51+
spawnAndAssert,
52+
transform,
53+
};

‎test/common/index.js‎

Lines changed: 45 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,31 @@ const hasOpenSSL3 = hasCrypto &&
6060

6161
consthasQuic=hasCrypto&&!!process.config.variables.openssl_quic;
6262

63+
functionparseTestFlags(filename=process.argv[1]){
64+
// The copyright notice is relatively big and the flags could come afterwards.
65+
constbytesToRead=1500;
66+
constbuffer=Buffer.allocUnsafe(bytesToRead);
67+
constfd=fs.openSync(filename,'r');
68+
constbytesRead=fs.readSync(fd,buffer,0,bytesToRead);
69+
fs.closeSync(fd);
70+
constsource=buffer.toString('utf8',0,bytesRead);
71+
72+
constflagStart=source.indexOf('// Flags: --')+10;
73+
74+
if(flagStart===9){
75+
return[];
76+
}
77+
letflagEnd=source.indexOf('\n',flagStart);
78+
// Normalize different EOL.
79+
if(source[flagEnd-1]==='\r'){
80+
flagEnd--;
81+
}
82+
returnsource
83+
.substring(flagStart,flagEnd)
84+
.replace(/_/g,'-')
85+
.split(' ');
86+
}
87+
6388
// Check for flags. Skip this for workers (both, the `cluster` module and
6489
// `worker_threads`) and child processes.
6590
// If the binary was built without-ssl then the crypto flags are
@@ -70,44 +95,25 @@ if (process.argv.length === 2 &&
7095
hasCrypto&&
7196
require('cluster').isPrimary&&
7297
fs.existsSync(process.argv[1])){
73-
// The copyright notice is relatively big and the flags could come afterwards.
74-
constbytesToRead=1500;
75-
constbuffer=Buffer.allocUnsafe(bytesToRead);
76-
constfd=fs.openSync(process.argv[1],'r');
77-
constbytesRead=fs.readSync(fd,buffer,0,bytesToRead);
78-
fs.closeSync(fd);
79-
constsource=buffer.toString('utf8',0,bytesRead);
80-
81-
constflagStart=source.indexOf('// Flags: --')+10;
82-
if(flagStart!==9){
83-
letflagEnd=source.indexOf('\n',flagStart);
84-
// Normalize different EOL.
85-
if(source[flagEnd-1]==='\r'){
86-
flagEnd--;
87-
}
88-
constflags=source
89-
.substring(flagStart,flagEnd)
90-
.replace(/_/g,'-')
91-
.split(' ');
92-
constargs=process.execArgv.map((arg)=>arg.replace(/_/g,'-'));
93-
for(constflagofflags){
94-
if(!args.includes(flag)&&
95-
// If the binary is build without `intl` the inspect option is
96-
// invalid. The test itself should handle this case.
97-
(process.features.inspector||!flag.startsWith('--inspect'))){
98-
console.log(
99-
'NOTE: The test started as a child_process using these flags:',
100-
inspect(flags),
101-
'Use NODE_SKIP_FLAG_CHECK to run the test with the original flags.',
102-
);
103-
constargs=[...flags, ...process.execArgv, ...process.argv.slice(1)];
104-
constoptions={encoding: 'utf8',stdio: 'inherit'};
105-
constresult=spawnSync(process.execPath,args,options);
106-
if(result.signal){
107-
process.kill(0,result.signal);
108-
}else{
109-
process.exit(result.status);
110-
}
98+
constflags=parseTestFlags();
99+
constargs=process.execArgv.map((arg)=>arg.replace(/_/g,'-'));
100+
for(constflagofflags){
101+
if(!args.includes(flag)&&
102+
// If the binary is build without `intl` the inspect option is
103+
// invalid. The test itself should handle this case.
104+
(process.features.inspector||!flag.startsWith('--inspect'))){
105+
console.log(
106+
'NOTE: The test started as a child_process using these flags:',
107+
inspect(flags),
108+
'Use NODE_SKIP_FLAG_CHECK to run the test with the original flags.',
109+
);
110+
constargs=[...flags, ...process.execArgv, ...process.argv.slice(1)];
111+
constoptions={encoding: 'utf8',stdio: 'inherit'};
112+
constresult=spawnSync(process.execPath,args,options);
113+
if(result.signal){
114+
process.kill(0,result.signal);
115+
}else{
116+
process.exit(result.status);
111117
}
112118
}
113119
}
@@ -917,6 +923,7 @@ const common = {
917923
mustSucceed,
918924
nodeProcessAborted,
919925
PIPE,
926+
parseTestFlags,
920927
platformTimeout,
921928
printSkipMessage,
922929
pwdCommand,

‎test/common/index.mjs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ const {
3737
getCallSite,
3838
mustNotCall,
3939
mustNotMutateObjectDeep,
40+
parseTestFlags,
4041
printSkipMessage,
4142
skip,
4243
nodeProcessAborted,
@@ -88,6 +89,7 @@ export {
8889
getCallSite,
8990
mustNotCall,
9091
mustNotMutateObjectDeep,
92+
parseTestFlags,
9193
printSkipMessage,
9294
skip,
9395
nodeProcessAborted,
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// Flags: --no-warnings
22
'use strict';
3-
require('../common');
3+
require('../../../common');
44
consttest=require('node:test');
55

66
test('promise timeout signal',{signal: AbortSignal.timeout(1)},async(t)=>{

test/message/test_runner_abort.out renamed to test/fixtures/test-runner/output/abort.snapshot

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,15 +31,15 @@ TAP version 13
3131
# Subtest: not ok 2
3232
not ok 6 - not ok 2
3333
---
34-
duration_ms: *
34+
duration_ms: ZERO
3535
failureType: 'cancelledByParent'
3636
error: 'test did not finish before its parent and was cancelled'
3737
code: 'ERR_TEST_FAILURE'
3838
...
3939
# Subtest: not ok 3
4040
not ok 7 - not ok 3
4141
---
42-
duration_ms: *
42+
duration_ms: ZERO
4343
failureType: 'testAborted'
4444
error: 'This operation was aborted'
4545
code: 20
@@ -59,7 +59,7 @@ TAP version 13
5959
# Subtest: not ok 4
6060
not ok 8 - not ok 4
6161
---
62-
duration_ms: *
62+
duration_ms: ZERO
6363
failureType: 'testAborted'
6464
error: 'This operation was aborted'
6565
code: 20
@@ -79,7 +79,7 @@ TAP version 13
7979
# Subtest: not ok 5
8080
not ok 9 - not ok 5
8181
---
82-
duration_ms: *
82+
duration_ms: ZERO
8383
failureType: 'testAborted'
8484
error: 'This operation was aborted'
8585
code: 20
@@ -161,15 +161,15 @@ not ok 2 - promise abort signal
161161
# Subtest: not ok 2
162162
not ok 6 - not ok 2
163163
---
164-
duration_ms: *
164+
duration_ms: ZERO
165165
failureType: 'cancelledByParent'
166166
error: 'test did not finish before its parent and was cancelled'
167167
code: 'ERR_TEST_FAILURE'
168168
...
169169
# Subtest: not ok 3
170170
not ok 7 - not ok 3
171171
---
172-
duration_ms: *
172+
duration_ms: ZERO
173173
failureType: 'testAborted'
174174
error: 'This operation was aborted'
175175
code: 20
@@ -189,7 +189,7 @@ not ok 2 - promise abort signal
189189
# Subtest: not ok 4
190190
not ok 8 - not ok 4
191191
---
192-
duration_ms: *
192+
duration_ms: ZERO
193193
failureType: 'testAborted'
194194
error: 'This operation was aborted'
195195
code: 20
@@ -209,7 +209,7 @@ not ok 2 - promise abort signal
209209
# Subtest: not ok 5
210210
not ok 9 - not ok 5
211211
---
212-
duration_ms: *
212+
duration_ms: ZERO
213213
failureType: 'testAborted'
214214
error: 'This operation was aborted'
215215
code: 20

test/message/test_runner_abort_suite.js renamed to test/fixtures/test-runner/output/abort_suite.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// Flags: --no-warnings
22
'use strict';
3-
require('../common');
3+
require('../../../common');
44
const{ describe, it }=require('node:test');
55

66
describe('describe timeout signal',{signal: AbortSignal.timeout(1)},(t)=>{

test/message/test_runner_abort_suite.out renamed to test/fixtures/test-runner/output/abort_suite.snapshot

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,31 +31,31 @@ TAP version 13
3131
# Subtest: not ok 2
3232
not ok 6 - not ok 2
3333
---
34-
duration_ms: *
34+
duration_ms: ZERO
3535
failureType: 'cancelledByParent'
3636
error: 'test did not finish before its parent and was cancelled'
3737
code: 'ERR_TEST_FAILURE'
3838
...
3939
# Subtest: not ok 3
4040
not ok 7 - not ok 3
4141
---
42-
duration_ms: *
42+
duration_ms: ZERO
4343
failureType: 'cancelledByParent'
4444
error: 'test did not finish before its parent and was cancelled'
4545
code: 'ERR_TEST_FAILURE'
4646
...
4747
# Subtest: not ok 4
4848
not ok 8 - not ok 4
4949
---
50-
duration_ms: *
50+
duration_ms: ZERO
5151
failureType: 'cancelledByParent'
5252
error: 'test did not finish before its parent and was cancelled'
5353
code: 'ERR_TEST_FAILURE'
5454
...
5555
# Subtest: not ok 5
5656
not ok 9 - not ok 5
5757
---
58-
duration_ms: *
58+
duration_ms: ZERO
5959
failureType: 'cancelledByParent'
6060
error: 'test did not finish before its parent and was cancelled'
6161
code: 'ERR_TEST_FAILURE'

test/message/test_runner_describe_it.js renamed to test/fixtures/test-runner/output/describe_it.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// Flags: --no-warnings
22
'use strict';
3-
require('../common');
3+
require('../../../common');
44
constassert=require('node:assert');
55
const{ describe, it, test }=require('node:test');
66
constutil=require('util');

0 commit comments

Comments
 (0)