From a9eba91f7f50a6d6ec8bcc9151239e460b69f0f9 Mon Sep 17 00:00:00 2001 From: Farbod Ahmadian Date: Fri, 21 Jun 2024 17:32:46 +0200 Subject: [PATCH] refactor: move bound_method repr from global version to rewrite module --- src/_pytest/_io/saferepr.py | 7 +------ src/_pytest/assertion/rewrite.py | 28 ++++++++++++++++++++++------ testing/io/test_saferepr.py | 23 ----------------------- testing/test_assertrewrite.py | 29 ++++++++++++++++++++++++++++- 4 files changed, 51 insertions(+), 36 deletions(-) diff --git a/src/_pytest/_io/saferepr.py b/src/_pytest/_io/saferepr.py index 520d6c4d1..497699137 100644 --- a/src/_pytest/_io/saferepr.py +++ b/src/_pytest/_io/saferepr.py @@ -60,12 +60,7 @@ class SafeRepr(reprlib.Repr): if self.use_ascii: s = ascii(x) else: - if isinstance(x, MethodType): - # for bound methods, skip redundant information - s = x.__name__ - else: - s = super().repr(x) - + s = super().repr(x) except (KeyboardInterrupt, SystemExit): raise except BaseException as exc: diff --git a/src/_pytest/assertion/rewrite.py b/src/_pytest/assertion/rewrite.py index 442fc5d9f..c3eeca87c 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -417,6 +417,10 @@ def _saferepr(obj: object) -> str: sequences, especially '\n{' and '\n}' are likely to be present in JSON reprs. """ + if isinstance(obj, types.MethodType): + # for bound methods, skip redundant information + return obj.__name__ + maxsize = _get_maxsize_for_saferepr(util._config) return saferepr(obj, maxsize=maxsize).replace("\n", "\\n") @@ -1014,7 +1018,9 @@ class AssertionRewriter(ast.NodeVisitor): ] ): pytest_temp = self.variable() - self.variables_overwrite[self.scope][v.left.target.id] = v.left # type:ignore[assignment] + self.variables_overwrite[self.scope][ + v.left.target.id + ] = v.left # type:ignore[assignment] v.left.target.id = pytest_temp self.push_format_context() res, expl = self.visit(v) @@ -1058,7 +1064,9 @@ class AssertionRewriter(ast.NodeVisitor): 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] + arg = self.variables_overwrite[self.scope][ + arg.id + ] # type:ignore[assignment] res, expl = self.visit(arg) arg_expls.append(expl) new_args.append(res) @@ -1066,7 +1074,9 @@ class AssertionRewriter(ast.NodeVisitor): if isinstance( keyword.value, ast.Name ) and keyword.value.id in self.variables_overwrite.get(self.scope, {}): - keyword.value = self.variables_overwrite[self.scope][keyword.value.id] # type:ignore[assignment] + keyword.value = self.variables_overwrite[self.scope][ + keyword.value.id + ] # type:ignore[assignment] res, expl = self.visit(keyword.value) new_kwargs.append(ast.keyword(keyword.arg, res)) if keyword.arg: @@ -1103,9 +1113,13 @@ class AssertionRewriter(ast.NodeVisitor): if isinstance( comp.left, ast.Name ) and comp.left.id in self.variables_overwrite.get(self.scope, {}): - comp.left = self.variables_overwrite[self.scope][comp.left.id] # type:ignore[assignment] + comp.left = self.variables_overwrite[self.scope][ + comp.left.id + ] # type:ignore[assignment] if isinstance(comp.left, ast.NamedExpr): - self.variables_overwrite[self.scope][comp.left.target.id] = comp.left # type:ignore[assignment] + self.variables_overwrite[self.scope][ + comp.left.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})" @@ -1123,7 +1137,9 @@ class AssertionRewriter(ast.NodeVisitor): and next_operand.target.id == left_res.id ): next_operand.target.id = self.variable() - self.variables_overwrite[self.scope][left_res.id] = next_operand # type:ignore[assignment] + self.variables_overwrite[self.scope][ + left_res.id + ] = next_operand # type:ignore[assignment] next_res, next_expl = self.visit(next_operand) if isinstance(next_operand, (ast.Compare, ast.BoolOp)): next_expl = f"({next_expl})" diff --git a/testing/io/test_saferepr.py b/testing/io/test_saferepr.py index a4226522c..7a3066ac6 100644 --- a/testing/io/test_saferepr.py +++ b/testing/io/test_saferepr.py @@ -194,26 +194,3 @@ def test_saferepr_unlimited_exc(): assert saferepr_unlimited(A()).startswith( "<[ValueError(42) raised in repr()] A object at 0x" ) - - -class TestSafereprUnbounded: - class Help: - def bound_method(self): # pragma: no cover - pass - - def test_saferepr_bound_method(self): - """saferepr() of a bound method should show only the method name""" - assert saferepr(self.Help().bound_method) == "bound_method" - - def test_saferepr_unbounded(self): - """saferepr() of an unbound method should still show the full information""" - obj = self.Help() - # using id() to fetch memory address fails on different platforms - pattern = re.compile( - r"", - ) - assert pattern.match(saferepr(obj)) - assert ( - saferepr(self.Help) - == f"" - ) diff --git a/testing/test_assertrewrite.py b/testing/test_assertrewrite.py index 185cd5ef2..6a27e52cd 100644 --- a/testing/test_assertrewrite.py +++ b/testing/test_assertrewrite.py @@ -18,6 +18,7 @@ from typing import Generator from typing import Mapping from unittest import mock import zipfile +import re import _pytest._code from _pytest._io.saferepr import DEFAULT_REPR_MAX_SIZE @@ -33,6 +34,7 @@ from _pytest.config import Config from _pytest.config import ExitCode from _pytest.pathlib import make_numbered_dir from _pytest.pytester import Pytester +from _pytest.assertion.rewrite import _saferepr import pytest @@ -2036,7 +2038,9 @@ class TestPyCacheDir: assert test_foo_pyc.is_file() # normal file: not touched by pytest, normal cache tag - bar_init_pyc = get_cache_dir(bar_init) / f"__init__.{sys.implementation.cache_tag}.pyc" + bar_init_pyc = ( + get_cache_dir(bar_init) / f"__init__.{sys.implementation.cache_tag}.pyc" + ) assert bar_init_pyc.is_file() @@ -2103,3 +2107,26 @@ class TestIssue11140: ) result = pytester.runpytest() assert result.ret == 0 + + +class TestSafereprUnbounded: + class Help: + def bound_method(self): # pragma: no cover + pass + + def test_saferepr_bound_method(self): + """saferepr() of a bound method should show only the method name""" + assert _saferepr(self.Help().bound_method) == "bound_method" + + def test_saferepr_unbounded(self): + """saferepr() of an unbound method should still show the full information""" + obj = self.Help() + # using id() to fetch memory address fails on different platforms + pattern = re.compile( + rf"<{Path(__file__).stem}.{self.__class__.__name__}.Help object at 0x[0-9a-fA-F]*>", + ) + assert pattern.match(_saferepr(obj)) + assert ( + _saferepr(self.Help) + == f"" + )