-
-
Notifications
You must be signed in to change notification settings - Fork 435
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
Lcov report fixes v2 #1849
Lcov report fixes v2 #1849
Conversation
- Rename LcovReporter.get_lcov to LcovReporter.lcov_file because the old name sounds like it returns something. lcov_file was selected by analogy with XmlReporter.xml_file. - Move the increment of self.total from lcov_file to the loop in LcovReporter.report for better separation of concerns.
Implement skip_empty mode in LcovReporter. Also, improve consistency with what you get by running ‘coverage xml’ and then ‘xml2lcov’ by not emitting any vacuous TN: lines, not emitting LF:0 LH:0 for empty files, and not emitting BRF:0 BRH:0 for files with no branches.
DA-line checksums bulk up the .lcov file and provide only a weak assurance that the source file being processed for analysis matches the source file that was used to generate coverage data. Current versions of the LCOV suite discourage their use. Add a boolean configuration option, lcov.line_checksums, which controls whether checksums are generated for DA lines. Consistent with the current behavior of the LCOV suite, the default is to not generate any checksums.
…rts. This fixes five serious bugs: - The first field of a BRDA: line may not be zero (nedbat#1846). - The first field of a BRDA: line is supposed to be the *source* line of each instrumented branch, not the destination line. - The fourth field of a BRDA: line is supposed to be “-” when the branch was *never reached*, not when it was reached but never/always taken (which is what a branch’s presence in missing_arcs means). As far as I can tell, coverage.py currently doesn’t know of the existence of branches that were never reached. - The decision of whether to emit DA: and BRDA: lines at all is now taken strictly according to what’s in analysis.statements. This is important because some lines may appear in analysis.executed and/or analysis.executed_branch_arcs but *not* in analysis.statements. For example, the beginnings of docstrings are like this, as is the phantom line 1 of an empty __init__.py in Python 3.10 and earlier. (I am pleased to note that the special casing of empty __init__.py in the test suite is no longer required after this patch.) - We no longer attempt to call branch-coverage-related Analysis methods when analysis.has_arcs is false. And two minor annoyances: - DA: and BRDA: lines are now emitted strictly in ascending order by (source) line number. - Source file records are now sorted by *relative* pathname, not absolute pathname from the coverage database.
Thank you so much for doing all this! I'm still curious about the checksums. They used to always be present. You want to disable them. There's a new style which we don't support. Help me understand the lcov community feelings about these checksums. What if we simply removed them with no option? |
I don't have any special insight into the lcov community. I'm just going by what I read in the lcov manpages and a bit of source archaeology. That said:
Putting those three things together, I am fairly confident in saying that DA: line checksums are considered obsolete by the lcov maintainers - but not obsolete enough to drop completely. Whether I think generation of VER: records by |
Thanks, let's merge this and see if people have concerns. |
Fixes #1846 (per my understanding of the lcov file format) and addresses several other issues. See individual commit logs for details. Should be ready to merge.
Changes from #1847:
tox
locally with Python 3.8 through 3.12 and PyPy installed