Fix broken mapped coverage map #6
Conversation
|
CLA is valid! |
| @@ -51,7 +51,7 @@ describe('mapped coverage', function () { | |||
| assert.equal(mc.fnMap[index].name,'(unknown_' + index + ')'); | |||
|
|
|||
| index = mc.addBranch('if', loc(1, 15, 1, 20), [loc(1, 15, 1, 20), loc(1, 21, 1, 40)], [1,0]); | |||
| assert.ok(index); | |||
| assert.ok(index === 0); | |||
There was a problem hiding this comment.
nit: why not just use assert.equal here? The assertion message if it fails would be more helpful.
There was a problem hiding this comment.
Fair point, replaced with strictEqual.
| index = meta.last.s; | ||
| meta.last.s += 1; |
There was a problem hiding this comment.
Let me see if I'm following here:
- there was logic in place that handled populating a statement the first time that it was observed; this logic used
!so was triggering on0, which currently counts as a valid mapping. - as a result, we accidentally fall outside of bounds in
istanbul-lib-coverage? could you better explain to me the cause of this?
This fix addresses the issue by looking for undefined rather than 0.
There was a problem hiding this comment.
@bcoe no, the main issue was that this code started from 1 when filling out statement, function, and branch stats, rather than 0 we switched to in istanbuljs-archived-repos/istanbul-lib-instrument#22. Then, when trying to merge stats generated by this code (1-based) with ones instrumented by instanbul-lib-instrument (0-based), it throwed because 0 element was undefined on the first.
|
@mourner published in |
Closes istanbuljs/nyc#400, closes istanbuljs-archived-repos/istanbul-lib-instrument#25.
@bcoe sorry this slipped past me! The bug was only visible when you run coverage on both source-mapped and non-source-mapped files and then it tried to merge the two.