Test: Added CondaFlowDecorator unit tests#3193
Conversation
Greptile SummaryThis PR fixes a bug in
Confidence Score: 5/5Safe to merge — the production change is a two-line addition that makes CondaFlowDecorator symmetric with CondaStepDecorator, and the tests correctly validate all affected code paths. The fix is minimal and clearly correct: it replicates a pattern already present and well-tested in CondaStepDecorator. All new tests exercise both decorator classes and cover the merge semantics, precedence rules, and user-tracking logic introduced by the fix. No files require special attention. Important Files Changed
Reviews (5): Last reviewed commit: "Clean up conda decorator tests" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Adds unit-level coverage for conda decorators’ attribute normalization/user-defined tracking, and fixes CondaFlowDecorator so legacy libraries inputs correctly mark packages as user-defined after merging.
Changes:
- Fix:
CondaFlowDecorator.init()now markspackagesas user-defined when legacylibrariespopulates it. - Tests: Expand
CondaStepDecoratorunit assertions (merge behavior, precedence, and normalization). - Tests: Add a new
TestCondaFlowDecoratorsuite covering defaults, user-defined tracking, merge behavior, and precedence.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
test/plugins/conda/test_conda_decorator_unit.py |
Adds/expands pure unit tests for CondaStepDecorator and CondaFlowDecorator covering merge/preference and user-defined tracking. |
metaflow/plugins/pypi/conda_decorator.py |
Bug fix to ensure flow-level packages is treated as user-defined when derived from legacy libraries. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3193 +/- ##
=========================================
Coverage ? 28.33%
=========================================
Files ? 381
Lines ? 52355
Branches ? 9243
=========================================
Hits ? 14833
Misses ? 36582
Partials ? 940 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
test/plugins/conda/test_conda_decorator_unit.py:111
- Docstring grammar nit: "'packages' and 'libraries' attribute have" should be pluralized/reworded for clarity (e.g., "When both attributes specify the same package, 'packages' takes precedence.").
def test_packages_precedence(self):
"""When both 'packages' and 'libraries' attribute have same packages, 'packages' attribute takes precedence."""
deco = CondaFlowDecorator(
| def test_default_attributes(self): | ||
| deco = CondaFlowDecorator() | ||
| deco.init() | ||
| assert deco.attributes["packages"] == {} | ||
| assert deco.attributes["python"] is None | ||
| assert not deco.attributes["disabled"] | ||
|
|
| assert deco.attributes["libraries"] == {} | ||
|
|
||
| def test_packages_precedence(self): | ||
| """When both 'packages' and 'libraries' attribute have same packages, 'packages' attribute takes precedence""" |
PR Type
Summary
This PR adds unit tests for the
CondaFlowDecoratorand expands the existing tests forCondaStepDecorator. It also includes a minor bug fix inCondaFlowDecoratorto ensure user-defined packages are properly tracked.CondaFlowDecoratorto correctly track user-definedpackages.CondaFlowDecorator.TestCondaStepDecoratorforpackagescorrectly take precedence overlibraries.Tests