Fix `assert mod not in mods` crash
Fix #27806. Co-authored-by: Loïc Estève <loic.esteve@ymail.com> Co-authored-by: Ran Benita <ran@unusedvar.com> Co-authored-by: Bruno Oliveira <nicoddemus@gmail.com>
This commit is contained in:
		
							parent
							
								
									d65bcd9a3b
								
							
						
					
					
						commit
						a7c2549321
					
				| 
						 | 
					@ -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*.
 | 
				
			||||||
| 
						 | 
					@ -634,7 +634,8 @@ class PytestPluginManager(PluginManager):
 | 
				
			||||||
    def _importconftest(
 | 
					    def _importconftest(
 | 
				
			||||||
        self, conftestpath: Path, importmode: Union[str, ImportMode], rootpath: Path
 | 
					        self, conftestpath: Path, importmode: Union[str, ImportMode], rootpath: Path
 | 
				
			||||||
    ) -> types.ModuleType:
 | 
					    ) -> 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:
 | 
					        if existing is not None:
 | 
				
			||||||
            return cast(types.ModuleType, existing)
 | 
					            return cast(types.ModuleType, existing)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
| 
						 | 
					@ -666,7 +667,7 @@ class PytestPluginManager(PluginManager):
 | 
				
			||||||
                    assert mod not in mods
 | 
					                    assert mod not in mods
 | 
				
			||||||
                    mods.append(mod)
 | 
					                    mods.append(mod)
 | 
				
			||||||
        self.trace(f"loading conftestmodule {mod!r}")
 | 
					        self.trace(f"loading conftestmodule {mod!r}")
 | 
				
			||||||
        self.consider_conftest(mod)
 | 
					        self.consider_conftest(mod, registration_name=conftestpath_plugin_name)
 | 
				
			||||||
        return mod
 | 
					        return mod
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    def _check_non_top_pytest_plugins(
 | 
					    def _check_non_top_pytest_plugins(
 | 
				
			||||||
| 
						 | 
					@ -746,9 +747,11 @@ class PytestPluginManager(PluginManager):
 | 
				
			||||||
                    del self._name2plugin["pytest_" + name]
 | 
					                    del self._name2plugin["pytest_" + name]
 | 
				
			||||||
            self.import_plugin(arg, consider_entry_points=True)
 | 
					            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:"""
 | 
					        """:meta private:"""
 | 
				
			||||||
        self.register(conftestmodule, name=conftestmodule.__file__)
 | 
					        self.register(conftestmodule, name=registration_name)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    def consider_env(self) -> None:
 | 
					    def consider_env(self) -> None:
 | 
				
			||||||
        """:meta private:"""
 | 
					        """:meta private:"""
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -1,6 +1,7 @@
 | 
				
			||||||
import dataclasses
 | 
					import dataclasses
 | 
				
			||||||
import importlib.metadata
 | 
					import importlib.metadata
 | 
				
			||||||
import os
 | 
					import os
 | 
				
			||||||
 | 
					import subprocess
 | 
				
			||||||
import sys
 | 
					import sys
 | 
				
			||||||
import types
 | 
					import types
 | 
				
			||||||
 | 
					
 | 
				
			||||||
| 
						 | 
					@ -1390,3 +1391,61 @@ def test_doctest_and_normal_imports_with_importlib(pytester: Pytester) -> None:
 | 
				
			||||||
    )
 | 
					    )
 | 
				
			||||||
    result = pytester.runpytest_subprocess()
 | 
					    result = pytester.runpytest_subprocess()
 | 
				
			||||||
    result.stdout.fnmatch_lines("*1 passed*")
 | 
					    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
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -98,6 +98,38 @@ class TestPytestPluginInteractions:
 | 
				
			||||||
        config.pluginmanager.register(A())
 | 
					        config.pluginmanager.register(A())
 | 
				
			||||||
        assert len(values) == 2
 | 
					        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:
 | 
					    def test_hook_tracing(self, _config_for_test: Config) -> None:
 | 
				
			||||||
        pytestpm = _config_for_test.pluginmanager  # fully initialized with plugins
 | 
					        pytestpm = _config_for_test.pluginmanager  # fully initialized with plugins
 | 
				
			||||||
        saveindent = []
 | 
					        saveindent = []
 | 
				
			||||||
| 
						 | 
					@ -368,7 +400,7 @@ class TestPytestPluginManager:
 | 
				
			||||||
            pytester.makepyfile("pytest_plugins='xyz'"), root=pytester.path
 | 
					            pytester.makepyfile("pytest_plugins='xyz'"), root=pytester.path
 | 
				
			||||||
        )
 | 
					        )
 | 
				
			||||||
        with pytest.raises(ImportError):
 | 
					        with pytest.raises(ImportError):
 | 
				
			||||||
            pytestpm.consider_conftest(mod)
 | 
					            pytestpm.consider_conftest(mod, registration_name="unused")
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					
 | 
				
			||||||
class TestPytestPluginManagerBootstrapming:
 | 
					class TestPytestPluginManagerBootstrapming:
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
		Loading…
	
		Reference in New Issue