fix(lib/test): stack traces when res.name is Error, not string #532
Conversation
|
@ljharb Not sure why it's failing on older Node versions TBH. Do you have any pro tips where I could start with fixing the CI on NodeJS 7, 6, etc.? Failing assertion here for example: Thank you in advance! |
When executing tests, the res.name property (as constructed in the ./lib/test.js#_assert method) is actually not a string but the error that was thrown. This (assuming edge case) combined with extra.error and opts.error both being falsy ends up producing this situation where feeding res.name into the Error constructor makes the original error's stack trace disappear and only it's message remains with the newly constructed Error instance holding a completely generic stack trace that's internal to tape's source code (and therefore not much use when a test is failing due to some code is throwing exceptions instead of failing an assertion). Signed-off-by: Peter Somogyvari <[email protected]>
ljharb
force-pushed
the
fix/stack-trace-res-name-error
branch
from
c40ec63 to
3e8cf85
Compare
Codecov Report
@@ Coverage Diff @@
## master #532 +/- ##
==========================================
+ Coverage 73.84% 73.87% +0.03%
==========================================
Files 19 19
Lines 757 758 +1
Branches 146 147 +1
==========================================
+ Hits 559 560 +1
Misses 198 198
Continue to review full report at Codecov.
|
|
@petermetz i've rebased this. it's passing ; but either way it needs a regression test. |
Thanks @ljharb I'll try to put a specific test case in there. |
|
@petermetz any chance you're still planning to add the regression test? |
|
@ljharb Apologies, still planning on it, yes. Might take some more time, but definitely have the intent. :-) |
|
No worries, whenever you can get to it is fine. |
ljharb
force-pushed
the
master
branch
3 times, most recently
from
c16dde3 to
2ad86d4
Compare
When executing tests, the res.name property
(as constructed in the ./lib/test.js#_assert method)
is actually not a string but the error that was thrown.
This (assuming edge case) combined with
extra.error and opts.error both being falsy ends up
producing this situation where feeding res.name into
the Error constructor makes the original error's stack
trace disappear and only it's message remains with the
newly constructed Error instance holding a completely
generic stack trace that's internal to tape's source code
(and therefore not much use when a test is failing due to
some code is throwing exceptions instead of failing an
assertion).
Signed-off-by: Peter Somogyvari [email protected]