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

TypeError: Cannot read property 'locations' of undefined #322

Closed
bennycode opened this issue Mar 6, 2019 · 34 comments
Closed

TypeError: Cannot read property 'locations' of undefined #322

bennycode opened this issue Mar 6, 2019 · 34 comments
Labels

Comments

@bennycode
Copy link

bennycode commented Mar 6, 2019

I created a coverage report using the nyc karma start command. It generated a HTML report but when I click on a class in that report I see the following error:

Cannot read property 'locations' of undefined
TypeError: Cannot read property 'locations' of undefined
at D:\dev\projects\temp\karma-test\node_modules\nyc\node_modules\istanbul-reports\lib\html\annotator.js:133:48
at Array.forEach ()
at annotateBranches (D:\dev\projects\temp\karma-test\node_modules\nyc\node_modules\istanbul-reports\lib\html\annotator.js:128:30)
at Object.annotateSourceCode (D:\dev\projects\temp\karma-test\node_modules\nyc\node_modules\istanbul-reports\lib\html\annotator.js:238:9)
at HtmlReport.onDetail (D:\dev\projects\temp\karma-test\node_modules\nyc\node_modules\istanbul-reports\lib\html\index.js:269:23)
at Visitor.(anonymous function) [as onDetail] (D:\dev\projects\temp\karma-test\node_modules\nyc\node_modules\istanbul-lib-report\lib\tree.js:34:30)
at ReportNode.Node.visit (D:\dev\projects\temp\karma-test\node_modules\nyc\node_modules\istanbul-lib-report\lib\tree.js:121:17)
at D:\dev\projects\temp\karma-test\node_modules\nyc\node_modules\istanbul-lib-report\lib\tree.js:114:23
at Array.forEach ()
at visitChildren (D:\dev\projects\temp\karma-test\node_modules\nyc\node_modules\istanbul-lib-report\lib\tree.js:113:32)

Scenario can be replayed with the following repository:

@coreyfarrell
Copy link
Member

It looks like karma-coverage depends on the deprecated istanbul module. I suspect this is why it is generating coverage files that are incompatible with nyc (lacking fields required by the HTML report). See karma-runner/karma-coverage#375 for the ticket about migrating away from the istanbul module.

@zhaochy1990
Copy link

hello @coreyfarrell , I also meet this error in my project, is there any update for this issue?

I'm writing Typescript codes, and only run these codes on Node.js environment.

I'm using
nyc: 14.1.1
node.js: 10.15.3
typescript: 3.5.3

I'm running my tests using
nyc mocha -r ts-node/register test/**/*.test.ts

And this is my nyc config:

"nyc": {
    "extends": "@istanbuljs/nyc-config-typescript",
    "all": true,
    "reporter": ["html", "text"]
}

WX20190716-163214@2x

coreyfarrell added a commit that referenced this issue Aug 2, 2019
Frequently users run `nyc --all` in a way that causes source files to be
transpiled for actual testing but not transpiled for `--all`.  This
produces incompatible coverage data and inconsistantly wrong reporting.

The work around here is to drop coverage produced by `--all` for any
file where we have coverage produced by actual test runs.  This ensures
that we prefer code that was transpiled in the way which tests actually
ran.

Fixes #123, #224, #260, #322, #413
@regevbr
Copy link

regevbr commented Nov 22, 2019

I have the same issue, but it is non consistent....

@stahloss
Copy link

stahloss commented Apr 13, 2020

Same issue here after merging cypress coverage with jest coverage using nyc. All of them use the same versions of the istanbul libraries. The separate reports look fine.

@coreyfarrell
Copy link
Member

@D0rmouse this issue is about karma. I don't know how cypress coverage works or even what module enables it, but I have seen a number of recent reports about issues with cypress. In any case please open a new bug including all required information.

For those who originally posted this issue karma-coverage updated to the latest istanbul libraries yesterday. I recommend updating to latest everything - note if your node_modules contains any istanbul-lib-* libraries that are not the latest version you are potentially hitting a bug that has already been fixed.

@stahloss
Copy link

I'd rather say this issue is about this error message. I got it through another route, as did @zhaochy1990 with Mocha.

My dependencies:
"istanbul-lib-coverage@3.0.0"
"istanbul-lib-hook@3.0.0"
"istanbul-lib-instrument@4.0.1"
"istanbul-lib-processinfo@2.0.2"
"istanbul-lib-report@3.0.0"
"istanbul-lib-source-maps@4.0.0"
"istanbul-reports@3.0.2"
"jest@25.2.4"

I use "babel-plugin-istanbul@6.0.0" to instrument my code that is tested by Cypress, which is also used by Jest.
@cypress/coverage uses "istanbul-lib-coverage@3.0.0", which uses "istanbul-lib-instrument@4.0.0".
So there is a difference in istanbul-lib-instrument, but it's only a patch version.

@craig-dae
Copy link

I just got code coverage up and am having this same issue on half my application files.

@JustinChristensen
Copy link
Contributor

JustinChristensen commented Jul 3, 2021

@coreyfarrell

I was able to reproduce this on a little demo project I've been playing around this when using v8toistanbul to convert v8 coverage reports and merge them into a coverage map.

What appears to be happening is that merging doesn't merge the branch/statement/functionMap's together, but it does merge the hit counts, and "annotating" expects those hit counts to have the same number of branches/statements/functions as the other FileCoverage object it's merging in. In my case, it doesn't.

On one such file I've got in my project that's demonstrating this problem, the first FileCoverage that gets created only has two branches in the branchMap that v8toistanbul extracts from the v8 coverage report, and when merging in a second FileCoverage for the same file (from a different converted v8 coverage report during the same test run), the second one has four branches in it's branchMap.

This results in the final merged FileCoverage having 4 entries in it's b property, but only 2 entries in it's branchMap property after merging, and when iterating during the "annotation" process, it then errors while iterating over the keys in b and accessing the branchMap's corresponding properties.

It's worth noting that as long as later FileCoverage objects that get merged in are a subset of the branches of the original File, then you won't see this problem, because the final merged File coverage will have the same number of entries between it's hit count and map.

Here's the relevant code in my project:
https://github.com/JustinChristensen/react-sample-app/blob/816a26a266ba7d4c7ec5febdb98aa9c4d7e05fa8/test/e2e/runner.js#L73-L78

And here are the pre-merge FileCoverage objects for the example file I described above:
https://gist.github.com/JustinChristensen/aa256e0187dc638f7db08a1391136861

I suspect the people above that reported this are encountering this same bug.

@JustinChristensen
Copy link
Contributor

A quick skim through the issues for this project shows lots of bugs related to this. It seems like that merge function needs to get smarter.

@eulersson
Copy link

Same, it happens to me when I follow this guide which shows how to merge coverage from Jest and Cypress

@petermanders89
Copy link

We are experiencing the same. Our situation is the following; The files are generated using the babel-loader in combination with webpack. The tests are initiated by cypress. The trouble occurs in a monorepo, with multiple libraries and applications. The focus is on the e2e tests and do units tests for the missing parts. So unit tests in a library do not always cover all the files, but are still mentioned in the generated out.json file.

I currently narrowed it down to two *.json files with coverage referencing a single file in which the error occurs. In the first test_01.json the coverage is generated. The 2nd file test_02.json does not have coverage for that file, since it is unused. So it the basic output is the following:

{
  "/test/grid.ts": {
    "path": "/test/grid.ts",
    "statementMap": {},
    "fnMap": {},
    "branchMap": {},
    "s": {},
    "f": {},
    "b": {}
  }
}

In test_01.json the content of the objects is filled with a lot of stuff, while in test_02.json it is as above. Merging one that has no coverage defined and one that has coverage defined, sometimes results in an error. I write sometimes, cause sometimes it works and sometimes it does not. It is inconsistent. If an error occurs, the file contents states:

'TypeError: Cannot read property 'locations' of undefined'

The two jsons are located in a folder .nyc_output. I tried running the command:
npx nyc report --reporter=text-summary --reporter=html

I also tried using the merge command first: nyc merge .nyc_ouptput coverage.json. However even inspecting that json output tells me that coverage is broken, without generating a report.

So this looks very similair to istanbuljs/nyc#1302

@bcoe
Copy link
Member

bcoe commented Sep 23, 2021

@JustinChristensen just released your changes, thank you for the contribution 👏

@bennycode, @petermanders89, etc., does this help your situation at all?

@petermanders89
Copy link

@bcoe Unfortunatly this caused a new error for me:
Cannot destructure property 'start' of 'undefined' as it is undefined.

This is probably caused by the following example. I have got two *.json files. Note that this is a striped so not all the coverage is shown. The first file does not show any coverage. Probably imported/compiled, but not used at all. The one the file is used and it has coverage. Mapping them together could probably indicate that start is undefined in the 0 object. Before the update it was working, but with the location errors. Now it doesnt work at all anymore.

One:

  "/grid.ts": {
    "path": "/grid.ts",
    "statementMap": {},
    "fnMap": {},
    "branchMap": {},
    "s": {},
    "f": {},
    "b": {}
  },

Two:

{
   "/grid.ts":{
      "path":"/grid.ts",
      "statementMap":{
         "0":{
            "start":{
               "line":59,
               "column":4
            },
            "end":{
               "line":59,
               "column":41
            }
         },
         "1":{
            "start":{
               "line":63,
               "column":4
            },
            "end":{
               "line":63,
               "column":35
            }
         }
      },
      "fnMap":{
         "0":{
            "name":"(anonymous_0)",
            "decl":{
               "start":{
                  "line":58,
                  "column":2
               },
               "end":{
                  "line":58,
                  "column":3
               }
            },
            "loc":{
               "start":{
                  "line":58,
                  "column":27
               },
               "end":{
                  "line":60,
                  "column":3
               }
            },
            "line":58
         }
      },
      "branchMap":{
         "0":{
            "loc":{
               "start":{
                  "line":73,
                  "column":4
               },
               "end":{
                  "line":75,
                  "column":5
               }
            },
            "type":"if",
            "locations":[
               {
                  "start":{
                     "line":73,
                     "column":4
                  },
                  "end":{
                     "line":75,
                     "column":5
                  }
               },
               {
                  "start":{
                     "line":73,
                     "column":4
                  },
                  "end":{
                     "line":75,
                     "column":5
                  }
               }
            ],
            "line":73
         }
      },
      "s":{
         "0":710,
         "1":408,
         "2":408,
         "3":408,
         "4":408,
         "5":408,
         "6":408,
         "7":408,
         "8":408
      },
      "f":{
         "0":710,
         "1":408,
         "2":2688,
         "3":1764,
         "4":1764
      },
      "b":{
         "0":[
            380,
            28
         ],
         "1":[
            58,
            1136
         ],
         "2":[
            88,
            1048
         ],
         "3":[
            30,
            58
         ]
      },
      "_coverageSchema":"1a1c01bbd47fc00a2c39e90264f33305004495a9",
      "hash":"ff6e72e6654cd65218b6f03b003f3b5946223bbc"
   }
}

@JustinChristensen
Copy link
Contributor

JustinChristensen commented Sep 23, 2021 via email

@JustinChristensen
Copy link
Contributor

JustinChristensen commented Sep 24, 2021

@petermanders89 @bcoe

This particular error you're running into is that you've got hit counters in "s", "f", and "b" that don't have corresponding entries in the "statementMap", "fnMap", and "branchMap".

I'm not sure how you'd even get into that state, but I don't believe it has anything to do with merging two FileCoverage objects together. I believe this is another bug happening somewhere else in this library causing FileCoverage objects to get initially created with an inconsistent state.

@JustinChristensen
Copy link
Contributor

At least, that's invariant I believe is expected to hold. That is, that there will always be a corresponding entry in a "xMap" for every hit counter in "x".

@JustinChristensen
Copy link
Contributor

My improvement makes it so two FileCoverage objects can each have a different number of corresponding properties, and get merged together into one FileCoverage that has all of the pairs of corresponding properties, but at the end of the day it is relying on the fact that the "maps" and the "hits" within a given FileCoverage will correspond.

@JustinChristensen
Copy link
Contributor

Oh, @petermanders89, you said those examples are from JSON files. If those were the output of some process that was using the previous merge it's possible they'd end up in that inconsistent state. The previous merge resulted in properties that didn't correspond.

@petermanders89
Copy link

@JustinChristensen The situation this occured for me is the following.

We've got a monorepo with multiple apps and libraries. All libraries and apps are tested with Cypress. The apps, tests are compiled with webpack using the babel-plugin-istanbul. The output that seems correct with all the 'xMap' comes from the app. The app is compiled before so all the FileCoverage listeners are probably in place.

The output without statementMap is from a library-test in which the file is not touched, but it is indexed by the coverage it seems. The lib is has a couple of unit tests, but the refered file does not have any unit test. The file is within the library so it is mentioned in the coverage. When I generate the html-output it shows up and it has zero coverage. That is what I expected and correct. In the unit tests, the library is never fully compiled, only the tests and the needed imports.

So in the first ouptut the file has coverage (the app output), but in the second one it has none (the lib output). Both are correct in my situtation. So the files are generated seperatly, not using any merge before.

I am not completely aware if the FileCoverage should always have a statementMap. If so, I guess the error is in the implementation, the file is probably not compiled (since there is no test for it). Therefor it can't generate any statementMap. That would make sense. But in my use case, it is fine that the file has no coverage at all in one situation, since its tested in another situation.

I am not completely aware on how the library works. Generating code-coverage based on the individual json shows me the correct output (at least expected). Merging them however, throws now this error. While before I did get the message Cannot read property 'locations' of undefined.

@JustinChristensen
Copy link
Contributor

JustinChristensen commented Sep 24, 2021

The empty JSON file you showed is perfectly valid and not a concern at all. The above rambling I was doing was referring to the fact that, for example, in your second file statementMap has 2 entries, and s has 9. These should have a corresponding set and same number of properties.

And this is why you're seeing the error. My merge logic iterates through the hit counters in s, of which there are 9 in your example, and when it gets to the 3rd counter (index 2), it tries to access statementMap[2], which of course is undefined and it then errors.

You shouldn't even be in this state coming into the merge.

@JustinChristensen
Copy link
Contributor

So the bug you're running into is pre-merge. I.e., not something my fix would have anything to do with.

@bcoe
Copy link
Member

bcoe commented Sep 24, 2021

@docwhite, @craig-dae, @bennycode would you be able to verify whether @JustinChristensen's fix addresses the issue for you?

@petermanders89
Copy link

@JustinChristensen Oh no, both are valid. But I stripped down the file. I didnt want to output a 500 line file here. If I run coverage seperatly on each file, the output is valid (html, or summary), but when merging it throws the error.

@JustinChristensen
Copy link
Contributor

JustinChristensen commented Sep 27, 2021

@petermanders89 The example you showed isn't valid. statemapMap and s in your second file should have an equal number of keys.

@etaoins
Copy link

etaoins commented Sep 28, 2021

istanbul-lib-coverage@3.0.1 caused the error to appear in our project:

 FAIL  src/events/MockSubscription.test.ts
  ● Test suite failed to run

    TypeError: Cannot read properties of undefined (reading 'locations')

      at keyFromLocationsProp (node_modules/istanbul-lib-coverage/lib/file-coverage.js:220:56)
      at node_modules/istanbul-lib-coverage/lib/file-coverage.js:49:15
          at Array.reduce (<anonymous>)
      at mergeProp (node_modules/istanbul-lib-coverage/lib/file-coverage.js:47:41)
      at FileCoverage.merge (node_modules/istanbul-lib-coverage/lib/file-coverage.js:232:23)
      at CoverageMap.addFileCoverage (node_modules/istanbul-lib-coverage/lib/coverage-map.js:112:29)
      at node_modules/istanbul-lib-coverage/lib/coverage-map.js:55:18
          at Array.forEach (<anonymous>)
      at CoverageMap.merge (node_modules/istanbul-lib-coverage/lib/coverage-map.js:54:35)
      at CoverageReporter.onTestResult (node_modules/@jest/reporters/build/CoverageReporter.js:226:25)

Downgrading to 3.0.0 fixes it

@JustinChristensen
Copy link
Contributor

JustinChristensen commented Sep 28, 2021

@etaoins @bcoe The reason this behavior wasn't happening pre 3.0.1 is because the old version of merge was making the assumption that the entries in statementMap, branchMap, and fnMap would be the same in each FileCoverage, and it was only just bumping the hit counters. This was incorrect behavior that my change did rectify, but it has now revealed this secondary problem where something is creating FileCoverage objects that have more hit counters in s, for example, then there are statements in statementMap.

So my new merge algorithm errors here because it's expecting Object.keys(this.s).length === Object.keys(this.statementMap).length for each pair of hit counters and item type.

itsMapleLeaf added a commit to itsMapleLeaf/thoughtbucket that referenced this issue Sep 29, 2021
getting this error when trying to view the html report for some files: istanbuljs/istanbuljs#322
@JustinChristensen
Copy link
Contributor

@bcoe I took a quick peek at this, and I that'll be all I'll have time for, but I think it has to do with Jest and other libraries' use of Istanbuls instrumentation faculties. Jest gets it's CoverageMapData that it passes into CoverageMap.merge from a global __coverage__ variable in the Jest runner runtime.

I see that packages/istanbul-lib-instrument/src/source-coverage.js subclasses FileCoverage, and seems to implement much of the logic that fills out a coverage map.

Hopefully that helps.

@bcoe
Copy link
Member

bcoe commented Oct 11, 2021

@etaoins would you be able to provide a repository with a minimum reproduction of the issue you're seeing? I believe I have a fix.

@bcoe bcoe closed this as completed in cdc28f3 Oct 12, 2021
@bcoe
Copy link
Member

bcoe commented Oct 12, 2021

@bennycode there's a chance you wandered on from this issue, as it's been a couple years. But, @JustinChristensen's fixes, along with a couple follow on fixes appear to have addressed your original issue:

Screen Shot 2021-10-11 at 7 04 31 PM

@etaoins
Copy link

etaoins commented Oct 13, 2021

@bcoe

I haven't minimised the repo but I can confirm it works again with istanbul-lib-coverage@3.0.2

@bennycode
Copy link
Author

@bcoe, three years later I updated the dependencies and verified the fix! Thank you for getting it done. Better late than never. 😊

@bcoe
Copy link
Member

bcoe commented Oct 18, 2021

Better late than never.

@bennycode there's a chance I'm a little over committed on open source, will do my best to hav a slightly quicker turn over for future bugs 😝

Thanks @JustinChristensen for dusting this off and getting the fix most of the way there.

@JustinChristensen
Copy link
Contributor

JustinChristensen commented Oct 18, 2021 via email

@loynoir
Copy link

loynoir commented Nov 14, 2021

Same issue here, when use nyc merge .... But solved.

Cannot destructure property 'start' of 'undefined' as it is undefined.


My minimal reproduce part is

interface foo {
  bar: Buffer
}

Within jest, no Buffer injection. Within karma, plugin guess Buffer is var but not type, thus need to inject library for browser.

Two ways both work.
1.

import type {Buffer} from 'buffer'
interface foo {
  bar: Buffer
}
  1. Move babel inject plugin to very last, after babel transform typescript. And use Program.exit.

But I guess jest babel-plugin-istanbul should get original source code, still no idea it effects nyc merge.

aarongable pushed a commit to chromium/chromium that referenced this issue Jul 15, 2022
Both these packages have gone up a version and provide new tooling
(specifically v8-to-istanbul) that is critical for code coverage
remapping. This also adds the istanbul-lib-coverage package explicitly
as a recent bug (istanbuljs/istanbuljs#322)
causes the reports to return with "Cannot find locations of undefined".
It is fixed when using version 3.0.2.

Bug: 1337530
Test: ./update_npm_deps
Change-Id: I6a10ec492707545e6b3ee90db4598997562eae3f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3761783
Commit-Queue: Ben Reich <benreich@chromium.org>
Reviewed-by: Tibor Goldschwendt <tiborg@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1024532}
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
Both these packages have gone up a version and provide new tooling
(specifically v8-to-istanbul) that is critical for code coverage
remapping. This also adds the istanbul-lib-coverage package explicitly
as a recent bug (istanbuljs/istanbuljs#322)
causes the reports to return with "Cannot find locations of undefined".
It is fixed when using version 3.0.2.

Bug: 1337530
Test: ./update_npm_deps
Change-Id: I6a10ec492707545e6b3ee90db4598997562eae3f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3761783
Commit-Queue: Ben Reich <benreich@chromium.org>
Reviewed-by: Tibor Goldschwendt <tiborg@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1024532}
NOKEYCHECK=True
GitOrigin-RevId: f434ed5f6727d647dd8a2aa82970a15e6f40b18b
vivek-freshworks pushed a commit to freshworks/istanbuljs that referenced this issue Oct 26, 2023
Frequently users run `nyc --all` in a way that causes source files to be
transpiled for actual testing but not transpiled for `--all`.  This
produces incompatible coverage data and inconsistantly wrong reporting.

The work around here is to drop coverage produced by `--all` for any
file where we have coverage produced by actual test runs.  This ensures
that we prefer code that was transpiled in the way which tests actually
ran.

Fixes istanbuljs#123, istanbuljs#224, istanbuljs#260, istanbuljs#322, istanbuljs#413
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.