test_runner: add getTestContext() public API

[MoLow]

Exposes getTestContext() function to access test context information from within tests and async operations.

My use case was exposing a pino logger that writes to test diagnostics, but there can be many other usecases:

import{prettyFactory}from'pino-pretty';constpretty=prettyFactory({colorize: true});constasyncId=executionAsyncId();constt=testResources.get(asyncId);t.diagnostic(pretty(obj).replace(/\n$/,''));

[nodejs-github-bot]

Review requested:

• @nodejs/test_runner

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/72266/

[codecov]

Codecov Report

❌ Patch coverage is 70.83333% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.71%. Comparing base (d9645d7) to head (28d1af6).
⚠️ Report is 29 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/internal/test_runner/harness.js	65.00%	6 Missing and 1 partial ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #62501 +/- ##
==========================================
+ Coverage 89.69% 89.71% +0.01% 
==========================================
Files 692 692 Lines 214039 214061 +22 Branches 41064 41066 +2 ==========================================
+ Hits 191985 192037 +52 + Misses 14134 14098 -36 - Partials 7920 7926 +6 

Files with missing lines	Coverage Δ
lib/internal/test_runner/test.js	96.80% <100.00%> (+<0.01%)	⬆️
lib/test.js	100.00% <100.00%> (ø)
lib/internal/test_runner/harness.js	86.14% <65.00%> (-0.99%)	⬇️

... and 36 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[ljharb]

I think this could be used by malicious code to behave well in tests, and then behave badly in prod. it'd be different if it was like t.getContext(), though, because malicious code wouldn't have access to t. As it is, it feels like an implicit global communications channel, and those are not advisable.

Only requesting changes to ensure that with multiple stamps, it doesn't land until this item is discussed.

[benjamingr]

@ljharb wdym? It's for tracing so you can get a consistent trace how is that related to behavior change compared to prod? It's just the "frameworkness" of a test-runner

[MoLow]

@ljharb not sure who uses node:test in prod, but even if one does:

• this only exposes behavior of something that is currently already possible using createHook, executionAsyncId from node:async_hooks
• not sure whatt.getTestContext() is? the global getTestContext just exposes the current t

[ljharb]

I'm not saying they'd use node:test in prod. I'm saying that this allows code to behave differently in tests versus not in tests because it creates the capability to determine if you're in tests or not.

the global getTestContext just exposes the current t

right - that's internal state of the test runner that I don't think is safe to expose implicitly.

I'm perfectly happy with the motivating use case - just not granting all code in the node process the ability to intercept it. Could this information be exposed in a more limited fashion, like in a loader or something?

[MoLow]

As I said this is already possible in user land tody, it just makes it less verbose. there is no new vector, just better DX.
here is a userland implementation of this feature:

import{createHook,executionAsyncId}from'node:async_hooks';consttestResources=newMap<number,any>();functiongetTestContext(){returntestResources.get(executionAsyncId());}createHook({init(asyncId,type,triggerAsyncId,resource: any){if(type==='Test'&&'diagnostic'inresource&&typeofresource.diagnostic==='function'){testResources.set(asyncId,resource);return;}constparent=testResources.get(triggerAsyncId);if(parent){testResources.set(asyncId,parent);}},}).enable();

this PR just simplifies this and makes it more accurate (uses instanceof instead of type === 'Test' && 'diagnostic' in resource && typeof resource.diagnostic === 'function')

[ljharb]

I wasn't aware of this - if you're saying that async_hooks already did the thing I think is a very bad idea, then rather than improve the DX for it, i'd prefer to see that capability removed from async_hooks. Code should not be able to undeniably distinguish whether it's running in tests or not.

I suppose an alternative would be to keep the capability but disable it by default, or provide an env var that can disable it?

[MoLow]

I dont think it is easy to disable, this is just how async hooks work in node. frankly I also dont understand the concern about being able to determing you are inside test mode (there are a dozen other easier ways to know that, such as enviroment variables we inject, process.argv, process.argsz and more)

[benjamingr]

I'm saying that this allows code to behave differently in tests versus not in tests because it creates the capability to determine if you're in tests or not.

You can already do that though? (Even without internal (but existant) stuff like NODE_TEST_CONTEXT, or parsing the stack trace - a user can easily define an environment variable (or any global or module value really) in a before hook and access it from within tests.

This actually has (some) legitimate use cases - typically in realms of telemetry (no visible side effect) code

[benjamingr]

I suppose an alternative would be to keep the capability but disable it by default, or provide an env var that can disable it?

I suspect it would be a big break - tooling like ide extensions already relies on this (which is reasonable imo)

[ljharb]

env vars and process properties can be altered by me before running my test suite if i want to prevent my deps from distinguishing.

My concern isn't about the test author being able to enable this - that's fine! It's about the test author being unable to prevent it.

All the "proper" use cases of this are of course legitimate - but it would be nice if we didn't have to enable a new category of attack vector just to satisfy some legit use cases.

[MoLow]

@ljharb the concerns you are rasing seem unrelated to this PR then, it does not enable anything new, juts makes it more accesible

[ljharb]

That turns out to be the case, yes. However, making a harmful thing more accessible is usually not a good thing either :-/

Certainly consider my block removed, but I strongly urge you to reconsider this.

capability sadly already exists

[nodejs-github-bot]

Landed in c5a83ab

[aduh95]

This makes test-doc fail on v24.x-staging and would require a manual backport PR if we want it there