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

fix: Resolved an ImportError crash when using --import-mode=importlib #13053

Merged
merged 2 commits into from
Dec 12, 2024

Conversation

dongfangtianyu
Copy link
Contributor

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)

  • main.py: Reproduces the error.
  • debug.py: Demonstrates the behavior of PathFinder.find_spec.

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

@psf-chronographer psf-chronographer bot added the bot:chronographer:provided (automation) changelog entry is part of PR label Dec 12, 2024
@dongfangtianyu
Copy link
Contributor Author

Thanks @valeriupredoi @michael-christen for feedback.
Could you help verify if this patch resolves the issue in your scenarios?

Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @dongfangtianyu, appreciate the PR!

@nicoddemus nicoddemus added the backport 8.3.x apply to PRs at any point; backports the changes to the 8.3.x branch label Dec 12, 2024
@nicoddemus nicoddemus enabled auto-merge (squash) December 12, 2024 14:22
@valeriupredoi
Copy link

many thanks @dongfangtianyu - I'm sure it does, I took a look at the code, and if @nicoddemus approved then I think it's all spiffy 🍺

@nicoddemus nicoddemus merged commit 28e1e25 into pytest-dev:main Dec 12, 2024
28 checks passed
Copy link

patchback bot commented Dec 12, 2024

Backport to 8.3.x: 💚 backport PR created

✅ Backport PR branch: patchback/backports/8.3.x/28e1e25a6782513db8a2963bd5ed5a9d66682f86/pr-13053

Backported as #13054

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Dec 12, 2024
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)
nicoddemus pushed a commit that referenced this pull request Dec 12, 2024
…#13054)

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)

Co-authored-by: dongfangtianyu <[email protected]>
@michael-christen
Copy link

Thanks @valeriupredoi @michael-christen for feedback. Could you help verify if this patch resolves the issue in your scenarios?

Can confirm, this fixed the issue I was experiencing, thanks again, you're the best @dongfangtianyu!

@dongfangtianyu dongfangtianyu deleted the hotfix branch December 13, 2024 07:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 8.3.x apply to PRs at any point; backports the changes to the 8.3.x branch bot:chronographer:provided (automation) changelog entry is part of PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants