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 ...)
4 changes: 4 additions & 0 deletions changelog/14817.improvement.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Assertion failure messages now show a method call on a single line, without the bound method as a separate intermediate::

assert 42 == 100
+ where 42 = Obj().compute()
86 changes: 73 additions & 13 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 @@ -1059,7 +1080,21 @@ def visit_Call(self, call: ast.Call) -> tuple[ast.Name, str]:
# 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)
if isinstance(call.func, ast.Attribute) and isinstance(call.func.ctx, ast.Load):
# obj.method(...) reads better flat -- "where 42 = Obj().compute()"
# rather than a separate "where compute = Obj().compute" line. The
# bound method still gets a temporary of its own, because Python
# looks it up before evaluating the arguments; that is also what
# keeps the receiver ordered ahead of them.
receiver, receiver_expl = self.visit(call.func.value)
new_func: ast.expr = self.assign(
ast.copy_location(
ast.Attribute(receiver, call.func.attr, ast.Load()), call.func
)
)
func_expl = f"{receiver_expl}.{call.func.attr}"
else:
new_func, func_expl = self.visit_operand(call.func, operands)
arg_expls = []
new_args = []
new_kwargs = []
Expand Down Expand Up @@ -1088,9 +1123,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 +1167,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
5 changes: 2 additions & 3 deletions testing/python/raises_group.py
Original file line number Diff line number Diff line change
Expand Up @@ -1237,11 +1237,10 @@ def test_assert_matches() -> None:
match=wrap_escape(
"`ValueError()` is not an instance of `TypeError`\n"
"assert False\n"
" + where False = matches(ValueError())\n"
" + where matches = RaisesExc(TypeError).matches"
" + where False = RaisesExc(TypeError).matches(ValueError())"
),
):
# you'd need to do this arcane incantation
# binding the RaisesExc is still how you get at ``fail_reason``
assert (m := RaisesExc(TypeError)).matches(e), m.fail_reason

# but even if we add assert_matches, will people remember to use it?
Expand Down
Loading