-
Notifications
You must be signed in to change notification settings - Fork 259
fix: differ between not defined and empty dependency list #892
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
fix: differ between not defined and empty dependency list #892
Conversation
Reviewer's GuideThis PR updates the package dependency setup in Factory._configure_package_dependencies to distinguish explicitly empty dependency lists from entirely missing dependencies, and adds new tests and fixtures to verify behavior when no dependencies are defined. Sequence diagram for dependency configuration logic in Factory._configure_package_dependenciessequenceDiagram
participant Factory
participant Project
participant DependencyGroup
Factory->>Project: get("dependencies")
Factory->>Project: get("optional-dependencies")
Factory->>Project: get("dynamic")
alt dependencies is not None or optional_dependencies exists
Factory->>DependencyGroup: create with MAIN_GROUP and mixed_dynamic
end
Class diagram for updated dependency handling in Factory._configure_package_dependenciesclassDiagram
class Factory {
_configure_package_dependencies(project)
}
class DependencyGroup {
MAIN_GROUP
mixed_dynamic
}
Factory --> DependencyGroup : creates
class Project {
dependencies
optional-dependencies
dynamic
}
Factory ..> Project : reads
class Dependency {}
class NormalizedName {}
DependencyGroup "1" -- "*" Dependency : contains
DependencyGroup "1" -- "*" NormalizedName : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `tests/test_factory.py:316-325` </location>
<code_context>
+def test_create_poetry_with_empty_dependencies() -> None:
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding a test for the case where dependencies are not defined at all.
Please add a test for when the dependencies key is missing from pyproject.toml to ensure both scenarios are covered.
Suggested implementation:
```python
def test_create_poetry_with_empty_dependencies() -> None:
project = "sample_project_new_no_deps"
poetry = Factory().create_poetry(fixtures_dir / project)
assert poetry.is_package_mode
package = poetry.package
assert "main" in package._dependency_groups
assert package._dependency_groups["main"].dependencies == []
def test_create_poetry_with_no_dependencies_key() -> None:
project = "sample_project_new_no_dependencies_key"
poetry = Factory().create_poetry(fixtures_dir / project)
assert poetry.is_package_mode
package = poetry.package
assert "main" in package._dependency_groups
assert package._dependency_groups["main"].dependencies == []
```
You will need to add a fixture directory (e.g., `fixtures_dir / "sample_project_new_no_dependencies_key"`) containing a `pyproject.toml` file that does NOT have a `[tool.poetry.dependencies]` section. This ensures the test is valid and passes.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
d0e821f to
c31dcfc
Compare
c31dcfc to
d1a9f33
Compare
Resolves: python-poetry/poetry#10557
Summary by Sourcery
Fix Factory._configure_package_dependencies to differentiate between an undefined and an explicitly empty dependencies list, ensuring that an empty list still produces a main dependency group with no entries.
Bug Fixes:
Tests: