test: fix tests that fail under coverage #20794
Conversation
nodejs-github-bot
added
the
build
test/message/core_line_numbers.out
Outdated
| throw new RangeError(errors[type]); | ||
| ^ | ||
|
|
||
| RangeError: Invalid input | ||
| at error (punycode.js:42:*) |
There was a problem hiding this comment.
The whole test is only about this line number. Therefore I would rather not touch this / find a different solution for this.
There was a problem hiding this comment.
any recommendations? this test will never work on the transpiled code. One thought, perhaps perhaps extend on the matcher to support something like this:
at error (punycode.js:(42|929):*)
There was a problem hiding this comment.
That does not seem to be a reliable way as the line number could likely also change again without anyone noticing it and it could also produce falsy expected entries in other output files.
What might work is to pass a flag / option to the test runner and then the test runner just ignores all numbers when receiving patterns that look like +at .*.js:(?:\d+|\*)(?::(?:\d+|\*))?.
That way this would never be a problem anymore even if new tests add line numbers and or columns.
There was a problem hiding this comment.
what if we just update the assertion to look at \d+; seems like the 42 is fragile ... and asserting that we have a line number would be sufficient for this test.
There was a problem hiding this comment.
That does not work either. The test is about the specific line number.
There was a problem hiding this comment.
See bcoe@b799a74#diff-fac9894e053cf4e578875b1166718a58 for more context.
There was a problem hiding this comment.
It seems like overkill to add a flag for one test; I'd be more tempted to add it to a skip list for coverage -- seems like a reasonable compromise.
|
@Trott looking at these failures, nothing is jumping out at me related to Makefile changes; any thoughts? |
|
@bcoe FreeBSD one is a known failure that is being worked on. The Windows failure for the trace api test is a new one to me but Windows has been having big problems the last 48 hours or so. Everything else looks good (green or yellow). Let's try again: FreeBSD: https://ci.nodejs.org/job/node-test-commit-freebsd/17932/ Windows: https://ci.nodejs.org/job/node-test-commit-windows-fanned/18164/ |
|
Landed in 1637714 |
Make test runner capable of skipping tests, which makes it possible to skip the failing test/message/core_line_numbers.js test. Make nyc no longer generate compact instrumentation (this causes significantly different code output, which leads to failing test assertions). PR-URL: #20794 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Khaidi Chu <i@2333.moe> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Make test runner capable of skipping tests, which makes it possible to skip the failing test/message/core_line_numbers.js test. Make nyc no longer generate compact instrumentation (this causes significantly different code output, which leads to failing test assertions). PR-URL: #20794 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Khaidi Chu <i@2333.moe> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Make test runner capable of skipping tests, which makes it possible to skip the failing test/message/core_line_numbers.js test. Make nyc no longer generate compact instrumentation (this causes significantly different code output, which leads to failing test assertions). PR-URL: #20794 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Khaidi Chu <i@2333.moe> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
A couple tests were failing when running
make coverage, to fix this problem I:core_line_numbers.jsno longer check line #s (coverage changes line #s, this was addressed in most other test files).nycno longer generate compact instrumentation (this was making significantly different code output, which lead to failing test assertions).Note: once unit tests all started passing, it became apparent that we probably shouldn't be running the linter when code is instrumented for coverage ... since the instrumentation results in 1000s of rules being broken.
CC: @addaleax
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes