Skip to content

Commit 7e58d07

Browse files
MoLowaduh95
authored andcommitted
test_runner: fix diagnostics channel context tracking
Signed-off-by: Moshe Atlow <moshe@atlow.co.il> PR-URL: #63283 Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
1 parent c322de2 commit 7e58d07

3 files changed

Lines changed: 67 additions & 11 deletions

File tree

lib/internal/test_runner/test.js

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1259,6 +1259,9 @@ class Test extends AsyncResource {
12591259

12601260
let stopPromise;
12611261

1262+
let publishEnd = () => testChannel.end.publish(channelContext);
1263+
let publishError = (err) => testChannel.error.publish({ __proto__: null, ...channelContext, error: err });
1264+
12621265
try {
12631266
if (this.parent?.hooks.before.length > 0) {
12641267
// This hook usually runs immediately, we need to wait for it to finish
@@ -1277,9 +1280,11 @@ class Test extends AsyncResource {
12771280
// not the runInAsyncScope call itself, to maintain AsyncLocalStorage bindings.
12781281
let testFn = this.fn;
12791282
if (channelContext !== null && testChannel.start.hasSubscribers) {
1280-
testFn = (...fnArgs) => testChannel.start.runStores(channelContext,
1281-
() => ReflectApply(this.fn, this, fnArgs),
1282-
);
1283+
testFn = (...fnArgs) => testChannel.start.runStores(channelContext, () => {
1284+
publishEnd = AsyncResource.bind(publishEnd);
1285+
publishError = AsyncResource.bind(publishError);
1286+
return ReflectApply(this.fn, this, fnArgs);
1287+
});
12831288
}
12841289

12851290
ArrayPrototypeUnshift(runArgs, testFn, ctx);
@@ -1331,9 +1336,8 @@ class Test extends AsyncResource {
13311336
await afterEach();
13321337
await after();
13331338
} catch (err) {
1334-
// Publish diagnostics_channel error event if the channel has subscribers
13351339
if (channelContext !== null && testChannel.error.hasSubscribers) {
1336-
testChannel.error.publish({ __proto__: null, ...channelContext, error: err });
1340+
publishError(err);
13371341
}
13381342
if (isTestFailureError(err)) {
13391343
if (err.failureType === kTestTimeoutFailure) {
@@ -1357,7 +1361,7 @@ class Test extends AsyncResource {
13571361

13581362
// Publish diagnostics_channel end event if the channel has subscribers (in both success and error cases)
13591363
if (channelContext !== null && testChannel.end.hasSubscribers) {
1360-
testChannel.end.publish(channelContext);
1364+
publishEnd();
13611365
}
13621366
}
13631367

@@ -1702,6 +1706,9 @@ class Suite extends Test {
17021706
file: this.entryFile,
17031707
type: this.reportedType,
17041708
};
1709+
let publishEnd = () => testChannel.end.publish(channelContext);
1710+
let publishError = (err) => testChannel.error.publish({ __proto__: null, ...channelContext, error: err });
1711+
17051712
try {
17061713
const { ctx, args } = this.getRunArgs();
17071714

@@ -1713,9 +1720,11 @@ class Suite extends Test {
17131720
let suiteFn = this.fn;
17141721
if (testChannel.start.hasSubscribers) {
17151722
const baseFn = this.fn;
1716-
suiteFn = (...fnArgs) => testChannel.start.runStores(channelContext,
1717-
() => ReflectApply(baseFn, this, fnArgs),
1718-
);
1723+
suiteFn = (...fnArgs) => testChannel.start.runStores(channelContext, () => {
1724+
publishEnd = AsyncResource.bind(publishEnd);
1725+
publishError = AsyncResource.bind(publishError);
1726+
return ReflectApply(baseFn, this, fnArgs);
1727+
});
17191728
}
17201729

17211730
const runArgs = [suiteFn, ctx];
@@ -1724,12 +1733,12 @@ class Suite extends Test {
17241733
await ReflectApply(this.runInAsyncScope, this, runArgs);
17251734
} catch (err) {
17261735
if (testChannel.error.hasSubscribers) {
1727-
testChannel.error.publish({ __proto__: null, ...channelContext, error: err });
1736+
publishError(err);
17281737
}
17291738
this.fail(new ERR_TEST_FAILURE(err, kTestCodeFailure));
17301739
} finally {
17311740
if (testChannel.end.hasSubscribers) {
1732-
testChannel.end.publish(channelContext);
1741+
publishEnd();
17331742
}
17341743
}
17351744

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
'use strict';
2+
const dc = require('node:diagnostics_channel');
3+
const { AsyncLocalStorage } = require('node:async_hooks');
4+
const { test } = require('node:test');
5+
6+
const als = new AsyncLocalStorage();
7+
const ch = dc.tracingChannel('node.test');
8+
9+
ch.start.bindStore(als, (data) => data.name);
10+
11+
const storeAtEnd = {};
12+
const storeAtError = {};
13+
14+
ch.end.subscribe((data) => {
15+
storeAtEnd[data.name] = als.getStore();
16+
});
17+
18+
ch.error.subscribe((data) => {
19+
storeAtError[data.name] = als.getStore();
20+
});
21+
22+
test('passing test', () => {});
23+
test('failing test', () => { throw new Error('boom'); });
24+
25+
process.on('exit', () => {
26+
console.log(JSON.stringify({ storeAtEnd, storeAtError }));
27+
});

test/parallel/test-runner-diagnostics-channel.js

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,26 @@ test('context is available in async operations within test', async () => {
119119
assert.strictEqual(valueInTimeout, testName);
120120
});
121121

122+
test('bindStore propagates store to end and error subscribers', async () => {
123+
// Spawn a fixture that records `als.getStore()` at end/error publish time so
124+
// we can assert subscribers see the bound store, not undefined.
125+
const fixturePath = join(__dirname, '../fixtures/test-runner/diagnostics-channel-bindstore-end.js');
126+
const result = spawnSync(process.execPath, [fixturePath], { encoding: 'utf8' });
127+
// The fixture contains an intentionally failing test, so exit is non-zero.
128+
assert.notStrictEqual(result.status, 0);
129+
const line = result.stdout.split('\n').find((l) => l.includes('storeAtEnd'));
130+
assert.ok(line, `expected storeAtEnd line in stdout:\n${result.stdout}`);
131+
const { storeAtEnd, storeAtError } = JSON.parse(line);
132+
assert.deepStrictEqual(storeAtEnd, {
133+
'<root>': '<root>',
134+
'passing test': 'passing test',
135+
'failing test': 'failing test',
136+
});
137+
assert.deepStrictEqual(storeAtError, {
138+
'failing test': 'failing test',
139+
});
140+
});
141+
122142
test('error events fire for failing tests in fixture', async () => {
123143
// Run the fixture test that intentionally fails
124144
const fixturePath = join(__dirname, '../fixtures/test-runner/diagnostics-channel-error-test.js');

0 commit comments

Comments
 (0)