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

Main and 8.0.x (not released yet): cannot find fixture when using --pyargs #11816

Closed
4 tasks done
woutdenolf opened this issue Jan 14, 2024 · 15 comments · Fixed by #11825
Closed
4 tasks done

Main and 8.0.x (not released yet): cannot find fixture when using --pyargs #11816

woutdenolf opened this issue Jan 14, 2024 · 15 comments · Fixed by #11825
Milestone

Comments

@woutdenolf
Copy link
Contributor

woutdenolf commented Jan 14, 2024

  • 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

While adding a comment to #9765 (comment) to reproduce that issue, I noticed another problem (not released yet, only the main and 8.0.x branches).

Reproduce the issue

python -m pip install "ewokscore[test]==0.7.1" git+https://github.com/pytest-dev/pytest.git@main
python -m pytest -v --pyargs ewokscore.tests

or also

python -m pip install "ewokscore[test]==0.7.1" git+https://github.com/pytest-dev/pytest.git@8.0.x
python -m pytest -v --pyargs ewokscore.tests

Released pytest versions do not show the problem. For example this works find:

python -m pip install "ewokscore[test]==0.7.1" "pytest==7.4.4"
python -m pytest -v --pyargs ewokscore.tests

Os and python version

  • Ubuntu 20.04, python 3.10.12: works
  • Windows 11, python 3.8.10: fails

Error message

The error says it cannot find the varinfo dependency which is defined in the module ewokscore.tests.conftest.

  @pytest.mark.parametrize("value", VALUES)
  def test_variable_references(value, varinfo):
E       fixture 'varinfo' not found
>       available fixtures: cache, capfd, capfdbinary, caplog, capsys, capsysbinary, doctest_namespace, monkeypatch, pytestconfig, record_property, record_testsuite_property, record_xml_attribute, recwarn, tmp_path, tmp_path_factory, tmpdir, tmpdir_factory
>       use 'pytest --fixtures [testpath]' for help on them.
@lesteve
Copy link
Contributor

lesteve commented Jan 15, 2024

Weird I don't get this error on Windows (Windows 10 but I doubt this matters), Python 3.8 and either main or 8.0.x ...

Could you try again, just to make sure it has not been fixed in main in the mean time?

It seems very similar to something that was seen in jdaviz, although that was a few days ago, in particular before #11708 was merged and that was using a custom pytest debug branch. There is a bit more context in spacetelescope/jdaviz#2654.

We were at the point that --trace-config showed that the conftest was loaded but for some unknown reason, the fixtures were not available. Switching to Python 3.11 was the work-around jdaviz went for, but of course, this may not be a reasonable work-around for your particular use case.

@woutdenolf
Copy link
Contributor Author

woutdenolf commented Jan 15, 2024

I just tried again and the error fixture 'varinfo' not found still occurs. Maybe the location of python and the virtual environment matter? Here is the full script I use to reproduce the error:

SET PYTHON_VER=3.8
SET "PATH=C:\\python\\%PYTHON_VER%;C:\\python\\%PYTHON_VER%\\Scripts;%PATH%"
SET "PATH_VENV=C:\\Users\\denolf\\virtualenvs\\testenv\\py%PYTHON_VER%\\"

rm -rf %PATH_VENV%
python -m venv --clear %PATH_VENV%

call  "%PATH_VENV%\\Scripts\\activate.bat"
python -m pip install --upgrade pip
python -m pip install --upgrade setuptools

python -m pip install "ewokscore[test]==0.7.1" git+https://github.com/pytest-dev/pytest.git@8.0.x --no-cache
python -m pytest -v --pyargs ewokscore.tests

@woutdenolf
Copy link
Contributor Author

woutdenolf commented Jan 15, 2024

We were at the point that --trace-config showed that the conftest was loaded but for some unknown reason, the fixtures were not available

This seems indeed the case for me as well:

PLUGIN registered: <_pytest.config.PytestPluginManager object at 0x0000020437C4D9A0>
PLUGIN registered: <_pytest.config.Config object at 0x000002043A483EE0>
PLUGIN registered: <module '_pytest.mark' from 'C:\\Users\\denolf\\virtualenvs\\testenv\\py3.8\\lib\\site-packages\\_pytest\\mark\\__init__.py'>
PLUGIN registered: <module '_pytest.main' from 'C:\\Users\\denolf\\virtualenvs\\testenv\\py3.8\\lib\\site-packages\\_pytest\\main.py'>
PLUGIN registered: <module '_pytest.runner' from 'C:\\Users\\denolf\\virtualenvs\\testenv\\py3.8\\lib\\site-packages\\_pytest\\runner.py'>
PLUGIN registered: <module '_pytest.fixtures' from 'C:\\Users\\denolf\\virtualenvs\\testenv\\py3.8\\lib\\site-packages\\_pytest\\fixtures.py'>
PLUGIN registered: <module '_pytest.helpconfig' from 'C:\\Users\\denolf\\virtualenvs\\testenv\\py3.8\\lib\\site-packages\\_pytest\\helpconfig.py'>
PLUGIN registered: <module '_pytest.python' from 'C:\\Users\\denolf\\virtualenvs\\testenv\\py3.8\\lib\\site-packages\\_pytest\\python.py'>
PLUGIN registered: <module '_pytest.terminal' from 'C:\\Users\\denolf\\virtualenvs\\testenv\\py3.8\\lib\\site-packages\\_pytest\\terminal.py'>
PLUGIN registered: <module '_pytest.debugging' from 'C:\\Users\\denolf\\virtualenvs\\testenv\\py3.8\\lib\\site-packages\\_pytest\\debugging.py'>
PLUGIN registered: <module '_pytest.unittest' from 'C:\\Users\\denolf\\virtualenvs\\testenv\\py3.8\\lib\\site-packages\\_pytest\\unittest.py'>
PLUGIN registered: <module '_pytest.capture' from 'C:\\Users\\denolf\\virtualenvs\\testenv\\py3.8\\lib\\site-packages\\_pytest\\capture.py'>
PLUGIN registered: <module '_pytest.skipping' from 'C:\\Users\\denolf\\virtualenvs\\testenv\\py3.8\\lib\\site-packages\\_pytest\\skipping.py'>
PLUGIN registered: <module '_pytest.legacypath' from 'C:\\Users\\denolf\\virtualenvs\\testenv\\py3.8\\lib\\site-packages\\_pytest\\legacypath.py'>
PLUGIN registered: <module '_pytest.tmpdir' from 'C:\\Users\\denolf\\virtualenvs\\testenv\\py3.8\\lib\\site-packages\\_pytest\\tmpdir.py'>
PLUGIN registered: <module '_pytest.monkeypatch' from 'C:\\Users\\denolf\\virtualenvs\\testenv\\py3.8\\lib\\site-packages\\_pytest\\monkeypatch.py'>
PLUGIN registered: <module '_pytest.recwarn' from 'C:\\Users\\denolf\\virtualenvs\\testenv\\py3.8\\lib\\site-packages\\_pytest\\recwarn.py'>
PLUGIN registered: <module '_pytest.pastebin' from 'C:\\Users\\denolf\\virtualenvs\\testenv\\py3.8\\lib\\site-packages\\_pytest\\pastebin.py'>
PLUGIN registered: <module '_pytest.nose' from 'C:\\Users\\denolf\\virtualenvs\\testenv\\py3.8\\lib\\site-packages\\_pytest\\nose.py'>
PLUGIN registered: <module '_pytest.assertion' from 'C:\\Users\\denolf\\virtualenvs\\testenv\\py3.8\\lib\\site-packages\\_pytest\\assertion\\__init__.py'>
PLUGIN registered: <module '_pytest.junitxml' from 'C:\\Users\\denolf\\virtualenvs\\testenv\\py3.8\\lib\\site-packages\\_pytest\\junitxml.py'>
PLUGIN registered: <module '_pytest.doctest' from 'C:\\Users\\denolf\\virtualenvs\\testenv\\py3.8\\lib\\site-packages\\_pytest\\doctest.py'>
PLUGIN registered: <module '_pytest.cacheprovider' from 'C:\\Users\\denolf\\virtualenvs\\testenv\\py3.8\\lib\\site-packages\\_pytest\\cacheprovider.py'>
PLUGIN registered: <module '_pytest.freeze_support' from 'C:\\Users\\denolf\\virtualenvs\\testenv\\py3.8\\lib\\site-packages\\_pytest\\freeze_support.py'>
PLUGIN registered: <module '_pytest.setuponly' from 'C:\\Users\\denolf\\virtualenvs\\testenv\\py3.8\\lib\\site-packages\\_pytest\\setuponly.py'>
PLUGIN registered: <module '_pytest.setupplan' from 'C:\\Users\\denolf\\virtualenvs\\testenv\\py3.8\\lib\\site-packages\\_pytest\\setupplan.py'>
PLUGIN registered: <module '_pytest.stepwise' from 'C:\\Users\\denolf\\virtualenvs\\testenv\\py3.8\\lib\\site-packages\\_pytest\\stepwise.py'>
PLUGIN registered: <module '_pytest.warnings' from 'C:\\Users\\denolf\\virtualenvs\\testenv\\py3.8\\lib\\site-packages\\_pytest\\warnings.py'>
PLUGIN registered: <module '_pytest.logging' from 'C:\\Users\\denolf\\virtualenvs\\testenv\\py3.8\\lib\\site-packages\\_pytest\\logging.py'>
PLUGIN registered: <module '_pytest.reports' from 'C:\\Users\\denolf\\virtualenvs\\testenv\\py3.8\\lib\\site-packages\\_pytest\\reports.py'>
PLUGIN registered: <module '_pytest.python_path' from 'C:\\Users\\denolf\\virtualenvs\\testenv\\py3.8\\lib\\site-packages\\_pytest\\python_path.py'>
PLUGIN registered: <module '_pytest.unraisableexception' from 'C:\\Users\\denolf\\virtualenvs\\testenv\\py3.8\\lib\\site-packages\\_pytest\\unraisableexception.py'>
PLUGIN registered: <module '_pytest.threadexception' from 'C:\\Users\\denolf\\virtualenvs\\testenv\\py3.8\\lib\\site-packages\\_pytest\\threadexception.py'>
PLUGIN registered: <module '_pytest.faulthandler' from 'C:\\Users\\denolf\\virtualenvs\\testenv\\py3.8\\lib\\site-packages\\_pytest\\faulthandler.py'>
PLUGIN registered: <CaptureManager _method='fd' _global_capturing=<MultiCapture out=<FDCapture 1 oldfd=5 _state='suspended' tmpfile=<_io.TextIOWrapper name='<tempfile._TemporaryFileWrapper object at 0x000002043A5418E0>' mode='r+' encoding='utf-8'>> err=<FDCapture 2 oldfd=7 _state='suspended' tmpfile=<_io.TextIOWrapper name='<tempfile._TemporaryFileWrapper object at 0x000002043A541760>' mode='r+' encoding='utf-8'>> in_=<FDCapture 0 oldfd=3 _state='started' tmpfile=<_io.TextIOWrapper name='nul' mode='r' encoding='utf-8'>> _state='suspended' _in_suspended=False> _capture_fixture=None>
PLUGIN registered: <Session  exitstatus=<ExitCode.OK: 0> testsfailed=0 testscollected=0>
PLUGIN registered: <_pytest.cacheprovider.LFPlugin object at 0x000002043A55C910>
PLUGIN registered: <_pytest.cacheprovider.NFPlugin object at 0x000002043A55CB80>
PLUGIN registered: <class '_pytest.legacypath.LegacyTmpdirPlugin'>
PLUGIN registered: <_pytest.terminal.TerminalReporter object at 0x000002043A55C310>
PLUGIN registered: <_pytest.logging.LoggingPlugin object at 0x000002043A575820>
PLUGIN registered: <_pytest.fixtures.FixtureManager object at 0x000002043A4B7220>
============================= test session starts =============================
platform win32 -- Python 3.8.10, pytest-8.0.0rc2.dev11+gb0c7f923a, pluggy-1.3.0 -- C:\Users\denolf\virtualenvs\testenv\py3.8\Scripts\python.exe
using: pytest-8.0.0rc2.dev11+gb0c7f923a
active plugins:
    2217138772384       : <_pytest.config.PytestPluginManager object at 0x0000020437C4D9A0>
    pytestconfig        : <_pytest.config.Config object at 0x000002043A483EE0>
    mark                : C:\Users\denolf\virtualenvs\testenv\py3.8\lib\site-packages\_pytest\mark\__init__.py
    main                : C:\Users\denolf\virtualenvs\testenv\py3.8\lib\site-packages\_pytest\main.py
    runner              : C:\Users\denolf\virtualenvs\testenv\py3.8\lib\site-packages\_pytest\runner.py
    fixtures            : C:\Users\denolf\virtualenvs\testenv\py3.8\lib\site-packages\_pytest\fixtures.py
    helpconfig          : C:\Users\denolf\virtualenvs\testenv\py3.8\lib\site-packages\_pytest\helpconfig.py
    python              : C:\Users\denolf\virtualenvs\testenv\py3.8\lib\site-packages\_pytest\python.py
    terminal            : C:\Users\denolf\virtualenvs\testenv\py3.8\lib\site-packages\_pytest\terminal.py
    debugging           : C:\Users\denolf\virtualenvs\testenv\py3.8\lib\site-packages\_pytest\debugging.py
    unittest            : C:\Users\denolf\virtualenvs\testenv\py3.8\lib\site-packages\_pytest\unittest.py
    capture             : C:\Users\denolf\virtualenvs\testenv\py3.8\lib\site-packages\_pytest\capture.py
    skipping            : C:\Users\denolf\virtualenvs\testenv\py3.8\lib\site-packages\_pytest\skipping.py
    legacypath          : C:\Users\denolf\virtualenvs\testenv\py3.8\lib\site-packages\_pytest\legacypath.py
    tmpdir              : C:\Users\denolf\virtualenvs\testenv\py3.8\lib\site-packages\_pytest\tmpdir.py
    monkeypatch         : C:\Users\denolf\virtualenvs\testenv\py3.8\lib\site-packages\_pytest\monkeypatch.py
    recwarn             : C:\Users\denolf\virtualenvs\testenv\py3.8\lib\site-packages\_pytest\recwarn.py
    pastebin            : C:\Users\denolf\virtualenvs\testenv\py3.8\lib\site-packages\_pytest\pastebin.py
    nose                : C:\Users\denolf\virtualenvs\testenv\py3.8\lib\site-packages\_pytest\nose.py
    assertion           : C:\Users\denolf\virtualenvs\testenv\py3.8\lib\site-packages\_pytest\assertion\__init__.py
    junitxml            : C:\Users\denolf\virtualenvs\testenv\py3.8\lib\site-packages\_pytest\junitxml.py
    doctest             : C:\Users\denolf\virtualenvs\testenv\py3.8\lib\site-packages\_pytest\doctest.py
    cacheprovider       : C:\Users\denolf\virtualenvs\testenv\py3.8\lib\site-packages\_pytest\cacheprovider.py
    freeze_support      : C:\Users\denolf\virtualenvs\testenv\py3.8\lib\site-packages\_pytest\freeze_support.py
    setuponly           : C:\Users\denolf\virtualenvs\testenv\py3.8\lib\site-packages\_pytest\setuponly.py
    setupplan           : C:\Users\denolf\virtualenvs\testenv\py3.8\lib\site-packages\_pytest\setupplan.py
    stepwise            : C:\Users\denolf\virtualenvs\testenv\py3.8\lib\site-packages\_pytest\stepwise.py
    warnings            : C:\Users\denolf\virtualenvs\testenv\py3.8\lib\site-packages\_pytest\warnings.py
    logging             : C:\Users\denolf\virtualenvs\testenv\py3.8\lib\site-packages\_pytest\logging.py
    reports             : C:\Users\denolf\virtualenvs\testenv\py3.8\lib\site-packages\_pytest\reports.py
    python_path         : C:\Users\denolf\virtualenvs\testenv\py3.8\lib\site-packages\_pytest\python_path.py
    unraisableexception : C:\Users\denolf\virtualenvs\testenv\py3.8\lib\site-packages\_pytest\unraisableexception.py
    threadexception     : C:\Users\denolf\virtualenvs\testenv\py3.8\lib\site-packages\_pytest\threadexception.py
    faulthandler        : C:\Users\denolf\virtualenvs\testenv\py3.8\lib\site-packages\_pytest\faulthandler.py
    capturemanager      : <CaptureManager _method='fd' _global_capturing=<MultiCapture out=<FDCapture 1 oldfd=5 _state='suspended' tmpfile=<_io.TextIOWrapper name='<tempfile._TemporaryFileWrapper object at 0x000002043A5418E0>' mode='r+' encoding='utf-8'>> err=<FDCapture 2 oldfd=7 _state='suspended' tmpfile=<_io.TextIOWrapper name='<tempfile._TemporaryFileWrapper object at 0x000002043A541760>' mode='r+' encoding='utf-8'>> in_=<FDCapture 0 oldfd=3 _state='started' tmpfile=<_io.TextIOWrapper name='nul' mode='r' encoding='utf-8'>> _state='suspended' _in_suspended=False> _capture_fixture=None>
    session             : <Session  exitstatus=<ExitCode.OK: 0> testsfailed=0 testscollected=0>
    lfplugin            : <_pytest.cacheprovider.LFPlugin object at 0x000002043A55C910>
    nfplugin            : <_pytest.cacheprovider.NFPlugin object at 0x000002043A55CB80>
    legacypath-tmpdir   : <class '_pytest.legacypath.LegacyTmpdirPlugin'>
    terminalreporter    : <_pytest.terminal.TerminalReporter object at 0x000002043A55C310>
    logging-plugin      : <_pytest.logging.LoggingPlugin object at 0x000002043A575820>
    funcmanage          : <_pytest.fixtures.FixtureManager object at 0x000002043A4B7220>
cachedir: .pytest_cache
rootdir: C:\Users\denolf
collecting ... PLUGIN registered: <module 'ewokscore.tests.conftest' from 'C:\\Users\\denolf\\virtualenvs\\testenv\\py3.8\\lib\\site-packages\\ewokscore\\tests\\conftest.py'>
collected 180 items

...


_____________________ ERROR at setup of test_method_task1 _____________________
file C:\Users\denolf\virtualenvs\testenv\py3.8\lib\site-packages\ewokscore\tests\test_method_task.py, line 9
  def test_method_task1(varinfo):
E       fixture 'varinfo' not found
>       available fixtures: cache, capfd, capfdbinary, caplog, capsys, capsysbinary, doctest_namespace, monkeypatch, pytestconfig, record_property, record_testsuite_property, record_xml_attribute, recwarn, tmp_path, tmp_path_factory, tmpdir, tmpdir_factory
>       use 'pytest --fixtures [testpath]' for help on them.

C:\Users\denolf\virtualenvs\testenv\py3.8\lib\site-packages\ewokscore\tests\test_method_task.py:9

So ewokscore.tests.conftest is registered (contains the varinfo fixture) but the fixture is not available.

So #11708 solved #9765 (assertion assert mod not in mods failed) which also occured in the use case I describe in this issue. I guess there are other places in the code that need the fix.

I'm guess all module.__file__ occurances are problematic. I believe getting the canonical path would solve the issues

In this particular case

from ewokscore.tests import conftest

conftest.__file__
'C:\\Users\\denolf\\virtualenvs\\testenv\\py3.8\\lib\\site-packages\\ewokscore\\tests\\conftest.py'

os.path.realpath(conftest.__file__)
'C:\\Users\\denolf\\virtualenvs\\testenv\\py3.8\\Lib\\site-packages\\ewokscore\\tests\\conftest.py'

@woutdenolf
Copy link
Contributor Author

I guess to reproduce this, you need two capitalized directory names? In my case

C:\\Users\\denolf\\virtualenvs\\testenv\\py3.8\\Lib\\site-packages\\ewokscore\\tests\\conftest.py

@lesteve Can you try in a virtual environment that starts with a capital letter? (for example python -m venv Myenv).

Also to be sure pip install git+https://github.com/pytest-dev/pytest.git@b0c7f923aa39f0765425c367a02f53b1d97915f6.

@woutdenolf woutdenolf changed the title Main and 8.0.x (not release): cannot find fixture when using --pyargs Main and 8.0.x (not released yet): cannot find fixture when using --pyargs Jan 15, 2024
@lesteve
Copy link
Contributor

lesteve commented Jan 15, 2024

I actually managed to reproduce your issue inside a Git bash console with your original instructions in your top post. Previously I was using a MSYS2 console which was not showing the issue for some unknown reason.

Quickly looking at your PR, you seem to be saying that there are still some places in Pytest code, where use of __file__ is problematic on Windows. This could well be the case ... I guess a Pytest maintainer would need to get involved, maybe @bluetech?

I would guess that since this issue was seen in jdaviz and your project ewokscore, this is likely something that a number of other projects will encounter.

@woutdenolf
Copy link
Contributor Author

woutdenolf commented Jan 15, 2024

Most of the project I know have this problem in fact. For example h5py/h5py#2364 and silx-kit/silx#4043.

Edit: well it doesn't really depend on the project, it depends on the module file paths.

@woutdenolf
Copy link
Contributor Author

woutdenolf commented Jan 15, 2024

Quickly looking at your PR, you seem to be saying that there are still some places in Pytest code, where use of __file__ is problematic on Windows.

Indeed. I basically replace all anymodule.__file__ occurrences with module_casesensitivepath(anymodule) which deals the casing.

@lesteve
Copy link
Contributor

lesteve commented Jan 16, 2024

I can reproduce the issue in the h5py CI: h5py/h5py#2367. I think this is a nice public way to show indeed that there is an issue, and also a quick and dirty way to double-check that this issue is fixed (in the future). You can see in this build log that all Windows builds fail with the same symptoms (fixtures are not defined).

As I said above, my guess is that this will likely affect a number of projects. @bluetech ideally it would be great to fix this before cutting a 8.0.0rc2 release.

@woutdenolf you probably have a good understanding of the issue, maybe it could help if you could do a quick summary. For now (sorry I did not have time to look into it closely), I get the vibe __file__ should not be used without normalization, but it would be great that you explain why in a bit more details.

For example in #9765, the issue was that the code expected that conftest_module.__file__ and str(contestpath) would match, which was not necessarily the case. Is there something similar in this issue?

@bluetech
Copy link
Member

The issue is here:

pytest/src/_pytest/fixtures.py

Lines 1486 to 1506 in 348e6de

def pytest_plugin_registered(self, plugin: _PluggyPlugin) -> None:
nodeid = None
try:
p = absolutepath(plugin.__file__) # type: ignore[attr-defined]
except AttributeError:
pass
else:
# Construct the base nodeid which is later used to check
# what fixtures are visible for particular tests (as denoted
# by their test id).
if p.name == "conftest.py":
try:
nodeid = str(p.parent.relative_to(self.config.rootpath))
except ValueError:
nodeid = ""
if nodeid == ".":
nodeid = ""
if os.sep != nodes.SEP:
nodeid = nodeid.replace(os.sep, nodes.SEP)
self.parsefactories(plugin, nodeid)

A bit dense technical explanation, hopefully you manage to follow :)

When a conftest plugin (and any other plugin) is registered, pytest calls the pytest_plugin_registered hook. The fixtures code implements this hook to register the fixtures defined in the plugin.

When you register a fixture, you need to define its "visibility" AKA baseid. The baseid is a nodeid, and a test can only "see" a fixture if the baseid is the nodeid of one of its parents (see matchfactories function).

When a fixture is defined in e.g. some test file, the visibility is the specific node it's defined in, e.g. the Module node (run pytest --collect-only to see the collection tree).

When a fixture is defined in a conftest plugin (the relevant case for this issue), the visibility logically is supposed to be the conftest's directory. In practice this is done in the code above -- the baseid (the nodeid variable in the code above) is synthesized from the plugin.__file__.

As we've already learned from #9765, relying on both __file__ and user-input paths is problematic. @woutdenolf solution in PR #11821 is to go back to relying on __file__ for registration and normalizing it more. But in #9765 the solution we've decided on is the opposite, that is, avoid relying on __file__ as much as possible.

Therefore, the PR I would like to see instead is to change the fixtures.py pytest_plugin_registered hook impl to not rely on plugin.__file__. Some ways I can think to do this:

  • Instead of using plugin.__file__, use the registration name, which for conftest plugins is the original conftest path as we registered it. The hookimpl can do plugin_name = manager.get_name(plugin) then use that instead of __file__.

  • Internally in pluggy (pytest's plugin system), get_name(plugin) is currently a linear search, which might get somewhat slow with many plugins. Maybe we don't care about it, but if we do, can do one of these instead:

    • Make the lookup faster in pluggy
    • Add a new plugin_name parameter to the pytest_plugin_registered hook.
  • Longer term, I really think we should tie conftests to Directory nodes (new in pytest 8.0), this is something I'm working toward but it's speculative and not applicable for the short term.

@woutdenolf @lesteve It'd be great if one of you could try this and see if it indeed fixes the issue.

@bluetech bluetech added this to the 8.0 milestone Jan 16, 2024
@woutdenolf
Copy link
Contributor Author

woutdenolf commented Jan 16, 2024

@woutdenolf you probably have a good understanding of the issue, maybe it could help if you could do a quick summary. For now (sorry I did not have time to look into it closely), I get the vibe __file__ should not be used without normalization, but it would be great that you explain why in a bit more details.

The core issue is that anymodule.__file__ on a case-insensitive filesystem like Windows does not have the correct casing. So we need to avoid using __file__.

Of course we could mix both solutions. I'm just worried that PytestPluginManager.consider_conftest has no control over whether the filename (called register_name) is in fact correct or not. Also consider_conftest is a public method so this can be called by anyone.

Are there other cases than anymodule.__file__ where the file path casing is messed up? No idea. But at least I found that fixing anymodule.__file__ solves #11816 (this issue fixture '...' not found) and #9765 (the assert mod not in mods issue) without the need for #11708.

@woutdenolf
Copy link
Contributor Author

woutdenolf commented Jan 16, 2024

I can reproduce the issue in the h5py CI: h5py/h5py#2367. I think this is a nice public way to show indeed that there is an issue, and also a quick and dirty way to double-check that this issue is fixed (in the future). You can see in this build log that all Windows builds fail with the same symptoms (fixtures are not defined).

@bluetech
Copy link
Member

Of course we could mix both solutions. I'm just worried that PytestPluginManager.consider_conftest has no control over whether the filename (called register_name) is in fact correct or not. Also consider_conftest is a public method so this can be called by anyone.

The method is not public, in fact we just broke it by adding registration_name :)

We know that normalization can work but it brings its own risks. So in the previous issue we've decided to avoid __file__ as much as possible. We just need to do the same for the conftest fixture loading bit.

@woutdenolf
Copy link
Contributor Author

woutdenolf commented Jan 16, 2024

Instead of using plugin.file, use the registration name, which for conftest plugins is the original conftest path as we registered it. The hookimpl can do plugin_name = manager.get_name(plugin) then use that instead of file.

I tried that suggestion

@woutdenolf
Copy link
Contributor Author

@bluetech So two potential fixes are

Both solve my use cases so whatever you want.

@lesteve
Copy link
Contributor

lesteve commented Jan 17, 2024

Nice to see this one closed, thanks a lot @woutdenolf and @bluetech!

I double-checked just to be sure in h5py/h5py#2367, and the problem that h5py (and other projects like jdaviz and ewokscore) was seeing is now gone from Pytest main branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment