[7.4.x] Fix duplicated imports with importlib mode and doctest-modules (#11164)
Co-authored-by: Bruno Oliveira <nicoddemus@gmail.com>
This commit is contained in:
		
							parent
							
								
									b6c55787fe
								
							
						
					
					
						commit
						a4d7254d18
					
				|  | @ -0,0 +1,2 @@ | ||||||
|  | Fixed issue when using ``--import-mode=importlib`` together with ``--doctest-modules`` that caused modules | ||||||
|  | to be imported more than once, causing problems with modules that have import side effects. | ||||||
|  | @ -523,6 +523,8 @@ def import_path( | ||||||
| 
 | 
 | ||||||
|     if mode is ImportMode.importlib: |     if mode is ImportMode.importlib: | ||||||
|         module_name = module_name_from_path(path, root) |         module_name = module_name_from_path(path, root) | ||||||
|  |         with contextlib.suppress(KeyError): | ||||||
|  |             return sys.modules[module_name] | ||||||
| 
 | 
 | ||||||
|         for meta_importer in sys.meta_path: |         for meta_importer in sys.meta_path: | ||||||
|             spec = meta_importer.find_spec(module_name, [str(path.parent)]) |             spec = meta_importer.find_spec(module_name, [str(path.parent)]) | ||||||
|  |  | ||||||
|  | @ -1317,3 +1317,38 @@ def test_function_return_non_none_warning(pytester: Pytester) -> None: | ||||||
|     ) |     ) | ||||||
|     res = pytester.runpytest() |     res = pytester.runpytest() | ||||||
|     res.stdout.fnmatch_lines(["*Did you mean to use `assert` instead of `return`?*"]) |     res.stdout.fnmatch_lines(["*Did you mean to use `assert` instead of `return`?*"]) | ||||||
|  | 
 | ||||||
|  | 
 | ||||||
|  | def test_doctest_and_normal_imports_with_importlib(pytester: Pytester) -> None: | ||||||
|  |     """ | ||||||
|  |     Regression test for #10811: previously import_path with ImportMode.importlib would | ||||||
|  |     not return a module if already in sys.modules, resulting in modules being imported | ||||||
|  |     multiple times, which causes problems with modules that have import side effects. | ||||||
|  |     """ | ||||||
|  |     # Uses the exact reproducer form #10811, given it is very minimal | ||||||
|  |     # and illustrates the problem well. | ||||||
|  |     pytester.makepyfile( | ||||||
|  |         **{ | ||||||
|  |             "pmxbot/commands.py": "from . import logging", | ||||||
|  |             "pmxbot/logging.py": "", | ||||||
|  |             "tests/__init__.py": "", | ||||||
|  |             "tests/test_commands.py": """ | ||||||
|  |                 import importlib | ||||||
|  |                 from pmxbot import logging | ||||||
|  | 
 | ||||||
|  |                 class TestCommands: | ||||||
|  |                     def test_boo(self): | ||||||
|  |                         assert importlib.import_module('pmxbot.logging') is logging | ||||||
|  |                 """, | ||||||
|  |         } | ||||||
|  |     ) | ||||||
|  |     pytester.makeini( | ||||||
|  |         """ | ||||||
|  |         [pytest] | ||||||
|  |         addopts= | ||||||
|  |             --doctest-modules | ||||||
|  |             --import-mode importlib | ||||||
|  |         """ | ||||||
|  |     ) | ||||||
|  |     result = pytester.runpytest_subprocess() | ||||||
|  |     result.stdout.fnmatch_lines("*1 passed*") | ||||||
|  |  | ||||||
|  | @ -7,6 +7,7 @@ from textwrap import dedent | ||||||
| from types import ModuleType | from types import ModuleType | ||||||
| from typing import Any | from typing import Any | ||||||
| from typing import Generator | from typing import Generator | ||||||
|  | from typing import Iterator | ||||||
| 
 | 
 | ||||||
| import pytest | import pytest | ||||||
| from _pytest.monkeypatch import MonkeyPatch | from _pytest.monkeypatch import MonkeyPatch | ||||||
|  | @ -282,29 +283,36 @@ class TestImportPath: | ||||||
|             import_path(tmp_path / "invalid.py", root=tmp_path) |             import_path(tmp_path / "invalid.py", root=tmp_path) | ||||||
| 
 | 
 | ||||||
|     @pytest.fixture |     @pytest.fixture | ||||||
|     def simple_module(self, tmp_path: Path) -> Path: |     def simple_module( | ||||||
|         fn = tmp_path / "_src/tests/mymod.py" |         self, tmp_path: Path, request: pytest.FixtureRequest | ||||||
|  |     ) -> Iterator[Path]: | ||||||
|  |         name = f"mymod_{request.node.name}" | ||||||
|  |         fn = tmp_path / f"_src/tests/{name}.py" | ||||||
|         fn.parent.mkdir(parents=True) |         fn.parent.mkdir(parents=True) | ||||||
|         fn.write_text("def foo(x): return 40 + x", encoding="utf-8") |         fn.write_text("def foo(x): return 40 + x", encoding="utf-8") | ||||||
|         return fn |         module_name = module_name_from_path(fn, root=tmp_path) | ||||||
|  |         yield fn | ||||||
|  |         sys.modules.pop(module_name, None) | ||||||
| 
 | 
 | ||||||
|     def test_importmode_importlib(self, simple_module: Path, tmp_path: Path) -> None: |     def test_importmode_importlib( | ||||||
|  |         self, simple_module: Path, tmp_path: Path, request: pytest.FixtureRequest | ||||||
|  |     ) -> None: | ||||||
|         """`importlib` mode does not change sys.path.""" |         """`importlib` mode does not change sys.path.""" | ||||||
|         module = import_path(simple_module, mode="importlib", root=tmp_path) |         module = import_path(simple_module, mode="importlib", root=tmp_path) | ||||||
|         assert module.foo(2) == 42  # type: ignore[attr-defined] |         assert module.foo(2) == 42  # type: ignore[attr-defined] | ||||||
|         assert str(simple_module.parent) not in sys.path |         assert str(simple_module.parent) not in sys.path | ||||||
|         assert module.__name__ in sys.modules |         assert module.__name__ in sys.modules | ||||||
|         assert module.__name__ == "_src.tests.mymod" |         assert module.__name__ == f"_src.tests.mymod_{request.node.name}" | ||||||
|         assert "_src" in sys.modules |         assert "_src" in sys.modules | ||||||
|         assert "_src.tests" in sys.modules |         assert "_src.tests" in sys.modules | ||||||
| 
 | 
 | ||||||
|     def test_importmode_twice_is_different_module( |     def test_remembers_previous_imports( | ||||||
|         self, simple_module: Path, tmp_path: Path |         self, simple_module: Path, tmp_path: Path | ||||||
|     ) -> None: |     ) -> None: | ||||||
|         """`importlib` mode always returns a new module.""" |         """`importlib` mode called remembers previous module (#10341, #10811).""" | ||||||
|         module1 = import_path(simple_module, mode="importlib", root=tmp_path) |         module1 = import_path(simple_module, mode="importlib", root=tmp_path) | ||||||
|         module2 = import_path(simple_module, mode="importlib", root=tmp_path) |         module2 = import_path(simple_module, mode="importlib", root=tmp_path) | ||||||
|         assert module1 is not module2 |         assert module1 is module2 | ||||||
| 
 | 
 | ||||||
|     def test_no_meta_path_found( |     def test_no_meta_path_found( | ||||||
|         self, simple_module: Path, monkeypatch: MonkeyPatch, tmp_path: Path |         self, simple_module: Path, monkeypatch: MonkeyPatch, tmp_path: Path | ||||||
|  | @ -317,6 +325,9 @@ class TestImportPath: | ||||||
|         # mode='importlib' fails if no spec is found to load the module |         # mode='importlib' fails if no spec is found to load the module | ||||||
|         import importlib.util |         import importlib.util | ||||||
| 
 | 
 | ||||||
|  |         # Force module to be re-imported. | ||||||
|  |         del sys.modules[module.__name__] | ||||||
|  | 
 | ||||||
|         monkeypatch.setattr( |         monkeypatch.setattr( | ||||||
|             importlib.util, "spec_from_file_location", lambda *args: None |             importlib.util, "spec_from_file_location", lambda *args: None | ||||||
|         ) |         ) | ||||||
|  |  | ||||||
		Loading…
	
		Reference in New Issue