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

[PR #12752/6486c3f3 backport][8.3.x] Fixed the issue of causes KeyError when using the parameter --import-mode=importlib in pytest>=8.2 . (#12592) #12843

Conversation

patchback[bot]
Copy link

@patchback patchback bot commented Sep 26, 2024

This is a backport of PR #12752 as merged into main (6486c3f).

closes #12592

Reproduction Steps:

  1. Create a namespace package in a non-root directory and include a test file.

  2. Create a directory with the same name in that location.

Directory Structure:

>tree ./a
a
└── b
    └── c
        ├── c
        └── test_demo.py

Execution Results:

>pytest --import-mode=importlib

==================== test session starts ===================
platform win32 -- Python 3.12.0, pytest-8.3.2, pluggy-1.5.0
rootdir: D:\app
plugins: allure-pytest-2.13.5
collected 0 items / 1 error

========================== ERRORS ==========================
____________ ERROR collecting a/b/c/test_demo.py ____________
<frozen importlib._bootstrap_external>:1533: in find_spec
    ???
<frozen importlib._bootstrap_external>:1334: in __init__
    ???
<frozen importlib._bootstrap_external>:1350: in _get_parent_path
    ???
E   KeyError: 'a.b'

=================== short test summary info =================
ERROR a/b/c/test_demo.py - KeyError: 'a.b'
!!!!!!!!!! Interrupted: 1 error during collection !!!!!!!!!!!
========================= 1 error in 0.10s ===================

Through tracing, it was discovered that the error occurs during the second call to pathlib._import_module_using_spec.

_import_module_using_spec  # Import module
 -> _import_module_using_spec # Import parent module
   -> PathFinder.find_spec
     -> _NamespacePath.__init__
       -> NamespacePath._get_parent_path
         -> sys.modules[parent_module_name]  # Retrieve ancestor module from sys.modules
           -> KeyError

This is the entire call process when the exception occurs.

In simple terms, when executing PathFinder.find_spec('a.b.c', ['/app/a/b/c']):

  1. Python assumes that we are trying to import a module named c (the tail module).

  2. At this point, there happens to be a directory c in /app/a/b/c, which is then treated as a namespace.

  3. According to convention, its parent module a.b should already exist in sys.modules, and it is accessed directly using the item method.

  4. However, when pytest uses the importlib mode, it doesn't follow this convention, leading to a KeyError.

So, from the observable behavior, it seems to be an upstream issue.

However, from a deeper perspective, there might also be a problem with pytest's handling of parent module imports.

I noticed that _import_module_using_spec attempts to follow the convention of importing the parent module, but the error occurs precisely during this process:
https://github.com/pytest-dev/pytest/blob/main/src/_pytest/pathlib.py#L634-L656

I searched for the usage of 'importlib', and the example provided in the Python documentation can import of any module or namespace package in this case:
https://docs.python.org/3/library/importlib.html#approximating-importlib-import-module.

Unlike _import_module_using_spec, the example function in the documentation first ensures that the necessary parent modules are present in sys.modules, and then imports the target module (or target namespace package).


Based on this, I attempted to modify the execution process in _import_module_using_spec, moving the import of the parent module to after insert_missing_modules() to avoid a KeyError due to sys.modules not being ready.

From the test results, it works quite well, but I'm not sure if there are potential flaws or better solutions (in fact, I've tried and discarded many solutions).

Welcome any further suggestions.

Directories inside a namespace package with the same name as the namespace package would cause a `KeyError` with `--import-mode=importlib`.

Fixes #12592

Co-authored-by: Bruno Oliveira <[email protected]>
(cherry picked from commit 6486c3f)
@nicoddemus nicoddemus enabled auto-merge (squash) September 26, 2024 01:09
@nicoddemus nicoddemus enabled auto-merge (squash) September 26, 2024 01:10
@nicoddemus nicoddemus merged commit 3d3ec57 into 8.3.x Sep 26, 2024
29 of 30 checks passed
@nicoddemus nicoddemus deleted the patchback/backports/8.3.x/6486c3f3a858a0c8043f5c3f7c24297b82a0abe4/pr-12752 branch September 26, 2024 01:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants