test: change jenkins reporter

[Ceres6]

This PR intends to change the test reporter in Jenkins CI to report errors only.

Follow up of #56739

[Ceres6]

@cjihrig I think this might fix it. The format is intended for GitHub actions so we might need to create a new option in test.py to account for that.

The view from #56739 (comment) shouldn't appear because that seems to be from

if [ -f "test.tap" ]; then
   tap2junit -i test.tap -o out/junit/test.xml
fi

in the ci and now we're no longer using tap. Not sure where the jenkins config is so I can't check

[cjihrig]

I don't think this is currently correct (but I'm also not 100% sure, so please bear with me). I think we still want the Python runner to output TAP when run in Jenkins. I believe there is other tooling in the CI that relies on that TAP output.

I think we want to update AboutToRun in the TapProgressIndicator in tools/test.py to set the reporter flags, similar to how ActionsAnnotationProgressIndicator does.

[Ceres6]

Do you think we want them directly in test.py or may it's better to add them in the commands in the Makefile?

[cjihrig]

I don't really have a preference, but since the flags are already set that way in test.py, it probably makes sense to keep things consistent.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.18%. Comparing base (f2d2747) to head (303b95e).
Report is 61 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #56808      +/-   ##
==========================================
- Coverage   89.21%   89.18%   -0.04%     
==========================================
  Files         663      665       +2     
  Lines      192012   192602     +590     
  Branches    36927    37061     +134     
==========================================
+ Hits       171296   171764     +468     
- Misses      13586    13641      +55     
- Partials     7130     7197      +67     

see 50 files with indirect coverage changes

[Ceres6]

I was asking because I'm not sure if test file might be used for other things that might not want those flags, but the makefile probably isn't used for anything else

[cjihrig]

I know test.py is used to run tests locally by some folks. I'm not sure beyond that, but I believe we'll always want that reporter specified since this is being done to make the failure output something more acceptable for core contributors.

[Ceres6]

Changed, I thought we wanted to get rid of all the

ok 4134 sequential/test-require-cache-without-stat
  ---
  duration_ms: 75.81300
  ...

[nodejs-github-bot]

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

[cjihrig]

Changed, I thought we wanted to get rid of all the

No, that's just TAP. We're trying to address the opinion that unit tests that use node:test create too much output when failing. Some people would like to only see the errors on failure. Node's Jenkins setup also doesn't seem to handle some unicode characters properly (all of the checkmarks and other special characters in the output)

So we're trying to get rid of this (copied from #56739 (comment)):

11:29:23     ✔ No args (1.35912ms)
11:29:23     ✔ One arg = message (0.185382ms)
11:29:23     ✔ One arg = Error (0.098171ms)
11:29:23     ✖ Object prototype get (0.197572ms)
11:29:23     ℹ tests 4
11:29:23     ℹ suites 0
11:29:23     ℹ pass 3
11:29:23     ℹ fail 1
11:29:23     ℹ cancelled 0
11:29:23     ℹ skipped 0
11:29:23     ℹ todo 0
11:29:23     ℹ duration_ms 7.237494

There is also the stack trace view show below. This one is particularly problematic because the plugin used to create it butchers the output into something like this:

stack: "\u2714 No args (1.328214ms)\n\u2714 One arg = message (0.18106ms)\n\u2714\
  \ One arg = Error (0.101721ms)\n\u2716 Object prototype get (0.177542ms)\n\u2139\
  \ tests 4\n\u2139 suites 0\n\u2139 pass 3\n\u2139 fail 1\n\u2139 cancelled 0\n\u2139\
  \ skipped 0\n\u2139 todo 0\n\u2139 duration_ms 6.872425\n\n\u2716 failing tests:\n\
  \ntest at test/parallel/test-assert-fail.js:46:1\n\u2716 Object prototype get (0.177542ms)\n\
  \  Error: failed\n      at TestContext.<anonymous> (/home/iojs/build/workspace/node-test-commit-linuxone/test/parallel/test-assert-fail.js:50:9)\n\
  \      at Test.runInAsyncScope (node:async_hooks:211:14)\n      at Test.run (node:internal/test_runner/test:980:25)\n\
  \      at Test.processPendingSubtests (node:internal/test_runner/test:678:18)\n\
  \      at Test.postRun (node:internal/test_runner/test:1091:19)\n      at Test.run\
  \ (node:internal/test_runner/test:1019:12)\n      at async Test.processPendingSubtests\
  \ (node:internal/test_runner/test:678:7)"

[cjihrig]

The good news is that this seems to mostly work:

09:01:19 not ok 392 parallel/test-assert-fail
09:01:19   ---
09:01:19   duration_ms: 57.51700
09:01:19   severity: fail
09:01:19   exitcode: 1
09:01:19   stack: |-
09:01:19     Test failure: 'Object prototype get'
09:01:19     Location: test/parallel/test-assert-fail.js:46:1
09:01:19     Error: fail
09:01:19         at TestContext.<anonymous> (/home/iojs/build/workspace/node-test-commit-linuxone/test/parallel/test-assert-fail.js:50:9)
09:01:19         at Test.runInAsyncScope (node:async_hooks:211:14)
09:01:19         at Test.run (node:internal/test_runner/test:980:25)
09:01:19         at Test.processPendingSubtests (node:internal/test_runner/test:678:18)
09:01:19         at Test.postRun (node:internal/test_runner/test:1091:19)
09:01:19         at Test.run (node:internal/test_runner/test:1019:12)
09:01:19         at async Test.processPendingSubtests (node:internal/test_runner/test:678:7)
09:01:19    

and stacktrace view:

---
duration_ms: 57.517
exitcode: 1
severity: fail
stack: |-
  Test failure: 'Object prototype get'
  Location: test/parallel/test-assert-fail.js:46:1
  Error: fail
      at TestContext.<anonymous> (/home/iojs/build/workspace/node-test-commit-linuxone/test/parallel/test-assert-fail.js:50:9)
      at Test.runInAsyncScope (node:async_hooks:211:14)
      at Test.run (node:internal/test_runner/test:980:25)
      at Test.processPendingSubtests (node:internal/test_runner/test:678:18)
      at Test.postRun (node:internal/test_runner/test:1091:19)
      at Test.run (node:internal/test_runner/test:1019:12)
      at async Test.processPendingSubtests (node:internal/test_runner/test:678:7)
...

The bad news is that we also run the test suite inside of a worker thread, and that appears to be breaking: https://ci.nodejs.org/job/node-test-commit-custom-suites-freestyle/40511/. The error looks like this:

---
duration_ms: 211.291
exitcode: 1
severity: fail
stack: |2-

  node:internal/event_target:1101
    process.nextTick(() => { throw err; });
                             ^
  Error: Cannot find module '/home/iojs/build/workspace/node/--test-reporter=./test/common/test-error-reporter.js'
      at Module._resolveFilename (node:internal/modules/cjs/loader:1394:15)
      at defaultResolveImpl (node:internal/modules/cjs/loader:1050:19)
      at resolveForCJSWithHooks (node:internal/modules/cjs/loader:1055:22)
      at Module._load (node:internal/modules/cjs/loader:1204:37)
      at TracingChannel.traceSync (node:diagnostics_channel:322:14)
      at wrapModuleLoad (node:internal/modules/cjs/loader:234:24)
      at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:151:5)
      at MessagePort.<anonymous> (node:internal/main/worker_thread:212:26)
      at [nodejs.internal.kHybridDispatch] (node:internal/event_target:827:20)
      at MessagePort.<anonymous> (node:internal/per_context/messageport:23:28) {
    code: 'MODULE_NOT_FOUND',
    requireStack: []
  }

  Node.js v24.0.0-pre
...

And the command that is run for that is /usr/bin/python3 tools/test.py -j 2 -p tap --logfile test.tap --mode=release --flaky-tests=dontcare --worker default. It looks like the --worker flag also appends arguments.

[cjihrig]

Perhaps this is the area where the flags should be set (including for the GitHub Actions runs).

[Ceres6]

I think I got it now, not sure if I might be missing the option somewhere else but now at least it would go after the worker one if both are passed

[nodejs-github-bot]

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

[cjihrig]

The CI is still running. It looks like this is working correctly everywhere except in the case where the suite is run within a worker: https://ci.nodejs.org/job/node-test-commit-custom-suites-freestyle/40530/testReport/junit/(root)/parallel/test_assert_fail/

---
duration_ms: 269.24
exitcode: 1
severity: fail
stack: "\u2714 No args (4.444589ms)\n\u2714 One arg = message (0.652588ms)\n\u2714\
  \ One arg = Error (0.474843ms)\n\u2716 Object prototype get (0.793434ms)\n\u2139\
  \ tests 4\n\u2139 suites 0\n\u2139 pass 3\n\u2139 fail 1\n\u2139 cancelled 0\n\u2139\
  \ skipped 0\n\u2139 todo 0\n\u2139 duration_ms 21.185476\n\n\u2716 failing tests:\n\
  \ntest at test/parallel/test-assert-fail.js:46:1\n\u2716 Object prototype get (0.793434ms)\n\
  \  Error: fail\n      at TestContext.<anonymous> (/home/iojs/build/workspace/node/test/parallel/test-assert-fail.js:50:9)\n\
  \      at Test.runInAsyncScope (node:async_hooks:211:14)\n      at Test.run (node:internal/test_runner/test:980:25)\n\
  \      at Test.processPendingSubtests (node:internal/test_runner/test:678:18)\n\
  \      at Test.postRun (node:internal/test_runner/test:1091:19)\n      at Test.run\
  \ (node:internal/test_runner/test:1019:12)\n      at process.processTicksAndRejections\
  \ (node:internal/process/task_queues:105:5)\n      at async Test.processPendingSubtests\
  \ (node:internal/test_runner/test:678:7)"
...

Here is the command being executed: /usr/bin/python3 tools/test.py -j 4 -p tap --logfile test.tap --mode=release --flaky-tests=dontcare --worker default

It might be worth enabling this new --error-reporter option by default so it is automatically picked up everywhere.

[nodejs-github-bot]

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

[nodejs-github-bot]

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

[cjihrig]

I think this got all of the cases (we'll have to double check once the CI finishes completely) 🎉

GitHub Actions:

=== release test-assert-fail ===
Path: parallel/test-assert-fail
Error: --- stdout ---
Test failure: 'Object prototype get'
Location: test/parallel/test-assert-fail.js:46:1
Error: fail
    at TestContext.<anonymous> (/Users/runner/work/node/node/node/test/parallel/test-assert-fail.js:50:9)
    at Test.runInAsyncScope (node:async_hooks:211:14)
    at Test.run (node:internal/test_runner/test:989:25)
    at Test.processPendingSubtests (node:internal/test_runner/test:680:18)
    at Test.postRun (node:internal/test_runner/test:1105:19)
    at Test.run (node:internal/test_runner/test:1033:12)
    at async Test.processPendingSubtests (node:internal/test_runner/test:680:7)
Command: out/Release/node --test-reporter=./test/common/test-error-reporter.js --test-reporter-destination=stdout /Users/runner/work/node/node/node/test/parallel/test-assert-fail.js

===
=== 1 tests failed
===

Failed tests:
out/Release/node --test-reporter=./test/common/test-error-reporter.js --test-reporter-destination=stdout /Users/runner/work/node/node/node/test/parallel/test-assert-fail.js
make[1]: *** [test-ci] Error 1

Jenkins console output view:

17:14:35 not ok 19 parallel/test-assert-fail
17:14:35   ---
17:14:35   duration_ms: 215.69800
17:14:35   severity: fail
17:14:35   exitcode: 1
17:14:35   stack: |-
17:14:35     Test failure: 'Object prototype get'
17:14:35     Location: test/parallel/test-assert-fail.js:46:1
17:14:35     Error: fail
17:14:35         at TestContext.<anonymous> (/home/iojs/build/workspace/node/test/parallel/test-assert-fail.js:50:9)
17:14:35         at Test.runInAsyncScope (node:async_hooks:211:14)
17:14:35         at Test.run (node:internal/test_runner/test:989:25)
17:14:35         at Test.processPendingSubtests (node:internal/test_runner/test:680:18)
17:14:35         at Test.postRun (node:internal/test_runner/test:1105:19)
17:14:35         at Test.run (node:internal/test_runner/test:1033:12)
17:14:35         at process.processTicksAndRejections (node:internal/process/task_queues:105:5)
17:14:35         at async Test.processPendingSubtests (node:internal/test_runner/test:680:7)
17:14:35     
17:14:35   ...

Jenkins stack trace view:

---
duration_ms: 215.698
exitcode: 1
severity: fail
stack: |-
  Test failure: 'Object prototype get'
  Location: test/parallel/test-assert-fail.js:46:1
  Error: fail
      at TestContext.<anonymous> (/home/iojs/build/workspace/node/test/parallel/test-assert-fail.js:50:9)
      at Test.runInAsyncScope (node:async_hooks:211:14)
      at Test.run (node:internal/test_runner/test:989:25)
      at Test.processPendingSubtests (node:internal/test_runner/test:680:18)
      at Test.postRun (node:internal/test_runner/test:1105:19)
      at Test.run (node:internal/test_runner/test:1033:12)
      at process.processTicksAndRejections (node:internal/process/task_queues:105:5)
      at async Test.processPendingSubtests (node:internal/test_runner/test:680:7)
...

[anonrig]

Nice!!

[nodejs-github-bot]

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

[cjihrig]

I see test/es-module/test-esm-no-addons.mjs is failing, and I think it might be legitimate. It has a process.execArgv.length === 0 check. We might need to update that to check !process.execArgv.includes('--no-addons').

[nodejs-github-bot]

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

[nodejs-github-bot]

Landed in 69502d8