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

Add support for conditional coverage #660

Open
nedbat opened this issue May 11, 2018 · 20 comments
Open

Add support for conditional coverage #660

nedbat opened this issue May 11, 2018 · 20 comments
Labels
enhancement New feature or request

Comments

@nedbat
Copy link
Owner

nedbat commented May 11, 2018

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
Copy link
Owner Author

nedbat commented May 11, 2018

@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.

@nedbat
Copy link
Owner Author

nedbat commented May 11, 2018

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
Copy link
Owner Author

nedbat commented May 11, 2018

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
Copy link
Owner Author

nedbat commented May 11, 2018

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
Copy link
Owner Author

nedbat commented May 21, 2018

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 nedbat added major enhancement New feature or request labels Jun 23, 2018
@luisfmelo
Copy link

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
Copy link
Owner Author

nedbat commented Aug 6, 2018

@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.

@nedbat nedbat removed the 4.5 label Aug 17, 2018
@yevgenypats
Copy link

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
Copy link
Owner Author

nedbat commented Nov 2, 2019

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
Copy link

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
Copy link
Owner Author

nedbat commented Nov 2, 2019

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
Copy link

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
Copy link
Owner Author

nedbat commented Nov 2, 2019

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

@nedbat nedbat removed the major label Jan 18, 2020
@mjpieters
Copy link

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

@tpansino
Copy link

@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
Copy link
Contributor

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

@nedbat
Copy link
Owner Author

nedbat commented Aug 10, 2020

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

@sobolevn
Copy link
Contributor

I was asking about this particular message #660 (comment)
I thought that it was referred to #660 (comment)

@nedbat
Copy link
Owner Author

nedbat commented Aug 10, 2020

I see. #660 (comment) is a suggestions by @tpansino. It is not implemented yet in coverage.py. I'm not sure the status of their work.

l0b0 added a commit to l0b0/pystac that referenced this issue Jun 29, 2021
Stricter than statement coverage.

The coverage library does not yet have support for conditional coverage
<nedbat/coveragepy#660>, which is even
stricter.
l0b0 added a commit to l0b0/pystac that referenced this issue Jun 30, 2021
Stricter than statement coverage.

The coverage library does not yet have support for conditional coverage
<nedbat/coveragepy#660>, which is even
stricter.
l0b0 added a commit to l0b0/pystac that referenced this issue Jun 30, 2021
Stricter than statement coverage, so I had to take the coverage minimum
down one percentage point.

The coverage library does not yet have support for conditional coverage
<nedbat/coveragepy#660>, which is even
stricter.
l0b0 added a commit to l0b0/pystac that referenced this issue Jul 1, 2021
Stricter than statement coverage, so I had to take the coverage minimum
down two percentage points.

The coverage library does not yet have support for conditional coverage
<nedbat/coveragepy#660>, which is even
stricter.
@autumnloveless
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

7 participants