Compare sizes only for RHEL 10 and higher. No less OS#377
Compare sizes only for RHEL 10 and higher. No less OS#377phracek merged 1 commit intosclorg:masterfrom
Conversation
Signed-off-by: Petr "Stone" Hracek <phracek@redhat.com>
📝 WalkthroughWalkthroughThe test file is refactored to extract Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Pull Request validationFailed🔴 Failed or pending statuses:
🔴 Review - Missing review from a member (1 required) |
Pull Request validationFailed🔴 Review - Missing review from a member (1 required) Success🟢 CI - All checks have passed |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/test_container_sizes.py (1)
36-40: Prefer a threshold check over enumerating old RHEL releases.The requirement is “RHEL 10 and higher,” but the code hard-codes
rhel7/rhel8/rhel9. Comparing the parsed major version makes the intent clearer and avoids stale skip logic if another pre-10 value appears.♻️ Proposed refactor
- if previous_os_version in ["rhel7", "rhel8", "rhel9"]: - pytest.skip("Skipping container size comparison for RHEL 7, 8 and 9.") + previous_os_major = int(previous_os_version[len("rhel") :]) + if previous_os_major < 10: + pytest.skip("Skipping container size comparison against pre-RHEL 10 images.")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/test_container_sizes.py` around lines 36 - 40, The test currently skips specific strings ("rhel7", "rhel8", "rhel9") which can become stale; change the logic in test_container_sizes.py to parse the major version from previous_os_version (e.g., detect the "rhel" prefix, extract the numeric major version) and skip when that major version is < 10 instead of enumerating values; update the block around previous_os_version and the subsequent call to get_public_image_name to use this version check so RHEL 10+ runs while all pre-10 releases are skipped.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@test/test_container_sizes.py`:
- Around line 36-40: The test currently skips specific strings ("rhel7",
"rhel8", "rhel9") which can become stale; change the logic in
test_container_sizes.py to parse the major version from previous_os_version
(e.g., detect the "rhel" prefix, extract the numeric major version) and skip
when that major version is < 10 instead of enumerating values; update the block
around previous_os_version and the subsequent call to get_public_image_name to
use this version check so RHEL 10+ runs while all pre-10 releases are skipped.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 08d265ba-9e21-440d-9041-ecddafd2aa31
📒 Files selected for processing (1)
test/test_container_sizes.py
|
[test-pytest] |
Testing Farm results
|
Summary by CodeRabbit