Skip to content

Commit ef2ed71

Browse files
jasnelladuh95
jasnell
authored and
aduh95
committed
test: rely less on duplicative common test harness utilities
There are several cleanups here that are not just style nits... 1. The `common.isMainThread` was just a passthrough to the `isMainThread` export on the worker_thread module. It's use was inconsistent and just obfuscated the fact that the test file depend on the `worker_threads` built-in. By eliminating it we simplify the test harness a bit and make it clearer which tests depend on the worker_threads check. 2. The `common.isDumbTerminal` is fairly unnecesary since that just wraps a public API check. 3. Several of the `common.skipIf....` checks were inconsistently used and really don't need to be separate utility functions. A key part of the motivation here is to work towards making more of the tests more self-contained and less reliant on the common test harness where possible. PR-URL: #56712 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
1 parent e654c8b commit ef2ed71

File tree

148 files changed

+672
-290
lines changed

Some content is hidden

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

148 files changed

+672
-290
lines changed

‎test/abort/test-abort-backtrace.js

+42-3
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,63 @@
11
'use strict';
2-
const common = require('../common');
2+
require('../common');
33
const assert = require('assert');
44
const cp = require('child_process');
55

6+
function getPrintedStackTrace(stderr) {
7+
const lines = stderr.split('\n');
8+
9+
let state = 'initial';
10+
const result = {
11+
message: [],
12+
nativeStack: [],
13+
jsStack: [],
14+
};
15+
for (let i = 0; i < lines.length; ++i) {
16+
const line = lines[i].trim();
17+
if (line.length === 0) {
18+
continue; // Skip empty lines.
19+
}
20+
21+
switch (state) {
22+
case 'initial':
23+
result.message.push(line);
24+
if (line.includes('Native stack trace')) {
25+
state = 'native-stack';
26+
} else {
27+
result.message.push(line);
28+
}
29+
break;
30+
case 'native-stack':
31+
if (line.includes('JavaScript stack trace')) {
32+
state = 'js-stack';
33+
} else {
34+
result.nativeStack.push(line);
35+
}
36+
break;
37+
case 'js-stack':
38+
result.jsStack.push(line);
39+
break;
40+
}
41+
}
42+
return result;
43+
}
44+
645
if (process.argv[2] === 'child') {
746
process.abort();
847
} else {
948
const child = cp.spawnSync(`${process.execPath}`, [`${__filename}`, 'child']);
1049
const stderr = child.stderr.toString();
1150

1251
assert.strictEqual(child.stdout.toString(), '');
13-
const { nativeStack, jsStack } = common.getPrintedStackTrace(stderr);
52+
const { nativeStack, jsStack } = getPrintedStackTrace(stderr);
1453

1554
if (!nativeStack.every((frame, index) => frame.startsWith(`${index + 1}:`))) {
1655
assert.fail(`Each frame should start with a frame number:\n${stderr}`);
1756
}
1857

1958
// For systems that don't support backtraces, the native stack is
2059
// going to be empty.
21-
if (!common.isWindows && nativeStack.length > 0) {
60+
if (process.platform !== 'win32' && nativeStack.length > 0) {
2261
const { getBinaryPath } = require('../common/shared-lib-util');
2362
if (!nativeStack.some((frame) => frame.includes(`[${getBinaryPath()}]`))) {
2463
assert.fail(`Some native stack frame include the binary name:\n${stderr}`);

‎test/async-hooks/init-hooks.js

+3-2
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
'use strict';
22
// Flags: --expose-gc
33

4-
const common = require('../common');
4+
require('../common');
55
const assert = require('assert');
66
const async_hooks = require('async_hooks');
7+
const { isMainThread } = require('worker_threads');
78
const util = require('util');
89
const print = process._rawDebug;
910

@@ -161,7 +162,7 @@ class ActivityCollector {
161162
const stub = { uid, type: 'Unknown', handleIsObject: true, handle: {} };
162163
this._activities.set(uid, stub);
163164
return stub;
164-
} else if (!common.isMainThread) {
165+
} else if (!isMainThread) {
165166
// Worker threads start main script execution inside of an AsyncWrap
166167
// callback, so we don't yield errors for these.
167168
return null;

‎test/async-hooks/test-crypto-pbkdf2.js

+5-2
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
'use strict';
22

33
const common = require('../common');
4-
if (!common.hasCrypto)
4+
if (!common.hasCrypto) {
55
common.skip('missing crypto');
6-
if (!common.isMainThread)
6+
}
7+
const { isMainThread } = require('worker_threads');
8+
if (!isMainThread) {
79
common.skip('Worker bootstrapping works differently -> different async IDs');
10+
}
811

912
const assert = require('assert');
1013
const tick = require('../common/tick');

‎test/async-hooks/test-crypto-randomBytes.js

+5-2
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
'use strict';
22

33
const common = require('../common');
4-
if (!common.hasCrypto)
4+
if (!common.hasCrypto) {
55
common.skip('missing crypto');
6-
if (!common.isMainThread)
6+
}
7+
const { isMainThread } = require('worker_threads');
8+
if (!isMainThread) {
79
common.skip('Worker bootstrapping works differently -> different async IDs');
10+
}
811

912
const assert = require('assert');
1013
const tick = require('../common/tick');

‎test/async-hooks/test-enable-disable.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,9 @@ const assert = require('assert');
8787
const tick = require('../common/tick');
8888
const initHooks = require('./init-hooks');
8989
const { checkInvocations } = require('./hook-checks');
90+
const { isMainThread } = require('worker_threads');
9091

91-
if (!common.isMainThread)
92+
if (!isMainThread)
9293
common.skip('Worker bootstrapping works differently -> different timing');
9394

9495
// Include "Unknown"s because hook2 will not be able to identify

‎test/async-hooks/test-fseventwrap.js

+5-2
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,15 @@ const initHooks = require('./init-hooks');
66
const tick = require('../common/tick');
77
const { checkInvocations } = require('./hook-checks');
88
const fs = require('fs');
9+
const { isMainThread } = require('worker_threads');
910

10-
if (!common.isMainThread)
11+
if (!isMainThread) {
1112
common.skip('Worker bootstrapping works differently -> different async IDs');
13+
}
1214

13-
if (common.isIBMi)
15+
if (common.isIBMi) {
1416
common.skip('IBMi does not support fs.watch()');
17+
}
1518

1619
const hooks = initHooks();
1720

‎test/async-hooks/test-fsreqcallback-readFile.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,11 @@ const tick = require('../common/tick');
66
const initHooks = require('./init-hooks');
77
const { checkInvocations } = require('./hook-checks');
88
const fs = require('fs');
9+
const { isMainThread } = require('worker_threads');
910

10-
if (!common.isMainThread)
11+
if (!isMainThread) {
1112
common.skip('Worker bootstrapping works differently -> different async IDs');
13+
}
1214

1315
const hooks = initHooks();
1416

‎test/async-hooks/test-getaddrinforeqwrap.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,11 @@ const tick = require('../common/tick');
66
const initHooks = require('./init-hooks');
77
const { checkInvocations } = require('./hook-checks');
88
const dns = require('dns');
9+
const { isMainThread } = require('worker_threads');
910

10-
if (!common.isMainThread)
11+
if (!isMainThread) {
1112
common.skip('Worker bootstrapping works differently -> different async IDs');
13+
}
1214

1315
const hooks = initHooks();
1416

‎test/async-hooks/test-getnameinforeqwrap.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,11 @@ const tick = require('../common/tick');
66
const initHooks = require('./init-hooks');
77
const { checkInvocations } = require('./hook-checks');
88
const dns = require('dns');
9+
const { isMainThread } = require('worker_threads');
910

10-
if (!common.isMainThread)
11+
if (!isMainThread) {
1112
common.skip('Worker bootstrapping works differently -> different async IDs');
13+
}
1214

1315
const hooks = initHooks();
1416

‎test/async-hooks/test-graph.signal.js

+5-2
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
'use strict';
22

33
const common = require('../common');
4-
if (common.isWindows)
4+
if (common.isWindows) {
55
common.skip('no signals on Windows');
6-
if (!common.isMainThread)
6+
}
7+
const { isMainThread } = require('worker_threads');
8+
if (!isMainThread) {
79
common.skip('No signal handling available in Workers');
10+
}
811

912
const initHooks = require('./init-hooks');
1013
const verifyGraph = require('./verify-graph');

‎test/async-hooks/test-no-assert-when-disabled.js

+3-2
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
'use strict';
22
// Flags: --no-force-async-hooks-checks --expose-internals
33
const common = require('../common');
4-
5-
if (!common.isMainThread)
4+
const { isMainThread } = require('worker_threads');
5+
if (!isMainThread) {
66
common.skip('Workers don\'t inherit per-env state like the check flag');
7+
}
78

89
const async_hooks = require('internal/async_hooks');
910

‎test/async-hooks/test-pipewrap.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,11 @@ const tick = require('../common/tick');
99
const initHooks = require('./init-hooks');
1010
const { checkInvocations } = require('./hook-checks');
1111
const { spawn } = require('child_process');
12+
const { isMainThread } = require('worker_threads');
1213

13-
if (!common.isMainThread)
14+
if (!isMainThread) {
1415
common.skip('Worker bootstrapping works differently -> different async IDs');
16+
}
1517

1618
const hooks = initHooks();
1719

‎test/async-hooks/test-promise.chain-promise-before-init-hooks.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,11 @@ const common = require('../common');
44
const assert = require('assert');
55
const initHooks = require('./init-hooks');
66
const { checkInvocations } = require('./hook-checks');
7+
const { isMainThread } = require('worker_threads');
78

8-
if (!common.isMainThread)
9+
if (!isMainThread) {
910
common.skip('Worker bootstrapping works differently -> different async IDs');
11+
}
1012

1113
const p = new Promise(common.mustCall(function executor(resolve) {
1214
resolve(5);

‎test/async-hooks/test-promise.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,11 @@ const common = require('../common');
55
const assert = require('assert');
66
const initHooks = require('./init-hooks');
77
const { checkInvocations } = require('./hook-checks');
8+
const { isMainThread } = require('worker_threads');
89

9-
if (!common.isMainThread)
10+
if (!isMainThread) {
1011
common.skip('Worker bootstrapping works differently -> different async IDs');
12+
}
1113

1214
const hooks = initHooks();
1315

‎test/async-hooks/test-signalwrap.js

+5-2
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
'use strict';
22
const common = require('../common');
33

4-
if (common.isWindows)
4+
if (common.isWindows) {
55
common.skip('no signals in Windows');
6-
if (!common.isMainThread)
6+
}
7+
const { isMainThread } = require('worker_threads');
8+
if (!isMainThread) {
79
common.skip('No signal handling available in Workers');
10+
}
811

912
const assert = require('assert');
1013
const initHooks = require('./init-hooks');

‎test/async-hooks/test-statwatcher.js

+4-1
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,11 @@ const initHooks = require('./init-hooks');
77
const { checkInvocations } = require('./hook-checks');
88
const fs = require('fs');
99

10-
if (!common.isMainThread)
10+
const { isMainThread } = require('worker_threads');
11+
12+
if (!isMainThread) {
1113
common.skip('Worker bootstrapping works differently -> different async IDs');
14+
}
1215

1316
tmpdir.refresh();
1417

‎test/async-hooks/test-unhandled-rejection-context.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,11 @@ const common = require('../common');
55
const assert = require('assert');
66
const initHooks = require('./init-hooks');
77
const async_hooks = require('async_hooks');
8+
const { isMainThread } = require('worker_threads');
89

9-
if (!common.isMainThread)
10+
if (!isMainThread) {
1011
common.skip('Worker bootstrapping works differently -> different async IDs');
12+
}
1113

1214
const promiseAsyncIds = [];
1315
const hooks = initHooks({

‎test/benchmark/test-benchmark-napi.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@ if (common.isWindows) {
66
common.skip('vcbuild.bat doesn\'t build the n-api benchmarks yet');
77
}
88

9-
if (!common.isMainThread) {
9+
const { isMainThread } = require('worker_threads');
10+
11+
if (!isMainThread) {
1012
common.skip('addons are not supported in workers');
1113
}
1214

‎test/common/README.md

-17
Original file line numberDiff line numberDiff line change
@@ -102,10 +102,6 @@ symlinks
102102
([SeCreateSymbolicLinkPrivilege](https://msdn.microsoft.com/en-us/library/windows/desktop/bb530716\(v=vs.85\).aspx)).
103103
On non-Windows platforms, this always returns `true`.
104104

105-
### `createZeroFilledFile(filename)`
106-
107-
Creates a 10 MiB file of all null characters.
108-
109105
### `enoughTestMem`
110106

111107
* [\<boolean>][<boolean>]
@@ -257,10 +253,6 @@ Platform check for Advanced Interactive eXecutive (AIX).
257253

258254
Attempts to 'kill' `pid`
259255

260-
### `isDumbTerminal`
261-
262-
* [\<boolean>][<boolean>]
263-
264256
### `isFreeBSD`
265257

266258
* [\<boolean>][<boolean>]
@@ -456,10 +448,6 @@ will not be run.
456448

457449
Logs '1..0 # Skipped: ' + `msg` and exits with exit code `0`.
458450

459-
### `skipIfDumbTerminal()`
460-
461-
Skip the rest of the tests if the current terminal is a dumb terminal
462-
463451
### `skipIfEslintMissing()`
464452

465453
Skip the rest of the tests in the current file when `ESLint` is not available
@@ -475,11 +463,6 @@ was disabled at compile time.
475463
Skip the rest of the tests in the current file when the Node.js executable
476464
was compiled with a pointer size smaller than 64 bits.
477465

478-
### `skipIfWorker()`
479-
480-
Skip the rest of the tests in the current file when not running on a main
481-
thread.
482-
483466
## ArrayStream module
484467

485468
The `ArrayStream` module provides a simple `Stream` that pushes elements from

0 commit comments

Comments
 (0)