node icon indicating copy to clipboard operation
node copied to clipboard

test_runner: throw on invalid source map

Open avivkeller opened this issue 1 year ago • 4 comments

This PR changes the code coverage test runner to throw ERR_SOURCE_MAP_CANNOT_PARSE when a sourcemap is not a valid JSON file, or does not exist.

avivkeller avatar Sep 21 '24 21:09 avivkeller

Review requested:

  • [ ] @nodejs/test_runner

nodejs-github-bot avatar Sep 21 '24 21:09 nodejs-github-bot

Codecov Report

Attention: Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.

Project coverage is 88.41%. Comparing base (8de2695) to head (87af23b). Report is 290 commits behind head on main.

Files with missing lines Patch % Lines
lib/internal/test_runner/coverage.js 50.00% 1 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #55055      +/-   ##
==========================================
- Coverage   88.41%   88.41%   -0.01%     
==========================================
  Files         652      652              
  Lines      186576   186579       +3     
  Branches    36049    36049              
==========================================
- Hits       164960   164955       -5     
- Misses      14889    14900      +11     
+ Partials     6727     6724       -3     
Files with missing lines Coverage Δ
lib/internal/errors.js 96.98% <100.00%> (+<0.01%) :arrow_up:
lib/internal/test_runner/coverage.js 64.65% <50.00%> (-0.05%) :arrow_down:

... and 35 files with indirect coverage changes

codecov[bot] avatar Sep 21 '24 22:09 codecov[bot]

~~The first commit is #55037, which will land shortly.~~ This PR adds ERR_SOURCE_MAP_CANNOT_PARSE for when the source map does not exist / is not valid JSON

avivkeller avatar Sep 22 '24 20:09 avivkeller

Please stop changing unrelated things in PRs. It makes PRs more difficult to review and is more likely to introduce issues. If you feel strongly that the source map fixtures should be renamed, please do that in a separate PR.

IMO it's related because this adds tests, and moving them into their own file is good for decluttering, but I'll move it feel it's unrelated.

avivkeller avatar Sep 24 '24 16:09 avivkeller

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

nodejs-github-bot avatar Oct 06 '24 18:10 nodejs-github-bot

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

nodejs-github-bot avatar Oct 06 '24 19:10 nodejs-github-bot

Landed in 9a9409ff1f45c968173118de4cd37dea784f8ec9

nodejs-github-bot avatar Oct 06 '24 20:10 nodejs-github-bot