Repository navigation
Conversation
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="tests/utils/test_extras.py" line_range="48" />
<code_context>
+ # Missing packages do not prevent resolving the available packages
+ (
+ [_PACKAGE_SPAM],
+ {"group0": ["foo", "spam"]},
+ ["group0"],
+ {"spam"},
</code_context>
<issue_to_address>
**Missing-package test misses later results**
When a future change stops traversal when it encounters a missing package, `stack.pop()` visits `spam` before the missing `foo`, so this assertion still passes if resolution returns immediately on a missing package; it does not verify that available packages queued after a miss are resolved.
Reverse the requested package order in this case, or add a case where an available package is popped after a missing one.
</issue_to_address>Sourcery assessment
Approval pending. 1 finding to address first.
Blocking findings: tests/utils/test_extras.py:48
| # Missing packages do not prevent resolving the available packages | ||
| ( | ||
| [_PACKAGE_SPAM], | ||
| {"group0": ["foo", "spam"]}, |
There was a problem hiding this comment.
🟡 Medium · Missing-package test misses later results
When a future change stops traversal when it encounters a missing package, stack.pop() visits spam before the missing foo, so this assertion still passes if resolution returns immediately on a missing package; it does not verify that available packages queued after a miss are resolved.
Reverse the requested package order in this case, or add a case where an available package is popped after a missing one.
Prompt for AI agents
In `tests/utils/test_extras.py` at line 48:
**Missing-package test misses later results**
When a future change stops traversal when it encounters a missing package, `stack.pop()` visits `spam` before the missing `foo`, so this assertion still passes if resolution returns immediately on a missing package; it does not verify that available packages queued after a miss are resolved.
Reverse the requested package order in this case, or add a case where an available package is popped after a missing one.This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Relates-to: #3155
I added four cases to the existing
get_extra_package_namesparametrization. They check that unknown extras are ignored, that available packages are still returned when another requested package is missing, and that a missing transitive dependency does not discard its parent.These tests protect the existing behavior; no production code changes are needed.
Validation on Linux with Python 3.12.14:
Full suite: 3,314 passed, 34 skipped, 2 xfailed, with no unexpected failures. I used
env -u REQUESTS_CA_BUNDLE CC=gcc CXX=g++ .venv/bin/pytest -n 4 --cov=src/poetryto avoid the runner’s injected Requests certificate override and use the available compiler. Global coverage: 94%.Extras tests: 11 passed, with 100% statement and branch coverage of
poetry.utils.extras.Mypy: all 380 source files passed. All applicable pre-commit hooks passed, including Ruff check and format.
Added tests.
Updated documentation — not needed for this test-only change.
CMGeorges
Prepared with Codex assistance.