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

Merging with nyc storybook test coverage with other reports generated by coverageProvider 'istanbul' result in wrong coverage #1559

Open
meanstackmax opened this issue Apr 30, 2024 · 4 comments

Comments

@meanstackmax
Copy link

Link to bug demonstration repository https://github.com/meanstackmax/demo-test-coverage/tree/main

Expected Behavior

Vitest and storybook should use the same source map outputted, thus the issue with different columns for the same stmt not affecting merged report

Observed Behavior

I am trying to generate a merged code report. Therefore, I am using for my unit tests vitest with istanbul. For Storybook I am using the addon which also uses istanbul. To merge the reports I am using nyc.

The issue I am having is that the reports of vitest and storybook are showing me different columns for the same stmt. This leads to not correct merging.

For example in one component vitest said that there is a stmt on line 38 start at col 18 and ends on the same line.

            "2": {
                "start": {
                    "line": 38,
                    "column": 18
                },
                "end": {
                    "line": 38,
                    "column": null
                }
            },

However, storybook coverage report says the statement is on line 38 in column 2:

      "3": {
        "start": {
          "line": 38,
          "column": 2
        },
        "end": {
          "line": 38,
          "column": null
        }
      },

Troubleshooting steps

-Tried to add this to package.json, but it didn't work:

"overrides": {
"istanbul-lib-instrument": "6.0.1",
"istanbul-lib-coverage": "3.2.0"
},
  • Tried to upgrade packages to "@storybook/addon-coverage" to v 1.0.1 and "@storybook/test-runner" to v 0.17.0 but it doesn't resolve the issue
  • Tried manually enabling source map for storybook and vitest and adding nyc configuration (installing "@istanbuljs/nyc-config-typescript" and adding .nycrc file for more control), but it didn't work as well
  • Tried with storybook v8 and storybook the dependancies updated but it doesn't solve the issue as well
  • Tried using istanbul-merge instead of nyc for merging the reports, but in the end it basically ends up with the same output as using nyc

Environment Information

  System:
    OS: macOS 14.4.1
    CPU: (8) arm64 Apple M1
    Memory: 122.00 MB / 16.00 GB
  Binaries:
    Node: 18.16.1 - ~/.nvm/versions/node/v18.16.1/bin/node
    Yarn: 1.22.19 - ~/.yarn/bin/yarn
    npm: 9.5.1 - ~/.nvm/versions/node/v18.16.1/bin/npm
  npmPackages:
    @babel/core: ^7.23.7 => 7.24.5 
    @vitest/coverage-istanbul: ^1.2.1 => 1.5.3 
    babel-loader: ^9.1.3 => 9.1.3 
    nyc: ^15.1.0 => 15.1.0 
    typescript: ^5.3.3 => 5.4.5 
@cenfun
Copy link

cenfun commented May 8, 2024

There are 3 different versions of istanbul-lib-instrument in use after running npm list istanbul-lib-instrument, it should be the cause of the problem.

[email protected] /demo-test-coverage
├─┬ @storybook/[email protected]
│ ├─┬ @jsdevtools/[email protected]
│ │ └── [email protected]
│ ├── [email protected]
│ └─┬ [email protected]
│   └── [email protected]
├─┬ @storybook/[email protected]
│ └─┬ @storybook/[email protected]
│   └─┬ @jest/[email protected]
│     └─┬ [email protected]
│       └── [email protected]
├─┬ @vitest/[email protected]
│ └── [email protected] deduped
├─┬ [email protected]
│ └─┬ @jest/[email protected]
│   └─┬ @jest/[email protected]
│     └── [email protected] deduped
└─┬ [email protected]
  └── [email protected]

@meanstackmax
Copy link
Author

@cenfun thank you, after syncing all istanbul-lib-instrument it seems like an issue still present. It looks like it doesn't count on storybook tests. So I did the following:

  • Added the following overrides section to my package.json and ran $npx npm-force-resolutions:
"overrides": {
    "@storybook/addon-coverage": {
      "istanbul-lib-instrument": "^6.0.2"
    },
    "@jsdevtools/coverage-istanbul-loader": {
      "istanbul-lib-instrument": "^6.0.2"
    },
    "vite-plugin-istanbul": {
      "istanbul-lib-instrument": "^6.0.2"
    },
    "@jest/reporters": {
      "istanbul-lib-instrument": "^6.0.2"
    },
    "@vitest/coverage-istanbul": {
      "istanbul-lib-instrument": "^6.0.2"
    },
    "nyc": {
      "istanbul-lib-instrument": "^6.0.2"
    },
    "babel-plugin-istanbul": {
      "istanbul-lib-instrument": "^6.0.2"
    }
  },

Even after adding specs and storybook the coverage is 95.45% (expected: 100%). The original idea was to have smoke test coverage report

@snake-py
Copy link

snake-py commented Sep 6, 2024

@cenfun I am working on this together with @meanstackmax. I am unsure but it seems it is not working even after harmonizing the dependency resolution. Maybe you have any other idea why we are unable to merge/generate the report properly?

@cenfun
Copy link

cenfun commented Sep 6, 2024

@snake-py Yes, the Istanbul ecosystem depends not only on istanbul-lib-instrument but also on istanbul-lib-source-maps and istanbul-lib-coverage. It is difficult to determine which one is causing the issue.

In fact, I don't have any other idea about Istanbul, but perhaps you could try V8, which is a native V8 engine coverage report. Following are some examples of merging V8 coverage reports with the mcr tool:

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

No branches or pull requests

3 participants