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

Don't change poetry.lock during poetry update --dry-run #3767

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/poetry/installation/installer.py
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@ def _do_install(self, local_repo: Repository) -> int:
# Execute operations
return self._execute(ops)

def _write_lock_file(self, repo: Repository, force: bool = True) -> None:
def _write_lock_file(self, repo: Repository, force: bool = False) -> None:
if force or (self._update and self._write_lock):
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if force or (self._update and self._write_lock):
if (force or self._update) and self._write_lock:

I think that line is also wrong since self._write_lock is only used for --dry-run, force should not overwrite this. From inspecting the code, I assume that fix is necessary for poetry add --lock --dry-run. I really think we need more coverage, here.

updated_lock = self._locker.set_lock_data(self._package, repo.packages)

Expand Down
2 changes: 0 additions & 2 deletions tests/console/commands/plugin/test_remove.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,6 @@ def test_remove_installed_package_dry_run(
Updating dependencies
Resolving dependencies...

Writing lock file

Package operations: 0 installs, 0 updates, 1 removal

• Removing poetry-plugin (1.2.3)
Expand Down
57 changes: 57 additions & 0 deletions tests/console/commands/test_update.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
from pathlib import Path
from typing import TYPE_CHECKING

import pytest

from poetry.packages import Locker
from tests.helpers import get_package


if TYPE_CHECKING:
from poetry.poetry import Poetry
from tests.helpers import TestRepository
from tests.types import CommandTesterFactory
from tests.types import FixtureDirGetter
from tests.types import ProjectFactory


@pytest.fixture
def source_dir(tmp_path: Path) -> Path:
return Path(tmp_path.as_posix())


@pytest.fixture
def poetry_with_old_lockfile(
project_factory: "ProjectFactory", fixture_dir: "FixtureDirGetter"
) -> "Poetry":
source = fixture_dir("old_lock")
pyproject_content = (source / "pyproject.toml").read_text(encoding="utf-8")
poetry_lock_content = (source / "poetry.lock").read_text(encoding="utf-8")
return project_factory(
name="foobar",
pyproject_content=pyproject_content,
poetry_lock_content=poetry_lock_content,
)


def test_dry_run_update(
Copy link
Member

Choose a reason for hiding this comment

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

Can you please add similar tests (or extend existing tests) for the following commands (if not yet existing):

  • add --dry-run
  • add --lock --dry-run
  • remove --dry-run

Further, it seems that tests for add check that the pyproject.toml is not changed for dry-run. Can you please add this check here, too?

Copy link
Member

Choose a reason for hiding this comment

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

And probably, you should rebase first before adding tests. 🙏

command_tester_factory: "CommandTesterFactory",
poetry_with_old_lockfile: "Poetry",
repo: "TestRepository",
) -> None:
repo.add_package(get_package("sampleproject", "1.3.1"))
repo.add_package(get_package("sampleproject", "2.0.0"))

lock = poetry_with_old_lockfile.pyproject.file.path.parent / "poetry.lock"
locker = Locker(
lock=lock,
local_config=poetry_with_old_lockfile.locker._local_config,
)
poetry_with_old_lockfile.set_locker(locker)

lock_content_before = lock.read_text(encoding="utf-8")

tester = command_tester_factory("update", poetry=poetry_with_old_lockfile)
tester.execute("--dry-run")

assert lock.read_text(encoding="utf-8") == lock_content_before