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

Ability to read relative paths? #29

Closed
ndegardin opened this issue Mar 16, 2017 · 11 comments
Closed

Ability to read relative paths? #29

ndegardin opened this issue Mar 16, 2017 · 11 comments
Assignees

Comments

@ndegardin
Copy link

A quick question: is it possible to use relative file paths in the LCOV file?

I didn't find any VS Code coverage extension allowing for relative paths, and I'm desperately looking for that, for my development tools to be completely on the server/container side.

@ryanluker ryanluker added this to the 0.3.0 milestone Mar 17, 2017
@ryanluker
Copy link
Owner

I dont see why the lcov compare couldnt have a relative option (compare equality based on file name rather then file path as it is now). I will add this to the 0.3.0 milestone and see if I can flesh something out 😄 . Thanks for the feedback!

@ndegardin
Copy link
Author

Just to be sure I'm clear, I'm speaking about the SF values of the LCOV file.
Some of the code coverage extensions were developed for VSCode Insiders and don't seem to work anymore. The few working extensions (VSCode LCOV and Code Cover) don't seem to accept a SF relative path (or it's not relative to the project path).

The current approach in my node/Typescript project is to generate on the server, after a project build, the Javascript LCOV file (with Istanbul and Mocha) and then to use Istanbul-remap to generate the Typescript LCOV file from the Javascript LCOV one and the source mapping.

But if Istanbul creates SF absolute paths, Remap-Istanbul creates SF relative paths... Which would be perfect for my needs, but isn't handled by the VS Code Coverage extension and led me to these threads:

lcov spec says the SF path should be an absolute path

In geninfo man page:

For each source file referenced in the .da file, there is a section containing filename and coverage data:
SF: absolute path to the source file

Well, not sure if the part describing LCOV files can really be considered as a spec. But I was wondering if SF being absolute should really be the case and if there's some reason why no VS Code coverage extension handles project-relative SF paths, which would be more portable than absolute ones (and would save me some transformation scripts). Maybe potential issues I didn't think of?

@ryanluker
Copy link
Owner

ryanluker commented Mar 18, 2017

Thanks for the extra info. I was planning on modifying / applying a configurable flag to the section around https://github.com/ryanluker/vscode-coverage-gutters/blob/master/src/gutters.ts#L95 that would turn on fuzzy file matching rather then the absolute version there is now. This fuzzy matching could be glob or regex based (or simply just the filename maybe).

@ndegardin
Copy link
Author

I didn't see any line 95 in your source file... ;)

I didn't debug the code (or code VS extensions), I don't know at my level what "window.activeTextEditor.document.fileName" returns, but I guess that's a file with its absolute path, and get the issue now.

If I follow correctly, the SF filename I was speaking about is the section.file.toLocaleLowerCase() of https://github.com/ryanluker/vscode-coverage-gutters/blob/master/src/indicators.ts#L43, and it can contain an absolute path or a relative one according to the tool that spawned the LCOV file. So I don't know if regex file matching is worth...
Wouldn't just defining a configuration source files root directory be enough, which would be prepended to the file path if it's relative? (that wouldn't be fuzzy matching). Or not defining this root in a configuration file and guessing it (if the path is relative, it can be relative to the LCOV file directory or the project directory)?

BTW your code is clear and concise, it's tempting to come contribute once I'll have more time...

@ryanluker
Copy link
Owner

ryanluker commented Mar 21, 2017

Yah line 95 went away when I refactored the gutters class to be more testable in a recent release 😄 . Thanks for the extra insight as well into the possible solutions, I will see about tinkering up a PR with a workable fixing sometime in the near future! Side note, I managed to recreate this with bash for windows generating my lcov and then vscode consuming it from my C drive.

SF:/mnt/c/dev/vscode-coverage-gutters/example/test.js
FN:6,(anonymous_1)
FN:7,(anonymous_2)
FNF:2
FNH:2
FNDA:1,(anonymous_1)
FNDA:1,(anonymous_2)
DA:3,1
DA:4,1
DA:6,1
DA:7,1
DA:8,1
DA:9,1
LF:6
LH:6
BRF:0
BRH:0
end_of_record
TN:
SF:/mnt/c/dev/vscode-coverage-gutters/example/test-coverage.js
FN:1,test
FNF:1
FNH:1
FNDA:1,test
DA:1,1
DA:2,1
DA:3,0
DA:5,1
DA:6,0
DA:8,1
LF:6
LH:4
BRDA:2,1,0,0
BRDA:2,1,1,1
BRDA:5,2,0,0
BRDA:5,2,1,1
BRF:4
BRH:2
end_of_record

As you can see mnt/c/dev would never match C:\dev! Thanks for the heads up on this @tomitm .

@ryanluker ryanluker self-assigned this Mar 21, 2017
@ndegardin
Copy link
Author

Another use case :)
Just have to send another request to Microsoft/Canonical to tell them to address all those troubles with permissions and symbolic links when working with containers, and I'm back to Windows!

@ryanluker
Copy link
Owner

@ndegardin I have a workable solution up in a pull request here #33
specifically related to your issue is the new relative option I added here:

private compareFilePaths(lcovFile: string, file: string): boolean {

backed by the test cases outlined here:
test("#extract: should find a matching file with absolute match mode", function(done) {

I made this option default to false for now but ideally next release you would be able to activate this feature 😄 . Looking forward to your feedback!

@ndegardin
Copy link
Author

I gave it a try... It's working perfectly! I'm still missing two features for a perfect daily usage, but I wouldn't want to get too greedy...For now I owe you that fifth star on VS Marketplace!

image

@ndegardin
Copy link
Author

ndegardin commented Mar 22, 2017

And to share:
My npm test command runs the following shell script to generate Typescript code coverage based on JS test execution (and mappings).

It runs on server/container side, once the compilation is done (by watching some build info file). Hence the importance for me of portable paths and of a coverage that can be refreshed on each build.

# Run the tests
node_modules/.bin/istanbul cover --dir lib/coverage node_modules/.bin/_mocha -- -R spec lib/**/test/**/*.js

# Create TS LCOV file (remap-istanbul outputs relative file paths - or maybe it's related to the source mapping style)
node_modules/.bin/remap-istanbul -i lib/coverage/coverage.json -t lcovonly -o lib/coverage/lcov.info

# Create TS cover report
node_modules/.bin/remap-istanbul -i lib/coverage/coverage.json -t html -o lib/coverage/lcov-report

Maybe there's a simpler way, I didn't have too much time to dig...

@ryanluker
Copy link
Owner

The way you are doing it makes sense seeming you need to have the server -> client option work, I cant see any simpler way from a high level. Side note, I created a release tonight with a few bug fixes and the relative path fix. When you get a chance to update let me know if the issue is still resolved. The option, altSfCompare, is false by default so make sure you change that to true and open / close the IDE to have the config load properly!

@ndegardin
Copy link
Author

Updated to 0.2.1, it's still working great with relative paths.
Definitely resolved! Thanks for the feature!

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

No branches or pull requests

2 participants