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

Fix files only covered by one LCOV report showing 100% coverage #433

Conversation

matsjoyce-refeyn
Copy link
Contributor

In our CI pipeline we are using pytest (+pytest-cov) to generate coverage for Python files, and cargo-llvm-cov to generate coverage for Rust files. Both tools can generate LCOV files, which we then pass to diff-cover. Since each tool only covers one language, the LCOV files do not overlap - a given file will only be present in one file or the other. This causes issues as diff-cover then reports 100% coverage for every file.

The root cause is that currently diff-cover always tries to find uncovered lines for each file in each report, and so if the file is not present in that report, the file is assumed to have no uncovered lines. This PR fixes the behaviour by skipping reports that the file does not appear in. I believe that matches the behaviour of the XML report parsing code.

@Bachmann1234
Copy link
Owner

neat! Thanks! @matsjoyce-refeyn can you add a test?

@matsjoyce-refeyn
Copy link
Contributor Author

@Bachmann1234 Done, I took the tests from one of the XML reporters and converted it to LCOV format. The test_different_files_in_inputs test fails without the bugfix from this PR.

@matsjoyce-refeyn
Copy link
Contributor Author

@Bachmann1234 Is there anything else you need for this PR to get merged?

@Bachmann1234
Copy link
Owner

@Bachmann1234 Is there anything else you need for this PR to get merged?

No, sorry! I'll merge this now and release this week

@Bachmann1234 Bachmann1234 merged commit e84643a into Bachmann1234:main Jan 28, 2025
7 checks passed
@matsjoyce-refeyn matsjoyce-refeyn deleted the fix-non-overlapping-merged-lcov-reports branch January 28, 2025 13:29
@matsjoyce-refeyn
Copy link
Contributor Author

No, sorry! I'll merge this now and release this week

Thank you!

@Bachmann1234
Copy link
Owner

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

Successfully merging this pull request may close these issues.

2 participants