nedbat / coveragepy

The code coverage tool for Python
https://coverage.readthedocs.io
Apache License 2.0
3.03k stars 435 forks source link

Lcov report fixes v2 #1849

Closed zackw closed 2 months ago

zackw commented 2 months ago

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:

nedbat commented 2 months ago

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?

zackw commented 2 months ago

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 coverage lcov should drop them completely is something I don't have a sense for. I would not miss them, but I don't know how many other people might.

I think generation of VER: records by coverage lcov can wait until someone comes around asking for it. For what I'm using coverage lcov for, I have a different way of ensuring generation-to-analysis consistency. And the implementation I had in #1847 is definitely no good. If coverage lcov is to generate VER: records at all, it needs to support using lcov's version scripts to generate VER: records, not just make up its own payloads.

nedbat commented 2 months ago

Thanks, let's merge this and see if people have concerns.