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

stubtest: Fix wrong assumption about relative path #12282

Merged
merged 1 commit into from
Mar 4, 2022

Conversation

stanislavlevin
Copy link
Contributor

@stanislavlevin stanislavlevin commented Mar 3, 2022

Have you read the Contributing Guidelines?

yes

Description

Fixes #11019

run_stubtest creates temp directory and prepend sys.path with relative path (dot .) wrongly assuming that the dot will be resolved to absolute path on every import attempt.

But in Python dot(.) in sys.path is actually resolved by PathFinder and cached in sys.path_importer_cache like:

sys.path_importer_cache['.']
FileFinder('/somepath/.')

later calls for find_module return None and import of test_module fails. This resulted in only the first test in stubtest's suite passed in non-pytest-xdist environments.

This issue was hidden with bug or feature in pytest-xdist < 2.3.0:
pytest-dev/pytest-xdist#421

It was fixed in pytest-xdist 2.3.0:
pytest-dev/pytest-xdist#667

  • sys.path for pytest-xdist < 2.3.0
    '.', 'project_path', ''

  • sys.path for pytest-xdist >= 2.3.0 or without xdist
    '.', 'project_path'

Proposed fix

In Python for denoting cwd the empty path '' can be used as a special case, but for readability sys.path is prepended with resolved absolute path of temp directory. Also it's essential to restore back sys.path after a test to not break subsequent tests.

Description:
`run_stubtest` creates temp directory and prepend `sys.path` with
relative path (dot `.`) wrongly assuming that the dot will be
resolved to absolute path on *every* import attempt.

But in Python dot(`.`) in sys.path is actually resolved by
PathFinder and cached in `sys.path_importer_cache` like:
```
sys.path_importer_cache['.']
FileFinder('/somepath/.')
```
later calls for `find_module` return None and import of
`test_module` fails.

This resulted in only the first test in stubtest's suite passed in
non-pytest-xdist environments.

This issue was hidden with bug or feature in pytest-xdist < 2.3.0:
pytest-dev/pytest-xdist#421

It was fixed in pytest-xdist 2.3.0:
pytest-dev/pytest-xdist#667

- sys.path for pytest-xdist < 2.3.0
  `'.', 'project_path', ''`

- sys.path for pytest-xdist >= 2.3.0 or without xdist
  `'.', 'project_path'`

Fix:
In Python for denoting cwd the empty path `''` can be used as a
special case, but for readability `sys.path` is prepended with
resolved absolute path of temp directory. Also it's essential to
restore back `sys.path` after a test to not break subsequent tests.

Fixes: python#11019
Signed-off-by: Stanislav Levin <[email protected]>
@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2022

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

Copy link
Collaborator

@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this!

@hauntsaninja hauntsaninja merged commit a562f0a into python:master Mar 4, 2022
hauntsaninja pushed a commit to hauntsaninja/mypy that referenced this pull request Mar 4, 2022
hauntsaninja added a commit that referenced this pull request Mar 4, 2022
Basically a follow up to #12282

Co-authored-by: hauntsaninja <>
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.

Tests from mypy/test/teststubtest.py fail without pytest-xdist or with v2.3.0
2 participants