Skip to content

Commit

Permalink
test: rely less on duplicative common test harness utilities
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
jasnell authored and aduh95 committed Jan 30, 2025
1 parent e654c8b commit ef2ed71
Show file tree
Hide file tree
Showing 148 changed files with 672 additions and 290 deletions.
45 changes: 42 additions & 3 deletions test/abort/test-abort-backtrace.js
Original file line number Diff line number Diff line change
@@ -1,24 +1,63 @@
'use strict';
const common = require('../common');
require('../common');
const assert = require('assert');
const cp = require('child_process');

function getPrintedStackTrace(stderr) {
const lines = stderr.split('\n');

let state = 'initial';
const result = {
message: [],
nativeStack: [],
jsStack: [],
};
for (let i = 0; i < lines.length; ++i) {
const line = lines[i].trim();
if (line.length === 0) {
continue; // Skip empty lines.
}

switch (state) {
case 'initial':
result.message.push(line);
if (line.includes('Native stack trace')) {
state = 'native-stack';
} else {
result.message.push(line);
}
break;
case 'native-stack':
if (line.includes('JavaScript stack trace')) {
state = 'js-stack';
} else {
result.nativeStack.push(line);
}
break;
case 'js-stack':
result.jsStack.push(line);
break;
}
}
return result;
}

if (process.argv[2] === 'child') {
process.abort();
} else {
const child = cp.spawnSync(`${process.execPath}`, [`${__filename}`, 'child']);
const stderr = child.stderr.toString();

assert.strictEqual(child.stdout.toString(), '');
const { nativeStack, jsStack } = common.getPrintedStackTrace(stderr);
const { nativeStack, jsStack } = getPrintedStackTrace(stderr);

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

// For systems that don't support backtraces, the native stack is
// going to be empty.
if (!common.isWindows && nativeStack.length > 0) {
if (process.platform !== 'win32' && nativeStack.length > 0) {
const { getBinaryPath } = require('../common/shared-lib-util');
if (!nativeStack.some((frame) => frame.includes(`[${getBinaryPath()}]`))) {
assert.fail(`Some native stack frame include the binary name:\n${stderr}`);
Expand Down
5 changes: 3 additions & 2 deletions test/async-hooks/init-hooks.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
'use strict';
// Flags: --expose-gc

const common = require('../common');
require('../common');
const assert = require('assert');
const async_hooks = require('async_hooks');
const { isMainThread } = require('worker_threads');
const util = require('util');
const print = process._rawDebug;

Expand Down Expand Up @@ -161,7 +162,7 @@ class ActivityCollector {
const stub = { uid, type: 'Unknown', handleIsObject: true, handle: {} };
this._activities.set(uid, stub);
return stub;
} else if (!common.isMainThread) {
} else if (!isMainThread) {
// Worker threads start main script execution inside of an AsyncWrap
// callback, so we don't yield errors for these.
return null;
Expand Down
7 changes: 5 additions & 2 deletions test/async-hooks/test-crypto-pbkdf2.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
'use strict';

const common = require('../common');
if (!common.hasCrypto)
if (!common.hasCrypto) {
common.skip('missing crypto');
if (!common.isMainThread)
}
const { isMainThread } = require('worker_threads');
if (!isMainThread) {
common.skip('Worker bootstrapping works differently -> different async IDs');
}

const assert = require('assert');
const tick = require('../common/tick');
Expand Down
7 changes: 5 additions & 2 deletions test/async-hooks/test-crypto-randomBytes.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
'use strict';

const common = require('../common');
if (!common.hasCrypto)
if (!common.hasCrypto) {
common.skip('missing crypto');
if (!common.isMainThread)
}
const { isMainThread } = require('worker_threads');
if (!isMainThread) {
common.skip('Worker bootstrapping works differently -> different async IDs');
}

const assert = require('assert');
const tick = require('../common/tick');
Expand Down
3 changes: 2 additions & 1 deletion test/async-hooks/test-enable-disable.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,9 @@ const assert = require('assert');
const tick = require('../common/tick');
const initHooks = require('./init-hooks');
const { checkInvocations } = require('./hook-checks');
const { isMainThread } = require('worker_threads');

if (!common.isMainThread)
if (!isMainThread)
common.skip('Worker bootstrapping works differently -> different timing');

// Include "Unknown"s because hook2 will not be able to identify
Expand Down
7 changes: 5 additions & 2 deletions test/async-hooks/test-fseventwrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,15 @@ const initHooks = require('./init-hooks');
const tick = require('../common/tick');
const { checkInvocations } = require('./hook-checks');
const fs = require('fs');
const { isMainThread } = require('worker_threads');

if (!common.isMainThread)
if (!isMainThread) {
common.skip('Worker bootstrapping works differently -> different async IDs');
}

if (common.isIBMi)
if (common.isIBMi) {
common.skip('IBMi does not support fs.watch()');
}

const hooks = initHooks();

Expand Down
4 changes: 3 additions & 1 deletion test/async-hooks/test-fsreqcallback-readFile.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@ const tick = require('../common/tick');
const initHooks = require('./init-hooks');
const { checkInvocations } = require('./hook-checks');
const fs = require('fs');
const { isMainThread } = require('worker_threads');

if (!common.isMainThread)
if (!isMainThread) {
common.skip('Worker bootstrapping works differently -> different async IDs');
}

const hooks = initHooks();

Expand Down
4 changes: 3 additions & 1 deletion test/async-hooks/test-getaddrinforeqwrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@ const tick = require('../common/tick');
const initHooks = require('./init-hooks');
const { checkInvocations } = require('./hook-checks');
const dns = require('dns');
const { isMainThread } = require('worker_threads');

if (!common.isMainThread)
if (!isMainThread) {
common.skip('Worker bootstrapping works differently -> different async IDs');
}

const hooks = initHooks();

Expand Down
4 changes: 3 additions & 1 deletion test/async-hooks/test-getnameinforeqwrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@ const tick = require('../common/tick');
const initHooks = require('./init-hooks');
const { checkInvocations } = require('./hook-checks');
const dns = require('dns');
const { isMainThread } = require('worker_threads');

if (!common.isMainThread)
if (!isMainThread) {
common.skip('Worker bootstrapping works differently -> different async IDs');
}

const hooks = initHooks();

Expand Down
7 changes: 5 additions & 2 deletions test/async-hooks/test-graph.signal.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
'use strict';

const common = require('../common');
if (common.isWindows)
if (common.isWindows) {
common.skip('no signals on Windows');
if (!common.isMainThread)
}
const { isMainThread } = require('worker_threads');
if (!isMainThread) {
common.skip('No signal handling available in Workers');
}

const initHooks = require('./init-hooks');
const verifyGraph = require('./verify-graph');
Expand Down
5 changes: 3 additions & 2 deletions test/async-hooks/test-no-assert-when-disabled.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
'use strict';
// Flags: --no-force-async-hooks-checks --expose-internals
const common = require('../common');

if (!common.isMainThread)
const { isMainThread } = require('worker_threads');
if (!isMainThread) {
common.skip('Workers don\'t inherit per-env state like the check flag');
}

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

Expand Down
4 changes: 3 additions & 1 deletion test/async-hooks/test-pipewrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@ const tick = require('../common/tick');
const initHooks = require('./init-hooks');
const { checkInvocations } = require('./hook-checks');
const { spawn } = require('child_process');
const { isMainThread } = require('worker_threads');

if (!common.isMainThread)
if (!isMainThread) {
common.skip('Worker bootstrapping works differently -> different async IDs');
}

const hooks = initHooks();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@ const common = require('../common');
const assert = require('assert');
const initHooks = require('./init-hooks');
const { checkInvocations } = require('./hook-checks');
const { isMainThread } = require('worker_threads');

if (!common.isMainThread)
if (!isMainThread) {
common.skip('Worker bootstrapping works differently -> different async IDs');
}

const p = new Promise(common.mustCall(function executor(resolve) {
resolve(5);
Expand Down
4 changes: 3 additions & 1 deletion test/async-hooks/test-promise.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@ const common = require('../common');
const assert = require('assert');
const initHooks = require('./init-hooks');
const { checkInvocations } = require('./hook-checks');
const { isMainThread } = require('worker_threads');

if (!common.isMainThread)
if (!isMainThread) {
common.skip('Worker bootstrapping works differently -> different async IDs');
}

const hooks = initHooks();

Expand Down
7 changes: 5 additions & 2 deletions test/async-hooks/test-signalwrap.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
'use strict';
const common = require('../common');

if (common.isWindows)
if (common.isWindows) {
common.skip('no signals in Windows');
if (!common.isMainThread)
}
const { isMainThread } = require('worker_threads');
if (!isMainThread) {
common.skip('No signal handling available in Workers');
}

const assert = require('assert');
const initHooks = require('./init-hooks');
Expand Down
5 changes: 4 additions & 1 deletion test/async-hooks/test-statwatcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,11 @@ const initHooks = require('./init-hooks');
const { checkInvocations } = require('./hook-checks');
const fs = require('fs');

if (!common.isMainThread)
const { isMainThread } = require('worker_threads');

if (!isMainThread) {
common.skip('Worker bootstrapping works differently -> different async IDs');
}

tmpdir.refresh();

Expand Down
4 changes: 3 additions & 1 deletion test/async-hooks/test-unhandled-rejection-context.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@ const common = require('../common');
const assert = require('assert');
const initHooks = require('./init-hooks');
const async_hooks = require('async_hooks');
const { isMainThread } = require('worker_threads');

if (!common.isMainThread)
if (!isMainThread) {
common.skip('Worker bootstrapping works differently -> different async IDs');
}

const promiseAsyncIds = [];
const hooks = initHooks({
Expand Down
4 changes: 3 additions & 1 deletion test/benchmark/test-benchmark-napi.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ if (common.isWindows) {
common.skip('vcbuild.bat doesn\'t build the n-api benchmarks yet');
}

if (!common.isMainThread) {
const { isMainThread } = require('worker_threads');

if (!isMainThread) {
common.skip('addons are not supported in workers');
}

Expand Down
17 changes: 0 additions & 17 deletions test/common/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,6 @@ symlinks
([SeCreateSymbolicLinkPrivilege](https://msdn.microsoft.com/en-us/library/windows/desktop/bb530716\(v=vs.85\).aspx)).
On non-Windows platforms, this always returns `true`.

### `createZeroFilledFile(filename)`

Creates a 10 MiB file of all null characters.

### `enoughTestMem`

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

Attempts to 'kill' `pid`

### `isDumbTerminal`

* [\<boolean>][<boolean>]

### `isFreeBSD`

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

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

### `skipIfDumbTerminal()`

Skip the rest of the tests if the current terminal is a dumb terminal

### `skipIfEslintMissing()`

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

### `skipIfWorker()`

Skip the rest of the tests in the current file when not running on a main
thread.

## ArrayStream module

The `ArrayStream` module provides a simple `Stream` that pushes elements from
Expand Down
Loading

0 comments on commit ef2ed71

Please sign in to comment.