Bug: "The code coverage on the diff in this pull request is 100.0%" but drilldown shows many +/- changes

[tim-gq]

What happened?

I'm not sure if this is a bug or just something I don't understand.

When we get a comment "The code coverage on the diff in this pull request is 100.0%", expanding the drilldown often shows many files that have changed coverage %'s, usually a mix of both + and - changes.

Does that indicate we have some indeterminant tests that sometimes cover certain lines of code and sometimes don't?

The email notifications for these is often several dozen lines long of files that have changed.

What did you expect to happen?

To understand what is going on. :)

Can you reproduce the problem?

Yes, consistently

CLI Version

latest

Relevant log output

[brynary]

Thanks @tim-gq. If you are able to share a specific example that can be helpful to make sure that the explanation is exactly matching your case.

That said, the confusion might stem from the two different ways we look at coverage: Total Coverage and Diff Coverage.

The message you pasted says that the code coverage of the diff is 100%. This I intended to mean that all of the lines of code modified in the PR's diff, for which coverage data is available, are covered.

The table, on the other hand, shows changes to the coverage for any lines in any files.

Crucially, these data sets are not expected to align, and there are many cases where the Diff Coverage may be 100% or NaN and the Total Coverage changes in anyways.

Based on the message you pasted, I would expect that if you look at the Files Changed section below the table that all of the lines modified in the diff are either covered (represented by a green block in the left gutter) or null.

On the other hand, with the message you pasted, if any lines in the Files Changed table are rendered with a red block in the gutter -- signaling they are cover-able but not covered -- then that would be unexpected / likely bug behavior.

[tim-gq]

@brynary Thanks for the explanation.

I think that makes sense for files that were touched in the diff (or their test files were).

What I'm confused about is files that weren't touched in the diff. i.e. we have one model file that appears quite frequently, that is entirely unrelated to the PRs in question. If I see that model frequently listed, but we've had no failing tests, does that mean one of the tests that touches the model can behave differently between runs?

I guess a semi-related question; if we have a github build on our main branch fail for some reason, then coverage won't be uploaded it for. Presumably this means the "coverage status" for the main branch remains as it was before this build was attempted.. And any diff comparisons to main will use the last successful coverage set as the base for the comparison?

[marschattha]

So I looked into this, and we can take one of our public PRs as an example.

The PR makes only one relatively small change to one file qlty-analysis/src/workspace_entries/args_source.rs, but you can see in the Drilldown of the coverage comment a number of different files have been named with changes:

Path	File Coverage Δ
qlty-check/src/tool/command_builder.rs	2.6
qlty-config/src/library.rs	0.5
qlty-check/src/tool/ruby/sys/macos.rs	3.1
qlty-check/src/tool/download.rs	0.7
qlty-check/src/executor/driver.rs	-0.9
qlty-check/src/tool/ruby/sys/linux.rs	-1.6

I checked the raw coverage data gathered by coverage reporter which we feed into qlty and indeed there were changes in those coverage files generated by the tests which align with these numbers.

So most likely it has something to do with either the test suite or the coverage generator and not something we can improve.

[tim-gq]

Thanks @marschattha. This is a great example.

If you know they're not part of the diff, what's the benefit in showing them?

On our project the deltas are much larger, so I can see them as a nudge towards looking into why our tests may not be consistently covering as much as we hoped - if that's what's actually going on.

[marschattha]

If you know they're not part of the diff, what's the benefit in showing them?

So you can make a change in a file which can increase or decrease the coverage in another file.

For example you can add a call to a function from another file which was previously uncovered and vice versa causing changes to coverage of files not within the diff.

Or make changes to your test suite that will affect the coverage of other files not in the diff.

We only have the coverage report to work off of as our source of truth and can't validate if that coverage report was accurate or not.

[tim-gq]

Ack, closing this issue.

There may be a better way to explain these in the UI to reduce confusion, but I don't have any decent suggestions off the top of my head.

[tim-gq]

@marschattha would it be possible on the Qlty coverage page, (i.e. https://qlty.sh/gh/GreatQuestion/projects/great_question/pull/11753/coverage) to show the coverage for these files that aren't part of the files changed?

i.e. have them collapsed by default maybe as they're not of primary importance, but be able to click to expand the coverage report for the file? That would allow me to easily tell which lines of the file have lost coverage due to a non-deterministic test, and add better coverage for the file.

I've just somewhat manually merged all the coverage reports for one of our files to find the line of code that wasn't being reliably covered. Unless I misunderstand something, I think Qlty could do that easily on every PR?

At the moment you're sending me alerts about changed/unreliable coverage .. but not providing me an easy way to retrieve the details I need to act on it.