nedbat / coveragepy

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

[internal] Make missing arc fragments useful to more report generators #1850

Closed zackw closed 1 month ago

zackw commented 1 month ago

This is following on from the discussion near the end of #1846. parser.py has a sophisticated algorithm for characterizing branch arc destinations - ordinary if statement, exiting the function, exiting a with clause, etc. - but the way it's presently implemented, it is only useful to the HTML report generator. To make it useful to more reporters, I'd like to ask for the following:

nedbat commented 1 month ago

I need to understand more about what you are requesting. I've been de-emphasizing arcs to focus more on true branches. I haven't checked the code, but I thought exit arcs from generator expressions were already removed.

zackw commented 1 month ago

You know what, this should have been two separate issues. I've split the exit-arcs-from-generator-expressions item to #1852 and I hope I've explained it better there. Let's focus this issue on the first two items.

The code at stake for the first two items is like this

def fn(pred):
    if pred:
        print("yes")
fn(True)
fn(False)

The LCOV reporter (using the code in #1851) currently generates these BRDA: records for this code:

BRDA:2,0,to line 3,1
BRDA:2,0,to exit,1

I want to make "to exit" be "return from 'fn'" instead, the way the HTML reporter would do it. But the problem is that the HTML reporter gets the text "return from 'fn'" from missing_arc_description, which produces a much longer sentence saying that the arc wasn't executed and why. The LCOV reporter only wants the "return from 'fn'" part, and it wants it whether or not the arc was executed, and I can't tell if it's legit to call missing_arc_description at all on arcs that were executed.

So I am suggesting there should be an API more like the internal missing_arc_fragments but exposed (to report generators), and which works on any arc, and uses words that don't imply anything about whether or not the arc was executed. The LCOV reporter only wants descriptions for arc destinations, but I can imagine other report generators might have a use for text describing when arcs would be executed, like the existing "cause" slot of a TArcFragment but reworded.

nedbat commented 1 month ago

I've added an arc_description method that I think does what you need, in branch https://github.com/nedbat/coveragepy/tree/nedbat/arc-description-for-1850. Try it out and let me know.

zackw commented 1 month ago

Thanks! I rebased https://github.com/MillionConcepts/coveragepy-fixes/tree/lcov-report-improvements-2 on top of https://github.com/nedbat/coveragepy/tree/nedbat/arc-description-for-1850 and adjusted the code in lcovreport.py to use arc_description. It definitely makes things simpler in the reporter to have this API available. However, it doesn't seem to work for exit arcs. I'm getting "jump to line -1" instead of "return from function 'foo'" consistently. It's supposed to be called via the FileReporter for the source file, right? That wasn't 100% clear to me.

The current batch of test failures for #1851 are because of this (and also #1852).

nedbat commented 1 month ago

I've made a pull request into your branch that fixes things.

zackw commented 1 month ago

Have now gotten around to checking your PR; it looks good on my end, let's see how it does in CI...

nedbat commented 1 month ago

Fixed in commit 6118798a5702a90dd89b726d44d4696bb0f82eec.

nedbat commented 4 weeks ago

This is now released as part of coverage 7.6.2.