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

Resolve cwd using os.path.realpath() when generating environment names #6110

Merged
merged 4 commits into from
Aug 5, 2022

Conversation

adamgreg
Copy link
Contributor

@adamgreg adamgreg commented Aug 3, 2022

Pull Request Check List

Resolves: #6109

  • Added tests for changed code.
  • Updated documentation for changed code.

@adamgreg
Copy link
Contributor Author

adamgreg commented Aug 3, 2022

I wasn't sure how to add a suitable new unit test, sorry.

src/poetry/utils/env.py Outdated Show resolved Hide resolved
@radoering
Copy link
Member

I wasn't sure how to add a suitable new unit test, sorry.

You probably have to mock realpath().

@adamgreg
Copy link
Contributor Author

adamgreg commented Aug 3, 2022

I wasn't sure how to add a suitable new unit test, sorry.

You probably have to mock realpath().

realpath() returns its argument unchanged if the path does not exist, so I don't think it's necessary to mock it unless we want to count the calls.

I meant to say I can't think of a way to unit test that this actually fixes the bug, short of mapping a drive on the test system.

@radoering
Copy link
Member

Let's say you have path1 and path2. You can mock realpath to return path1 for both (i.e. path2 is a symlink pointing to path1). Then, generate_env_name() should return the same value no matter if it's called with path1 or path2. A test case similar to

poetry/tests/utils/test_env.py

Lines 1356 to 1362 in 62701fe

def test_generate_env_name_ignores_case_for_case_insensitive_fs(tmp_dir: str):
venv_name1 = EnvManager.generate_env_name("simple-project", "MyDiR")
venv_name2 = EnvManager.generate_env_name("simple-project", "mYdIr")
if sys.platform == "win32":
assert venv_name1 == venv_name2
else:
assert venv_name1 != venv_name2
should be sufficient (just with mocking and without platform distinction).

Alternatively, instead of mocking you could create a symlink similar to

def test_lock_file_resolves_file_url_symlinks(root: ProjectPackage):
although that's probably a bit more overhead.

@radoering
Copy link
Member

pre-commit.ci autofix

Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 29, 2024
@adamgreg adamgreg deleted the fix_win_mapped_drive_env branch February 29, 2024 10:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Windows drive mappings break virtual environment name
2 participants