Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pytest_assertrepr_compare not being used? #1784

Closed
nicoddemus opened this issue Aug 2, 2016 · 7 comments
Closed

pytest_assertrepr_compare not being used? #1784

nicoddemus opened this issue Aug 2, 2016 · 7 comments
Labels
status: critical grave problem or usability issue that affects lots of users type: bug problem that needs to be addressed
Milestone

Comments

@nicoddemus
Copy link
Member

Running regendoc in #1780, @The-Compiler noticed that an example from the docs regarding pytest_assertrepr_compare was not working as it should:

# conftest.py
from test_foocompare import Foo
def pytest_assertrepr_compare(op, left, right):
   if isinstance(left, Foo) and isinstance(right, Foo) and op == "==":
       return ['Comparing Foo instances:',
               '   vals: %s != %s' % (left.val, right.val)]

# test_foocompare
class Foo:
    def __init__(self, val):
        self.val = val

    def __eq__(self, other):
        return self.val == other.val


def test_compare():
    f1 = Foo(1)
    f2 = Foo(2)
    assert f1 == f2
$ py.test

================================== FAILURES ===================================
________________________________ test_compare _________________________________

    def test_compare():
        f1 = Foo(1)
        f2 = Foo(2)
>       assert f1 == f2
E       AssertionError

test_foocompare.py:12: AssertionError
@flub
Copy link
Member

flub commented Aug 2, 2016

This looks like it's using plain assertion instead of re-write at first glance. So the hook probably doesn't come to the question. I suspect this would have been done by reinterpret before but that's now gone...

@nicoddemus
Copy link
Member Author

Hmm that's weird, I have the same effect even if I explicitly pass --assert=rewrite... why would this fall back to plain? 😮

@RonnyPfannschmidt
Copy link
Member

at first glance it seems the rewritten import of conftest triggers a non-rewritten import of the test module before attempting a rewritten import of the test module
how/when does the rewriting logic add test modules to the paths to consider, and is there a way to do this earlier?

@flub flub added this to the 3.0 milestone Aug 2, 2016
@nicoddemus
Copy link
Member Author

Good point. Does it work if test_foocompare is imported locally inside the hook function? (not in front of a computer to test this myself)

@RonnyPfannschmidt RonnyPfannschmidt added type: bug problem that needs to be addressed status: critical grave problem or usability issue that affects lots of users labels Aug 2, 2016
nicoddemus added a commit to nicoddemus/pytest that referenced this issue Aug 2, 2016
@nicoddemus
Copy link
Member Author

PR opened at #1787

nicoddemus added a commit to nicoddemus/pytest that referenced this issue Aug 3, 2016
Also now match modules which start with any of the names registered
using register_assert_rewrite as discussed in pytest-dev#1787

Fix pytest-dev#1784
nicoddemus added a commit to nicoddemus/pytest that referenced this issue Aug 3, 2016
Also now match modules which start with any of the names registered
using register_assert_rewrite as discussed in pytest-dev#1787

Fix pytest-dev#1784
@The-Compiler
Copy link
Member

PR merged, so I'm closing this. I guess it wasn't auto-closed because it was merged to features?

@nicoddemus
Copy link
Member Author

Yep! Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: critical grave problem or usability issue that affects lots of users type: bug problem that needs to be addressed
Projects
None yet
Development

No branches or pull requests

4 participants