Chore/remove test reporter duplicate

[Ceres6]

Currently the test reporter is specified both in workflow files and test.py. This PR removes it from the formers to avoid duplication

[nodejs-github-bot]

Review requested:

• @nodejs/actions

[Ceres6]

see #56468 (comment)

[Ceres6]

cc @cjihrig

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.72%. Comparing base (08eeddf) to head (9fc6be1).
Report is 40 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #56739      +/-   ##
==========================================
- Coverage   89.21%   88.72%   -0.49%     
==========================================
  Files         662      662              
  Lines      191968   191919      -49     
  Branches    36955    36599     -356     
==========================================
- Hits       171269   170286     -983     
- Misses      13535    14471     +936     
+ Partials     7164     7162       -2     

see 121 files with indirect coverage changes

[cjihrig]

It looks like the CI is not happy yet. It looks like there is an extra or missing --test-reporter-destination somewhere based on the error message.

[Ceres6]

@cjihrig Based on the error there seems to be an extra one, probably the one defined below in the workflow files, just removed it

[cjihrig]

Looking at the GitHub Actions output, I see two failures. One is #56768 and can be ignored. The other is the intentional failure from this PR:

Test failure: 'Object prototype get'
Location: test/parallel/test-assert-fail.js:46:1
Error: failed
    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: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)

This includes only the test title, location, and actual error as expected. Nice work.

I'll kick off a Jenkins CI run to see how that looks.

[nodejs-github-bot]

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

[Ceres6]

Cool! In the meantime, lmk if there's anything else I can help with

[cjihrig]

It looks like this does not use the error only reporter properly in Jenkins:

11:29:23 not ok 29 parallel/test-assert-fail
11:29:23   ---
11:29:23   duration_ms: 152.94200
11:29:23   severity: fail
11:29:23   exitcode: 1
11:29:23   stack: |-
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
11:29:23     
11:29:23     ✖ failing tests:
11:29:23     
11:29:23     test at test/parallel/test-assert-fail.js:46:1
11:29:23     ✖ Object prototype get (0.197572ms)
11:29:23       Error: failed
11:29:23           at TestContext.<anonymous> (/home/iojs/build/workspace/node-test-commit-linuxone/test/parallel/test-assert-fail.js:50:9)
11:29:23           at Test.runInAsyncScope (node:async_hooks:211:14)
11:29:23           at Test.run (node:internal/test_runner/test:980:25)
11:29:23           at Test.processPendingSubtests (node:internal/test_runner/test:678:18)
11:29:23           at Test.postRun (node:internal/test_runner/test:1091:19)
11:29:23           at Test.run (node:internal/test_runner/test:1019:12)
11:29:23           at async Test.processPendingSubtests (node:internal/test_runner/test:678:7)

I think we should land this change and address Jenkins in a followup (unless you want to try getting Jenkins fixed up here).

[cjihrig]

FWIW, I believe this "stack trace" view is the biggest problem: https://ci.nodejs.org/job/node-test-commit-linuxone/47790/nodes=rhel9-s390x/testReport/junit/(root)/parallel/test_assert_fail/

---
duration_ms: 57.108
exitcode: 1
severity: fail
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)"
...

This should be addressed by getting the correct reporter in place though.

[Ceres6]

I'm okay with landing this and following up with a new one. Should I remove the failing test, then?

[cjihrig]

Yea, if you want to get this landed now, please remove the failing test.

[Ceres6]

Done!

[Ceres6]

@cjihrig seems like a reported flaky test, should we keep trying?

[nodejs-github-bot]

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

[nodejs-github-bot]

Landed in 0741d69