Test reporter - #56438
Conversation
This commit introduces a node:test reporter to the common utils. This reporter can be used to silence output other than errors from node:test. This is useful because in Node's own test suite, the output of node:test is included in the output of the Python test runner. Refs: nodejs#49120
|
Review requested:
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #56438 +/- ##
==========================================
+ Coverage 88.53% 89.17% +0.63%
==========================================
Files 657 662 +5
Lines 190741 191745 +1004
Branches 36607 36906 +299
==========================================
+ Hits 168881 170988 +2107
+ Misses 15036 13616 -1420
- Partials 6824 7141 +317 |
|
I kept the failing test so the output can be seen. I will remove it once we agree that output is good enough |
cjihrig
left a comment
There was a problem hiding this comment.
Personally, this seems fine to me. We can also iterate on it pretty quickly since it's internal only. I'm not sure if I can sign off on this or not since I'm the original author, but throwing my LGTM here.
Also, we'll have to remove that intentional failure before landing.
On a side note, I definitely think we (you?) should open another PR to get rid of the GitHub reporter. It generates far too much output noise.
|
Sure, I can open the PR |
|
This is what the output currently looks like: All of the lines starting with @Ceres6 can you remove the intentionally failing test now so that we can progress towards landing this. |
|
@cjihrig done! |
|
@MoLow it is intentional. See #56438 (comment). I pinged you in #52189 (comment) as well. Is it possible to make the GitHub reporter stop printing to stdout? If it is possible, I'd love to keep the GitHub annotations. |
I'l take a look, sure |
Co-authored-by: Colin Ihrig <cjihrig@gmail.com>
PR-URL: nodejs#48409 Reviewed-By: Jacob Smith <jacob@frende.me>
|
Landed in 0576deb |
|
No, I don't think it should be documented because it's internal. I don't have a strong opinion on exposing it. I wouldn't, but I'll leave that decision up to other folks. |


Building on top of #52189
Rebased with main to fix conflicts and added
FAIL_FASTenvvar.cc @cjihrig @JakobJingleheimer