style: apply safe ruff lint fixes repo-wide#4755
Open
rnetser wants to merge 3 commits intoRedHatQE:mainfrom
Open
style: apply safe ruff lint fixes repo-wide#4755rnetser wants to merge 3 commits intoRedHatQE:mainfrom
rnetser wants to merge 3 commits intoRedHatQE:mainfrom
Conversation
Apply automated ruff lint fixes across 33 files:
- SIM118: remove redundant .keys() in dict membership tests
- SIM201: not x == y to x != y
- PLC0206: dict iteration with .items() instead of bracket access
- PERF102: .items() to .values() where only value used
- C403: set([...]) to {...}
- C408: dict(...) to {...}
- C414: sorted(list(...)) to sorted(...)
- UP031: % format to f-string
- B008: mutable default arg to module constant
- LOG015: logging.error() to LOGGER.error()
Assisted-by: Claude <noreply@anthropic.com>
Signed-off-by: rnetser <rnetser@redhat.com>
Contributor
📝 WalkthroughWalkthroughThis PR applies systematic code quality improvements across the test suite and utilities, primarily removing redundant ChangesDictionary Membership & Code Style Refactoring
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
github-actions
Bot
added
sig-network
sig-iuo
sig-infra
sig-scale
sig-storage
sig-virt
labels
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4755 +/- ##
==========================================
+ Coverage 98.63% 98.67% +0.03%
==========================================
Files 25 25
Lines 2420 2485 +65
==========================================
+ Hits 2387 2452 +65
Misses 33 33
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:running
Collaborator
Author
|
/build-and-push-container |
|
Report bugs in Issues Welcome! 🎉This pull request will be automatically processed with the following features: 🔄 Automatic Actions
📋 Available CommandsPR Status Management
Review & Approval
Testing & Validation
Container Operations
Cherry-pick Operations
Label Management
✅ Merge RequirementsThis PR will be automatically approved when the following conditions are met:
📊 Review ProcessApprovers and ReviewersApprovers:
Reviewers:
Available Labels
AI Features
💡 Tips
For more information, please refer to the project documentation or contact the maintainers. |
openshift-virtualization-qe-bot-4
requested review from
EdDev,
SamAlber,
albarker-rh,
dalia-frank,
dshchedr,
kshvaika and
rlobillo
openshift-virtualization-qe-bot-4
requested review from
Ahmad-Hafe,
Anatw,
acinko-rh,
azhivovk,
ema-aka-young,
guchen11,
jpeimer,
kbidarkar and
yossisegev
…zation-tests into chore/safe-ruff-fixes
|
Clean rebase detected — no code changes compared to previous head ( |
Collaborator
Author
|
/build-and-push-container |
Contributor
|
New container for quay.io/openshift-cnv/openshift-virtualization-tests:pr-4755 published |
1 similar comment
Contributor
|
New container for quay.io/openshift-cnv/openshift-virtualization-tests:pr-4755 published |
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:running
Contributor
|
/retest all Auto-triggered: Files in this PR were modified by merged PR #4756. Overlapping filesutilities/virt.py |
…zation-tests into chore/safe-ruff-fixes
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/tests_analyzer/pytest_marker_analyzer.py`:
- Around line 2013-2014: The condition in _analyze_single_test_dependencies was
corrected to skip non-Python files and avoid revisiting files (the check using
dep_file and visited was inverted previously), which changes behavior and fixes
a bug that caused Python files to be skipped; update the PR description to
explicitly state this is a behavior-changing bug fix (not just a lint refactor),
explain that the fix is in pytest_marker_analyzer.py within
_analyze_single_test_dependencies and references the dep_file/visited guard, and
call out the impact on transitive dependency traversal and test-impact results
so reviewers give it appropriate scrutiny.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f4b9674e-60e1-45fa-aa27-e90efbefd8ad
📒 Files selected for processing (33)
conftest.pyscripts/quarantine_stats/generate_dashboard.pyscripts/tests_analyzer/pytest_marker_analyzer.pytests/conftest.pytests/infrastructure/golden_images/update_boot_source/test_ssp_data_sources.pytests/infrastructure/instance_types/test_common_vm_preference.pytests/infrastructure/vm_console_proxy/utils.pytests/install_upgrade_operators/csv/csv_permissions_audit/test_csv_permissions_audit.pytests/install_upgrade_operators/json_patch/test_json_patch_annotation_multiple_updates.pytests/install_upgrade_operators/node_component/utils.pytests/install_upgrade_operators/resource_params/utils.pytests/network/kubemacpool/utils.pytests/network/l2_bridge/libl2bridge.pytests/network/sriov/libsriov.pytests/scale/test_scale_benchmark.pytests/storage/cdi_upload/test_upload_virtctl.pytests/storage/golden_image/test_cached_snapshots.pytests/storage/storage_migration/utils.pytests/storage/test_cdi_resources.pytests/utils.pytests/virt/cluster/common_templates/custom_namespace/utils.pytests/virt/cluster/common_templates/general/test_base_template.pytests/virt/cluster/general/mass_machine_type_transition_tests/test_mass_machine_type_transition.pytests/virt/cluster/migration_and_maintenance/test_evictionstrategy.pytests/virt/node/descheduler/conftest.pytests/virt/node/gpu/vgpu/test_rhel_vm_with_vgpu.pytests/virt/node/high_performance_vm/test_numa.pytests/virt/node/hyperv_support/test_hyperv_features_in_vm.pytests/virt/upgrade/utils.pyutilities/infra.pyutilities/network.pyutilities/storage.pyutilities/virt.py
Comment on lines
+2013
to
2014
| if dep_file in visited or dep_file.suffix != ".py": | ||
| continue |
Contributor
There was a problem hiding this comment.
Correct fix — but this is a behavior-changing bug fix, not a lint transformation.
The repaired condition is logically sound:
if dep_file in visited or dep_file.suffix != ".py":
continuedep_file in visited→ prevents re-processing (cycle guard). ✓dep_file.suffix != ".py"→ skips non-Python files that cannot carry importable dependencies. ✓
Before this fix, the inverted predicate caused the transitive traversal to skip every Python file and attempt import extraction on non-Python files (.yaml, .json, etc.) — making the 1–2 level import expansion silently a no-op for .py files.
WHY this matters: The PR description asserts all changes are "behavior-preserving mechanical transformations". This change is not. It alters which files are included in _analyze_single_test_dependencies's transitive dependency closure, directly affecting which tests are considered impacted by a PR. Reviewers should be aware this is a correctness fix bundled into the lint PR.
Please update the PR description to call this out explicitly as a bug fix so approvers review it with the appropriate scrutiny.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/tests_analyzer/pytest_marker_analyzer.py` around lines 2013 - 2014,
The condition in _analyze_single_test_dependencies was corrected to skip
non-Python files and avoid revisiting files (the check using dep_file and
visited was inverted previously), which changes behavior and fixes a bug that
caused Python files to be skipped; update the PR description to explicitly state
this is a behavior-changing bug fix (not just a lint refactor), explain that the
fix is in pytest_marker_analyzer.py within _analyze_single_test_dependencies and
references the dep_file/visited guard, and call out the impact on transitive
dependency traversal and test-impact results so reviewers give it appropriate
scrutiny.
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
5 participants
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.
What this PR does / why we need it:
Apply automated ruff lint fixes across 33 files. All fixes are behavior-preserving mechanical transformations:
.keys()in dict membership testsnot x == y→x != y.items()instead of bracket access.items()→.values()where only value usedset([...])→{...}dict(...)→{...}sorted(list(...))→sorted(...)%format → f-stringlogging.error()→LOGGER.error()Part of the incremental lint cleanup series:
.flake8ignore rules ✅ mergedpyproject.tomlruff ignore rules ✅ mergedAssisted-by: Claude noreply@anthropic.com
Which issue(s) this PR fixes:
Special notes for reviewer:
All changes are automated ruff fixes. No manual edits. Each fix category is a well-known safe transformation.
jira-ticket:
Summary by CodeRabbit
Release Notes