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

Should warn when using pytest_plugins in a test file (since it's also not the root conftest.py) #13030

Open
4 tasks done
ryangalamb opened this issue Dec 5, 2024 · 4 comments
Labels
type: deprecation feature that will be removed in the future

Comments

@ryangalamb
Copy link

  • a detailed description of the bug or problem you are having
  • output of pip list from the virtual environment you are using
  • pytest and operating system versions
  • minimal example if possible

Description

If I load a plugin using pytest_plugins in a test file, pytest should issue a warning similar to the warning given when using pytest_plugins in a non-root conftest.py.

I'm opening this as a bug because this seems unintentional. When pytest started discouraging pytest_plugins in non-root conftest.py, it should have been discouraged in test files too. If folks would prefer this as a feature request, I'm happy to change it.

Why this is a problem

The same issues with plugins in non-root conftest.py : The plugin gets activated globally for the rest of the test run, even though it seems like it'd only be activated for that one test file. (#3625 and #2062 have some discussion on pitfalls of pytest_plugins outside root conftest.py)

Moreover, since there's now a warning/error when using pytest_plugins in non-root conftest.py, the lack of a warning when using pytest_plugins in test files implies that using pytest_plugins in test files doesn't have the same problems.

Of course, the problems still apply to test files. From my experiments (using pytest 8.3.4):

  • plugin fixtures are available in all test files (even if requested before running the test file with pytest_plugins)
  • autouse fixtures are used in all tests that execute after the test file with pytest_plugins executes
  • hooks get registered/run in all test files (including the ones run before the file with pytest_plugins)

And then if the file with pytest_plugins is omitted from the test run (e.g., by selecting a specific file), then the plugin is not loaded at all. (This behavior is reasonable/good, but it can cause surprising/confusing test failures when combined with plugins in test files.)

Minimal example

Expand for code snippets

# content of tests/some_plugin.py

import pytest

@pytest.fixture()
def some_fixture():
    return "some value"
# content of tests/test_1_before_plugin.py

def test_some_fixture(some_fixture):
    assert some_fixture == "some value"
# content of tests/test_2_load_plugin.py

pytest_plugins = ["tests.some_plugin"]

def test_some_fixture(some_fixture):
    assert some_fixture == "some value"
# content of tests/test_3_after_plugin.py

def test_some_fixture(some_fixture):
    assert some_fixture == "some value"
$ pytest tests

All tests will pass, and no warning is emitted. (This template can be tweaked to experiment with the other behaviors I described.)

Other info

  • pytest: 8.3.4
  • OS: NixOS
$ pip list
Package                Version Editable project location
---------------------- ------- ----------------------------------------------------------
attrs                  24.2.0
fancycompleter         0.9.1
iniconfig              2.0.0
packaging              24.2
pdbpp                  0.10.3
pip                    23.2.1
pluggy                 1.5.0
Pygments               2.18.0
pyrepl                 0.9.0
pytest                 8.3.4
pytest-fixture-scoping 0.1.0   [...]/bug-repros/pytest-fixture-scoping
setuptools             68.2.0
wmctrl                 0.5
@dongfangtianyu
Copy link
Contributor

So no matter where it is written, activated globally?

consider removing the previous warning and displaying it in the 'plugin' of the terminal ? (Just an idea)

@The-Compiler
Copy link
Member

I'm concerned this will cause a lot of churn for simple single-file pytest plugins, where you have a someplugin.py and a test_someplugin.py which does pytest_plugins = ["pytester"].

From a quick look at a few smaller pytest-dev plugins:

This kind of thing seems quite common in general:

So if such a warning is introduced, I think it...

  • At the very minimum needs to be suppressed if there's only a single test file.
  • Or alternatively, it only gets emitted if a plugin was loaded from pytest_plugins from another test_*.py but it is not contained in the current test_*.py in pytest_plugins.

@ryangalamb
Copy link
Author

this will cause a lot of churn for simple single-file pytest plugins, where you have a someplugin.py and a test_someplugin.py which does pytest_plugins = ["pytester"].

That's a fair concern. I like your idea to suppress the warning when there's only a single test file.

There could still be issues when the plugin uses hooks that have already been run. (e.g., pytest_addoption) IMO, that behavior is documented and nowhere near as surprising as the global effects of pytest_plugins in a test file, so it's probably not a big deal.

This kind of thing seems quite common in general

Yeah, that's what I was afraid of. Up until this week, pytest_plugins in test files has been my preferred way to share fixtures 🙈 Since pytest_plugins is a good way to split up the root conftest.py, using pytest_plugins in test files seemed like a good way to explicitly reuse fixtures between tests.

Overall, emitting a warning if pytest_plugins is used in a test file that's not the only test file sounds like a good way to communicate the surprising behavior.


But while we're talking about this: What if we made pytest_plugins in a test file only be active for that test file?

I'm working on a proof of concept to see if it's viable. So far it seems doable, but tricky. It'd be a breaking change. But hopefully no one is intentionally relying on test files pytest_plugins being global.

If that seems worth exploring, I'm happy to keep working on my proof of concept and share it.

@RonnyPfannschmidt
Copy link
Member

I think it's fair to warn in any case with more than one test file

I think it's also fair to eventually deprecated it altogether

@Zac-HD Zac-HD added the type: deprecation feature that will be removed in the future label Dec 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: deprecation feature that will be removed in the future
Projects
None yet
Development

No branches or pull requests

5 participants