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.

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

Jump to bottom

fix: excluding non-existing groups for pdm remove #3420

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

fix: excluding non-existing groups for pdm remove #3420

wants to merge 6 commits into from
+17 −1

Conversation

huxuan
Copy link
Contributor

@huxuan huxuan commented Mar 11, 2025

Pull Request Checklist

  • A news fragment is added in news/ describing what is new.
  • Test cases added for changed code.

Describe what you have changed in this PR.

Fix #3404

Same test case before the changes

$ pytest -k "test_remove_exclude_non_existing_dev_group_in_lockfile"
================================================================================ test session starts ================================================================================
platform linux -- Python 3.10.12, pytest-8.3.3, pluggy-1.5.0
rootdir: /home/huxuan/Code/pdm
configfile: pyproject.toml
testpaths: tests/
plugins: anyio-4.3.0, mock-3.14.0, rerunfailures-14.0, pytest_httpserver-1.1.0, cov-5.0.0, xdist-3.6.1
collected 987 items / 986 deselected / 1 selected                                                                                                                                   

tests/cli/test_remove.py F                                                                                                                                                    [100%]

===================================================================================== FAILURES ======================================================================================
______________________________________________________________ test_remove_exclude_non_existing_dev_group_in_lockfile _______________________________________________________________

project = <Project '/tmp/pytest-of-huxuan/pytest-4/test_remove_exclude_non_existi0'>, pdm = <function pdm.<locals>.caller at 0x7dc7251bca60>
mocker = <pytest_mock.plugin.MockerFixture object at 0x7dc7252aa440>

    @pytest.mark.usefixtures("working_set")
    def test_remove_exclude_non_existing_dev_group_in_lockfile(project, pdm, mocker):
        pdm(["add", "requests"], obj=project, strict=True)
        project.add_dependencies(["pytz"], to_group="tz", dev=True)
        assert project.lockfile.groups == ["default"]
        result = pdm(["remove", "requests"], obj=project)
>       assert result.exit_code == 0
E       AssertionError: assert 1 == 0
E        +  where 1 = RunResult(exit_code=1, stdout='Removing packages from default dependencies: requests\nChanges are written to pyproject....\n  0:00:00 🔒 Lock successful.  \n', stderr='[PdmUsageError]: Requested groups not in lockfile: tz\n', exception=None).exit_code

tests/cli/test_remove.py:115: AssertionError
------------------------------------------------------------------------------- Captured stdout setup -------------------------------------------------------------------------------
Changes are written to pyproject.toml.
Changes are written to pyproject.toml.
------------------------------------------------------------------------------- Captured stdout call --------------------------------------------------------------------------------
Changes are written to pyproject.toml.
--------------------------------------------------------------------------------- Captured log call ---------------------------------------------------------------------------------
INFO     pdm.termui:reporters.py:23 ======== Start resolving requirements ========
INFO     pdm.termui:reporters.py:117   requests
INFO     pdm.termui:reporters.py:36   Adding requirement python>=3.7
INFO     pdm.termui:reporters.py:36   Adding requirement requests
INFO     pdm.termui:reporters.py:23 ======== Starting round 0 ========
INFO     pdm.termui:reporters.py:146 Adding new pin: python None
INFO     pdm.termui:reporters.py:23 ======== Starting round 1 ========
INFO     pdm.termui:reporters.py:36   Adding requirement certifi>=2017.4.17(from requests 2.19.1)
INFO     pdm.termui:reporters.py:36   Adding requirement chardet<3.1.0,>=3.0.2(from requests 2.19.1)
INFO     pdm.termui:reporters.py:36   Adding requirement idna<2.8,>=2.5(from requests 2.19.1)
INFO     pdm.termui:reporters.py:36   Adding requirement urllib3<1.24,>=1.21.1(from requests 2.19.1)
INFO     pdm.termui:reporters.py:146 Adding new pin: requests 2.19.1
INFO     pdm.termui:reporters.py:23 ======== Starting round 2 ========
INFO     pdm.termui:reporters.py:146 Adding new pin: chardet 3.0.4
INFO     pdm.termui:reporters.py:23 ======== Starting round 3 ========
INFO     pdm.termui:reporters.py:146 Adding new pin: idna 2.7
INFO     pdm.termui:reporters.py:23 ======== Starting round 4 ========
INFO     pdm.termui:reporters.py:146 Adding new pin: urllib3 1.22
INFO     pdm.termui:reporters.py:23 ======== Starting round 5 ========
INFO     pdm.termui:reporters.py:146 Adding new pin: certifi 2018.11.17
INFO     pdm.termui:reporters.py:23 ======== Starting round 6 ========
INFO     pdm.termui:reporters.py:23 ======== Resolution Result ========
INFO     pdm.termui:reporters.py:49     python None
INFO     pdm.termui:reporters.py:49   requests 2.19.1
INFO     pdm.termui:reporters.py:49    chardet 3.0.4
INFO     pdm.termui:reporters.py:49       idna 2.7
INFO     pdm.termui:reporters.py:49    urllib3 1.22
INFO     pdm.termui:reporters.py:49    certifi 2018.11.17
INFO     unearth.preparer:preparer.py:329 The file /tmp/pytest-of-huxuan/pytest-4/test_remove_exclude_non_existi0 is a local directory, use it directly
INFO     pdm.termui:reporters.py:23 ======== Start resolving requirements ========
INFO     pdm.termui:reporters.py:36   Adding requirement python>=3.7
INFO     pdm.termui:reporters.py:23 ======== Starting round 0 ========
INFO     pdm.termui:reporters.py:146 Adding new pin: python None
INFO     pdm.termui:reporters.py:23 ======== Starting round 1 ========
INFO     pdm.termui:reporters.py:23 ======== Resolution Result ========
INFO     pdm.termui:reporters.py:49   python None
============================================================================== short test summary info ==============================================================================
FAILED tests/cli/test_remove.py::test_remove_exclude_non_existing_dev_group_in_lockfile - AssertionError: assert 1 == 0
 +  where 1 = RunResult(exit_code=1, stdout='Removing packages from default dependencies: requests\nChanges are written to pyproject....\n  0:00:00 🔒 Lock successful.  \n', stderr='[PdmUsageError]: Requested groups not in lockfile: tz\n', exception=None).exit_code
========================================================================= 1 failed, 986 deselected in 1.84s =========================================================================

@huxuan huxuan changed the title Xuan.hu/intersect translated groups fix: excluding non-existing groups for pdm remove Mar 11, 2025
@codecov Codecov
Copy link

codecov bot commented Mar 11, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.28%. Comparing base (0531be8) to head (446e388).

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #3420      +/-   ##
==========================================
- Coverage   85.52%   85.28%   -0.24%     
==========================================
  Files         112      112              
  Lines       11509    11512       +3     
  Branches     2526     2528       +2     
==========================================
- Hits         9843     9818      -25     
- Misses       1140     1169      +29     
+ Partials      526      525       -1
Flag Coverage Δ
unittests 85.08% <100.00%> (-0.24%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@huxuan huxuan mentioned this pull request Mar 11, 2025
Closed
2 tasks
@frostming
Copy link
Collaborator

No I mean you only need to intersect for this two branches

pdm/src/pdm/cli/filters.py

Lines 88 to 92 in c582a94

elif dev:
groups_set.update(dev_groups)
if ":all" in groups:
groups_set.discard(":all")
groups_set.update(optional_groups)

Otherwise, the groups are given explicitly by name and shouldn't do intersect to miss user errors.

And no need to use a switch, just always intersect

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

pdm remove will raise PdmUsageError when spliting prod and dev dependencies in different lockfiles.
2 participants
@huxuan @frostming