pytest-dev / pytest-dev/pytest

Assertion rewriting does not short-circuit chained comparisons

Open
#14,819 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
14.5k
Forks
3.4k
Avg merge
2d 9h
Merged PRs (30d)
35

Description

Python evaluates a comparison chain lazily: in a < b < c, if a < b is false, c is never evaluated. The assertion rewriter evaluates every comparator unconditionally, so a rewritten assert can call things Python would not call, and can fail with an unrelated exception instead of AssertionError.

def test_raises_the_wrong_error():
    assert 1 < 0 < 1 / 0

def test_calls_what_it_should_not():
    calls = []
    def boom():
        calls.append("boom")
        return 5
    try:
        assert 1 < 0 < boom()
    except AssertionError:
        pass
    assert calls == []

Both pass under --assert=plain and on unrewritten Python. Rewritten, the first raises ZeroDivisionError and the second finds calls == ["boom"].

The cause is in AssertionRewriter.visit_Compare: it walks the comparators in a loop, appending @py_assertN = <left> <op> <next> for each, and only then combines them with ast.BoolOp(ast.And(), ...). By the time the and runs, everything has already been evaluated.

This is the same family as #57 ("New assertion logic no longer follows Python short circuit logic"), which was fixed for and/orvisit_BoolOp nests each subsequent operand inside an ast.If on the previous result. Comparison chains never got the same treatment.

Tested on main (f306da747), Python 3.12.

Found while sweeping the rewriter for evaluation-order divergences alongside #14445.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start at AssertionRewriter.visit_Compare and compare its comparator loop with the nesting approach described for visit_BoolOp. Add regression coverage for the two chained-comparison examples, then run the assertion-rewriting tests; done means rewritten assertions preserve Python's lazy comparator evaluation and still raise AssertionError.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
testing
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
74/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.