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

Install import hook earlier during pytest initialization #143

Merged
merged 1 commit into from Aug 14, 2020
Merged

Install import hook earlier during pytest initialization #143

merged 1 commit into from Aug 14, 2020

Conversation

wbolster
Copy link
Contributor

@wbolster wbolster commented Aug 14, 2020

The pytest plugin does not work at all in this very common scenario:

  • Application/library uses a conftest.py file with pytest configuration
  • This conftest.py imports application/library modules.

Even worse, no matter what is passed as the ‘--typeguard-packages=’
command line argument, no warnings or errors will be reported at all.
Typeguard will be fully silent. This makes it seem the code is
fully correct, while in reality the code is not checked by typeguard
at all. Oops. 💥 🙈 😱

The problem is that the pytest plugin used the ‘pytest_sessionstart()’
hook, which triggers only after conftest.py files (or other pytest
configuration files) have been loaded already. This is too late to
install the import hook: the targeted modules may have been imported
already at this point!

This pull request fixes this problem in the following way:

  • Use ‘pytest_configure()’ instead of ‘pytest_sessionstart()’, since
    the former triggers before any conftest files are loaded.

  • Make it impossible to silently skip checks for packages specified on
    the command line that cannot actually be checked. In those situations,
    raise an error with all the details. Example for
    ‘--typeguard-packages=sys’:

    INTERNALERROR> RuntimeError: typeguard cannot check these packages that are already imported: sys

See also:

The pytest plugin does not work at all in this very common scenario:

- Application/library uses a conftest.py file with pytest configuration
- This conftest.py imports application/library modules.

Even worse, no matter what is passed as the ‘--typeguard-packages=’
command line argument, no warnings or errors will be reported at all.
Typeguard will be fully silent. this this makes it seem the code is
fully correct, while in reality the code is not checked by typeguard
*at all*. Oops. 💥 🙈 😱

The problem is that the pytest plugin used the ‘pytest_sessionstart()’
hook, which triggers only after conftest.py files (or other pytest
configuration files) have been loaded already. This is too late to
install the import hook: the targeted modules may have been imported
already at this point!

This pull request fixes this problem in the following way:

- Use ‘pytest_configure()’ instead of ‘pytest_sessionstart()’, since
  the former triggers before any conftest files are loaded.

- Make it impossible to silently skip checks for packages specified on
  the command line that cannot actually be checked. In those situations,
  raise an error with all the details. Example for
  ‘--typeguard-packages=sys’:

  INTERNALERROR> RuntimeError: typeguard cannot check these packages that are already imported: sys

See also:

- https://docs.pytest.org/en/stable/reference.html#pytest.hookspec.pytest_configure
- https://docs.pytest.org/en/stable/reference.html#pytest.hookspec.pytest_sessionstart
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.5%) to 88.462% when pulling 92f46ee on wbolster:pytest-configure-hook into 461cfd5 on agronholm:master.

@agronholm
Copy link
Owner

Looks good to me. Thanks!

@agronholm agronholm merged commit 9673ad1 into agronholm:master Aug 14, 2020
@wbolster wbolster deleted the pytest-configure-hook branch August 14, 2020 17:49
@jtbeach
Copy link
Contributor

jtbeach commented Oct 20, 2020

@wbolster @agronholm Something odd is going on here for me -- I have the following defined in my root conftest.py test file (inside a tests folder):

pytest_plugins = ["blaze_util.testfixtures"]

If I run test on typeguard 2.10.1, using command line:

py.test -vvx --trace-config --junitxml=/tmp/junit-report.xml --typeguard=blaze_util tests

This triggers the typeguard import hook after blaze_util has been loaded. It seems like conftest.py files are parsed at least for plugin discovery before the import hook is called. The final error is:

INTERNALERROR> RuntimeError: typeguard cannot check these packages because they are already imported: blaze_util

@agronholm
Copy link
Owner

Can you suggest a fix?

@jtbeach
Copy link
Contributor

jtbeach commented Oct 27, 2020

I don't have a suggestion at the top of my head -- I need to go back and check on older versions that the original issue was biting me (that typeguard wasn't instrumenting the modules during the test run).

If I was getting hit by that, it's good that we detect that and fail rather than silently pass. I just hope that there is some kind of pytest plugin hook that runs before anything is imported (even earlier than pytest_configure) or it might be difficult to fix this

@AckslD
Copy link

AckslD commented Sep 9, 2021

I just stumbled on this issue as well, when I run

pytest tests --typeguard-packages=my_package

I get the error

INTERNALERROR> RuntimeError: typeguard cannot check these packages because they are already imported: my_package

but specifying a single submodule works, ie:

pytest tests --typeguard-packages=my_package.some_module

Any ideas how to run the whole package if a conftest file is used?

@wbolster
Copy link
Contributor Author

wbolster commented Sep 9, 2021

i have forgotten the nitty gritty details since i worked on this, but i ran into a similar issue in a project with a larger and more complex test suite, and i was not able to fix it

@jtbeach
Copy link
Contributor

jtbeach commented Sep 9, 2021

Yeah -- I also was unable to resolve this, I think this is a limitation of the import hook and the integration with pytest to be honest, but I do not know how to fix. I ended up pinning typeguard to 2.9.1 in my project. Whilst it might not be perfect and may silently skip some files, it still seems to work for me and gives better results than the complete failure in 2.9.2. YMMV

@AckslD
Copy link

AckslD commented Sep 14, 2021

Not importing the package directly in conftest is I guess a good solution, probably shouldn't do that anyway :)

@wbolster
Copy link
Contributor Author

Not importing the package directly in conftest is I guess a good solution, probably shouldn't do that anyway :)

otoh, conftest is a useful place to put some reusable @pytest.fixture definitions, and those often use the package…

@AckslD
Copy link

AckslD commented Sep 14, 2021

But the package import can be done inside the fixtures rather than at the top-level. Then if something goes wrong while importing you also getting better messages from pytest :)

@hotenov
Copy link

hotenov commented Nov 17, 2021

Thanks @AckslD package import inside each fixture is worked for me.

But what to do with fixture type annotations? What if I want to use specific type from my package? but I cannot import it in this case and I have to replace it with common Any type. Is it "good" way for pytest fixture or not, preserving typeguard checking?

Or it's "better" to disable typeguard and I should rely only on static analysis only (with precisely annotated pytest fixtures)

Does anyone have the experience of choosing which way to go? 🤓⛕

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

Successfully merging this pull request may close these issues.

None yet

6 participants