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';

const pretty = prettyFactory({ colorize: true  });
const asyncId = executionAsyncId();
const t = 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 9 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';

const testResources = new Map<number, any>();

function getTestContext() {
  return testResources.get(executionAsyncId());
}

createHook({
  init(asyncId, type, triggerAsyncId, resource: any) {
    if (type === 'Test' && 'diagnostic' in resource && typeof resource.diagnostic === 'function') {
      testResources.set(asyncId, resource);
      return;
    }
    const parent = 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)