Skip to content

Proposed Fix for Optional Dependency Removal Bug (#10703) #10718

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.

Sign up for GitHub

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

Closed
jontyhuang wants to merge 1 commit into from
+11 −0
Closed

Proposed Fix for Optional Dependency Removal Bug (#10703) #10718

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
11 changes: 11 additions & 0 deletions test_optional_dependency_removal.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# Test script to reproduce issue #10703 in poetry
# Expected: poetry remove pylint should succeed
Copy link

@sourcery-ai sourcery-ai bot Feb 7, 2026

Choose a reason for hiding this comment

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

suggestion (testing): The test should reflect the actual CLI behavior described in the PR (using poetry remove), not just the internal API.

The comment’s expectation is about the CLI (poetry remove pylint), but this script only exercises internal classes. To reliably demonstrate the CLI bug is fixed, consider adding an integration/end-to-end test that runs poetry remove pylint (or equivalent) via the existing CLI test harness in a temporary project and asserts on exit code and output, matching the behavior described in issue #10703.

Suggested implementation:

import subprocess
import tempfile
from pathlib import Path

import pytest


@pytest.mark.integration
def test_optional_dependency_removal_via_cli_succeeds():
    """
    Reproduces issue #10703 via the CLI.

    The expectation from the original report is that running:
        poetry remove pylint
    in a project where pylint is declared as an optional dependency
    should succeed (exit code 0) instead of erroring.
    """
    with tempfile.TemporaryDirectory() as tmp_dir:
        project_dir = Path(tmp_dir)

        pyproject = project_dir / "pyproject.toml"
        pyproject.write_text(
            """
[tool.poetry]
name = "test_project"
version = "0.1.0"
description = ""
authors = ["Test <test@example.com>"]

[tool.poetry.dependencies]
python = "^3.8"
pylint = { version = "*", optional = true }

[tool.poetry.extras]
lint = ["pylint"]
""".lstrip(),
            encoding="utf-8",
        )

        # Run the CLI as a user would.
        result = subprocess.run(
            ["poetry", "remove", "pylint"],
            cwd=project_dir,
            capture_output=True,
            text=True,
        )

        # The primary signal that the bug is fixed is a successful exit code.
        assert (
            result.returncode == 0
        ), f"'poetry remove pylint' failed.\nstdout:\n{result.stdout}\nstderr:\n{result.stderr}"

Depending on the existing test conventions in the Poetry codebase, you may want to:

  1. Move this test into the existing CLI/integration test package (e.g. tests/console/ or similar) and adjust the module name accordingly.
  2. Replace the direct subprocess.run usage with Poetry's existing CLI test harness/runner (for example, a CommandTester, CliRunner, or repository-specific helper) if one is already used elsewhere for poetry CLI tests.
  3. Align the marker (@pytest.mark.integration) with whatever marker or naming convention the project uses for end‑to‑end or CLI tests (e.g. @pytest.mark.e2e, @pytest.mark.cli, or none at all).
  4. If the test suite assumes poetry is invoked via a specific entry point (like python -m poetry or a helper function), adjust the subprocess.run arguments to match that convention.

from poetry.factory import Factory
from poetry.project.project import Project

# Simulated project setup
project = Project('test_project')
poetry = Factory().create_poetry(project)
poetry.add_dependency('pylint', {'optional': True})
poetry.remove_dependency('pylint')
print("Optional dependency removed successfully.")
Comment on lines +10 to +11
Copy link

@sourcery-ai sourcery-ai bot Feb 7, 2026

Choose a reason for hiding this comment

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

suggestion (testing): Add assertions to verify the dependency was actually removed instead of relying on the absence of an exception and a print.

Right now this test passes as long as no exception is raised and a message is printed. To make it a meaningful automated test, add an assertion that pylint is actually removed (e.g., it no longer appears in the dependencies collection or in the relevant section of pyproject.toml / lockfile).

Suggested implementation:

# Test script to reproduce issue #10703 in poetry
# Expected: poetry remove pylint should succeed
from poetry.factory import Factory
from poetry.project.project import Project

# Simulated project setup
project = Project('test_project')
poetry = Factory().create_poetry(project)

# Add optional dependency
poetry.add_dependency('pylint', {'optional': True})
# Sanity check that the dependency was added
assert 'pylint' in poetry.package.dependencies

# Remove the dependency
poetry.remove_dependency('pylint')

# Assert that the dependency was actually removed
assert 'pylint' not in poetry.package.dependencies

If the Poetry API in your codebase exposes dependencies differently (for example via poetry.locker, poetry.pyproject, or another attribute instead of poetry.package.dependencies), adjust the two assert lines to target the correct collection or pyproject/lockfile representation.

For example, you might need to change them to something like:

  • assert 'pylint' in poetry.pyproject.poetry_config['tool']['poetry']['dependencies']
  • assert 'pylint' not in poetry.pyproject.poetry_config['tool']['poetry']['dependencies']

depending on how add_dependency and remove_dependency are implemented in your version of Poetry.