ternary ifs not taken into account by branch coverage

[nedbat]

Originally reported by Antony Lee (Bitbucket: anntzer, GitHub: anntzer)

---

coverage 4.1, python 3.5.2, arch linux

test.py

cond = True

if cond:
    x = 1
else:
    x = 2

x = 1 if cond else 2
x = cond and 1 or 2

Getting branch coverage for this file shows partial coverage of the first if cond:, but not of the ternary (inline) if cond, or of the short-circuiting binary operators.

---

• Bitbucket: https://bitbucket.org/ned/coveragepy/issue/509

[nedbat]

The Python settrace facility only offers feedback about lines executed. Supporting ternary-if and short-circuiting binary operators requires more details than Python offers. There are experiments about getting those details (http://nedbatchelder.com/blog/200804/wicked_hack_python_bytecode_tracing.html), but it's impractical to support in coverage.py for now.

[nedbat]

Original comment by Antony Lee (Bitbucket: anntzer, GitHub: anntzer)

---

Perhaps it may be worth revisiting this now that Py3.7 optionally supports opcode tracing (if I understood https://bugs.python.org/issue31344 correctly)?

[nedbat]

Bytecode tracing is definitely an enabling technology, but there is still a lot to be done, including how to correlate tokens in the source with bytecodes at run-time. There's no information in the compiled code object about that. If someone wants to do some experiments, I'll be very interested in the results.

[graingert]

this also applies to any sort of short circuiting operator eg:

def true():
    return True


def throw_error():
    raise Exception


def error():
    a = true()
    b = throw_error()
    return (a or b)


def no_error():
    return (true() or throw_error())  # True


def test():
    print(no_error())  # True
    print(error())     # Exception()


if __name__ == '__main__':
    test()

of course when expanded to

def no_error():
    if x := true():
        return x
    return throw_error()

it's caught by branch and line coverage

[nedbat]

This would be an amazing feature to implement, but has a number of challenges:

• How do we know what byte codes could be executed? I think it's likely that there are dead op codes that are not reachable.
• How do we report on the results? Python doesn't track a mapping from tokens to byte codes.

[ocehugo]

This thing just happened to me since I was curious about why I was not getting a lack of coverage.

1. Maybe put a warning msg for some ternary operator cases ?

2. Silly solution: why not just expand the source code into ifs for the analysis? Like storing original AST/FST, calculating/piping-in modified AST/FSTs (expanded) and then remapping results back to original AST/FST!? maybe baron package helps!?

[snejus]

Have been asking the same question today as well once I noticed that a file which contains some rather complex conditional logic has been reported to have 0 branches. It had the following structure:

def resolve_status(data):
    DEFAULT = -1

    def priority1():
        return 1 if something(data) else None

    def priority2():
        return 2 if something_else(data) else None

    def priority3():
        return 3 if data else None

    return priority1() or priority2() or priority3() or DEFAULT

I find this sort of style both concise and readable and tend to use the or operator quite a lot when branched conditions need to be considered. I found that rewriting my function in

if condition():
    return something
return None

style yielded 16 branches. That's quite a significant difference compared with 0.

Are there any plans to support this?

[ammaraskar]

Just fyi: it might be possible to implement this on 3.11 where we now expose column information for bytecode: https://docs.python.org/3.11/reference/datamodel.html#codeobject.co_positions

[markusschaber]

See also: https://www.python.org/dev/peps/pep-0657/ for the PEP "Include Fine Grained Error Locations in Tracebacks" :-)

[saroad2]

Hey @nedbat .
If one would have want to try and tackle this issue, where would you recommend to begin?
Any tips on how to tackle this 6 years old problem?

[nedbat]

@saroad2 I haven't even looked at bytecode tracing. You could experiment with the information that sys.settrace offers for bytecode-level tracing. There's a lot to do to make it useful I think.

[abcnishant007]

https://docs.python.org/3.11/whatsnew/3.11.html#summary-release-highlights
With python 3.11 byte code mapping will be available; they mention on their release notes that expression/byte level coverage is possible now. would this be included in pycoverage?

[flying-sheep]

Either it will or pycoverage will be replaced by whatever new project comes along and does it. Sub-expression-level branching is simply the the correct way to measure coverage, and now it’s finally possible!

[nedbat]

I'm always interested in exploring how these things might work, even if it's just a discussion here. What would reporting look like? Are there other languages with this kind of instrumentation that we could learn from?

[flying-sheep]

It’s just regular branch coverage reporting. Instead of “lines of code that have been executed” it’s “nonbranching expressions that have been executed” or a similar concept.

If you mean “how to visualize region coverage”, coverage.py already visualizes “partial hits” for if statements without else branches: https://coverage.readthedocs.io/en/latest/branch.html

As a first step, that could also be used for ternaries, and/or expressions, and so on, like in this example from the Eclipse IDE:

The real deal however is to map code regions (or “spans”) that can be executed, as explained here here:

Once that’s done, we can actually visualize which code regions are executed and which aren’t:

example	screenshot
rust
c++

In a HTML report, hovering regions would then show how often they’ve been executed e.g. like this:

[flying-sheep]

I created a small proof of concept package that shows that it’s possible: https://github.com/flying-sheep/fine-coverage

It visualizes coverage of and, or, and if else expressions, here an excerpt of its own code:

(it would be trivial to enhance it to do more, but it’s only a proof of concept)

[nedbat]

Thanks, I didn't doubt that it was possible :)

[flying-sheep]

Ah I just wanted to be the first who does it 😄 and gains some insight how to do this

The issue here is that the ideal data structure to represent this isn’t line based, and indeed having a line based data structure hinders implementation. I assume coverage.py uses a line based data structure to track coverage. If I’m right, it would probably need quite some refactoring to get it to a point where adding this is feasible.

[obaltian]

Hello @nedbat, do you have any updates on this? I think I could try to dive into concept from @flying-sheep and further refactoring, if you are interested in this feature.

I have couple of projects at work using large conditionals and lambdas a lot, and I would love to see this work soon, since now coverage.py doesn't provide truthful measurements.

[xunto]

Want to express my interest in this feature after we almost hit a dangerous bug, because the line was marked as tested when it wasn't actually tested (bc of a ternary operator).

Maybe someone can suggest another coverage tool, that supports that?

[zmievsa]

I'm afraid no one was strong enough to attempt to replace coverage.py successfully :)

I stopped using ternaries entirely in my production code because of this.

[xunto]

I stopped using ternaries entirely in my production code because of this.

This is what we are discussing with the team right now.

[altendky]

Until this ticket is resolved, are there existing tools to apply such bans automatically?

But, I do also want to point back to where this round of comments started at #509 (comment) where interest was expressed in working on this.

[zmievsa]

@altendky https://github.com/afonasev/flake8-if-expr

[earonesty]

i want to point out that it's not just ternaries, it's also the and and the or operators, which are short-circuited:

if x and y:
  ...

operates like this:

if x:
   if y:
       ...

which means the exit branch when x is false might never be tested (and any side effect of evaluating y never happens!)

it means all and and or conditions in our code are now linted out... devs get an ugly error use nested ifs instead, so we can guarantee coverage