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

Coverage paths are no longer fully resolved #529

Open
flovilmart opened this issue Feb 14, 2020 · 5 comments · May be fixed by #532
Open

Coverage paths are no longer fully resolved #529

flovilmart opened this issue Feb 14, 2020 · 5 comments · May be fixed by #532

Comments

@flovilmart
Copy link

Since #492, we have issues when reporting our coverage to tools like Sonaqube on our monorepo projects.

This PR effectively makes all the coverage reports file paths relative to the projectRoot. In our case, using jest in a mono repo context, this prevents us from properly analyzing the code through sonarqube. (sonarqube analysis occurs at the root of the mono repo and all the relative paths cannot be resolved.)
Temporarily, we use a custom lcov reporter in jest, that sets the appropriate projectRoot relative to the mono repo root, but it's not ideal, as this need to be done for every module of the mono-repo.

Currently, jest does not support configuring / passing additional options to the coverage reporters.

I can see multiple ways out:

  1. when not passing any projectRoot we could default to full paths.
  2. add ability to configure default Istanbul-reporter options .istanbulrc or something similar
  3. Improve jest support for custom reporter configurations.
@MartinNuc
Copy link

MartinNuc commented Apr 22, 2020

My monorepo coverage broke due to relative paths too.

If it helps I solved the issue by providing projectRoot which points the root of the monorepo in the Jest config:

// server/jest.config.js
coverageReporters: [['lcov', { projectRoot: '..' }]],

@sebastianhaeni
Copy link

Paths should be relative IMHO.

I'm currently user of karma-coverage-istanbul-reporter that still uses an older version of istanbuljs, which does have absolute paths.
We cache the results of the tests if the sources didn't change. And if the tests run again, we hit the cache and have the coverage reports back from a previous build.
This report now contains absolute paths, but is not run in the same project directory. Thus the paths are now wrong since they are not relative.
Then our SonarQube analysis fails because it can rightfully not resolve the paths.

So I think relative paths are the better default.
I would fix it exactly as @MartinNuc described if necessary.

@flovilmart
Copy link
Author

@sebastianhaeni what @MartinNuc describes is the fix I put in jest to support reporter configuration.

@cpupower
Copy link

For me it seems like the lcov tracefile format was defined by the Linux Test Project (http://ltp.sourceforge.net/coverage/lcov/geninfo.1.php), if that is not the original source, please correct me.

There it is defined, that source files should be referenced by absolute paths:

SF:<absolute path to the source file>

Even if relative paths are more flexible, IMHO for compatibility reasons the default format should therefore be absolute paths. Other tools might depend on this, see for example Webstorm/WEB-46615, sonarcloud.io karma-runner/karma-coverage#415.

Note that in karma-coverage, unfortunately the fileformat change (absolute -> relative paths) was introduced in a patch version (2.0.2), therefore possibly in violation with semantic versioning, as it is not backwards compatible.

In istanbuljs/nyc#1277 it was suggested that instead of adding more options

a separate lcovabsolute reporter

should be introduced (which another user created an npm package for). However, from my point of view it would make more sense to use the absolute format by default, if the original format definition states that absolute paths should be used, adding an lcovrelative reporter for other use cases (possibly included in instanbul-reports?).

Also #532 could possibly at least allow both relative and absolute paths to be used, without creating an additional reporter.

Furthermore it seems that absolute paths are currently not supported by lcov/lcovonly, even if projectRoot is configured differently, e.g. if "/" is supplied, the path will still be relative, the empty string or null will result in the current directory being used as projectRoot.

Alternatively all depending projects would have to detect, whether relative paths are used and have to be made compatible. Either way I think there are / will be some breaking changes for developers.

@noelia-lencina
Copy link

I totally agree that if a standard format exists, it should be followed. This is causing issues for the Code Climate coverage tool, too.

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

Successfully merging a pull request may close this issue.

5 participants