-
Notifications
You must be signed in to change notification settings - Fork 165
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
skip directories with perm error when building autoimport index #733
Conversation
I ran into this minor issue when working on my other PR so here's a quick patch. |
@MrBago thanks for that! LGTM. |
ropetest/contrib/autoimporttest.py
Outdated
with tempfile.TemporaryDirectory() as dir: | ||
import os | ||
os.chmod(dir, 0) | ||
sys.path.append(dir) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test lacks isolation, changing the sys.path here is global mutation that won't be undone by the end of the test so it will affect other tests as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MrBago I think it's enough to remove the dir at the end of the context manager.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lieryan I updated the test to use prefs.python_paths
instead of sys.path.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests shouldn't be changing the sys.path globally
@lieryan Happy new year! When you have a min, can you look the update I made to this PR? |
- comment is not a docstring - use octal syntax for chmod
Thanks for fixing this issue @MrBago |
@all-contributors add @MrBago for code contribution |
I've put up a pull request to add @MrBago! 🎉 |
Description
WIth this fix, rope will skip folders in sys.path that the user does not have permission to access instead of failing the entire cache build.
Fixes #(issue)
Checklist (delete if not relevant):