From acb3e8e8a797a1fd321322d27114ef15ff99d440 Mon Sep 17 00:00:00 2001 From: Anton Lodder Date: Thu, 10 Jan 2019 14:12:50 -0500 Subject: [PATCH 1/3] Test rewriting assertion when __name__ fails Pytest rewrites assertions so that the items on each side of a comoparison will have easier-to-read names in case of an assertion error. Before doing this, it checks to make sure the object doesn't have a __name__ attribute; however, it uses `hasattr` so if the objects __getattr__ is broken then the test failure message will be the stack trace for this failure instead of a rewritten assertion. --- testing/test_assertrewrite.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/testing/test_assertrewrite.py b/testing/test_assertrewrite.py index 4187e365b6d..840fda2ca9a 100644 --- a/testing/test_assertrewrite.py +++ b/testing/test_assertrewrite.py @@ -180,6 +180,27 @@ class X(object): assert getmsg(f, {"cls": X}) == "assert cls == 42" + def test_dont_rewrite_if_hasattr_fails(self): + class Y(object): + """ A class whos getattr fails, but not with `AttributeError` """ + + def __getattr__(self, attribute_name): + raise KeyError() + + def __repr__(self): + return "Y" + + def __init__(self): + self.foo = 3 + + def f(): + assert cls().foo == 2 # noqa + + message = getmsg(f, {"cls": Y}) + assert "assert 3 == 2" in message + assert "+ where 3 = Y.foo" in message + assert "+ where Y = cls()" in message + def test_assert_already_has_message(self): def f(): assert False, "something bad!" From 3241fc31030cc2d9cc9870fb388fabcab541fbe5 Mon Sep 17 00:00:00 2001 From: Anton Lodder Date: Thu, 10 Jan 2019 14:13:17 -0500 Subject: [PATCH 2/3] Don't fail if hasattr fails when rewriting assertions When rewriting assertions, pytest makes a call to `__name__` on each object in a comparision. If one of the objects has reimplemented `__getattr__`, they could fail trying to fetch `__name__` with an error other than `AttributeError`, which is what `hasattr` catches. In this case, the stack trace for the failed `__getattr__` call will show up in the pytest output, even though it isn't related to the test failing. This change fixes that by catching exceptions that `hasattr` throws. --- src/_pytest/assertion/rewrite.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/_pytest/assertion/rewrite.py b/src/_pytest/assertion/rewrite.py index 1d2c27ed152..df8b9becbb4 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -525,7 +525,13 @@ def _format_assertmsg(obj): def _should_repr_global_name(obj): - return not hasattr(obj, "__name__") and not callable(obj) + if callable(obj): + return False + + try: + return not hasattr(obj, "__name__") + except Exception: + return True def _format_boolop(explanations, is_or): From 77da4f118c81a6c28b4be0f931fa4d8bd512fb69 Mon Sep 17 00:00:00 2001 From: Anton Lodder Date: Thu, 10 Jan 2019 14:27:10 -0500 Subject: [PATCH 3/3] Add changelog 4631 and AUTHOR credits for Anton Lodder --- AUTHORS | 1 + changelog/4631.bugfix.rst | 1 + 2 files changed, 2 insertions(+) create mode 100644 changelog/4631.bugfix.rst diff --git a/AUTHORS b/AUTHORS index 7ce822b4329..06947d17be9 100644 --- a/AUTHORS +++ b/AUTHORS @@ -24,6 +24,7 @@ Andy Freeland Anthon van der Neut Anthony Shaw Anthony Sottile +Anton Lodder Antony Lee Armin Rigo Aron Coyle diff --git a/changelog/4631.bugfix.rst b/changelog/4631.bugfix.rst new file mode 100644 index 00000000000..df929e8afd4 --- /dev/null +++ b/changelog/4631.bugfix.rst @@ -0,0 +1 @@ +Don't rewrite assertion when ``__getattr__`` is broken