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

pytest cache files contain absolute paths #3968

Open
scw opened this issue Sep 11, 2018 · 6 comments
Open

pytest cache files contain absolute paths #3968

scw opened this issue Sep 11, 2018 · 6 comments
Labels
topic: rewrite related to the assertion rewrite mechanism type: enhancement new feature or API change, should be merged into features branch

Comments

@scw
Copy link

scw commented Sep 11, 2018

The .pyc files that pytest creates with the PYTEST suffix include hardcoded full paths in their embedded source representation. This means that if the file moves, it will no longer be valid if the original location is removed, and pytest will throw errors trying to import these cached files. I checked what the CPython .pyc files are doing differently (which work after moving), and it looks like this is due to how pytest writes the cache files. Here's a quick test using uncompyle6, showing the headers only (decompiled file contents are exactly the same) using a file from scipy:

uncompyle6 conftest.cpython-36.pyc
# uncompyle6 version 3.2.3
# Python bytecode 3.6 (3379)
# Decompiled from: Python 3.6.6 |Anaconda, Inc.| (default, Jun 28 2018, 11:27:44) [MSC v.1900 64 bit (AMD64)]
# Embedded file name: Lib\site-packages\scipy\conftest.py
# Compiled at: 2018-05-05 10:07:41
# Size of source mod 2**32: 999 bytes

uncompyle6 conftest.cpython-36-PYTEST.pyc
# uncompyle6 version 3.2.3
# Python bytecode 3.6 (3379)p
# Decompiled from: Python 3.6.6 |Anaconda, Inc.| (default, Jun 28 2018, 11:27:44) [MSC v.1900 64 bit (AMD64)]
# Embedded file name: C:\apps\miniconda_x\envs\up\lib\site-packages\scipy\conftest.py
# Compiled at: 2018-05-05 10:07:41
# Size of source mod 2**32: 999 bytes

You can see from the "Embedded file name" line that a CPython .pyc contains a relative path, but the PYTEST version contains an absolute path. Systems such as conda typically distribute compiled components, and it would be nice to be able to distribute PYTEST files this way, and more generally be able to move environments.

Here's an oversimplified fix:

import sys
from pathlib import Path

relative_path = Path(fn.strpath).relative_to(sys.prefix)

and use this value for fn.strpath in

co = compile(tree, fn.strpath, "exec", dont_inherit=True)
.

I can make a PR, but it wasn't clear from my reading if the full path is explicitly required somehow by pytest, and figured I'd ask as an issue first.

Environment:

x64 Windows 10, pytest 3.7.4, Python 3.6.6, conda

@RonnyPfannschmidt
Copy link
Member

i clearly see the appeal of this desire - but i'd like to get a clear understandign of what it means with regard to sys.path and locations of files

we should match the cpython behaviour more closely there and allow people to sensibly rename things

but we definitively also need to be very careful about import mismatches relating to different locations of files which are accidentally different but share a common importation prefix

@scw
Copy link
Author

scw commented Sep 12, 2018

Yes, it makes sense to be careful about sourcing, sys.path and site-packages are wildly variable dependent on the platform used. I think for the purposes of .pyc files, this shouldn't be too onerous to resolve, from what I understand .pyc files will either end up in an adjacent __pycache__ folder on Python 3, or in the same directory as the source file itself on Python 2. I see that the loader passes in the full path as stored in the embedded source, but think this could be constructed based on absolute path of the .pyc. Are there other contexts where the PYTEST generated .pyc files are stored independently of the location Python uses, or not in proximity to the source itself?

@RonnyPfannschmidt
Copy link
Member

@scw i am currently under the impression the problem is mainly when pytest alters sys.path to be able to import other modules - we need to be able to keep apart "copies" or test files with the same name but different locations

@scw
Copy link
Author

scw commented Sep 14, 2018

OK, how about this -- when parsing the co_filename property, if that doesn't exist, then fall back to behavior which instead uses a relative path? That would keep the current behavior in most cases, but would also prevent pytest from loading tests from non-existent locations.

@RonnyPfannschmidt
Copy link
Member

@scw the problem i fear is files with tests from 2 different locations ending up with the same import name and relative filename

additionally some of that might be tied with py.path.local limitations (i havent digged in deep yet)

@scw
Copy link
Author

scw commented Sep 19, 2018

OK, can you help me understand what needs to be researched in order to fix this? Here's my understanding of how to handle this if there are multiple duplicates on sys.path:

  1. For each file location, start by trying to parse the co_filename property, a fully qualified path. If this succeeds, continue with existing code. If this fails, continue to step 2.
  2. Construct a fully qualified path from the existing PYTEST file. Get the absolute directory name of the file in question. So, if the file is C:\apps\miniconda_x\envs\up\lib\site-packages\scipy\__pycache__\conftest-PYTEST.pyc this would return C:\apps\miniconda_x\envs\up\lib\site-packages\scipy\__pycache__. On Python 2, this returned value can be used directly, on Python 3, strip the final __pycache__ entry. This leaves C:\apps\miniconda_x\envs\up\lib\site-packages\scipy in both cases. This is the directory of the file being tested, in this case conftest.py. Use this for the current value of co_filename.

Say there are two copies of conftest.cpython-36-PYTEST.pyc that exist in sys.path, the above approach will still find the file that's paired with its appropriate entry in sys.path.

@Zac-HD Zac-HD added type: enhancement new feature or API change, should be merged into features branch topic: rewrite related to the assertion rewrite mechanism labels Oct 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: rewrite related to the assertion rewrite mechanism type: enhancement new feature or API change, should be merged into features branch
Projects
None yet
Development

No branches or pull requests

3 participants