Skip to content

installer: remove unrequested extras if requires_synchronization is set#8621

Merged
radoering merged 1 commit into
python-poetry:masterfrom
radoering:installer-extras-test-coverage
Nov 21, 2023
Merged

installer: remove unrequested extras if requires_synchronization is set#8621
radoering merged 1 commit into
python-poetry:masterfrom
radoering:installer-extras-test-coverage

Conversation

@radoering

@radoering radoering commented Nov 4, 2023

Copy link
Copy Markdown
Member

tl;dr

  • fix an issue where poetry install --sync does not remove unrequested extras (funny enough poetry install without --sync does remove them)
  • increase test coverage of the installer regarding extras
  • Added tests for changed code.
  • Updated documentation for changed code.

Long story

Originally, I was just poking around with python-poetry/poetry-core#613 (comment). However, I noticed that all installer tests still passed if one of the following lines were removed:

Thus, I decided to improve the test coverage before trying to fix bugs. I merged some quite similar tests and used pytest.mark.parametrize to cover the missing cases. However,

if locked.optional and locked.name not in extra_packages:
# Installed but optional and not requested in extras
ops.append(Uninstall(locked))
was still irrelevant.

After some code spelunking, I suppose that these lines became irrelevant with ea8fb8c. However, that introduced the mentioned bug. After adding @pytest.mark.parametrize("do_sync", [False, True]) to the test and fixing the bug, the mentioned lines of code are relevant again.

@radoering
radoering force-pushed the installer-extras-test-coverage branch from 0aef22d to a8d2002 Compare November 10, 2023 16:33
@radoering
radoering requested a review from a team November 10, 2023 16:39
@radoering
radoering force-pushed the installer-extras-test-coverage branch 3 times, most recently from d6d3895 to cd4f7b3 Compare November 19, 2023 16:58

@Secrus Secrus left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

… set, increase test coverage of installer regarding extras
@radoering
radoering force-pushed the installer-extras-test-coverage branch from cd4f7b3 to 9eb5363 Compare November 21, 2023 15:33
@radoering
radoering merged commit e6c1c99 into python-poetry:master Nov 21, 2023
MrGreenTea pushed a commit to MrGreenTea/poetry that referenced this pull request Dec 18, 2023
… set (python-poetry#8621)

... and increase test coverage of installer regarding extras
@github-actions

github-actions Bot commented Mar 3, 2024

Copy link
Copy Markdown

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 Mar 3, 2024
@radoering
radoering deleted the installer-extras-test-coverage branch August 16, 2025 10:11
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.

2 participants