test_runner: print formatted errors on summary #56911
Conversation
|
Review requested:
|
nodejs-github-bot
added
needs-ci
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #56911 +/- ##
==========================================
+ Coverage 89.16% 89.21% +0.04%
==========================================
Files 665 665
Lines 192602 192511 -91
Branches 37050 37045 -5
==========================================
+ Hits 171732 171747 +15
+ Misses 13678 13606 -72
+ Partials 7192 7158 -34
|
pmarchini
force-pushed
the
issue/56902
branch
from
72e82e9 to
9e21914
Compare
| ArrayPrototypePush(results, formattedErr); | ||
| } | ||
| callback(null, ArrayPrototypeJoin(results, '\n')); | ||
| callback(null, this.#formatFailedTestResults()); |
There was a problem hiding this comment.
We shouldn't still need this, right?
There was a problem hiding this comment.
in theory no but since it's part of _flush, I think it shouldn't be a big deal to have it there as well.
Wdyt?
There was a problem hiding this comment.
In non-watch mode, wouldn't it cause the errors to be printed here and for the summary event?
There was a problem hiding this comment.
nope as on each "print" we're cleaning this.#failedTests = []; // Clean up the failed tests.
But still we can remove it
There was a problem hiding this comment.
Ah ok. I guess the question is, "is this expected to do anything?" My thoughts are no, so we should probably remove it.
There was a problem hiding this comment.
I've just checked, and I was wrong.
Removing _flush breaks the following tests:
Following this discussion: #56662 (comment)
This PR should address #56902