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

Istanbul/coverage reporter generate LCOV file with relative path for SF parameter instead of absolute path #104

Closed
flambert70 opened this issue Oct 7, 2013 · 16 comments

Comments

@flambert70
Copy link

We are using Istanbul with Karma to generate a javascript code coverage report. We are using "lcovonly" option for the reporter type.
At present, the file generated by Karma/Istanbul contains a relative path for the SF parameter of lcov.info while it must be an absolute path. (see http://ltp.sourceforge.net/coverage/lcov/geninfo.1.php)
This is causing problems when we try to read this lcov file with sonar javascript plugin because the source files are not found.

We finally end to "patch" some source files of istanbul in order to fix this problem for lcovonly, but I think this should be done for lcov also.
What modifications ?

  1. /usr/lib/node_modules/karma-coverage/node_modules/istanbul/lib/report/lcov.js
 29 function LcovReport(opts) {
 30     Report.call(this);
 31     opts = opts || {};
 32     var baseDir = path.resolve(opts.dir || process.cwd()),
 33         htmlDir = path.resolve(baseDir, 'lcov-report');
 34     //patch
 35     var rootDir = path.resolve(opts.root || process.cwd());
 36     mkdirp.sync(baseDir);
 37     //patch
 38     //this.lcov = new LcovOnlyReport({ dir: baseDir });
 39     this.lcov = new LcovOnlyReport({ dir: baseDir, root: rootDir });
 40     this.html = new HtmlReport({ dir: htmlDir, sourceStore: opts.sourceStore});
 41 }
  1. /usr/lib/node_modules/karma-coverage/node_modules/istanbul/lib/report/lcovonly.js
 function LcovOnlyReport(opts) {
 27     this.opts = opts || {};
 28     this.opts.dir = this.opts.dir || process.cwd();
 29     this.opts.writer = this.opts.writer || null;
 30     //patch
 31     this.opts.root = this.opts.root || process.cwd();
 32 }
...
 45         writer.println('TN:'); //no test name
 46         //patch
 47         //writer.println('SF:' +fc.path);
 48         writer.println('SF:' + this.opts.root + fc.path.replace(/\./,''));

This way lcov file generated has absolute path for SF in lcov.info.

Could this be taken into account for the next version of Istanbul. It will save us a lot of time ? I don't think it is a big fix.

This bug is similar to karma-runner/karma#528, but was not fixed.

pgokul added a commit to pgokul/grunt-karma-sonar that referenced this issue Nov 26, 2013
http://ltp.sourceforge.net/coverage/lcov/geninfo.1.php

However istanbul is generating a releative path.
gotwarlost/istanbul#104

refix can help fix this issue by prefixing with an absolute path.
prefix currently prepends ./ to the prefix which makes it a releative path and breaks it on windows.

fixes  mdasberg#1
@drkibitz
Copy link

I know what the spec says, but should this be a security concern at all? I mean check this path on the sample, and now I'm noticing it in all my projects.

http://gotwarlost.github.io/istanbul/public/coverage/std-lcov/Users/ananthk/screwdriver-git/istanbul/index.html

@gotwarlost
Copy link
Owner

@drkibitz , absolutely - showing absolute paths in reports is terrible from a security viewpoint.

@gotwarlost
Copy link
Owner

Closing this - reopen if still an issue. I agree with @drkibitz 's security concerns,

@CodisRedding
Copy link

Anyone else dealing with this again? I'm seeing the same issue again with the latest version v0.3.5

@pocesar
Copy link

pocesar commented Apr 14, 2015

seems to be a problem again yeah

@vptech20nn
Copy link

Facing same issue, are there workarounds so that lcov.info has relative paths instead?

@rogerpadilla
Copy link

+1 for using absolute paths (sonarqube expects that).

@vladistan
Copy link

+1

For sonarqube I use a dirty hack by running this sed command

sed -i.bak 's@^SF:/app/@SF:@' coverage/lcov.info

in between the unit tests and sonar scan.

Because my build runs in docker container I can get away with hard-coding absolute path /app

@RobotLimeLtd
Copy link

RobotLimeLtd commented Sep 2, 2016

-1 for absolute paths. In my recent experience, Sonar works perfectly fine with relative paths, so long as you configure sonar-project.properties properly.

But with absolute paths, you're stuck with an lcov.info file that can only be used in one location. My build pipeline zips up enough of the current working build directory so I can run the Sonar analysis in a complete independent build job. The absolute paths make this impossible, as they point to a specific build directory path.

At the very least, there should be an option to choose absolute or relative.

@RachelSatoyama
Copy link

I don't quite get why sonarqube expects absolute paths. We can run tests on one job on some machine and ran report analysis on some different machine. Absolute paths are acceptable in some cases but in some others they definitely are not.

@benjamine
Copy link

+1 for having an option to choose between absolute and relative

@miguelrincon
Copy link

miguelrincon commented Mar 1, 2017

@benjamine and @gotwarlost I have sent a PR (#771) requesting with precisely this feature. I hope to revisit this topic.

@MuraliM
Copy link

MuraliM commented Aug 16, 2018

@vladistan and @miguelrincon any work around for this in teamcity?

I need to make a similar thing for teamcity. Is there any command like sed available for teamcity? I tried the below but it fails

$workdir="%teamcity.build.checkoutDir%"
sed -i -- 's/\/$workdir/Web.Spa/app/g' coverage/report/lcov.info

gives error

sed: -e expression #1, char 23: unknown option to `s'

Lcov with absolute path

TN:
SF:E:\a03\work\bb52cb33e083fc9\src\Web.Spa\app\History\history.service.js
FN:5,(anonymous_1)
FN:6,HistoryService
FN:8,(anonymous_3)
FN:9,(anonymous_4)

I need to update this path *E:\a03\work\bb52cb33e083fc9\src* to relative

Before running sed command SonarQube failed with error

Could not resolve file paths

[14:20:53][Step 13/13] INFO: Integration Test Coverage Sensor is started
[14:20:53][Step 13/13] INFO: Overall Coverage Sensor is started
[14:20:53][Step 13/13] INFO: Analysing [E:\a03\work\bb52cb33e083fc9\src\Web.Spa\coverage\report\lcov.info]
[14:20:53][Step 13/13] INFO: Sensor JavaScript Squid Sensor [javascript] (done) | time=13034ms
[14:20:53][Step 13/13] WARN: Could not resolve 44 file paths in [E:\a03\work\bb52cb33e083fc9\src\Web.Spa\coverage\report\lcov.info], first unresolved path: E:\a03\work\bb52cb33e083fc9\src\Web.Spa\app\History\history.service.js

Installed Versions

"jasmine-core": "2.8.0",
"karma": "^1.3.0",
"karma-chrome-launcher": "^2.0.0",
"karma-cli": "^1.0.1",
"karma-coverage": "^1.1.1",
"karma-coverage-istanbul-reporter": "^1.3.3",
"karma-jasmine": "^1.0.2",
"karma-jasmine-html-reporter": "^0.2.2",
"karma-teamcity-reporter": "^1.1.0",

@michael-mckenna
Copy link

michael-mckenna commented Dec 5, 2018

To provide another example passed off @vladistan 's logic:

  pattern="/usr/src/app/"
  replacement="output/myName/src/app"
  sed -i "s@$pattern@$replacement@g" output/coverage/lcov.info

I'm adding this in the execute shell phase in the "Build" phase in jenkins after I run the tests.

@jsmithe
Copy link

jsmithe commented Jan 6, 2020

Is there another way to get relative paths now?

Our build is within a docker container separate from where analysis are run which is failing because it cannot find the JS files.

@donmahallem
Copy link

It would be great if there was a way to have an option to toggle which behavior we want. Relative as default and absolute as option.

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