Skip to content

Commit

Permalink
Merge pull request #11708 from fcharras/FIX/crash_during_conftest_col…
Browse files Browse the repository at this point in the history
…lection

FIX key formating divergence when inspecting plugin dictionary.
  • Loading branch information
bluetech committed Jan 13, 2024
2 parents d65bcd9 + a7c2549 commit e403bbf
Show file tree
Hide file tree
Showing 4 changed files with 102 additions and 5 deletions.
3 changes: 3 additions & 0 deletions changelog/9765.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Fixed a frustrating bug that afflicted some users with the only error being ``assert mod not in mods``. The issue was caused by the fact that ``str(Path(mod))`` and ``mod.__file__`` don't necessarily produce the same string, and was being erroneously used interchangably in some places in the code.

This fix also broke the internal API of ``PytestPluginManager.consider_conftest`` by introducing a new parameter -- we mention this in case it is being used by external code, even if marked as *private*.
11 changes: 7 additions & 4 deletions src/_pytest/config/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -634,7 +634,8 @@ def _rget_with_confmod(
def _importconftest(
self, conftestpath: Path, importmode: Union[str, ImportMode], rootpath: Path
) -> types.ModuleType:
existing = self.get_plugin(str(conftestpath))
conftestpath_plugin_name = str(conftestpath)
existing = self.get_plugin(conftestpath_plugin_name)
if existing is not None:
return cast(types.ModuleType, existing)

Expand Down Expand Up @@ -666,7 +667,7 @@ def _importconftest(
assert mod not in mods
mods.append(mod)
self.trace(f"loading conftestmodule {mod!r}")
self.consider_conftest(mod)
self.consider_conftest(mod, registration_name=conftestpath_plugin_name)
return mod

def _check_non_top_pytest_plugins(
Expand Down Expand Up @@ -746,9 +747,11 @@ def consider_pluginarg(self, arg: str) -> None:
del self._name2plugin["pytest_" + name]
self.import_plugin(arg, consider_entry_points=True)

def consider_conftest(self, conftestmodule: types.ModuleType) -> None:
def consider_conftest(
self, conftestmodule: types.ModuleType, registration_name: str
) -> None:
""":meta private:"""
self.register(conftestmodule, name=conftestmodule.__file__)
self.register(conftestmodule, name=registration_name)

def consider_env(self) -> None:
""":meta private:"""
Expand Down
59 changes: 59 additions & 0 deletions testing/acceptance_test.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import dataclasses
import importlib.metadata
import os
import subprocess
import sys
import types

Expand Down Expand Up @@ -1390,3 +1391,61 @@ def test_boo(self):
)
result = pytester.runpytest_subprocess()
result.stdout.fnmatch_lines("*1 passed*")


@pytest.mark.skip(reason="Test is not isolated")
def test_issue_9765(pytester: Pytester) -> None:
"""Reproducer for issue #9765 on Windows
https://github.com/pytest-dev/pytest/issues/9765
"""
pytester.makepyprojecttoml(
"""
[tool.pytest.ini_options]
addopts = "-p my_package.plugin.my_plugin"
"""
)
pytester.makepyfile(
**{
"setup.py": (
"""
from setuptools import setup
if __name__ == '__main__':
setup(name='my_package', packages=['my_package', 'my_package.plugin'])
"""
),
"my_package/__init__.py": "",
"my_package/conftest.py": "",
"my_package/test_foo.py": "def test(): pass",
"my_package/plugin/__init__.py": "",
"my_package/plugin/my_plugin.py": (
"""
import pytest
def pytest_configure(config):
class SimplePlugin:
@pytest.fixture(params=[1, 2, 3])
def my_fixture(self, request):
yield request.param
config.pluginmanager.register(SimplePlugin())
"""
),
}
)

subprocess.run([sys.executable, "setup.py", "develop"], check=True)
try:
# We are using subprocess.run rather than pytester.run on purpose.
# pytester.run is adding the current directory to PYTHONPATH which avoids
# the bug. We also use pytest rather than python -m pytest for the same
# PYTHONPATH reason.
subprocess.run(
["pytest", "my_package"], capture_output=True, check=True, text=True
)
except subprocess.CalledProcessError as exc:
raise AssertionError(
f"pytest command failed:\n{exc.stdout=!s}\n{exc.stderr=!s}"
) from exc
34 changes: 33 additions & 1 deletion testing/test_pluginmanager.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,38 @@ def pytest_configure(self):
config.pluginmanager.register(A())
assert len(values) == 2

@pytest.mark.skipif(
not sys.platform.startswith("win"),
reason="requires a case-insensitive file system",
)
def test_conftestpath_case_sensitivity(self, pytester: Pytester) -> None:
"""Unit test for issue #9765."""
config = pytester.parseconfig()
pytester.makepyfile(**{"tests/conftest.py": ""})

conftest = pytester.path.joinpath("tests/conftest.py")
conftest_upper_case = pytester.path.joinpath("TESTS/conftest.py")

mod = config.pluginmanager._importconftest(
conftest,
importmode="prepend",
rootpath=pytester.path,
)
plugin = config.pluginmanager.get_plugin(str(conftest))
assert plugin is mod

mod_uppercase = config.pluginmanager._importconftest(
conftest_upper_case,
importmode="prepend",
rootpath=pytester.path,
)
plugin_uppercase = config.pluginmanager.get_plugin(str(conftest_upper_case))
assert plugin_uppercase is mod_uppercase

# No str(conftestpath) normalization so conftest should be imported
# twice and modules should be different objects
assert mod is not mod_uppercase

def test_hook_tracing(self, _config_for_test: Config) -> None:
pytestpm = _config_for_test.pluginmanager # fully initialized with plugins
saveindent = []
Expand Down Expand Up @@ -368,7 +400,7 @@ def test_consider_conftest_deps(
pytester.makepyfile("pytest_plugins='xyz'"), root=pytester.path
)
with pytest.raises(ImportError):
pytestpm.consider_conftest(mod)
pytestpm.consider_conftest(mod, registration_name="unused")


class TestPytestPluginManagerBootstrapming:
Expand Down

0 comments on commit e403bbf

Please sign in to comment.