ryanluker / vscode-coverage-gutters

Display test coverage generated by lcov and xml - works with many languages
https://marketplace.visualstudio.com/items?itemName=ryanluker.vscode-coverage-gutters
MIT License
460 stars 88 forks source link

Unable to get coverage to display #133

Closed justfalter closed 6 years ago

justfalter commented 6 years ago

I can't get coverage-gutters to render the gutters. After having done some tracing, I think the issue is in/around renderer.findIntersect(), as it always return "".

Information about my setup:

Snippet of cov.xml:

<coverage branch-rate="0" branches-covered="0" branches-valid="0" complexity="0" line-rate="0.7871" lines-covered="196" lines-valid="249" timestamp="1525805187668" version="4.5.1">
    <!-- Generated by coverage.py: https://coverage.readthedocs.io -->
    <!-- Based on https://raw.githubusercontent.com/cobertura/web/master/htdocs/xml/coverage-04.dtd -->
    <sources>
        <source>/Users/justfalter/code/myprojectname/myprojectname</source>
    </sources>
    <packages>
        <package branch-rate="0" complexity="0" line-rate="0.7871" name=".">
            <classes>
                <class branch-rate="0" complexity="0" filename="messages.py" line-rate="0.9691" name="messages.py">
                    <methods/>
                    <lines>
                        <line hits="1" number="3"/>
                        <line hits="1" number="4"/>
                        <line hits="1" number="5"/>
                        <line hits="1" number="7"/>
                    </lines>
                </class>
            </classes>
        </package>
    </packages>
</coverage>

For example, findTopSectionForEditor calls findIntersect(base: editorFile, comparee: sectionFile):

subInt ends up being messages.py, here: https://github.com/ryanluker/vscode-coverage-gutters/blob/f06458d0ae307118fe6ea54e2efa187dc6bc08e7/src/renderer.ts#L140

But the function always seems to return "" here: https://github.com/ryanluker/vscode-coverage-gutters/blob/f06458d0ae307118fe6ea54e2efa187dc6bc08e7/src/renderer.ts#L142-L144

justfalter commented 6 years ago

And, FWIW, everything works fine when after downgrading to version 1.3.1.

ryanluker commented 6 years ago

@justfalter ahh yes the new intersect / scoring system was added in 2.0 so that is definitely it. The bad part with this is I didnt want just names of files to match the editorFile so I added the requirement to have atleast one folder level in the filename. You can see an example of that in https://github.com/ryanluker/vscode-coverage-gutters/blob/master/example/python/cov.xml#L55 .

Are you able to generate your coverage with more file precision? having just the filename can make for incorrect coverage being added to the wrong editor if you dont have unique filenames across all workspaces.

ryanluker commented 6 years ago

@spodym might be able to weigh in, as well, on generating python based cov.xml with more precision.

justfalter commented 6 years ago

@ryanluker I don't see a straightforward way to increase precision. I can point the coverage tool at the root of my repository, but it doesn't allow me to see the coverage analysis for files in that root (for example: https://github.com/ryanluker/vscode-coverage-gutters/blob/master/example/python/cov.xml#L11 ).

As you can see in the source for coverage XML (line 148: https://bitbucket.org/ned/coveragepy/src/506e670e23857674786236bbad60feee15ed5fd9/coverage/xmlreport.py?at=default&fileviewer=file-view-default#xmlreport.py-148), it really wants to make the path relative to one of the source directories. Ideally, they'd be using a single root to establish the relative paths. I realize this isn't your problem, but I'm unfamiliar with any other tools that generate this kind of coverage XML to compare against.

What about detecting situations that would make for applying coverage to the wrong files?

spodym commented 6 years ago

Hi! I just checked my main project and coverage for it looks the same <class branch-rate="0" complexity="0" filename="__init__.py" line-rate="0.8571" name="__init__.py"> so most probably I will be affected by this change too (right now I am using old version). I don't know how to make xml with more precise paths.

Maybe we could add switch for this ### checking? On default it would be on.

Or check coverage file for <!-- Generated by coverage.py: https://coverage.readthedocs.io --> ?

spodym commented 6 years ago

@ryanluker I have a question. Is there use case when user will be using one coverage file in multiple workspaces? I have intuition that coverage file should be bounded to workspace it was found in.

justfalter commented 6 years ago

If you take into account the <sources>, following the suggestion I made, you wouldn't have to care about who generated the XML. I would bet that most people are only going to have a single source directory in their coverage XML. See https://github.com/ryanluker/vscode-coverage-gutters/blob/master/example/python/cov.xml#L5-L7

ryanluker commented 6 years ago

@spodym part of the 2.0 refactor is that it consumes converge from everywhere in order to efficiently cache the results for later use. This extremely simplified the process of consuming lcov (.info or .xml) as the results could live together in one data structure (with copies of the same result for a file being merged, although that would be an extreme edge case).

ryanluker commented 6 years ago

@justfalter @spodym regarding the sources in the xml file. my understanding is that the cobertura parser (https://github.com/vokal/cobertura-parse) is supposed to add these to the filename in each section for downstream consumers to utilize. you can see in the snippet below, of the cache I grabbed at runtime, the difference between the cobertura parse and the lcov parse (https://github.com/davglass/lcov-parse) modules in how they give back the file pathing.

this.cache
Map(8) {, …}
[[Entries]]:Array(8)
length:8
0:{"/home/ryanluker/dev/vscode-coverage-gutters/example/node/test.js" => Object} {key: "/home/ryanluker/dev/vscode-coverage-gutters/exampl…", value: Object}
1:{"/home/ryanluker/dev/vscode-coverage-gutters/example/node/test-coverage.js" => Object} {key: "/home/ryanluker/dev/vscode-coverage-gutters/exampl…", value: Object}
key:"/home/ryanluker/dev/vscode-coverage-gutters/example/node/test-coverage.js"
value:Object {lines: Object, functions: Object, branches: Object, …}
branches:Object {hit: 16, found: 23, details: Array(23)}
file:"/home/ryanluker/dev/vscode-coverage-gutters/example/node/test-coverage.js"
functions:Object {hit: 3, found: 3, details: Array(3)}
lines:Object {found: 25, hit: 21, details: Array(25)}
title:""
__proto__:Object {__defineGetter__: , __defineSetter__: , hasOwnProperty: , …}
2:{"__init__.py" => Object} {key: "__init__.py", value: Object}
3:{"tests/test_sample.py" => Object} {key: "tests/test_sample.py", value: Object}
key:"tests/test_sample.py"
value:Object {title: "test_sample.py", file: "tests/test_sample.py", functions: Object, …}
branches:Object {found: 0, hit: 0, details: Array(0)}
file:"tests/test_sample.py"
functions:Object {found: 0, hit: 0, details: Array(0)}
lines:Object {found: 4, hit: 4, details: Array(4)}
title:"test_sample.py"
__proto__:Object {__defineGetter__: , __defineSetter__: , hasOwnProperty: , …}
4:{"tests/bar/__init__.py" => Object} {key: "tests/bar/__init__.py", value: Object}
key:"tests/bar/__init__.py"
value:Object {title: "__init__.py", file: "tests/bar/__init__.py", functions: Object, …}
branches:Object {found: 0, hit: 0, details: Array(0)}
file:"tests/bar/__init__.py"
functions:Object {found: 0, hit: 0, details: Array(0)}
lines:Object {found: 0, hit: 0, details: Array(0)}
title:"__init__.py"
__proto__:Object {__defineGetter__: , __defineSetter__: , hasOwnProperty: , …}
5:{"tests/bar/a.py" => Object} {key: "tests/bar/a.py", value: Object}
key:"tests/bar/a.py"
value:Object {title: "a.py", file: "tests/bar/a.py", functions: Object, …}
branches:Object {found: 0, hit: 0, details: Array(0)}
file:"tests/bar/a.py"
functions:Object {found: 0, hit: 0, details: Array(0)}
lines:Object {found: 6, hit: 3, details: Array(6)}
title:"a.py"
__proto__:Object {__defineGetter__: , __defineSetter__: , hasOwnProperty: , …}
6:{"tests/foo/__init__.py" => Object} {key: "tests/foo/__init__.py", value: Object}
7:{"tests/foo/a.py" => Object} {key: "tests/foo/a.py", value: Object}
size:8
__proto__:Map {constructor: , size: <accessor>, get: , …}

I would also encourage you both to open the output tab and get me copy while the extension is running, while you have watch mode turned on. You should get something like the image below, it will assist with seeing what the extension is getting for the file path as it consumes the sections from the two parsing modules. 20180518-1

spodym commented 6 years ago

@ryanluker You are correct.

The output is based on, and intended to be compatible with, https://github.com/davglass/lcov-parse This is from https://github.com/vokal/cobertura-parse

lcov-parse is taking it's file attribute from lcov's SF part (SF:<absolute path to the source file>)

            case 'SF':
                item.file = parts.slice(1).join(':').trim();

The clean solution is to update cobertura-parse and extend paths to absolute paths. But I also see benefits of adding switch in this project for allowing less strict path matching.

Since I recently made small update in cobertura-parse I will make PR with absolute paths update later today.

ryanluker commented 6 years ago

@spodym sounds good, I am happy to remove the ### requirement as it won't be used very often anymore once you make that update. I am thinking we will make a 2.0.1 release with these two changes, I should be able to release that whenever the upstream library gets a new NPM version.

spodym commented 6 years ago

@ryanluker https://github.com/vokal/cobertura-parse/pull/9 - let's wait for review and npm release.

ryanluker commented 6 years ago

@spodym sounds good thanks!

ryanluker commented 6 years ago

@spodym I see that the commit was merged into master but no npm release yet? I can install the module from the git commit hash instead if the maintainer is being slow :package: .

spodym commented 6 years ago

@ryanluker Lets do it this way. I hope it will work properly :)

supersonicclay commented 6 years ago

Perhaps related. I'm getting coverage gutters on my test classes (which I don't expect to be there). I place my source files in src/.../myfile.js and my tests are under src/__tests__/.../myfile.js.

If I rename my test file from src/__tests__/.../myfile.js to src/__tests__/.../myfile.test.js, then the coverage is removed (which is what I expect).

Is there a way to get it to only consider exact matches?

FYI, here's the log before and after the rename:

[1528325149168][coverageservice]: Loading 1 file(s)
[1528325149172][coverageservice]: Caching 44 coverage(s)
[1528325149172][coverageservice]: READY
[1528325149172][coverageservice]: RENDERING
[1528325149173][renderer][section file path]: .../src/.../myfile.js [exactness score]: 100
[1528325149174][renderer][section file path]: .../src/.../myfile.js [exactness score]: 22.35294117647059
[1528325149174][coverageservice]: READY
[1528325168004][coverageservice]: RENDERING
[1528325168006][renderer][section file path]: .../src/.../myfile.js [exactness score]: 100
[1528325168014][coverageservice]: READY
ryanluker commented 6 years ago

@supersonicclay yes that looks related as the exactness score is matching the test files instead of the real ones, the next release (2.0.1) should help with this. Are you using xml or regular lcov by the way?

supersonicclay commented 6 years ago

I'm using the default config. I have the following files in a ./coverage folder at the root of my workspace. I'm guessing it is pulling in ./coverage/lcov.info.

ryanluker commented 6 years ago

@supersonicclay hmm odd, when I do the next release QA I will see if I get similar results locally thanks for the extra info!

ryanluker commented 6 years ago

2.0.1 release is going out shortly and will have the improvements to the upstream xml parser included! thanks again for submitting the ticket.

ryanluker commented 6 years ago

@supersonicclay FYI this issue here is semi related to clover.xml #140