test_runner: add context subtests expectFailure only skip todo

[JakobJingleheimer]

Closes #61251
Closes #62156
Fixes #50665

[nodejs-github-bot]

Review requested:

• @nodejs/test_runner

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.79%. Comparing base (1989f4d) to head (bc3b228).
⚠️ Report is 1834 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #62263      +/-   ##
==========================================
+ Coverage   89.65%   89.79%   +0.13%     
==========================================
  Files         676      697      +21     
  Lines      206555   215809    +9254     
  Branches    39547    41306    +1759     
==========================================
+ Hits       185195   193787    +8592     
- Misses      13495    14112     +617     
- Partials     7865     7910      +45     

Files with missing lines	Coverage Δ
lib/internal/test_runner/test.js	97.00% <100.00%> (+0.19%)	⬆️

... and 154 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.

[JakobJingleheimer]

Okee I finally figure out what is wrong:

  at Test.start (node:internal/test_runner/test:…)
- at TestContext.test (node:internal/test_runner/test:377:20)
+ at #runTest (node:internal/test_runner/test:378:20)
+ at test.ObjectAssign.__proto__ [as test] (node:internal/test_runner/test:383:59)
  at TestContext.<anonymous> (<project-root>/test/fixtures/test-runner/output/hooks.js:197:11)
- at Test.runInAsyncScope (node:async_hooks:226:14)

full diff screenshot

The problem is the new implementation replaces 1 entry (at TestContext.test) in the middle of the stack with 2 entries (at #runTest and at test.ObjectAssign.__proto__), and getCallerLocation() returns 10 entries, causing the bottom Test.runInAsyncScope to get lobbed off.

There are 2 things to consider here:

• at #runTest and at test.ObjectAssign.__proto__ is not as pretty as at TestContext.test
  • moving the ObjectAssign to the constructor (ObjectAssign(this.test, {…}) does make the stack prettier:

    at Test.start (node:internal/test_runner/test:1136:17)
    at #runTest (node:internal/test_runner/test:406:20)
    at TestContext.test (node:internal/test_runner/test:412:25)
    at TestContext.<anonymous> (<project-root>/test/fixtures/test-runner/output/hooks.js:197:11)
    

• is that bottom Test.runInAsyncScope important?

[github-actions]

This pull request has been marked as stale due to 90 days of inactivity.
It will be automatically closed in 30 days if no further activity occurs. If this is still relevant, please leave a comment or update it to keep it open.

[github-actions]

This pull request has been automatically closed after 30 days of inactivity following its stale status (no activity for a total of 120 days).
If this is still relevant, feel free to reopen it or leave a comment with additional details so we can continue the discussion.

[H4ad]

@JakobJingleheimer there's anything missing here apart from the test breaking due to the update in the call stack? I thought this was good to merge

[JakobJingleheimer]

Correct, that's all. I can easily fix it, I just need some input on how it should be fixed.

[H4ad]

You likely just need to regenerate those snapshot tests, change in the call stack is not an issue

To improve the call stack test, maybe you can move the ObjectAssign to constructor?

class Test {
constructor(){ 
  ObjectAssign(this.test, { only: ... })
}
test() { ... } 

[JakobJingleheimer]

It is an issue because the utility that composes the stack is limited to like 7 lines, and a maybe important line gets clipped. We need to decide whether to clip the line or expand the stack to include an extra line (which requires using a different utility, so a bit of refactoring)