nedbat / coveragepy

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

Add support for conditional coverage #660

Open nedbat opened 6 years ago

nedbat commented 6 years ago

Originally reported by Thomas Pansino (Bitbucket: tpansino, GitHub: tpansino)


Coming from the Perl world with its excellent Devel::Cover module, I'm quite surprised that the most popular equivalent coverage package in Python doesn't have any support for conditional coverage. This can be a useful metric similiar to branch coverage to ensure all combinations of boolean conditionals are tested properly.

The best tool I've found so far to measure this is @desmaj's instrumental utility, which is in need of some updates for Python 3 compatibility and some general polishing and love.

I'm thinking there may be an opportunity here to incorporate some of the work done in instrumental into coverage.py. I don't know that I have the Python expertise to do the work myself, but I'm happy to learn and contribute in whatever way I can.

First thing's first - where does this idea fall on the coverage.py roadmap?


nedbat commented 6 years ago

Original comment by Thomas Pansino (Bitbucket: tpansino, GitHub: tpansino)


Any feedback here? I'd like to work on this in my spare time as a contribution to the community, but I need input on which direction we think we'd like to go.

nedbat commented 6 years ago

Original comment by Thomas Pansino (Bitbucket: tpansino, GitHub: tpansino)


Ok, I've played around with coverage.py's reporting, and I think I have an initial idea I can show.

Here is the test script run.py that I ran coverage on to report the screenshots further down:

#!python

#!/usr/bin/env python3
class MyClass():
    def run():
        a = True
        b = True

        if (a):
            pass

        if (a and b):
            pass

MyClass.run()

So I have 2 branches that aren't covered, and 2/3 conditions uncovered.

With branching and conditional coverage enabled, the text report might look like: python_coverage_report.png

And the HTML report: python_coverage_html.png

And the detailed HTML report for run.py (with hover text visible): python_coverage_file.png

For some context, here is the coverage report using Devel::Cover and cover on the equivalent code in Perl. First the text: perl_coverage_report.png

And in HTML: perl_coverage_html.png

And the run.pl detail HTML page: perl_coverage_file.png

And the additional Conditional Coverage page which links from line 18, conditional column: perl_coverage_condition.png

I like the color-coding visual truth table that the Perl report offers quite a bit, and would love to do that in lieu of all the sentences in the hover box, but the implementation of the coverage.py file detail page right now uses <span>, which doesn't allow inserting a table into it (or anything else). So if we wanted to do something like that, we would need to use a different element there in the coverage.py HTML report (I'm pretty new to front-end web dev, so I'm not sure what the alternatives are)

Overall I think implementing the visual reporting changes would be much less difficult than the underlying coverage tracking changes. For that, we have a few options.

The first of course is trying to directly leverage the techniques @desmaj (Matt) is using. This seems like it would be a significant departure from what coverage.py is doing today, however, and essentially wraps each underlying function with tracking code, thus modifying the program on the fly. For most developers, this probably isn't much of an issue, but there may be some for which it is, so that's a consideration here. Matt can elaborate more than I can since he wrote the code.

Second, Matt has alternatively suggested that we take the approach of using instrumental as a sort of alternative driver to the normal coverage.py coverage collector. In essence, this would mean changes to the output format of instrumental to make it coverage.py compatible, and then of course the visual changes to the coverage.py reports to interpret the results. This would almost definitely be faster and easier to implement, even if not as well-integrated with coverage.py.

The third option here as I see it would be to go the other direction and port some of the reporting code in coverage.py over to instrumental, to improve the polish, and just keep the two tools divorced. The trouble is that coverage.py has the brand recognition, so an effort like that may not end up with much visibility, or get much use from people, and will have less community and therefore support around it for long-term maintenance.

I'm curious what @nedbat and maybe others in the community think of these approaches.

nedbat commented 6 years ago

Original comment by d (Bitbucket: desmaj, GitHub: desmaj)


There are condition / decision coverage report examples in the instrumental docs here. I think it would be straightforward to incorporate this in textual coverage reports. I'll be very interested to see the ideas that @tpansino has for displaying instrumental's coverage information.

nedbat commented 6 years ago

Original comment by Thomas Pansino (Bitbucket: tpansino, GitHub: tpansino)


Generally, behavior within a line is not visible to coverage.py, since the Python trace function works on a line-by-line basis.

That's the problem where I think instrumental could help, it looks like the dev there has solved this.

Can you show some examples of how the results would be reported? That's often another challenge with information-rich features like this.

Agreed. I have some initial thoughts on this, but since a picture is worth a thousand words, I'll work on a screenshot of how I think the text report and HTML report could be modified to display this efficiently.

nedbat commented 6 years ago

@tpansino I haven't thought about how hard it would be to implement something like this. Generally, behavior within a line is not visible to coverage.py, since the Python trace function works on a line-by-line basis.

Can you show some examples of how the results would be reported? That's often another challenge with information-rich features like this.

luisfmelo commented 6 years ago

Hi, I would like this too. In my code I have:

    def is_completed(self) -> bool:
        return self.json['status'] == 'completed'

But coverage cannot check the 2 possible return values. And this is not pythonic:

    def is_completed(self) -> bool:
        if self.json['status'] == 'completed':
            return True
        return False

Especially now that python3.7 was released coming with inline tracing capabilities. If you can give me some insights to start working on this project, I can give it a try.

Cheers.

nedbat commented 6 years ago

@luisfmelo Even if we implement conditional coverage, it wouldn't help in your situation. You don't have a condition. Look at the disassembly:

>>> import dis
>>> def is_completed(self) -> bool:
...         return self.json['status'] == 'completed'
...
>>> dis.dis(is_completed)
  2           0 LOAD_FAST                0 (self)
              2 LOAD_ATTR                0 (json)
              4 LOAD_CONST               1 ('status')
              6 BINARY_SUBSCR
              8 LOAD_CONST               2 ('completed')
             10 COMPARE_OP               2 (==)
             12 RETURN_VALUE
>>>

The same sequence of instructions is executed no matter what.

yevgenypats commented 4 years ago

Hey @nedbat wanted pump this issue back. I'm particular interested in this feature for the pythonfuzz library we recently released (which uses coverage.py) . As conditional branch coverage is critical to fuzzing for example:

def my_function(buf):
    if len(buf) >= 3 and buf[0] == 'F' and buf[1] == 'U' and buf[2] == 'Z':
        raise Exception('You got me')
    else:
        return False

For a coverage guided fuzzer without conditional branch coverage it would be pretty much impossible to generate 'FUZ' but with conditional branch coverage feedback it would be pretty easy and quick.

nedbat commented 4 years ago

We can talk some more about how this could be accomplished, though it feels like a big lift, and I should get 5.0 out the door before taking on more large changes.

BTW: I see this line in pythonfuzz: for filename in cov_data._arcs:. Is there some way to use the supported interfaces instead?

yevgenypats commented 4 years ago

Thanks for the quick reply. I would be happy to either help with this feature myself or even sponsor this feature if there is someone who is more familiar with coveragepy and have some spare time.

Regarding the for filename in cov_data._arcs - sure. What would be the right way to use the supported interface to count total coverage without involving some sort of copying as speed is important in this particular part (as this is collected after each iteration).

nedbat commented 4 years ago

At a quick look, you are doing this:

            for filename in cov_data._arcs:
                total_coverage += len(cov_data._arcs[filename])

which could instead be:

for filename in cov_data.measured_files():
    total_coverage += len(cov_data.arcs(filename))

There is one list created in the second way that isn't in the first. I hope you can afford one list. The _arcs attribute is gone in coverage.py 5.0, but the second way will still work. You should try coverage.py 5.0 to make sure it still suits you. We may need higher-level APIs in CoverageData to keep your use-case viable.

yevgenypats commented 4 years ago

Thanks, sounds good. I'll push this change and do a benchmark I think it should work for me. when do you think 5.0 will be out (estimate)?

nedbat commented 4 years ago

5.0 alphas are available now, and feedback is appreciated. I'm hoping to have the beta this month.

mjpieters commented 4 years ago

This is very closely related, if not exactly the same thing as #292, or vice versa, isn't it?

tpansino commented 4 years ago

@mjpieters Yes, it's the same feature. We could potentially close as a duplicate, though I think the examples I provided of what the coverage report looks like in Perl/could look like with coverage.py are helpful to note if the decision is made to fund this feature.

sobolevn commented 4 years ago

Sorry, I am not able to find this feature anywhere in 5.x versions. Was it released?

nedbat commented 4 years ago

@sobolevn this issue has meandered a bit. What feature exactly are you looking for?

sobolevn commented 4 years ago

I was asking about this particular message https://github.com/nedbat/coveragepy/issues/660#issuecomment-549057096 I thought that it was referred to https://github.com/nedbat/coveragepy/issues/660#issuecomment-399708946

nedbat commented 4 years ago

I see. https://github.com/nedbat/coveragepy/issues/660#issuecomment-399708946 is a suggestions by @tpansino. It is not implemented yet in coverage.py. I'm not sure the status of their work.

autumnloveless commented 1 year ago

Is there any update on this? Is this feature possible to implement? It would be very useful.