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

Inaccurate Coverage Reported #158

Open
wkirby opened this issue Mar 6, 2017 · 21 comments
Open

Inaccurate Coverage Reported #158

wkirby opened this issue Mar 6, 2017 · 21 comments

Comments

@wkirby
Copy link

wkirby commented Mar 6, 2017

Test coverage is being reported as:

Statements   : 100% ( 79/79 ), 3 ignored
Branches     : 96.15% ( 75/78 ), 12 ignored
Functions    : 100% ( 20/20 ), 1 ignored
Lines        : 100% ( 38/38 )

The travis build can be found here. The generated lcov.info file matches the reported output.

When viewing the coverage in Coveralls, the report shows only 63 of 78 branches covered, instead of 75 of 78 as shown in the lcov.info file, and project coverage of 86.957%.

@nickmerwin
Copy link
Owner

@wkirby could you upload your lcov file so I can see what's going on?

Thank you!

@wkirby
Copy link
Author

wkirby commented Mar 6, 2017

@nickmerwin No problem. I had to change the extension to .txt to get it to upload, but this is what comes out of mocha when I run locally.

lcov.info.txt

@wkirby
Copy link
Author

wkirby commented Mar 6, 2017

@nickmerwin Doing some math that I failed to do earlier, it looks like the 12 ignored branches are not ignored by coveralls (75-12 = 63). Is this intentional?

@anehx
Copy link

anehx commented Mar 6, 2017

I'm experiencing the same issue.. It reports stuff like:

import moment from 'moment'  // Branches [[2, 1], [8, 1], [10, 1], [12, 1]] missed.

Which is absolutely wrong

@wkirby
Copy link
Author

wkirby commented Mar 6, 2017

@anehx I'm pretty sure these numbers are correct — behind the scenes the es6 is still being transpiled. Those ignored branches are ignored because they're the result of the transpiling.

The real issue, to me, seems to be that the actual report pushed to coveralls doesn't also ignore those branches, which results in a lower reported test coverage.

@geoff-reid
Copy link

geoff-reid commented Mar 9, 2017

I am also seeing the same issue. Here is an example: https://coveralls.io/builds/10518271

Coveralls is reporting: COVERAGE DECREASED (-3.6%) TO 85.523%

Here is the output from the Travis job: https://travis-ci.org/RackHD/on-tasks/jobs/209507039

istanbul reports:

===================== Coverage summary ===============================
Statements   : 89.19% ( 6422/7200 )
Branches     : 73.82% ( 1686/2284 )
Functions    : 86.29% ( 1598/1852 )
Lines        : 89.24% ( 6413/7186 )
=================================================================

@nickmerwin
Copy link
Owner

Hi @geoff-reid, we now factor branches into the overall coverage %, so that 85% is:

(1686 covered branches + 6413 covered lines) / (2284 branches + 7186 relevant lines)

@wkirby
Copy link
Author

wkirby commented Mar 9, 2017

@nickmerwin Any luck on the original issue, where it appears ignored branches aren't being sent to coveralls correctly?

@nickmerwin
Copy link
Owner

@wkirby we're only passing along the BRDA:.. lines from the lcov.info, and I see 78 in your's but no indication of which should be ignored -- do you know where that's specified?

@wkirby
Copy link
Author

wkirby commented Mar 9, 2017

I don't see anything that's output into the lcov.info file that would indicate which branches are hit or skipped, but the test suite is correctly outputting the BRF and BRH numbers at the end of the file.

I have no idea what the coveralls API expects, but does it allow sending up the BRF and BRH values in addition to the BRDA lines?

@nickmerwin
Copy link
Owner

@wkirby ok I see, so it looks like we'll need to add a BRH override parameter to the API. It's unfortunate that it won't be reflected in the source file view since all missed branches will be highlighted whether or not they were supposed to be ignored.

Maybe worth asking the lcov folks about that.

@wkirby
Copy link
Author

wkirby commented Mar 10, 2017

For sure. I'm pretty confident that this is a side effect of testing an es6 class.

Our command using isparta in place of istanbul seems to transpile the es6 class in the background and flag the generated branches to be ignored. This is great for actually running the test, but, like you said, the generated lcov still has the branches in the final output file.

@vitaly-t
Copy link

vitaly-t commented Mar 11, 2017

I can confirm the issue, have been struggling to understand why suddenly coverage started misreporting since recently.

Turns out, the latest release broke it. All lines that were marked as istanbul ignore if and instanbul ignore next were suddenly included into the coverage on Coveralls, even though Travis CI reported the coverage correctly.

I've just downgraded to version 2.11.16, and the problem is gone.

@wkirby
Copy link
Author

wkirby commented Mar 12, 2017

@vitaly-t Thanks! We bumped down to 2.11.16 and it fixed our coverage issue as well.

@nickmerwin On version 2.11.16 we're sending up the exact same lcov file, but getting a 100% coverage report from Coveralls. Something definitely changed with the 2.12.0 release that broke the ignored branch reporting.

@selfcontained
Copy link

@nickmerwin was the change to include branches as part of the coverage calculation a patch release? It definitely changes functionality (some projects may have been setup to enforce 100% statements only) so was a bit surprising to see coverage % drop when no changes in code were made. Is there a way to control if lines and/or branches are included as the coverage calculation?

@nickmerwin
Copy link
Owner

@selfcontained we've added a toggle for this in the repo settings if you'd prefer to disable it:

image

@vitaly-t
Copy link

vitaly-t commented Mar 14, 2017

@nickmerwin shouldn't the default be the original behaivour? Everyone's code coverage got broken, and you just drop it in casually that everybody needs to update their projects?

@nickmerwin
Copy link
Owner

@vitaly-t that's a good point, while we wanted existing repo owners to be immediately aware of uncovered branches, it should be opt-in so as not to disrupt automated build workflows.

Moving forward we're going to opt-out all existing repos, and opt-in new ones by default.

@leggsimon
Copy link

leggsimon commented Mar 29, 2017

@nickmerwin does that mean it's on or off 😉 https://twitter.com/_simon__says/status/844106636235083777

(Kinda half sarcastic/half serious. Thanks for the tool 👍 )

@danday74
Copy link

danday74 commented Sep 29, 2017

can we allow istanbul ignore using coveralls.yml without having to rely on the web UI? See discussion on lemurheavy/coveralls-public#939

@davidshare
Copy link

davidshare commented Sep 16, 2018

can someone help with how to interpret coverage report on coveralls and terminal?
image
image

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

9 participants