Short-circuit comparison chains in rewritten asserts

AUTHORS

@@ -236,6 +236,7 @@ Jeff Rackauckas
 Jeff Widman
 Jenni Rinker
 Jens Tröger
+Jhon Alvarez
 Jiajun Xu
 John Eddie Ayson
 John Litborn

changelog/14819.bugfix.rst

@@ -0,0 +1,6 @@
+Comparison chains in an ``assert`` are now evaluated lazily, as Python evaluates them.
+
+Previously the rewriter evaluated every comparator, so ``assert a < b < c`` could
+evaluate ``c`` even when ``a < b`` was false. A rewritten assertion could therefore
+call what Python would not, or fail with an unrelated exception instead of
+``AssertionError``. ``and`` and ``or`` already short-circuited; chains now do too.

src/_pytest/assertion/rewrite.py

@@ -1108,7 +1108,31 @@ def visit_Compare(self, comp: ast.Compare) -> tuple[ast.expr, str]:
         expls: list[ast.expr] = []
         syms: list[ast.expr] = []
         results = [left_res]
+        # Python evaluates a comparison chain lazily: in "a < b < c", "c" is
+        # never evaluated when "a < b" is false. Nest every comparison after the
+        # first inside an "if" on the previous result so the rewritten form does
+        # the same, the way visit_BoolOp already does for "and" and "or".
+        outer_statements = self.statements
+        outer_expl_stmts = self.expl_stmts
+        # comp.left is already emitted; the chain's own work starts here.
+        chain_starts_at = len(self.statements)
+        conditional_blocks: list[list[ast.stmt]] = []
+        fail_ifs: list[tuple[list[ast.stmt], ast.If]] = []
         for i, op, next_operand in it:
+            if i:
+                inner: list[ast.stmt] = []
+                self.statements.append(ast.If(load_names[i - 1], inner, []))
+                self.statements = inner
+                # An operand can also contribute statements that run only when
+                # the assertion fails, such as a nested boolop appending to its
+                # explanation list. Skip those too, or they run against names
+                # the skipped operand never bound.
+                fail_inner: list[ast.stmt] = []
+                fail_if = ast.If(load_names[i - 1], fail_inner, [])
+                fail_ifs.append((self.expl_stmts, fail_if))
+                self.expl_stmts.append(fail_if)
+                self.expl_stmts = fail_inner
+                conditional_blocks += (inner, fail_inner)
             match (next_operand, left_res):
                 case (
                     ast.NamedExpr(target=ast.Name(id=target_id)),
@@ -1128,6 +1152,32 @@ def visit_Compare(self, comp: ast.Compare) -> tuple[ast.expr, str]:
             res_expr = ast.copy_location(ast.Compare(left_res, [op], [next_res]), comp)
             self.statements.append(ast.Assign([store_names[i]], res_expr))
             left_res, left_expl = next_res, next_expl
+        self.statements = outer_statements
+        self.expl_stmts = outer_expl_stmts
+        # Most operands contribute nothing to the explanation, and an "if" with
+        # an empty body is not valid ast.
+        for parent, fail_if in reversed(fail_ifs):  # innermost first
+            if not fail_if.body:
+                parent.remove(fail_if)
+        # The explanation builds its arguments eagerly, so whatever the skipped
+        # statements would have bound is set to None first. Nothing reads those
+        # values: _call_reprcompare stops at the first false result, and a chain
+        # only short-circuits after one.
+        may_be_skipped = dict.fromkeys(
+            node.id
+            for block in conditional_blocks
+            for stmt in block
+            for node in ast.walk(stmt)
+            if isinstance(node, ast.Name) and isinstance(node.ctx, ast.Store)
+        )
+        if may_be_skipped:
+            self.statements.insert(
+                chain_starts_at,
+                ast.Assign(
+                    [ast.Name(name, ast.Store()) for name in may_be_skipped],
+                    ast.Constant(None),
+                ),
+            )
         # Use pytest.assertion.util._reprcompare if that's available.
         expl_call = self.helper(
             "_call_reprcompare",

testing/test_assertrewrite.py

@@ -889,6 +889,43 @@ def f5() -> None:
 
         getmsg(f5, must_pass=True)
 
+    def test_comparison_chain_short_circuits(self) -> None:
+        def f1() -> None:
+            assert 1 < 0 < 1 / 0
+
+        # Not a ZeroDivisionError: Python never evaluates the last comparator.
+        assert getmsg(f1) == """assert 1 < 0"""
+
+        def f2() -> None:
+            calls = []
+
+            def sentinel() -> int:
+                calls.append("called")
+                return 5
+
+            try:
+                assert 1 < 0 < sentinel()
+            except AssertionError:
+                pass
+            assert calls == []
+
+        getmsg(f2, must_pass=True)
+
+        def f3() -> None:
+            a, b, c = range(3)
+            assert c < b < a < 1 / 0
+
+        # The chain is walked left to right, so the failure at "2 < 1" stops it.
+        assert getmsg(f3) == """assert 2 < 1"""
+
+        def f4() -> None:
+            a = b = 0
+            # A boolop in a skipped comparator also contributes statements that
+            # only run when the assertion fails. Those have to be skipped too.
+            assert 1 < 0 < (a or b)
+
+        assert getmsg(f4) == """assert 1 < 0"""
+
     def test_len(self, request) -> None:
         def f():
             values = list(range(10))