Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/14814.bugfix.rst
Original file line number Diff line number Diff line change
@@ -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``.
4 changes: 4 additions & 0 deletions changelog/14815.improvement.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Assertion failure messages now decompose subscript expressions, showing the container and the key that produced a value::

assert 1 == 99
+ where 1 = {'a': 1, 'b': 2}['a']
4 changes: 4 additions & 0 deletions changelog/14816.improvement.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Assertion failure messages now show the condition of a conditional expression::

assert 0 == 99
+ where 0 = (... if True else ...)
70 changes: 58 additions & 12 deletions src/_pytest/assertion/rewrite.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]:
Expand Down Expand Up @@ -1088,9 +1109,36 @@ def visit_Starred(self, starred: ast.Starred) -> tuple[ast.Starred, str]:
new_starred = ast.Starred(res, starred.ctx)
return new_starred, "*" + expl

def visit_IfExp(self, ifexp: ast.IfExp) -> tuple[ast.Name, str]:
# Introspect the condition but keep the branches as they are: only the
# selected one may be evaluated, so neither can be hoisted. That also
# keeps them ordered after the condition, which is where Python puts
# them, so no freeze is needed here.
cond_res, cond_expl = self.visit(ifexp.test)
res = self.assign(
ast.copy_location(ast.IfExp(cond_res, ifexp.body, ifexp.orelse), ifexp)
)
res_expl = self.explanation_param(self.display(res))
pat = "%s\n{%s = (... if %s else ...)\n}"
expl = pat % (res_expl, res_expl, cond_expl)
return res, expl

def visit_Subscript(self, subscript: ast.Subscript) -> tuple[ast.Name, str]:
# For Slice objects (a[1:3]), fall back to generic — decomposing
# start/stop/step is rarely useful in assertion messages.
if isinstance(subscript.slice, ast.Slice):
return self.generic_visit(subscript)
value, value_expl = self.visit_operand(subscript.value, [subscript.slice])
slice_res, slice_expl = self.visit(subscript.slice)
res = self.assign(
ast.copy_location(ast.Subscript(value, slice_res, ast.Load()), subscript)
)
res_expl = self.explanation_param(self.display(res))
pat = "%s\n{%s = %s[%s]\n}"
expl = pat % (res_expl, res_expl, value_expl, slice_expl)
return res, expl

def visit_Attribute(self, attr: ast.Attribute) -> tuple[ast.Name, str]:
if not isinstance(attr.ctx, ast.Load):
return self.generic_visit(attr)
value, value_expl = self.visit(attr.value)
res = self.assign(
ast.copy_location(ast.Attribute(value, attr.attr, ast.Load()), attr)
Expand All @@ -1105,8 +1153,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]
Expand Down
94 changes: 94 additions & 0 deletions testing/test_assertrewrite_coverage.py
Original file line number Diff line number Diff line change
Expand Up @@ -535,6 +535,26 @@ def check():
class TestIntrospectionSubscript:
"""Subscript / indexing."""

def test_dict_subscript_shows_key_and_container(self) -> None:
assert_introspects(
"""
def check():
d = {"a": 1, "b": 2}
assert d["a"] == 99
""",
must_contain=["where 1 = ", "['a']"],
)

def test_list_subscript_shows_index_and_container(self) -> None:
assert_introspects(
"""
def check():
items = [10, 20, 30]
assert items[1] == 99
""",
must_contain=["where 20 = ", "[1]"],
)

def test_subscript_semantics_preserved(self) -> None:
assert_semantically_equivalent("""
def check():
Expand All @@ -557,13 +577,33 @@ def check():
class TestIntrospectionIfExp:
"""Ternary / if-expression."""

def test_ifexp_shows_condition_value(self) -> None:
assert_introspects(
"""
def check():
flag = True
assert (0 if flag else 1) == 1
""",
must_contain=["if True else"],
)

def test_ifexp_semantics_preserved(self) -> None:
assert_semantically_equivalent("""
def check():
flag = True
assert (0 if flag else 1) == 1
""")

def test_ifexp_in_compare_shows_result(self) -> None:
assert_introspects(
"""
def check():
flag = True
assert (0 if flag else 1) == 99
""",
must_contain=["assert 0 == 99", "if True else"],
)

def test_ifexp_short_circuit_true(self) -> None:
"""Orelse branch must NOT be evaluated when condition is True."""
assert_passes_when_true("""
Expand Down Expand Up @@ -976,6 +1016,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():
Expand All @@ -989,6 +1044,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("""
Expand Down Expand Up @@ -1120,6 +1190,18 @@ def identity(v):
class TestEdgeCases:
"""Regression and edge-case tests combining multiple expression types."""

def test_subscript_with_variable_key(self) -> None:
"""Subscript where the key is a variable (not constant)."""
assert_introspects(
"""
def check():
d = {"hello": 42}
key = "hello"
assert d[key] == 100
""",
must_contain=["where 42 = ", "['hello']"],
)

def test_subscript_with_call_key(self) -> None:
"""Subscript where the key is a function call."""
assert_introspects(
Expand Down Expand Up @@ -1160,6 +1242,18 @@ def __repr__(self):
must_contain=["42", "100"],
)

def test_ifexp_with_call_condition(self) -> None:
"""IfExp where condition is a function call."""
assert_introspects(
"""
def check():
def is_ready():
return False
assert (1 if is_ready() else 0) == 1
""",
must_contain=["if False else"],
)

def test_walrus_in_subscript(self) -> None:
"""Walrus operator used as subscript key."""
assert_semantically_equivalent("""
Expand Down