Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] Unable to merge coverage for the same file #753

Closed
sashuk opened this issue Nov 6, 2023 · 5 comments · Fixed by #754 or #755
Closed

[Bug] Unable to merge coverage for the same file #753

sashuk opened this issue Nov 6, 2023 · 5 comments · Fixed by #754 or #755

Comments

@sashuk
Copy link

sashuk commented Nov 6, 2023

One of my projects started failing with the following message (I use latest https://github.com/ljharb/istanbul-merge that pulls latest https://www.npmjs.com/package/istanbul-lib-coverage)

aleksandr@Alexanders-MacBook-Pro-2 test-portal % npx istanbul-merge --out "coverage/coverage-final-1.json" "./**/coverage/coverage-final.json"
/Users/aleksandr/.npm/_npx/98709a0041747bf7/node_modules/istanbul-lib-coverage/lib/file-coverage.js:74
            itemLoc.start.line - mapLoc.start.line,
                    ^

TypeError: Cannot read properties of undefined (reading 'start')
    at findNearestContainer (/Users/aleksandr/.npm/_npx/98709a0041747bf7/node_modules/istanbul-lib-coverage/lib/file-coverage.js:74:21)
    at addNearestContainerHits (/Users/aleksandr/.npm/_npx/98709a0041747bf7/node_modules/istanbul-lib-coverage/lib/file-coverage.js:121:23)
    at mergeProp (/Users/aleksandr/.npm/_npx/98709a0041747bf7/node_modules/istanbul-lib-coverage/lib/file-coverage.js:147:25)
    at FileCoverage.merge (/Users/aleksandr/.npm/_npx/98709a0041747bf7/node_modules/istanbul-lib-coverage/lib/file-coverage.js:301:27)
    at CoverageMap.addFileCoverage (/Users/aleksandr/.npm/_npx/98709a0041747bf7/node_modules/istanbul-lib-coverage/lib/coverage-map.js:112:29)
    at /Users/aleksandr/.npm/_npx/98709a0041747bf7/node_modules/istanbul-lib-coverage/lib/coverage-map.js:55:18
    at Array.forEach (<anonymous>)
    at CoverageMap.merge (/Users/aleksandr/.npm/_npx/98709a0041747bf7/node_modules/istanbul-lib-coverage/lib/coverage-map.js:54:35)
    at /Users/aleksandr/.npm/_npx/98709a0041747bf7/node_modules/istanbul-merge/bin/istanbul-merge:36:6
    at forEachArray (/Users/aleksandr/.npm/_npx/98709a0041747bf7/node_modules/for-each/index.js:12:17)

Node.js v18.16.0

It started failing right after https://github.com/istanbuljs/istanbuljs/pull/750/files was published as latest.

After doing some research locally I was able to narrow the problem down to the coverage file, coming from one of the dependencies, particularly node_modules/tsconfig-paths-webpack-plugin/coverage/coverage-final-test.json (istanbul-merge looks into node_modules, there is no easy way to exclude node_modules via glob).

How to reproduce

Please check out https://github.com/sashuk/demo-coverage and run npx istanbul-merge --out "coverage-final-1.json" "./**/coverage-final.json"

aleksandr@Alexanders-MacBook-Pro-2 demo-coverage % npx istanbul-merge --out "coverage-final-1.json" "./**/coverage-final.json"
@@@ other FileCoverage {
  data: {
    path: '/random-file.ts',
    ... some debugging output ...
  }
}
/Users/aleksandr/.npm/_npx/98709a0041747bf7/node_modules/istanbul-lib-coverage/lib/file-coverage.js:74
            itemLoc.start.line - mapLoc.start.line,
                    ^

TypeError: Cannot read properties of undefined (reading 'start')
    at findNearestContainer (/Users/aleksandr/.npm/_npx/98709a0041747bf7/node_modules/istanbul-lib-coverage/lib/file-coverage.js:74:21)
    at addNearestContainerHits (/Users/aleksandr/.npm/_npx/98709a0041747bf7/node_modules/istanbul-lib-coverage/lib/file-coverage.js:121:23)
    at mergeProp (/Users/aleksandr/.npm/_npx/98709a0041747bf7/node_modules/istanbul-lib-coverage/lib/file-coverage.js:147:25)
    at FileCoverage.merge (/Users/aleksandr/.npm/_npx/98709a0041747bf7/node_modules/istanbul-lib-coverage/lib/file-coverage.js:302:27)
    at CoverageMap.addFileCoverage (/Users/aleksandr/.npm/_npx/98709a0041747bf7/node_modules/istanbul-lib-coverage/lib/coverage-map.js:112:29)
    at /Users/aleksandr/.npm/_npx/98709a0041747bf7/node_modules/istanbul-lib-coverage/lib/coverage-map.js:55:18
    at Array.forEach (<anonymous>)
    at CoverageMap.merge (/Users/aleksandr/.npm/_npx/98709a0041747bf7/node_modules/istanbul-lib-coverage/lib/coverage-map.js:54:35)
    at /Users/aleksandr/.npm/_npx/98709a0041747bf7/node_modules/istanbul-merge/bin/istanbul-merge:36:6
    at forEachArray (/Users/aleksandr/.npm/_npx/98709a0041747bf7/node_modules/for-each/index.js:12:17)

Node.js v19.0.0
npm notice 
npm notice New major version of npm available! 8.19.2 -> 10.2.3
npm notice Changelog: https://github.com/npm/cli/releases/tag/v10.2.3
npm notice Run npm install -g npm@10.2.3 to update!
npm notice 

cc @isaacs @SimenB

@isaacs
Copy link
Contributor

isaacs commented Nov 6, 2023

It's weird to me that you'd get a range entry that doesn't have a loc property, but since apparently that can happen, adding some defenses to that case seems fine enough.

@SimenB care to take a look at #754 when you get a chance?

@isaacs
Copy link
Contributor

isaacs commented Nov 6, 2023

Ohhh, I see. The statementMap is just the location at the top level. It doesn't have a nested loc field, like branchMap and functionMap do. I hadn't hit that in my testing, because it looks like v8-to-istanbul always tracks each entire line separately, so they're always a match and don't have to walk up to find a containing element.

isaacs added a commit that referenced this issue Nov 6, 2023
This prevents a thrown when the statement map needs to be processed for
container hits, because the statement map entries do not contain a "loc"
field, they just have this inforation at the top level.

Fix: #753
@mattd-tg
Copy link

mattd-tg commented Nov 7, 2023

Thanks for making this reproducible issue @sashuk. I am running into the same exact issue. Came here to check and found you already did the leg work 🥇

@sashuk
Copy link
Author

sashuk commented Nov 7, 2023

Thank you @isaacs for looking into it so fast!

Happy to help @mattd-tg!

SimenB pushed a commit that referenced this issue Nov 7, 2023
This prevents a thrown when the statement map needs to be processed for
container hits, because the statement map entries do not contain a "loc"
field, they just have this inforation at the top level.

Fix: #753
@SimenB
Copy link
Member

SimenB commented Nov 8, 2023

Forgot to do the release 😅

https://github.com/istanbuljs/istanbuljs/releases/tag/istanbul-lib-coverage-v3.2.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants