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

No assertion rewriting in files imported from conftest #1871

Closed
The-Compiler opened this issue Aug 25, 2016 · 7 comments
Closed

No assertion rewriting in files imported from conftest #1871

The-Compiler opened this issue Aug 25, 2016 · 7 comments

Comments

@The-Compiler
Copy link
Member

I guess I can't be the only one importing some utility code from conftest.py, or can I? 😉

With this conftest.py:

import pytest
from testutils import SomeClass

@pytest.fixture
def some_obj():
    return SomeClass()

And a testutils.py:

class SomeClass():

    def __init__(self):
        x = 23
        y = 42
        assert x + y == 1337

And a test_foo.py:

def test_foo(some_obj):
    pass

With pytest 2.9, I guess assertion reinterpretion kicks in and I get:

>       assert x + y == 1337
E       AssertionError

With 3.0, that's removed and I get plain asserts:

>       assert x + y == 1337
E       AssertionError

Since I have some custom assertion helpers and stuff in separated files, that's quite a bummer.

I'm not sure what the best way to fix this would be - ideas I had so far:

  • Rewrite everything under the testpath if an explicit one is given
  • Rewrite everything in directories where there are conftest.py files
  • Rewrite everything imported from conftest.py files (probably cleanest, but maybe hard to implement?)
@The-Compiler The-Compiler changed the title No assertion rewriting in files imported to conftest No assertion rewriting in files imported from conftest Aug 25, 2016
The-Compiler added a commit to qutebrowser/qutebrowser that referenced this issue Aug 25, 2016
pytest 3.0 removed the assertion reinterpretation which made this nice:
pytest-dev/pytest#1871
@nicoddemus
Copy link
Member

You will have to call register_assert_rewrite before trying to import testutils:

import pytest
pytest.register_assert_rewrite('testutils')
from testutils import SomeClass

@pytest.fixture
def some_obj():
    return SomeClass()

This now works as expected:

    def __init__(self):
        x = 23
        y = 42
>       assert x + y == 1337
E       assert (23 + 42) == 1337

@The-Compiler
Copy link
Member Author

Oh - thanks, that worked beautifully (even with a package)!

Do you think we should do something automatically (like I outlined above) or should I close this? I don't really have a strong opinion on it.

@nicoddemus
Copy link
Member

nicoddemus commented Aug 26, 2016

Rewrite everything under the testpath if an explicit one is given

This would mean that projects which decide to place tests next to the code would get their own non-test code rewritten, which is something @flub and @RonnyPfannschmidt are against (and with good reason).

Rewrite everything in directories where there are conftest.py files

Same problem as above.

Rewrite everything imported from conftest.py files (probably cleanest, but maybe hard to implement?)

That might not be safe as well because conftest.py files can easily import application code to implement fixtures etc.

I would really love some way to automate this in some way, but I don't really have an idea for that at the moment. 😁

@flub
Copy link
Member

flub commented Aug 26, 2016

Ideally you'd do that register_assert_rewrite() in the __init__.py.

More importantly however is that you did not find the documentation for this when you upgraded to 3.0. How can we improve that?

@nicoddemus
Copy link
Member

nicoddemus commented Aug 26, 2016

Ideally you'd do that register_assert_rewrite() in the init.py.

Agreed, it's just that @The-Compiler's test suite doesn't have any __init__.py files.

More importantly however is that you did not find the documentation for this when you upgraded to 3.0. How can we improve that?

Good point. We have some docs here already, but maybe we should've mentioned that more prominently on the CHANGELOG, I don't know... suggestions are welcome.

@The-Compiler
Copy link
Member Author

Agreed, it's just that @The-Compiler's test suite doesn't have any __init__.py files.

Do we have some agreed-upon recommendation whether to do that or not yet? Seems like the answer changes depending on whom I ask 😆

More importantly however is that you did not find the documentation for this when you upgraded to 3.0. How can we improve that?

As Bruno pointed out, what I usually do when doing an upgrade (and certainly with a major one) is reading the changelog. register_assert_rewrite seems to be not mentioned at all - something like this would've helped me:

  • Reinterpretation mode has now been removed. Only plain and rewrite mode are available, consequently the --assert=reinterp option is no longer available. This also means files imported from conftest.py will not benefit from improved assertions by default, you should use pytest.register_assert_rewrite to explicitly turn assertion rewriting on for those files. Thanks @flub for the PR.

@nicoddemus
Copy link
Member

FWIW I think it is still valid to add that to the CHANGELOG, it might be helpful for someone else down the road. 😁

flub added a commit to flub/pytest that referenced this issue Sep 5, 2016
It helps mentioning this explicitly in the changelog.
Fixes pytest-dev#1871.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants