Skip to content

Commit

Permalink
Fix ImportError crash when using --import-mode=importlib (#13053)
Browse files Browse the repository at this point in the history
Regression in #12716

In short: `PathFinder.find_spec` received the argument `/cow/moo` but loaded `/cow/moo/moo.py` instead.

**Trigger conditions:**
1. `/cow/moo/moo.py` exists (a file and directory with the same name).
2. `/cow/moo/test_moo.py` exists (test case resides in the directory).

When pytest loads test files in `importlib` mode, it continues recursive loading upward:
- When loading `cow.moo`, it should return a namespace but unexpectedly returns a module.
- When loading `cow.moo.moo`, it should return a module but unexpectedly returns a namespace.

**Complete example:** [[GitHub repository](https://github.com/dongfangtianyu/pytest_importlib_issue)](https://github.com/dongfangtianyu/pytest_importlib_issue)
- `main.py`: Reproduces the error.
- `debug.py`: Demonstrates the behavior of `PathFinder.find_spec`.

**Context:**
#12592 (comment)
#12592 (comment)

---------

Co-authored-by: Bruno Oliveira <[email protected]>
(cherry picked from commit 28e1e25)
  • Loading branch information
dongfangtianyu authored and patchback[bot] committed Dec 12, 2024
1 parent 179b690 commit 1c9e8a3
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 3 deletions.
1 change: 1 addition & 0 deletions changelog/13053.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed a regression in pytest 8.3.4 where, when using ``--import-mode=importlib``, a directory containing py file with the same name would cause an ``ImportError``
13 changes: 10 additions & 3 deletions src/_pytest/pathlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -694,9 +694,16 @@ def _import_module_using_spec(
# Checking with sys.meta_path first in case one of its hooks can import this module,
# such as our own assertion-rewrite hook.
for meta_importer in sys.meta_path:
spec = meta_importer.find_spec(
module_name, [str(module_location), str(module_path)]
)
module_name_of_meta = getattr(meta_importer.__class__, "__module__", "")

Check warning on line 697 in src/_pytest/pathlib.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/pathlib.py#L697

Added line #L697 was not covered by tests
if module_name_of_meta == "_pytest.assertion.rewrite" and module_path.is_file():
# Import modules in subdirectories by module_path
# to ensure assertion rewrites are not missed (#12659).
find_spec_path = [str(module_location), str(module_path)]

Check warning on line 701 in src/_pytest/pathlib.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/pathlib.py#L701

Added line #L701 was not covered by tests
else:
find_spec_path = [str(module_location)]

Check warning on line 703 in src/_pytest/pathlib.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/pathlib.py#L703

Added line #L703 was not covered by tests

spec = meta_importer.find_spec(module_name, find_spec_path)

Check warning on line 705 in src/_pytest/pathlib.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/pathlib.py#L705

Added line #L705 was not covered by tests

if spec_matches_module_path(spec, module_path):
break
else:
Expand Down
28 changes: 28 additions & 0 deletions testing/test_pathlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -1498,6 +1498,34 @@ def test():
]
)

def test_ns_multiple_levels_import_error(
self,
tmp_path: Path,
pytester: Pytester,
) -> None:
# Trigger condition 1: ns and file with the same name
file = pytester.path / "cow/moo/moo.py"
file.parent.mkdir(parents=True)
file.write_text("data=123", encoding="utf-8")

# Trigger condition 2: tests are located in ns
tests = pytester.path / "cow/moo/test_moo.py"

tests.write_text(
dedent(
"""
from cow.moo.moo import data
def test_moo():
print(data)
"""
),
encoding="utf-8",
)

result = pytester.runpytest("--import-mode=importlib")
assert result.ret == ExitCode.OK

@pytest.mark.parametrize("import_mode", ["prepend", "append", "importlib"])
def test_incorrect_namespace_package(
self,
Expand Down

0 comments on commit 1c9e8a3

Please sign in to comment.