Add fixes for new inc update dep solving default: false - #22280
Conversation
Reviewer's GuideAdds API tests to verify the behavior of incremental content view updates with dependency solving enabled, disabled, and by default, and updates an existing incremental update test to explicitly enable dependency solving so it continues to pass under the new default (False). File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The new
test_positive_inc_update_resolve_dependencieshardcodes the expected RPM NEVRA names for direct and dependency packages; consider deriving these from the FAKE_9 fixtures or a shared helper so the test is less brittle to future fixture/content changes. - The test currently relies on
response['output']['changed_content'][0]always referring to the CVV under test; if the API ever returns multiple entries or changes ordering, this could become flaky—consider filteringchanged_contentbycontent_view_version_idinstead of indexing.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `test_positive_inc_update_resolve_dependencies` hardcodes the expected RPM NEVRA names for direct and dependency packages; consider deriving these from the FAKE_9 fixtures or a shared helper so the test is less brittle to future fixture/content changes.
- The test currently relies on `response['output']['changed_content'][0]` always referring to the CVV under test; if the API ever returns multiple entries or changes ordering, this could become flaky—consider filtering `changed_content` by `content_view_version_id` instead of indexing.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
The
This is only a worry for composite content view versions. |
|
|
PRT Result |
| assert set(added_errata) == set(FAKE_9_YUM_SECURITY_ERRATUM) | ||
|
|
||
| # Packages listed directly in the 3 security errata | ||
| direct_packages = { |
There was a problem hiding this comment.
Same as prev comment.
You could prob use FAKE_0_CUSTOM_PACKAGE_GROUP constants/_init__.py
sambible
left a comment
There was a problem hiding this comment.
Aiden's suggestion is solid, good otherwise.
I just saw your previous comment about the constants, after I posted my comments. Maybe add a comment making it more clear why you hard coded the package names and didn't you anything from the constants. |
|
I'm going to put the packages into constants, test locally, and then merge. |
|
Passed locally and comments settled! |
Problem Statement
Incremental updates no longer use dependency solving by default.
Solution
Adds testing for the new incremental update default for no dependency solving. Also fixes
test_positive_incremental_update_apply_to_envs_cvswhich would fail due to a lack of dependency solving.Related Issues
https://redhat.atlassian.net/browse/SAT-46503
Summary by Sourcery
Add API tests verifying incremental content view updates honor the resolve_dependencies flag and server default, and update existing errata incremental update test to explicitly enable dependency solving.
Tests: