test_runner: reimplement assert.ok to allow stack parsing

[RedYetiDev]

Fixes #54760
Ref: nodejs/help#4461

By default, assert.ok uses itself as a reference for where to start checking the stacktrace. In the test runner, there is a wrapper around this call (for test.plan), so it can't use itself as a reference.

This PR pulls out innerOk into it's own file, allowing the test runner to re-implement assert.ok using it's own function as a reference for the stacktrace.

(Hopefully that makes sense).

[nodejs-github-bot]

Review requested:

• @nodejs/test_runner

[RedYetiDev]

CC @nodejs/assert

[codecov]

Codecov Report

Attention: Patch coverage is 90.30100% with 29 lines in your changes missing coverage. Please review.

Project coverage is 88.05%. Comparing base (53cdffe) to head (0a06a62).
Report is 46 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/internal/assert/utils.js	89.89%	28 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #54776      +/-   ##
==========================================
- Coverage   88.06%   88.05%   -0.01%     
==========================================
  Files         651      652       +1     
  Lines      183387   183439      +52     
  Branches    35794    35823      +29     
==========================================
+ Hits       161494   161533      +39     
+ Misses      15166    15157       -9     
- Partials     6727     6749      +22     

Files with missing lines	Coverage Δ
lib/assert.js	99.87% <100.00%> (+2.65%)	⬆️
lib/internal/test_runner/test.js	96.99% <100.00%> (+0.06%)	⬆️
lib/internal/assert/utils.js	89.89% <89.89%> (ø)

... and 44 files with indirect coverage changes

[mcollina]

lgtm

[nodejs-github-bot]

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

[RedYetiDev]

I've figured out a much less hacky way to acheive the same result.

[RedYetiDev]

I'm currently facing the following issue:
I've extended the stacktrace call-finding to search for the most recent userland function that called the assert. However, this causes the following case:

myEvent.on("event", assert);
myEvent.emit("event", false);

Will cause myEvent.emit("event", false) to be the error, not true === false. I'm not opposed to that behavior, but that changes the existing behavior, making this semver-major, so I want some feedback on that first? WDYT?

[cjihrig]

making this semver-major

We should try to avoid that if possible.

[RedYetiDev]

making this semver-major

We should try to avoid that if possible.

We can go back to what it was, that is, reimplemented in the test runner, but it's kinda hacky.

[cjihrig]

The thing about semver major changes is not only that they are breaking and can cause users headaches, but they can't be backported, so Node 20 will have this bug until April of 2026.

[nodejs-github-bot]

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

[nodejs-github-bot]

Commit Queue failed

- Loading data for nodejs/node/pull/54776
✔  Done loading data for nodejs/node/pull/54776
----------------------------------- PR info ------------------------------------
Title      test_runner: reimplement `assert.ok` to allow stack parsing (#54776)
Author     Aviv Keller <redyetidev@gmail.com> (@RedYetiDev)
Branch     RedYetiDev:test-runner-assert-stack -> nodejs:main
Labels     assert, author ready, test_runner
Commits    1
 - test_runner: reimplement `assert.ok` to allow stack parsing
Committers 1
 - RedYetiDev <38299977+RedYetiDev@users.noreply.github.com>
PR-URL: https://github.com/nodejs/node/pull/54776
Refs: https://github.com/nodejs/help/issues/4461
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/54776
Refs: https://github.com/nodejs/help/issues/4461
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Wed, 04 Sep 2024 22:39:16 GMT
   ✔  Approvals: 3
   ✔  - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/54776#pullrequestreview-2282105721
   ✔  - Chemi Atlow (@atlowChemi): https://github.com/nodejs/node/pull/54776#pullrequestreview-2282187629
   ✔  - Colin Ihrig (@cjihrig): https://github.com/nodejs/node/pull/54776#pullrequestreview-2305376460
   ✘  Last GitHub CI failed
   ℹ  Last Full PR CI on 2024-09-14T17:21:15Z: https://ci.nodejs.org/job/node-test-pull-request/62431/
- Querying data for job/node-test-pull-request/62431/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

https://github.com/nodejs/node/actions/runs/10871984761

[RedYetiDev]

The linters were stopped, so the GitHub CI counted as a failure. Can someone restart them / manually land the PR?

[nodejs-github-bot]

Landed in 028c586