From 792fa39660ef95eca9d34196d661f6fe6696c3d8 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Fri, 8 May 2026 10:37:47 +0200 Subject: [PATCH 01/11] fix(rewrite): prevent walrus operator double evaluation in assertions Fixes #14445 - assertion rewriting evaluated NamedExpr (:=) expressions multiple times, causing side effects to fire repeatedly. The root cause was the `variables_overwrite` mechanism which stored and re-evaluated NamedExpr AST nodes in subsequent assertions, in `_call_reprcompare`'s results tuple, and in explanation formatting. The fix: - visit_NamedExpr: reference the target variable in explanations instead of re-evaluating the full expression - visit_Compare: assign left-side NamedExpr to a temp before right-side hoisting; freeze left_res when a comparator walrus targets the same name; replace NamedExpr entries in `results` with target variables - visit_BoolOp: capture short-circuit condition in a stable temp for the explanation path; remove walrus target rename logic - visit_Call: remove variables_overwrite substitution (walrus now properly assigns to user variables in its natural evaluation position) - Remove variables_overwrite, scope tracking, Sentinel class Co-authored-by: Cursor AI Co-authored-by: Anthropic Claude Sonnet 4 --- src/_pytest/assertion/rewrite.py | 102 +++++++++++-------------------- testing/test_assertrewrite.py | 68 ++++++++++++++++++++- 2 files changed, 102 insertions(+), 68 deletions(-) diff --git a/src/_pytest/assertion/rewrite.py b/src/_pytest/assertion/rewrite.py index 362c93d7253..7b3be3fb107 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -3,7 +3,6 @@ from __future__ import annotations import ast -from collections import defaultdict from collections.abc import Callable from collections.abc import Iterable from collections.abc import Iterator @@ -57,10 +56,6 @@ from _pytest.assertion import AssertionState -class Sentinel: - pass - - assertstate_key = StashKey["AssertionState"]() # pytest caches rewritten pycs in pycache dirs @@ -68,9 +63,6 @@ class Sentinel: PYC_EXT = ".py" + ((__debug__ and "c") or "o") PYC_TAIL = "." + PYTEST_TAG + PYC_EXT -# Special marker that denotes we have just left a scope definition -_SCOPE_END_MARKER = Sentinel() - class AssertionRewritingHook(importlib.abc.MetaPathFinder, importlib.abc.Loader): """PEP302/PEP451 import hook which rewrites asserts.""" @@ -642,14 +634,8 @@ class AssertionRewriter(ast.NodeVisitor): .push_format_context() and .pop_format_context() which allows to build another %-formatted string while already building one. - :scope: A tuple containing the current scope used for variables_overwrite. - - :variables_overwrite: A dict filled with references to variables - that change value within an assert. This happens when a variable is - reassigned with the walrus operator - - This state, except the variables_overwrite, is reset on every new assert - statement visited and used by the other visitors. + This state is reset on every new assert statement visited and used by + the other visitors. """ def __init__( @@ -665,10 +651,6 @@ def __init__( else: self.enable_assertion_pass_hook = False self.source = source - self.scope: tuple[ast.AST, ...] = () - self.variables_overwrite: defaultdict[tuple[ast.AST, ...], dict[str, str]] = ( - defaultdict(dict) - ) def run(self, mod: ast.Module) -> None: """Find all assert statements in *mod* and rewrite them.""" @@ -718,16 +700,9 @@ def run(self, mod: ast.Module) -> None: mod.body[pos:pos] = imports # Collect asserts. - self.scope = (mod,) - nodes: list[ast.AST | Sentinel] = [mod] + nodes: list[ast.AST] = [mod] while nodes: node = nodes.pop() - if isinstance(node, ast.FunctionDef | ast.AsyncFunctionDef | ast.ClassDef): - self.scope = tuple((*self.scope, node)) - nodes.append(_SCOPE_END_MARKER) - if node == _SCOPE_END_MARKER: - self.scope = self.scope[:-1] - continue assert isinstance(node, ast.AST) for name, field in ast.iter_fields(node): if isinstance(field, list): @@ -954,15 +929,17 @@ def visit_Assert(self, assert_: ast.Assert) -> list[ast.stmt]: return self.statements def visit_NamedExpr(self, name: ast.NamedExpr) -> tuple[ast.NamedExpr, str]: - # This method handles the 'walrus operator' repr of the target - # name if it's a local variable or _should_repr_global_name() - # thinks it's acceptable. + # Return the NamedExpr as-is so it evaluates in its natural position + # (preserving left-to-right evaluation order). For the explanation, + # reference the target variable (already assigned by the walrus) to + # avoid re-evaluating the expression. locs = ast.Call(self.builtin("locals"), [], []) target_id = name.target.id + target_name = ast.Name(target_id, ast.Load()) inlocs = ast.Compare(ast.Constant(target_id), [ast.In()], [locs]) - dorepr = self.helper("_should_repr_global_name", name) + dorepr = self.helper("_should_repr_global_name", target_name) test = ast.BoolOp(ast.Or(), [inlocs, dorepr]) - expr = ast.IfExp(test, self.display(name), ast.Constant(target_id)) + expr = ast.IfExp(test, self.display(target_name), ast.Constant(target_id)) return name, self.explanation_param(expr) def visit_Name(self, name: ast.Name) -> tuple[ast.Name, str]: @@ -988,20 +965,9 @@ def visit_BoolOp(self, boolop: ast.BoolOp) -> tuple[ast.Name, str]: for i, v in enumerate(boolop.values): if i: fail_inner: list[ast.stmt] = [] - # cond is set in a prior loop iteration below - self.expl_stmts.append(ast.If(cond, fail_inner, [])) # noqa: F821 + # expl_cond is set in a prior loop iteration below + self.expl_stmts.append(ast.If(expl_cond, fail_inner, [])) # noqa: F821 self.expl_stmts = fail_inner - match v: - # Check if the left operand is an ast.NamedExpr and the value has already been visited - case ast.Compare( - left=ast.NamedExpr(target=ast.Name(id=target_id)) - ) if target_id in [ - e.id for e in boolop.values[:i] if hasattr(e, "id") - ]: - pytest_temp = self.variable() - self.variables_overwrite[self.scope][target_id] = v.left # type:ignore[assignment] - # mypy's false positive, we're checking that the 'target' attribute exists. - v.left.target.id = pytest_temp # type:ignore[attr-defined] self.push_format_context() res, expl = self.visit(v) body.append(ast.Assign([ast.Name(res_var, ast.Store())], res)) @@ -1012,8 +978,16 @@ def visit_BoolOp(self, boolop: ast.BoolOp) -> tuple[ast.Name, str]: cond: ast.expr = res if is_or: cond = ast.UnaryOp(ast.Not(), cond) + # Capture the condition in a temp variable so the explanation + # path (which runs after walrus operators may have modified + # the original variable) sees the correct truthiness. + cond_var = self.variable() + body.append(ast.Assign([ast.Name(cond_var, ast.Store())], cond)) + expl_cond: ast.expr = ast.Name(cond_var, ast.Load()) # noqa: F841 inner: list[ast.stmt] = [] - self.statements.append(ast.If(cond, inner, [])) + self.statements.append( + ast.If(ast.Name(cond_var, ast.Load()), inner, []) + ) self.statements = body = inner self.statements = save self.expl_stmts = fail_save @@ -1043,19 +1017,10 @@ def visit_Call(self, call: ast.Call) -> tuple[ast.Name, str]: new_args = [] new_kwargs = [] for arg in call.args: - if isinstance(arg, ast.Name) and arg.id in self.variables_overwrite.get( - self.scope, {} - ): - arg = self.variables_overwrite[self.scope][arg.id] # type:ignore[assignment] res, expl = self.visit(arg) arg_expls.append(expl) new_args.append(res) for keyword in call.keywords: - match keyword.value: - case ast.Name(id=id) if id in self.variables_overwrite.get( - self.scope, {} - ): - keyword.value = self.variables_overwrite[self.scope][id] # type:ignore[assignment] res, expl = self.visit(keyword.value) new_kwargs.append(ast.keyword(keyword.arg, res)) if keyword.arg: @@ -1090,17 +1055,13 @@ def visit_Attribute(self, attr: ast.Attribute) -> tuple[ast.Name, str]: def visit_Compare(self, comp: ast.Compare) -> tuple[ast.expr, str]: self.push_format_context() - # We first check if we have overwritten a variable in the previous assert - match comp.left: - case ast.Name(id=name_id) if name_id in self.variables_overwrite.get( - self.scope, {} - ): - comp.left = self.variables_overwrite[self.scope][name_id] # type: ignore[assignment] - case ast.NamedExpr(target=ast.Name(id=target_id)): - self.variables_overwrite[self.scope][target_id] = comp.left # type: ignore[assignment] left_res, left_expl = self.visit(comp.left) if isinstance(comp.left, ast.Compare | ast.BoolOp): left_expl = f"({left_expl})" + # If the left operand is a NamedExpr, assign it to a temp so the + # walrus executes before any right-side expressions are hoisted. + if isinstance(left_res, ast.NamedExpr): + left_res = self.assign(left_res) res_variables = [self.variable() for i in range(len(comp.ops))] load_names: list[ast.expr] = [ast.Name(v, ast.Load()) for v in res_variables] store_names = [ast.Name(v, ast.Store()) for v in res_variables] @@ -1109,13 +1070,16 @@ def visit_Compare(self, comp: ast.Compare) -> tuple[ast.expr, str]: syms: list[ast.expr] = [] results = [left_res] for i, op, next_operand in it: + # If the next operand is a walrus that assigns to the same name as + # the current left_res, we must freeze left_res's value before the + # walrus modifies it. match (next_operand, left_res): case ( ast.NamedExpr(target=ast.Name(id=target_id)), ast.Name(id=name_id), ) if target_id == name_id: - next_operand.target.id = self.variable() - self.variables_overwrite[self.scope][name_id] = next_operand # type: ignore[assignment] + left_res = self.assign(left_res) + results[-1] = left_res next_res, next_expl = self.visit(next_operand) if isinstance(next_operand, ast.Compare | ast.BoolOp): @@ -1128,6 +1092,12 @@ 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 + # Replace NamedExpr entries in results with their target variable + # to avoid re-evaluating walrus operators in the explanation path. + results = [ + ast.Name(r.target.id, ast.Load()) if isinstance(r, ast.NamedExpr) else r + for r in results + ] # Use pytest.assertion.util._reprcompare if that's available. expl_call = self.helper( "_call_reprcompare", diff --git a/testing/test_assertrewrite.py b/testing/test_assertrewrite.py index 12e12449693..e667ffe03f1 100644 --- a/testing/test_assertrewrite.py +++ b/testing/test_assertrewrite.py @@ -1773,7 +1773,7 @@ def test_walrus_operator_change_boolean_value(): ) result = pytester.runpytest() assert result.ret == 1 - result.stdout.fnmatch_lines(["*assert not (True and False is False)"]) + result.stdout.fnmatch_lines(["*assert not (False and False is False)"]) def test_assertion_walrus_operator_boolean_none_fails( self, pytester: Pytester @@ -1787,7 +1787,7 @@ def test_walrus_operator_change_boolean_value(): ) result = pytester.runpytest() assert result.ret == 1 - result.stdout.fnmatch_lines(["*assert not (True and None is None)"]) + result.stdout.fnmatch_lines(["*assert not (None and None is None)"]) def test_assertion_walrus_operator_value_changes_cleared_after_each_test( self, pytester: Pytester @@ -1931,6 +1931,70 @@ def test_2(): assert result.ret == 0 +class TestIssue14445: + """Regression tests for #14445: walrus operator double evaluation.""" + + def test_walrus_no_double_eval_basic(self, pytester: Pytester) -> None: + """Walrus captures the value at assignment time, not re-evaluated later.""" + pytester.makepyfile( + """ + class Counter: + def __init__(self): + self.value = 0 + def increment(self): + self.value += 1 + + def test_walrus_in_assertion_basic(): + c = Counter() + assert (before := c.value) == 0 + c.increment() + assert before != (after := c.value) + """ + ) + result = pytester.runpytest() + assert result.ret == 0 + + def test_walrus_no_double_eval_running_counter(self, pytester: Pytester) -> None: + """Walrus increments fire exactly once per assert statement.""" + pytester.makepyfile( + """ + def test_walrus_running_counter(): + count = 0 + items = [] + items.append("a") + assert (count := count + 1) == len(items) + items.append("b") + assert (count := count + 1) == len(items) + items.append("c") + assert (count := count + 1) == len(items) + assert count == 3 + """ + ) + result = pytester.runpytest() + assert result.ret == 0 + + def test_walrus_no_double_eval_in_function_call(self, pytester: Pytester) -> None: + """Walrus in function call arguments not evaluated twice.""" + pytester.makepyfile( + """ + call_count = 0 + + def side_effect(): + global call_count + call_count += 1 + return call_count + + def test_walrus_side_effect(): + assert (val := side_effect()) == 1 + assert val == 1 + assert (val := side_effect()) == 2 + assert val == 2 + """ + ) + result = pytester.runpytest() + assert result.ret == 0 + + @pytest.mark.skipif( sys.maxsize <= (2**31 - 1), reason="Causes OverflowError on 32bit systems" ) From 54bd8e911aa376e715ec297aba0948924f2f24f5 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Fri, 8 May 2026 12:59:20 +0200 Subject: [PATCH 02/11] Add changelog fragment for #14445 Co-authored-by: Cursor AI Co-authored-by: Anthropic Claude Sonnet 4 --- changelog/14445.bugfix.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog/14445.bugfix.rst diff --git a/changelog/14445.bugfix.rst b/changelog/14445.bugfix.rst new file mode 100644 index 00000000000..aaae0c615f5 --- /dev/null +++ b/changelog/14445.bugfix.rst @@ -0,0 +1 @@ +Fixed assertion rewriting evaluating walrus operator (``:=``) expressions multiple times, causing incorrect test results when the expression had side effects (e.g., incrementing a counter or calling a function). From 32829ddda4e7649d3a827958a85191e4ce3af88d Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Fri, 8 May 2026 13:04:00 +0200 Subject: [PATCH 03/11] test(rewrite): add xfail tests for remaining walrus edge cases Add tests for two remaining walrus double-evaluation scenarios: - Bare NamedExpr as BoolOp operand evaluated twice via condition check - Same walrus target in chained comparison evaluated multiple times Co-authored-by: Cursor AI Co-authored-by: Anthropic Claude Sonnet 4 --- testing/test_assertrewrite.py | 40 +++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/testing/test_assertrewrite.py b/testing/test_assertrewrite.py index e667ffe03f1..147df42eb0b 100644 --- a/testing/test_assertrewrite.py +++ b/testing/test_assertrewrite.py @@ -1994,6 +1994,46 @@ def test_walrus_side_effect(): result = pytester.runpytest() assert result.ret == 0 + @pytest.mark.xfail(reason="BoolOp condition re-evaluates walrus operand") + def test_walrus_no_double_eval_in_boolop(self, pytester: Pytester) -> None: + """Bare walrus as a BoolOp operand must not be evaluated twice.""" + pytester.makepyfile( + """ + call_count = 0 + + def side_effect(): + global call_count + call_count += 1 + return call_count + + def test_walrus_boolop(): + assert (x := side_effect()) and x == 1 + assert call_count == 1 + """ + ) + result = pytester.runpytest() + assert result.ret == 0 + + @pytest.mark.xfail(reason="Chained compare re-evaluates walrus with same target") + def test_walrus_no_double_eval_chained_compare(self, pytester: Pytester) -> None: + """Same walrus target in chained comparison must evaluate each once.""" + pytester.makepyfile( + """ + call_count = 0 + + def track(value): + global call_count + call_count += 1 + return value + + def test_walrus_chained(): + assert (x := track(1)) < (x := track(3)) < (x := track(5)) + assert call_count == 3 + """ + ) + result = pytester.runpytest() + assert result.ret == 0 + @pytest.mark.skipif( sys.maxsize <= (2**31 - 1), reason="Causes OverflowError on 32bit systems" From 960a58cb5b4641d3d20ff7ad5ea220a9652bd426 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Fri, 8 May 2026 13:05:41 +0200 Subject: [PATCH 04/11] fix(rewrite): avoid double evaluation of walrus in BoolOp condition Use the already-assigned res_var to build the short-circuit condition instead of the raw visitor result, preventing bare NamedExpr operands from being evaluated a second time when checking truthiness. Co-authored-by: Cursor AI Co-authored-by: Anthropic Claude Sonnet 4 --- src/_pytest/assertion/rewrite.py | 9 +++++---- testing/test_assertrewrite.py | 1 - 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/_pytest/assertion/rewrite.py b/src/_pytest/assertion/rewrite.py index 7b3be3fb107..f015b703b6a 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -975,12 +975,13 @@ def visit_BoolOp(self, boolop: ast.BoolOp) -> tuple[ast.Name, str]: call = ast.Call(app, [expl_format], []) self.expl_stmts.append(ast.Expr(call)) if i < levels: - cond: ast.expr = res + # Use res_var (already assigned above) rather than res directly, + # so that NamedExpr operands aren't evaluated a second time. + cond: ast.expr = ast.Name(res_var, ast.Load()) if is_or: cond = ast.UnaryOp(ast.Not(), cond) - # Capture the condition in a temp variable so the explanation - # path (which runs after walrus operators may have modified - # the original variable) sees the correct truthiness. + # Capture the condition in a stable temp for the explanation + # path — res_var is overwritten by subsequent operands. cond_var = self.variable() body.append(ast.Assign([ast.Name(cond_var, ast.Store())], cond)) expl_cond: ast.expr = ast.Name(cond_var, ast.Load()) # noqa: F841 diff --git a/testing/test_assertrewrite.py b/testing/test_assertrewrite.py index 147df42eb0b..513eec42f3f 100644 --- a/testing/test_assertrewrite.py +++ b/testing/test_assertrewrite.py @@ -1994,7 +1994,6 @@ def test_walrus_side_effect(): result = pytester.runpytest() assert result.ret == 0 - @pytest.mark.xfail(reason="BoolOp condition re-evaluates walrus operand") def test_walrus_no_double_eval_in_boolop(self, pytester: Pytester) -> None: """Bare walrus as a BoolOp operand must not be evaluated twice.""" pytester.makepyfile( From 298ca957094b940d6e0e91723a1d6442a0028bf0 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Fri, 8 May 2026 13:08:10 +0200 Subject: [PATCH 05/11] fix(rewrite): assign walrus comparators to temps in chained comparisons In a chained comparison like `(x := f()) < (x := g()) < (x := h())`, each NamedExpr comparator is now assigned to a temp variable so it evaluates exactly once. Previously the raw NamedExpr node would be reused as left_res in the next iteration, causing double evaluation. Co-authored-by: Cursor AI Co-authored-by: Anthropic Claude Sonnet 4 --- src/_pytest/assertion/rewrite.py | 11 +++++------ testing/test_assertrewrite.py | 1 - 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/_pytest/assertion/rewrite.py b/src/_pytest/assertion/rewrite.py index f015b703b6a..0bc34eb3463 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -1085,6 +1085,11 @@ def visit_Compare(self, comp: ast.Compare) -> tuple[ast.expr, str]: next_res, next_expl = self.visit(next_operand) if isinstance(next_operand, ast.Compare | ast.BoolOp): next_expl = f"({next_expl})" + # Assign NamedExpr comparators to a temp so each walrus evaluates + # exactly once — critical for chained comparisons where the same + # node would otherwise be re-evaluated as left_res next iteration. + if isinstance(next_res, ast.NamedExpr): + next_res = self.assign(next_res) results.append(next_res) sym = BINOP_MAP[op.__class__] syms.append(ast.Constant(sym)) @@ -1093,12 +1098,6 @@ 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 - # Replace NamedExpr entries in results with their target variable - # to avoid re-evaluating walrus operators in the explanation path. - results = [ - ast.Name(r.target.id, ast.Load()) if isinstance(r, ast.NamedExpr) else r - for r in results - ] # Use pytest.assertion.util._reprcompare if that's available. expl_call = self.helper( "_call_reprcompare", diff --git a/testing/test_assertrewrite.py b/testing/test_assertrewrite.py index 513eec42f3f..103b900cd6a 100644 --- a/testing/test_assertrewrite.py +++ b/testing/test_assertrewrite.py @@ -2013,7 +2013,6 @@ def test_walrus_boolop(): result = pytester.runpytest() assert result.ret == 0 - @pytest.mark.xfail(reason="Chained compare re-evaluates walrus with same target") def test_walrus_no_double_eval_chained_compare(self, pytester: Pytester) -> None: """Same walrus target in chained comparison must evaluate each once.""" pytester.makepyfile( From c90c70fc5fd2dbad4bfa5cfcdecadf1044ec4026 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Wed, 3 Jun 2026 10:55:00 +0200 Subject: [PATCH 06/11] fix(rewrite): show correct walrus values in BoolOp explanations When multiple walrus operators target the same variable in a BoolOp (e.g., `assert (x := side_effect()) and (x := False)`), the assertion explanation previously showed the final value of `x` for all operands because the format context evaluated lazily after all operands ran. Fix by tracking Name/NamedExpr operand values in stable @py_assert variables (via self.assign) immediately after evaluation, then pointing the explanation format context at the tracked copy. This uses the same value-tracking mechanism already used by visit_Call, visit_Attribute, etc. Fixes the case reported by @bluetech in PR review. Co-authored-by: Cursor AI Co-authored-by: Anthropic Claude Sonnet 4 --- src/_pytest/assertion/rewrite.py | 19 +++++++++---------- testing/test_assertrewrite.py | 22 ++++++++++++++++++++-- 2 files changed, 29 insertions(+), 12 deletions(-) diff --git a/src/_pytest/assertion/rewrite.py b/src/_pytest/assertion/rewrite.py index 0bc34eb3463..3ad244a0ff8 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -930,9 +930,8 @@ def visit_Assert(self, assert_: ast.Assert) -> list[ast.stmt]: def visit_NamedExpr(self, name: ast.NamedExpr) -> tuple[ast.NamedExpr, str]: # Return the NamedExpr as-is so it evaluates in its natural position - # (preserving left-to-right evaluation order). For the explanation, - # reference the target variable (already assigned by the walrus) to - # avoid re-evaluating the expression. + # (preserving left-to-right evaluation order in function calls, etc.). + # For the explanation, reference the target variable. locs = ast.Call(self.builtin("locals"), [], []) target_id = name.target.id target_name = ast.Name(target_id, ast.Load()) @@ -971,12 +970,17 @@ def visit_BoolOp(self, boolop: ast.BoolOp) -> tuple[ast.Name, str]: self.push_format_context() res, expl = self.visit(v) body.append(ast.Assign([ast.Name(res_var, ast.Store())], res)) + # For Name/NamedExpr operands, track the value in a stable + # @py_assert variable so the explanation shows the value at + # evaluation time — even if a later walrus overwrites the name. + if isinstance(v, ast.NamedExpr | ast.Name): + tracked = self.assign(ast.Name(res_var, ast.Load())) + for key in self.stack[-1]: + self.stack[-1][key] = self.display(tracked) expl_format = self.pop_format_context(ast.Constant(expl)) call = ast.Call(app, [expl_format], []) self.expl_stmts.append(ast.Expr(call)) if i < levels: - # Use res_var (already assigned above) rather than res directly, - # so that NamedExpr operands aren't evaluated a second time. cond: ast.expr = ast.Name(res_var, ast.Load()) if is_or: cond = ast.UnaryOp(ast.Not(), cond) @@ -1059,8 +1063,6 @@ def visit_Compare(self, comp: ast.Compare) -> tuple[ast.expr, str]: left_res, left_expl = self.visit(comp.left) if isinstance(comp.left, ast.Compare | ast.BoolOp): left_expl = f"({left_expl})" - # If the left operand is a NamedExpr, assign it to a temp so the - # walrus executes before any right-side expressions are hoisted. if isinstance(left_res, ast.NamedExpr): left_res = self.assign(left_res) res_variables = [self.variable() for i in range(len(comp.ops))] @@ -1085,9 +1087,6 @@ def visit_Compare(self, comp: ast.Compare) -> tuple[ast.expr, str]: next_res, next_expl = self.visit(next_operand) if isinstance(next_operand, ast.Compare | ast.BoolOp): next_expl = f"({next_expl})" - # Assign NamedExpr comparators to a temp so each walrus evaluates - # exactly once — critical for chained comparisons where the same - # node would otherwise be re-evaluated as left_res next iteration. if isinstance(next_res, ast.NamedExpr): next_res = self.assign(next_res) results.append(next_res) diff --git a/testing/test_assertrewrite.py b/testing/test_assertrewrite.py index 103b900cd6a..b9464f5aec3 100644 --- a/testing/test_assertrewrite.py +++ b/testing/test_assertrewrite.py @@ -1773,7 +1773,7 @@ def test_walrus_operator_change_boolean_value(): ) result = pytester.runpytest() assert result.ret == 1 - result.stdout.fnmatch_lines(["*assert not (False and False is False)"]) + result.stdout.fnmatch_lines(["*assert not (True and False is False)"]) def test_assertion_walrus_operator_boolean_none_fails( self, pytester: Pytester @@ -1787,7 +1787,7 @@ def test_walrus_operator_change_boolean_value(): ) result = pytester.runpytest() assert result.ret == 1 - result.stdout.fnmatch_lines(["*assert not (None and None is None)"]) + result.stdout.fnmatch_lines(["*assert not (True and None is None)"]) def test_assertion_walrus_operator_value_changes_cleared_after_each_test( self, pytester: Pytester @@ -2032,6 +2032,24 @@ def test_walrus_chained(): result = pytester.runpytest() assert result.ret == 0 + def test_walrus_boolop_same_target_correct_explanation( + self, pytester: Pytester + ) -> None: + """Multiple walrus operators to the same name in a BoolOp must show + each operand's value at evaluation time, not the final value.""" + pytester.makepyfile( + """ + def side_effect(): + return True + + def test_walrus_boolop(): + assert (x := side_effect()) and (x := False) + """ + ) + result = pytester.runpytest() + assert result.ret == 1 + result.stdout.fnmatch_lines(["*assert (True and False)"]) + @pytest.mark.skipif( sys.maxsize <= (2**31 - 1), reason="Causes OverflowError on 32bit systems" From 9f9e73872f92dd71938670f3ca1f9b7bca7ec68a Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Wed, 3 Jun 2026 11:56:28 +0200 Subject: [PATCH 07/11] refactor(rewrite): minimal snapshots in BoolOp for walrus conflicts Replace the blanket snapshot-all-operands approach with a targeted one: pre-scan the BoolOp to find walrus targets, then only snapshot operands whose value a later walrus would corrupt. Snapshot rules: - NamedExpr (non-last): always, to avoid re-evaluating side effects - Name with later walrus conflict: to freeze the pre-overwrite value - Everything else: use res directly (stable @py_assert or plain name) Non-walrus BoolOps now generate identical code to 8.3.5 (no snapshots). Co-authored-by: Cursor AI Co-authored-by: Anthropic Claude Opus 4 --- src/_pytest/assertion/rewrite.py | 52 +++++++++++++++++++++----------- 1 file changed, 34 insertions(+), 18 deletions(-) diff --git a/src/_pytest/assertion/rewrite.py b/src/_pytest/assertion/rewrite.py index 3ad244a0ff8..781cb27b45c 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -959,40 +959,56 @@ def visit_BoolOp(self, boolop: ast.BoolOp) -> tuple[ast.Name, str]: body = save = self.statements fail_save = self.expl_stmts levels = len(boolop.values) - 1 + # Pre-scan: for each operand position, collect the set of variable + # names that a *later* operand's walrus operator will overwrite. + # An operand needs a snapshot only when its value references a name + # in this set (otherwise the explanation would show the post-walrus + # value instead of the value at evaluation time). + later_walrus_targets: list[set[str]] = [set() for _ in boolop.values] + seen: set[str] = set() + for idx in range(len(boolop.values) - 1, -1, -1): + later_walrus_targets[idx] = set(seen) + for node in ast.walk(boolop.values[idx]): + if isinstance(node, ast.NamedExpr): + seen.add(node.target.id) self.push_format_context() - # Process each operand, short-circuiting if needed. + # Process each operand, short-circuiting as needed. for i, v in enumerate(boolop.values): if i: fail_inner: list[ast.stmt] = [] - # expl_cond is set in a prior loop iteration below - self.expl_stmts.append(ast.If(expl_cond, fail_inner, [])) # noqa: F821 + # cond is set in a prior loop iteration below + self.expl_stmts.append(ast.If(cond, fail_inner, [])) # noqa: F821 self.expl_stmts = fail_inner self.push_format_context() res, expl = self.visit(v) body.append(ast.Assign([ast.Name(res_var, ast.Store())], res)) - # For Name/NamedExpr operands, track the value in a stable - # @py_assert variable so the explanation shows the value at - # evaluation time — even if a later walrus overwrites the name. - if isinstance(v, ast.NamedExpr | ast.Name): - tracked = self.assign(ast.Name(res_var, ast.Load())) + # Snapshot when the raw ``res`` node would be unsafe to reuse + # as a condition or explanation reference: + # - NamedExpr (non-last): reusing the node re-evaluates the + # walrus expression including any side effects. + # - Name whose variable a later walrus overwrites: the + # explanation would show the post-walrus value. + needs_snapshot = (isinstance(v, ast.NamedExpr) and i < levels) or ( + isinstance(v, ast.Name) and v.id in later_walrus_targets[i] + ) + if needs_snapshot: + snapshot = self.assign(ast.Name(res_var, ast.Load())) + res = snapshot for key in self.stack[-1]: - self.stack[-1][key] = self.display(tracked) + self.stack[-1][key] = self.display(snapshot) expl_format = self.pop_format_context(ast.Constant(expl)) call = ast.Call(app, [expl_format], []) self.expl_stmts.append(ast.Expr(call)) if i < levels: - cond: ast.expr = ast.Name(res_var, ast.Load()) + # Short-circuit: and → continue if truthy; or → if falsy. + # ``res`` is a stable reference (Name vars are only + # snapshotted when a later walrus would corrupt them; + # calls/compares return @py_assert vars from assign()). + cond: ast.expr = res if is_or: cond = ast.UnaryOp(ast.Not(), cond) - # Capture the condition in a stable temp for the explanation - # path — res_var is overwritten by subsequent operands. - cond_var = self.variable() - body.append(ast.Assign([ast.Name(cond_var, ast.Store())], cond)) - expl_cond: ast.expr = ast.Name(cond_var, ast.Load()) # noqa: F841 inner: list[ast.stmt] = [] - self.statements.append( - ast.If(ast.Name(cond_var, ast.Load()), inner, []) - ) + self.statements.append(ast.If(cond, inner, [])) self.statements = body = inner self.statements = save self.expl_stmts = fail_save From 72f4a7db9000c733f12bfaac4fe4ec6a2b957764 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Fri, 31 Jul 2026 19:52:36 +0200 Subject: [PATCH 08/11] fix(rewrite): freeze operands a later walrus would clobber The rewriter hoists each operand into its own statement, but a plain name is left as a bare load evaluated when the enclosing expression is assembled -- after the statements of the operands that follow it. A walrus operator in a later operand rebinds the name in between, so both the value used and the value reported were the post-walrus one, while Python evaluates the earlier operand first: assert value != identity(value := value.lower()) visit_BoolOp already guarded against this; extract its pre-scan as _walrus_targets() and add visit_operand() to apply the same freeze in visit_Compare, visit_Call and visit_BinOp. visit_Compare previously matched only a comparator that *was* a NamedExpr, missing walrus operators nested inside it; visit_Call did not guard at all, so an earlier argument saw a later argument's assignment. These cases predate the walrus rework -- they fail on main too. Closes the single-eval-walrus, order-compare-left, order-call-argument and order-binop-left groups in the coverage matrix. order-call-argument keeps one entry: a bare walrus argument is still substituted into a later one, which visit_operand does not yet see because the operand is a NamedExpr rather than a Name. Reported-by: Denis Scapin Co-Authored-By: Claude Opus 5 (1M context) --- changelog/14445.bugfix.rst | 2 + src/_pytest/assertion/rewrite.py | 79 +++++++++------- testing/test_assertrewrite_coverage.py | 123 +++++++++++++++++++++++++ 3 files changed, 173 insertions(+), 31 deletions(-) diff --git a/changelog/14445.bugfix.rst b/changelog/14445.bugfix.rst index aaae0c615f5..a9547581073 100644 --- a/changelog/14445.bugfix.rst +++ b/changelog/14445.bugfix.rst @@ -1 +1,3 @@ Fixed assertion rewriting evaluating walrus operator (``:=``) expressions multiple times, causing incorrect test results when the expression had side effects (e.g., incrementing a counter or calling a function). + +Operands preceding a walrus operator are now evaluated -- and reported -- before it rebinds their name, so ``assert value != identity(value := value.lower())`` keeps Python's left-to-right evaluation order. diff --git a/src/_pytest/assertion/rewrite.py b/src/_pytest/assertion/rewrite.py index 781cb27b45c..27953336c5c 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -530,6 +530,16 @@ def traverse_node(node: ast.AST) -> Iterator[ast.AST]: yield from traverse_node(child) +def _walrus_targets(nodes: Iterable[ast.expr]) -> set[str]: + """Return the names any walrus operator in *nodes* rebinds.""" + return { + sub.target.id + for node in nodes + for sub in ast.walk(node) + if isinstance(sub, ast.NamedExpr) + } + + @functools.lru_cache(maxsize=1) def _get_assertion_exprs(src: bytes) -> dict[int, str]: """Return a mapping from {lineno: "assertion test expression"}.""" @@ -951,6 +961,27 @@ def visit_Name(self, name: ast.Name) -> tuple[ast.Name, str]: expr = ast.IfExp(test, self.display(name), ast.Constant(name.id)) return name, self.explanation_param(expr) + def visit_operand( + self, operand: ast.expr, later: Sequence[ast.expr] + ) -> tuple[ast.expr, str]: + """Visit an operand, freezing it against walrus operators in *later*. + + Operands are rewritten into statements that run in source order, but + a plain name is left as a bare load evaluated at the very end, when + the enclosing expression is assembled. A walrus operator in a later + operand rebinds that name in between, so both the value used and the + value reported would be the post-walrus one -- Python evaluates the + earlier operand first. Copy the value into a temporary instead. + """ + specifiers = set(self.explanation_specifiers) + res, expl = self.visit(operand) + if isinstance(res, ast.Name) and res.id in _walrus_targets(later): + snapshot = self.assign(res) + for key in set(self.explanation_specifiers) - specifiers: + self.explanation_specifiers[key] = self.display(snapshot) + res = snapshot + return res, expl + def visit_BoolOp(self, boolop: ast.BoolOp) -> tuple[ast.Name, str]: res_var = self.variable() expl_list = self.assign(ast.List([], ast.Load())) @@ -959,18 +990,10 @@ def visit_BoolOp(self, boolop: ast.BoolOp) -> tuple[ast.Name, str]: body = save = self.statements fail_save = self.expl_stmts levels = len(boolop.values) - 1 - # Pre-scan: for each operand position, collect the set of variable - # names that a *later* operand's walrus operator will overwrite. - # An operand needs a snapshot only when its value references a name - # in this set (otherwise the explanation would show the post-walrus - # value instead of the value at evaluation time). - later_walrus_targets: list[set[str]] = [set() for _ in boolop.values] - seen: set[str] = set() - for idx in range(len(boolop.values) - 1, -1, -1): - later_walrus_targets[idx] = set(seen) - for node in ast.walk(boolop.values[idx]): - if isinstance(node, ast.NamedExpr): - seen.add(node.target.id) + later_walrus_targets = [ + _walrus_targets(boolop.values[idx + 1 :]) + for idx in range(len(boolop.values)) + ] self.push_format_context() # Process each operand, short-circuiting as needed. for i, v in enumerate(boolop.values): @@ -1024,7 +1047,7 @@ def visit_UnaryOp(self, unary: ast.UnaryOp) -> tuple[ast.Name, str]: def visit_BinOp(self, binop: ast.BinOp) -> tuple[ast.Name, str]: symbol = BINOP_MAP[binop.op.__class__] - left_expr, left_expl = self.visit(binop.left) + left_expr, left_expl = self.visit_operand(binop.left, [binop.right]) right_expr, right_expl = self.visit(binop.right) explanation = f"({left_expl} {symbol} {right_expl})" res = self.assign( @@ -1033,16 +1056,19 @@ def visit_BinOp(self, binop: ast.BinOp) -> tuple[ast.Name, str]: return res, explanation def visit_Call(self, call: ast.Call) -> tuple[ast.Name, str]: - new_func, func_expl = self.visit(call.func) + # The callee and every argument are evaluated left to right, so each of + # them has to be frozen against walrus operators in what follows. + operands = [*call.args, *(keyword.value for keyword in call.keywords)] + new_func, func_expl = self.visit_operand(call.func, operands) arg_expls = [] new_args = [] new_kwargs = [] - for arg in call.args: - res, expl = self.visit(arg) + for i, arg in enumerate(call.args): + res, expl = self.visit_operand(arg, operands[i + 1 :]) arg_expls.append(expl) new_args.append(res) - for keyword in call.keywords: - res, expl = self.visit(keyword.value) + for i, keyword in enumerate(call.keywords, start=len(call.args)): + res, expl = self.visit_operand(keyword.value, operands[i + 1 :]) new_kwargs.append(ast.keyword(keyword.arg, res)) if keyword.arg: arg_expls.append(keyword.arg + "=" + expl) @@ -1076,7 +1102,7 @@ def visit_Attribute(self, attr: ast.Attribute) -> tuple[ast.Name, str]: def visit_Compare(self, comp: ast.Compare) -> tuple[ast.expr, str]: self.push_format_context() - left_res, left_expl = self.visit(comp.left) + left_res, left_expl = self.visit_operand(comp.left, comp.comparators) if isinstance(comp.left, ast.Compare | ast.BoolOp): left_expl = f"({left_expl})" if isinstance(left_res, ast.NamedExpr): @@ -1089,18 +1115,9 @@ def visit_Compare(self, comp: ast.Compare) -> tuple[ast.expr, str]: syms: list[ast.expr] = [] results = [left_res] for i, op, next_operand in it: - # If the next operand is a walrus that assigns to the same name as - # the current left_res, we must freeze left_res's value before the - # walrus modifies it. - match (next_operand, left_res): - case ( - ast.NamedExpr(target=ast.Name(id=target_id)), - ast.Name(id=name_id), - ) if target_id == name_id: - left_res = self.assign(left_res) - results[-1] = left_res - - next_res, next_expl = self.visit(next_operand) + next_res, next_expl = self.visit_operand( + next_operand, comp.comparators[i + 1 :] + ) if isinstance(next_operand, ast.Compare | ast.BoolOp): next_expl = f"({next_expl})" if isinstance(next_res, ast.NamedExpr): diff --git a/testing/test_assertrewrite_coverage.py b/testing/test_assertrewrite_coverage.py index 3bc16d2917a..7517efb4210 100644 --- a/testing/test_assertrewrite_coverage.py +++ b/testing/test_assertrewrite_coverage.py @@ -763,6 +763,33 @@ def __getitem__(self, key): assert d["a"] == 100 """) + def test_walrus_in_compare_evaluated_once(self) -> None: + assert_single_evaluation(""" + def check(): + def side_effect(): + counter[0] += 1 + return 42 + assert (x := side_effect()) == 100 + """) + + def test_walrus_in_boolean_evaluated_once(self) -> None: + assert_single_evaluation(""" + def check(): + def side_effect(): + counter[0] += 1 + return 42 + assert (x := side_effect()) and False + """) + + def test_walrus_in_chained_compare_evaluated_once(self) -> None: + assert_single_evaluation(""" + def check(): + def side_effect(): + counter[0] += 1 + return 5 + assert 1 < (x := side_effect()) < 3 + """) + def test_method_call_evaluated_once(self) -> None: assert_single_evaluation(""" def check(): @@ -836,6 +863,72 @@ class TestEvaluationOrder: given it. """ + def test_compare_left_operand_precedes_walrus(self) -> None: + assert_evaluation_order(""" + def check(): + def identity(v): + return v + value = "Hello" + try: + assert value != identity(value := value.lower()) + except AssertionError: + return "raised", value + return "passed", value + """) + + def test_compare_reports_left_operand(self) -> None: + assert_introspects( + """ + def check(): + def identity(v): + return v + value = 2 + assert value == identity(value := 3) + """, + must_contain=["assert 2 == 3"], + ) + + def test_call_earlier_argument_precedes_walrus(self) -> None: + assert_evaluation_order(""" + def check(): + def identity(v): + return v + def collect(*values): + return values + value = "Hello" + try: + assert collect(value, identity(value := value.lower())) == ("Hello", "hello") + except AssertionError: + return "raised", value + return "passed", value + """) + + def test_binop_left_operand_precedes_walrus(self) -> None: + assert_evaluation_order(""" + def check(): + def identity(v): + return v + value = 1 + try: + assert value + identity(value := 5) == 6 + except AssertionError: + return "raised", value + return "passed", value + """) + + def test_chained_compare_operands_in_order(self) -> None: + assert_evaluation_order(""" + def check(): + def identity(v): + return v + value = 1 + try: + assert value < identity(value := 5) < 9 + except AssertionError: + return "raised", value + return "passed", value + """) + def test_container_literal_operand_in_order(self) -> None: """Guard: ``generic_visit`` hoists container literals into a temporary.""" assert_evaluation_order(""" @@ -914,6 +1007,36 @@ def take(self, value): return "passed", obj """) + def test_keyword_argument_precedes_walrus(self) -> None: + assert_evaluation_order(""" + def check(): + def identity(v): + return v + def collect(**kwargs): + return kwargs + value = 1 + try: + assert collect(a=value, b=identity(value := 2)) == {"a": 1, "b": 2} + except AssertionError: + return "raised", value + return "passed", value + """) + + def test_double_star_argument_precedes_walrus(self) -> None: + assert_evaluation_order(""" + def check(): + def identity(v): + return v + def collect(**kwargs): + return kwargs + mapping = {"a": 1} + try: + assert collect(**mapping, b=identity(mapping := {"a": 9})) == {"a": 1, "b": {"a": 9}} + except AssertionError: + return "raised", mapping + return "passed", mapping + """) + def test_ifexp_branches_in_order(self) -> None: """Guard: the condition is evaluated before the selected branch.""" assert_evaluation_order(""" From 025318045c231e98d894b3d3f44502fdea706011 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Wed, 26 Aug 2026 11:32:02 +0200 Subject: [PATCH 09/11] test(rewrite): retire the walrus tests the matrix subsumes TestAssertionRewriteWalrusOperator predates the coverage matrix and asks its questions through runpytest(): twelve tests that mostly check ret == 0. Nine of them the matrix already answers in-process and more precisely -- what the failure message says, what the operands saw, how often each ran. Two are worth more than a deletion, so they move rather than vanish: assert not (a and ((a := False) is False)) reads back as an introspection case, and the composite chain assert a and True and ((a := False) is False) and (a is False) and ... as an evaluation-order one. That second is why the deletion is not just tidying: it passes on main, and it passes there because of the bug. Main rewrites the walrus target to an internal temp and substitutes the stored NamedExpr into every later read of the name -- including the next statement, so the `assert a is None` that is supposed to verify the outcome re-runs the walrus and creates it. Asked through returned values instead of ret == 0, main fails the case. TestIssue14445 loses the four tests that restate matrix single-evaluation entries this PR already adds, and keeps the two reproducers from the issue. One test survives in place: nothing the rewriter stores may outlive a statement, which needs two tests in one module to observe and so cannot be said in-process. Co-Authored-By: Claude Opus 5 (1M context) --- testing/test_assertrewrite.py | 278 ++----------------------- testing/test_assertrewrite_coverage.py | 60 ++++++ 2 files changed, 80 insertions(+), 258 deletions(-) diff --git a/testing/test_assertrewrite.py b/testing/test_assertrewrite.py index b9464f5aec3..df40e1faa32 100644 --- a/testing/test_assertrewrite.py +++ b/testing/test_assertrewrite.py @@ -1637,190 +1637,29 @@ def test_simple_failure(): result.stdout.fnmatch_lines(["*E*assert (1 + 1) == 3"]) -class TestAssertionRewriteWalrusOperator: - """See #10743""" - - def test_assertion_walrus_operator(self, pytester: Pytester) -> None: - pytester.makepyfile( - """ - def my_func(before, after): - return before == after - - def change_value(value): - return value.lower() - - def test_walrus_conversion(): - a = "Hello" - assert not my_func(a, a := change_value(a)) - assert a == "hello" - """ - ) - result = pytester.runpytest() - assert result.ret == 0 - - def test_assertion_walrus_operator_dont_rewrite(self, pytester: Pytester) -> None: - pytester.makepyfile( - """ - 'PYTEST_DONT_REWRITE' - def my_func(before, after): - return before == after - - def change_value(value): - return value.lower() - - def test_walrus_conversion_dont_rewrite(): - a = "Hello" - assert not my_func(a, a := change_value(a)) - assert a == "hello" - """ - ) - result = pytester.runpytest() - assert result.ret == 0 - - def test_assertion_inline_walrus_operator(self, pytester: Pytester) -> None: - pytester.makepyfile( - """ - def my_func(before, after): - return before == after - - def test_walrus_conversion_inline(): - a = "Hello" - assert not my_func(a, a := a.lower()) - assert a == "hello" - """ - ) - result = pytester.runpytest() - assert result.ret == 0 - - def test_assertion_inline_walrus_operator_reverse(self, pytester: Pytester) -> None: - pytester.makepyfile( - """ - def my_func(before, after): - return before == after - - def test_walrus_conversion_reverse(): - a = "Hello" - assert my_func(a := a.lower(), a) - assert a == 'hello' - """ - ) - result = pytester.runpytest() - assert result.ret == 0 - - def test_assertion_walrus_no_variable_name_conflict( - self, pytester: Pytester - ) -> None: - pytester.makepyfile( - """ - def test_walrus_conversion_no_conflict(): - a = "Hello" - assert a == (b := a.lower()) - """ - ) - result = pytester.runpytest() - assert result.ret == 1 - result.stdout.fnmatch_lines(["*AssertionError: assert 'Hello' == 'hello'"]) - - def test_assertion_walrus_operator_true_assertion_and_changes_variable_value( - self, pytester: Pytester - ) -> None: - pytester.makepyfile( - """ - def test_walrus_conversion_succeed(): - a = "Hello" - assert a != (a := a.lower()) - assert a == 'hello' - """ - ) - result = pytester.runpytest() - assert result.ret == 0 - - def test_assertion_walrus_operator_fail_assertion(self, pytester: Pytester) -> None: - pytester.makepyfile( - """ - def test_walrus_conversion_fails(): - a = "Hello" - assert a == (a := a.lower()) - """ - ) - result = pytester.runpytest() - assert result.ret == 1 - result.stdout.fnmatch_lines(["*AssertionError: assert 'Hello' == 'hello'"]) - - def test_assertion_walrus_operator_boolean_composite( - self, pytester: Pytester - ) -> None: - pytester.makepyfile( - """ - def test_walrus_operator_change_boolean_value(): - a = True - assert a and True and ((a := False) is False) and (a is False) and ((a := None) is None) - assert a is None - """ - ) - result = pytester.runpytest() - assert result.ret == 0 - - def test_assertion_walrus_operator_compare_boolean_fails( - self, pytester: Pytester - ) -> None: - pytester.makepyfile( - """ - def test_walrus_operator_change_boolean_value(): - a = True - assert not (a and ((a := False) is False)) - """ - ) - result = pytester.runpytest() - assert result.ret == 1 - result.stdout.fnmatch_lines(["*assert not (True and False is False)"]) - - def test_assertion_walrus_operator_boolean_none_fails( - self, pytester: Pytester - ) -> None: - pytester.makepyfile( - """ - def test_walrus_operator_change_boolean_value(): - a = True - assert not (a and ((a := None) is None)) - """ - ) - result = pytester.runpytest() - assert result.ret == 1 - result.stdout.fnmatch_lines(["*assert not (True and None is None)"]) - - def test_assertion_walrus_operator_value_changes_cleared_after_each_test( - self, pytester: Pytester - ) -> None: - pytester.makepyfile( - """ - def test_walrus_operator_change_value(): - a = True - assert (a := None) is None +def test_walrus_rebinding_does_not_outlive_its_statement( + pytester: Pytester, +) -> None: + """A walrus target must not be rebound by a later, unrelated assertion. - def test_walrus_operator_not_override_value(): - a = True - assert a is True + The rest of #10743's suite moved to the coverage matrix, which runs + in-process. This one stays here because it needs two tests in one module + to say anything: the rewriter may keep no state that survives a statement, + let alone a test. The matrix cannot express that. + """ + pytester.makepyfile( """ - ) - result = pytester.runpytest() - assert result.ret == 0 + def test_walrus_operator_change_value(): + a = True + assert (a := None) is None - def test_assertion_namedexpr_compare_left_overwrite( - self, pytester: Pytester - ) -> None: - pytester.makepyfile( - """ - def test_namedexpr_compare_left_overwrite(): - a = "Hello" - b = "World" - c = "Test" - assert (a := b) == c and (a := "Test") == "Test" - """ - ) - result = pytester.runpytest() - assert result.ret == 1 - result.stdout.fnmatch_lines(["*assert ('World' == 'Test'*"]) + def test_walrus_operator_not_override_value(): + a = True + assert a is True + """ + ) + result = pytester.runpytest() + assert result.ret == 0 class TestIssue11028: @@ -1973,83 +1812,6 @@ def test_walrus_running_counter(): result = pytester.runpytest() assert result.ret == 0 - def test_walrus_no_double_eval_in_function_call(self, pytester: Pytester) -> None: - """Walrus in function call arguments not evaluated twice.""" - pytester.makepyfile( - """ - call_count = 0 - - def side_effect(): - global call_count - call_count += 1 - return call_count - - def test_walrus_side_effect(): - assert (val := side_effect()) == 1 - assert val == 1 - assert (val := side_effect()) == 2 - assert val == 2 - """ - ) - result = pytester.runpytest() - assert result.ret == 0 - - def test_walrus_no_double_eval_in_boolop(self, pytester: Pytester) -> None: - """Bare walrus as a BoolOp operand must not be evaluated twice.""" - pytester.makepyfile( - """ - call_count = 0 - - def side_effect(): - global call_count - call_count += 1 - return call_count - - def test_walrus_boolop(): - assert (x := side_effect()) and x == 1 - assert call_count == 1 - """ - ) - result = pytester.runpytest() - assert result.ret == 0 - - def test_walrus_no_double_eval_chained_compare(self, pytester: Pytester) -> None: - """Same walrus target in chained comparison must evaluate each once.""" - pytester.makepyfile( - """ - call_count = 0 - - def track(value): - global call_count - call_count += 1 - return value - - def test_walrus_chained(): - assert (x := track(1)) < (x := track(3)) < (x := track(5)) - assert call_count == 3 - """ - ) - result = pytester.runpytest() - assert result.ret == 0 - - def test_walrus_boolop_same_target_correct_explanation( - self, pytester: Pytester - ) -> None: - """Multiple walrus operators to the same name in a BoolOp must show - each operand's value at evaluation time, not the final value.""" - pytester.makepyfile( - """ - def side_effect(): - return True - - def test_walrus_boolop(): - assert (x := side_effect()) and (x := False) - """ - ) - result = pytester.runpytest() - assert result.ret == 1 - result.stdout.fnmatch_lines(["*assert (True and False)"]) - @pytest.mark.skipif( sys.maxsize <= (2**31 - 1), reason="Causes OverflowError on 32bit systems" diff --git a/testing/test_assertrewrite_coverage.py b/testing/test_assertrewrite_coverage.py index 7517efb4210..43a3f668f73 100644 --- a/testing/test_assertrewrite_coverage.py +++ b/testing/test_assertrewrite_coverage.py @@ -674,6 +674,41 @@ def check(): assert (y := x * 2) == 100 """) + def test_walrus_in_boolop_reports_each_operand(self) -> None: + """Two walrus assignments to one name: each operand shows what it saw.""" + assert_introspects( + """ + def check(): + def side_effect(): + return True + assert (x := side_effect()) and (x := False) + """, + must_contain=["assert (True and False)"], + ) + + def test_walrus_in_boolop_reports_assigned_value(self) -> None: + assert_introspects( + """ + def check(): + a = True + assert not (a and ((a := False) is False)) + """, + must_contain=["assert not (True and False is False)"], + ) + + def test_walrus_in_boolop_reports_left_operand(self) -> None: + """A comparator walrus must not overwrite the left operand's report.""" + assert_introspects( + """ + def check(): + a = "Hello" + b = "World" + c = "Test" + assert (a := b) == c and (a := "Test") == "Test" + """, + must_contain=["assert ('World' == 'Test'"], + ) + # --------------------------------------------------------------------------- # Single-evaluation tests: ensure no expression is evaluated multiple times @@ -903,6 +938,31 @@ def collect(*values): return "passed", value """) + def test_call_later_argument_sees_walrus_value(self) -> None: + """The mirror of the case above: a later operand sees the new value.""" + assert_evaluation_order(""" + def check(): + def collect(*values): + return values + value = "Hello" + try: + assert collect(value := value.lower(), value) == ("hello", "hello") + except AssertionError: + return "raised", value + return "passed", value + """) + + def test_boolop_chain_rebinds_in_order(self) -> None: + assert_evaluation_order(""" + def check(): + a = True + try: + assert a and True and ((a := False) is False) and (a is False) and ((a := None) is None) + except AssertionError: + return "raised", a + return "passed", a + """) + def test_binop_left_operand_precedes_walrus(self) -> None: assert_evaluation_order(""" def check(): From 5663cf5c1b5be31cab88700fcfbc9f08a5a83d50 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Fri, 31 Jul 2026 20:42:11 +0200 Subject: [PATCH 10/11] fix(rewrite): freeze walrus and starred operands too visit_operand() only froze a bare name, so two other unhoisted operands kept being evaluated after everything that follows them: assert collect((x := 1), identity(x := 2)) == (1, 2) assert collect(*items, identity(items := [9])) == (1, [9]) A walrus operator left in place assigns once the enclosing expression is assembled, which is after the later arguments have run -- so the earlier argument saw the later assignment. A starred argument hid its value inside an ast.Starred, where the existing Name check could not see it. Closes the order-starred-argument group and the remaining order-call-argument entry in the coverage matrix. Co-Authored-By: Claude Opus 5 (1M context) --- changelog/14814.bugfix.rst | 1 + src/_pytest/assertion/rewrite.py | 37 ++++++++++++++++++++------ testing/test_assertrewrite_coverage.py | 30 +++++++++++++++++++++ 3 files changed, 60 insertions(+), 8 deletions(-) create mode 100644 changelog/14814.bugfix.rst diff --git a/changelog/14814.bugfix.rst b/changelog/14814.bugfix.rst new file mode 100644 index 00000000000..6e44ab4976f --- /dev/null +++ b/changelog/14814.bugfix.rst @@ -0,0 +1 @@ +Fixed assertion rewriting evaluating a walrus operator (``:=``) or a starred argument out of order when a later argument assigned to the same name, so ``assert collect(*items, identity(items := [9]))`` now passes the pre-assignment ``items``. diff --git a/src/_pytest/assertion/rewrite.py b/src/_pytest/assertion/rewrite.py index 27953336c5c..d9d9d0b1390 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -967,19 +967,40 @@ def visit_operand( """Visit an operand, freezing it against walrus operators in *later*. Operands are rewritten into statements that run in source order, but - a plain name is left as a bare load evaluated at the very end, when - the enclosing expression is assembled. A walrus operator in a later - operand rebinds that name in between, so both the value used and the - value reported would be the post-walrus one -- Python evaluates the - earlier operand first. Copy the value into a temporary instead. + two of them stay unhoisted and are evaluated at the very end, when the + enclosing expression is assembled -- after everything that follows + them: + + * a plain name, which a walrus operator in a later operand rebinds in + between, so the value used and the value reported would be the + post-walrus one; + * a walrus operator itself, which would then assign in the wrong + order, and be visible to the operands that were meant to precede it. + + Either way Python evaluates the earlier operand first, so copy it into + a temporary here. A starred argument is unwrapped and rewrapped, its + value being subject to the same problem. """ specifiers = set(self.explanation_specifiers) res, expl = self.visit(operand) - if isinstance(res, ast.Name) and res.id in _walrus_targets(later): - snapshot = self.assign(res) + value = res.value if isinstance(res, ast.Starred) else res + if isinstance(value, ast.NamedExpr): + needs_freeze = bool(later) + else: + # Every other operand arrives as a temporary: the visit_* methods + # hoist what they build, and generic_visit assigns whatever is left + # -- a literal included -- so a name is all that can reach here. + assert isinstance(value, ast.Name) + needs_freeze = value.id in _walrus_targets(later) + if needs_freeze: + snapshot = self.assign(value) for key in set(self.explanation_specifiers) - specifiers: self.explanation_specifiers[key] = self.display(snapshot) - res = snapshot + res = ( + ast.copy_location(ast.Starred(snapshot, res.ctx), res) + if isinstance(res, ast.Starred) + else snapshot + ) return res, expl def visit_BoolOp(self, boolop: ast.BoolOp) -> tuple[ast.Name, str]: diff --git a/testing/test_assertrewrite_coverage.py b/testing/test_assertrewrite_coverage.py index 43a3f668f73..6ac2cecb4fe 100644 --- a/testing/test_assertrewrite_coverage.py +++ b/testing/test_assertrewrite_coverage.py @@ -976,6 +976,21 @@ def identity(v): return "passed", value """) + def test_starred_argument_precedes_walrus(self) -> None: + assert_evaluation_order(""" + def check(): + def identity(v): + return v + def collect(*values): + return values + items = [1] + try: + assert collect(*items, identity(items := [9])) == (1, [9]) + except AssertionError: + return "raised", items + return "passed", items + """) + def test_chained_compare_operands_in_order(self) -> None: assert_evaluation_order(""" def check(): @@ -989,6 +1004,21 @@ def identity(v): return "passed", value """) + def test_bare_walrus_argument_in_order(self) -> None: + """A walrus argument is evaluated in place, before the ones after it.""" + assert_evaluation_order(""" + def check(): + def identity(v): + return v + def collect(*values): + return values + try: + assert collect((x := 1), identity(x := 2)) == (1, 2) + except AssertionError: + return "raised", x + return "passed", x + """) + def test_container_literal_operand_in_order(self) -> None: """Guard: ``generic_visit`` hoists container literals into a temporary.""" assert_evaluation_order(""" From 338cd6778fa26c0f656c33475c8cc5a410d9f642 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Mon, 10 Aug 2026 09:11:18 +0200 Subject: [PATCH 11/11] refactor(rewrite): drop the walrus snapshot visit_Compare no longer needs visit_operand freezes a walrus operand whenever anything follows it, and a comparison always has at least one comparator -- so by the time visit_Compare looks at its left operand, a NamedExpr has already been copied into a temporary. The special case that did it here can never run. Co-Authored-By: Claude Opus 5 (1M context) --- src/_pytest/assertion/rewrite.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/_pytest/assertion/rewrite.py b/src/_pytest/assertion/rewrite.py index d9d9d0b1390..7d5c620316a 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -1126,8 +1126,6 @@ def visit_Compare(self, comp: ast.Compare) -> tuple[ast.expr, str]: left_res, left_expl = self.visit_operand(comp.left, comp.comparators) if isinstance(comp.left, ast.Compare | ast.BoolOp): left_expl = f"({left_expl})" - if isinstance(left_res, ast.NamedExpr): - left_res = self.assign(left_res) res_variables = [self.variable() for i in range(len(comp.ops))] load_names: list[ast.expr] = [ast.Name(v, ast.Load()) for v in res_variables] store_names = [ast.Name(v, ast.Store()) for v in res_variables]