Skip to content

Commit

Permalink
Update env remove logic
Browse files Browse the repository at this point in the history
Update remove handling, so that if other project's venv name is passed, raise IncorrectEnvError for better error message
  • Loading branch information
dhvcc committed Aug 19, 2022
1 parent 4d91dad commit 2700195
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 5 deletions.
15 changes: 11 additions & 4 deletions src/poetry/utils/env.py
Original file line number Diff line number Diff line change
Expand Up @@ -744,13 +744,14 @@ def list(self, name: str | None = None) -> list[VirtualEnv]:
env_list.insert(0, VirtualEnv(venv))
return env_list

def check_env_is_for_current_project(self, env: str) -> bool:
@staticmethod
def check_env_is_for_current_project(env: str, base_env_name: str) -> bool:
"""
Check if env name starts with projects name.
This is done to prevent action on other project's envs.
"""
return env.startswith(self._poetry.package.name)
return env.startswith(base_env_name)

def remove(self, python: str) -> Env:
venv_path = self._poetry.config.virtualenvs_path
Expand All @@ -770,12 +771,12 @@ def remove(self, python: str) -> Env:
)
).strip("\n")
env_name = Path(env_dir).name
if not self.check_env_is_for_current_project(env_name):
if not self.check_env_is_for_current_project(env_name, base_env_name):
raise IncorrectEnvError(env_name)
except CalledProcessError as e:
raise EnvCommandError(e)

if self.check_env_is_for_current_project(python):
if self.check_env_is_for_current_project(python, base_env_name):
venvs = self.list()
for venv in venvs:
if venv.path.name == python:
Expand Down Expand Up @@ -806,6 +807,12 @@ def remove(self, python: str) -> Env:
raise ValueError(
f'<warning>Environment "{python}" does not exist.</warning>'
)
else:
venv_path = self._poetry.config.virtualenvs_path
# Get all the poetry envs, even for other projects
env_names = [Path(p).name for p in sorted(venv_path.glob("*-*-py*"))]
if python in env_names:
raise IncorrectEnvError(python)

try:
python_version = Version.parse(python)
Expand Down
56 changes: 55 additions & 1 deletion tests/utils/test_env.py
Original file line number Diff line number Diff line change
Expand Up @@ -727,7 +727,7 @@ def test_remove_by_full_path_to_python(
assert not expected_venv_path.exists()


def test_doesnt_act_on_different_projects_env_when_passing_full_path(
def test_raises_if_acting_on_different_project_by_full_path(
tmp_dir: str,
manager: EnvManager,
poetry: Poetry,
Expand All @@ -754,6 +754,60 @@ def test_doesnt_act_on_different_projects_env_when_passing_full_path(
manager.remove(str(python_path))


def test_raises_if_acting_on_different_project_by_name(
tmp_dir: str,
manager: EnvManager,
poetry: Poetry,
config: Config,
):
config.merge({"virtualenvs": {"path": str(tmp_dir)}})

different_venv_name = (
EnvManager.generate_env_name(
"different-project",
str(poetry.file.parent),
)
+ "-py3.6"
)
different_venv_path = Path(tmp_dir) / different_venv_name
different_venv_bin_path = different_venv_path / "bin"
different_venv_bin_path.mkdir(parents=True)

python_path = different_venv_bin_path / "python"
python_path.touch(exist_ok=True)

with pytest.raises(IncorrectEnvError):
manager.remove(different_venv_name)


def test_raises_when_passing_old_env_after_dir_rename(
tmp_dir: str,
manager: EnvManager,
poetry: Poetry,
config: Config,
venv_name: str,
):
# Make sure that poetry raises when trying to remove old venv after you've renamed
# root directory of the project, which will create another venv with new name.
# This is not ideal as you still "can't" remove it by name, but it at least doesn't
# cause any unwanted side effects
config.merge({"virtualenvs": {"path": str(tmp_dir)}})

previous_venv_name = EnvManager.generate_env_name(
poetry.package.name,
"previous_dir_name",
)
venv_path = Path(tmp_dir) / f"{venv_name}-py3.6"
venv_path.mkdir()

previous_venv_name = f"{previous_venv_name}-py3.6"
previous_venv_path = Path(tmp_dir) / previous_venv_name
previous_venv_path.mkdir()

with pytest.raises(IncorrectEnvError):
manager.remove(previous_venv_name)


def test_remove_also_deactivates(
tmp_dir: str,
manager: EnvManager,
Expand Down

0 comments on commit 2700195

Please sign in to comment.