test_runner: consider --enable-source-maps option in coverage report#55146
test_runner: consider --enable-source-maps option in coverage report#55146geeksilva97 wants to merge 1 commit intonodejs:mainfrom
Conversation
|
Review requested:
|
| const { result } = coverage; | ||
| const sourceMapCache = coverage['source-map-cache']; | ||
| if (!sourceMapCache) { | ||
| if (!sourceMapEnabled || !sourceMapCache) { |
There was a problem hiding this comment.
Don't use getOptionValue. Instead, check whether the test runners sourceMaps is true.
There was a problem hiding this comment.
@redyetidev , do you have any clue on how I could do that?
There was a problem hiding this comment.
See my previous implementation for more information
|
This was tried in #55039, however it couldn't land until a regression was fixed. IMHO this PR will have the same result. Maybe looking into-and fixing?-the regression will help this move along? |
Thanks for letting me know. Let me close this then and look into this regression stuff. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #55146 +/- ##
==========================================
- Coverage 88.25% 88.24% -0.01%
==========================================
Files 651 651
Lines 183915 183900 -15
Branches 35867 35857 -10
==========================================
- Hits 162307 162276 -31
- Misses 14895 14910 +15
- Partials 6713 6714 +1
|
|
@redyetidev I had started some implementation related to non-mapped lines in that same issue. Following @jaydenseric's suggestion it worked. Is it worth opening a PR or does it also depend on regression? |
|
Feel free to open a PR. Only --enable-source-maps has the regression |
Ref: #54753