refactor(virt): migrate virt to direct constants submodule imports#5318
Conversation
…orts Replace all `from utilities.constants import X` with `from utilities.constants.<submodule> import X` across 76 files in tests/virt/. Imports of `Images` are intentionally kept via `from utilities.constants import Images` (computed alias in __init__.py; pending circular-import cleanup in utilities/architecture.py). Generated-by: Claude Sonnet 4.6 Signed-off-by: vsibirsk <vsibirsk@redhat.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughImports across ChangesImport path migration to utilities.constants submodules
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsLinked repositories: Your configuration references 1 linked repositories, but your current plan allows 0. Analyzed ``, skipped 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 |
openshift-virtualization-qe-bot
added
the
tox:verify-tc-requirement-polarion:running
|
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
Branch Management
Label Management
✅ Merge RequirementsThis PR will be automatically approved when the following conditions are met:
📊 Review ProcessApprovers and ReviewersApprovers:
Reviewers:
Available Labels
AI Features
Security Checks
💡 Tips
For more information, please refer to the project documentation or contact the maintainers. |
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:passed
|
@coderabbitai full review |
✅ Action performedFull review finished. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@tests/virt/node/workload_density/test_kernel_samepage_merging.py`:
- Around line 12-15: For this import-only refactor that changes imports from
utilities.constants.timeouts without modifying any test logic, validate the
changes by running uv run pytest --collect-only on the
test_kernel_samepage_merging.py module rather than executing the full test
suite. This collect-only approach is sufficient to verify that the import paths
are correctly resolved without the overhead of full test execution.
🪄 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: 52f6cd69-bc70-4cc7-97f5-5f2c4d04d3c2
📒 Files selected for processing (76)
tests/virt/cluster/aaq/conftest.pytests/virt/cluster/aaq/constants.pytests/virt/cluster/aaq/test_arq.pytests/virt/cluster/aaq/utils.pytests/virt/cluster/common_templates/centos/test_centos_os_support.pytests/virt/cluster/common_templates/conftest.pytests/virt/cluster/common_templates/custom_namespace/conftest.pytests/virt/cluster/common_templates/custom_namespace/test_custom_namespace.pytests/virt/cluster/common_templates/custom_namespace/utils.pytests/virt/cluster/common_templates/fedora/test_fedora_os_support.pytests/virt/cluster/common_templates/general/test_base_custom_templates.pytests/virt/cluster/common_templates/general/test_base_template.pytests/virt/cluster/common_templates/rhel/test_rhel_os_support.pytests/virt/cluster/common_templates/rhel/test_rhel_tablet_device.pytests/virt/cluster/common_templates/utils.pytests/virt/cluster/common_templates/windows/test_windows_custom_options.pytests/virt/cluster/common_templates/windows/test_windows_os_support.pytests/virt/cluster/common_templates/windows/test_windows_tablet_device.pytests/virt/cluster/general/mass_machine_type_transition_tests/conftest.pytests/virt/cluster/longevity_tests/constants.pytests/virt/cluster/longevity_tests/utils.pytests/virt/cluster/migration_and_maintenance/rbac_hardening/test_migration_rights.pytests/virt/cluster/migration_and_maintenance/test_evictionstrategy.pytests/virt/cluster/migration_and_maintenance/test_migration_policy.pytests/virt/cluster/operator_tests/test_critical_pods.pytests/virt/cluster/operator_tests/test_ssp_custom_resources.pytests/virt/cluster/sysprep/test_sysprep.pytests/virt/cluster/vm_cloning/conftest.pytests/virt/cluster/vm_cloning/test_vm_cloning.pytests/virt/cluster/vm_cloning/utils.pytests/virt/cluster/vm_lifecycle/test_vm_data_persistency.pytests/virt/cluster/vm_lifecycle/test_vm_run_strategy.pytests/virt/conftest.pytests/virt/node/conftest.pytests/virt/node/descheduler/conftest.pytests/virt/node/descheduler/test_descheduler_psi_metrics.pytests/virt/node/descheduler/utils.pytests/virt/node/general/test_cloud_init_disk_vm.pytests/virt/node/general/test_cloud_init_vm.pytests/virt/node/general/test_disk_io_option.pytests/virt/node/general/test_machinetype.pytests/virt/node/general/test_networkinterfacemultiqueue_feature.pytests/virt/node/general/test_oom.pytests/virt/node/general/test_static_ssh_key_injection.pytests/virt/node/general/test_windows_vtpm_bitlocker.pytests/virt/node/general/test_wsl2.pytests/virt/node/gpu/conftest.pytests/virt/node/gpu/gpu_pci_passthrough/conftest.pytests/virt/node/gpu/gpu_pci_passthrough/test_rhel_vm_with_gpu_pci_passthrough.pytests/virt/node/gpu/utils.pytests/virt/node/high_performance_vm/test_numa.pytests/virt/node/hotplug/test_cpu_memory_hotplug.pytests/virt/node/hyperv_support/test_hyperv_features_in_vm.pytests/virt/node/hyperv_support/test_windows_crash_detection.pytests/virt/node/log_verbosity/test_log_verbosity.pytests/virt/node/log_verbosity/test_log_virt_launcher.pytests/virt/node/migration_and_maintenance/test_node_maintenance.pytests/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/virt/node/migration_and_maintenance/test_post_copy_migration.pytests/virt/node/migration_and_maintenance/test_vm_disk_load_with_migration.pytests/virt/node/migration_and_maintenance/test_vm_memory_load_with_migration.pytests/virt/node/node_labeller/cpu_features/test_node_feature_discovery.pytests/virt/node/node_labeller/cpu_features/test_vm_with_cpu_features.pytests/virt/node/node_labeller/cpu_features/test_vm_with_cpu_flag.pytests/virt/node/node_labeller/test_node_labeller_annotation.pytests/virt/node/readonly_disk/test_vm_with_windows_guest_tools.pytests/virt/node/workload_density/test_kernel_samepage_merging.pytests/virt/node/workload_density/test_swap.pytests/virt/upgrade/conftest.pytests/virt/upgrade/test_upgrade_virt.pytests/virt/upgrade/utils.pytests/virt/upgrade_custom/aaq/conftest.pytests/virt/upgrade_custom/aaq/constants.pytests/virt/upgrade_custom/aaq/test_upgrade_virt_aaq.pytests/virt/upgrade_custom/vgpu/conftest.pytests/virt/utils.py
|
/verified collect only since it's only import path updates (no logic change) |
Test execution plan request detailsCRITICAL: You MUST post an inline review comment on the first changed line of the first file. As an expert software testing engineer, analyze all modified files in this PR and create a targeted test execution plan. Analysis Requirements:
Output rules: Your deliverable: Test Execution Plan
Use these formats:
Real test commands (MANDATORY when changes affect session/runtime code): When the affected code runs at session/collection time (conftest fixtures, pytest plugins,
Example output for a session-startup fix: Guidelines:
Hardware-Related Checks (SR-IOV, GPU, DPDK): When PR modifies fixtures for hardware-specific resources:
CRITICAL WORKFLOW COMPLETION RULES: When responding to this test execution plan request, you MUST follow these rules EXACTLY:
ABSOLUTE PROHIBITIONS (violating these creates empty/meaningless reviews):
Remember: The pull request review is visible to users. Additional comments are redundant noise. CRITICAL — Verification rules for smoke and gating decisions: |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
PR_NUMBER=5318
REPO="RedHatQE/openshift-virtualization-tests"
# Get list of changed files to find the first one
gh pr diff "$PR_NUMBER" --repo "$REPO" --name-only | head -5
echo "---DIFF_HEADER---"
# Get the first changed file and its first changed line number
gh pr diff "$PR_NUMBER" --repo "$REPO" | head -60Length of output: 3215 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Check for smoke markers in changed test files under tests/virt/
rg -l '`@pytest.mark.smoke`' tests/virt/ 2>/dev/null | head -20
echo "---GATING---"
rg -l '`@pytest.mark.gating`' tests/virt/ 2>/dev/null | head -20Length of output: 1687 🏁 Script executed: #!/bin/bash
set -euo pipefail
PR_NUMBER=5318
REPO="RedHatQE/openshift-virtualization-tests"
# Get the list of changed files in this PR
CHANGED_FILES=$(gh pr diff "$PR_NUMBER" --repo "$REPO" --name-only)
echo "Changed files:"
echo "$CHANGED_FILES" | head -10
FIRST_FILE=$(echo "$CHANGED_FILES" | head -1)
echo "First file: $FIRST_FILE"
# Get the diff for the first file to find the first changed line number
gh pr diff "$PR_NUMBER" --repo "$REPO" -- "$FIRST_FILE" 2>/dev/null | head -30Length of output: 1417 🏁 Script executed: #!/bin/bash
set -euo pipefail
PR_NUMBER=5318
REPO="RedHatQE/openshift-virtualization-tests"
# Check if test_container_disk_vm.py is in the PR changes
gh pr diff "$PR_NUMBER" --repo "$REPO" --name-only | grep -E "test_container_disk_vm|test_rhel_os_support" || echo "Not found in PR"Length of output: 488 🏁 Script executed: #!/bin/bash
set -euo pipefail
PR_NUMBER=5318
REPO="RedHatQE/openshift-virtualization-tests"
# Get the commit SHA for the PR head (needed for inline comment)
HEAD_SHA=$(gh pr view "$PR_NUMBER" --repo "$REPO" --json headRefOid --jq '.headRefOid')
echo "Head SHA: $HEAD_SHA"
# Post a non-blocking inline review comment (COMMENT, not REQUEST_CHANGES or APPROVE)
# First changed line in tests/virt/cluster/aaq/conftest.py is position in diff
# The hunk is @@ -23,15 +23,19 @@ — first changed line ("+") is line 26 in the new file
gh api \
--method POST \
"/repos/${REPO}/pulls/${PR_NUMBER}/reviews" \
--field commit_id="$HEAD_SHA" \
--field event="COMMENT" \
--field "comments[][path]=tests/virt/cluster/aaq/conftest.py" \
--field "comments[][line]=26" \
--field "comments[][side]=RIGHT" \
--field "comments[][body]=## Test Execution Plan
**Run smoke tests: True**
Dependency path: \`tests/virt/cluster/common_templates/rhel/test_rhel_os_support.py\` (smoke) → imports \`LINUX_STR\` directly changed from \`utilities.constants\` to \`utilities.constants.instance_types\` in this PR.
**Run gating tests: True**
Dependency paths (gating-marked files directly modified in this PR — import paths changed):
- \`tests/virt/upgrade/test_upgrade_virt.py\`
- \`tests/virt/cluster/vm_lifecycle/test_vm_run_strategy.py\`
- \`tests/virt/cluster/vm_cloning/test_vm_cloning.py\`
- \`tests/virt/cluster/migration_and_maintenance/test_migration_policy.py\`
- \`tests/virt/cluster/migration_and_maintenance/test_evictionstrategy.py\`
- \`tests/virt/node/node_labeller/cpu_features/test_vm_with_cpu_flag.py\`
- \`tests/virt/node/node_labeller/cpu_features/test_node_feature_discovery.py\`
- \`tests/virt/node/general/test_machinetype.py\`
- \`tests/virt/node/general/test_static_ssh_key_injection.py\`
- \`tests/virt/node/general/test_disk_io_option.py\`
**Affected tests to run**
All 76 modified files under \`tests/virt/\` have import-path-only changes (no logic). The PR author already validated collection succeeds:
\`\`\`
pytest tests/virt --collect-only # 535/560 collected ✅
\`\`\`
Prioritized subset for runtime validation:
**Smoke (verify import resolution at runtime):**
- \`tests/virt/cluster/common_templates/rhel/test_rhel_os_support.py\` (smoke)
**Gating (verify import resolution at runtime):**
- \`tests/virt/upgrade/test_upgrade_virt.py\` (gating)
- \`tests/virt/cluster/vm_lifecycle/test_vm_run_strategy.py\` (gating)
- \`tests/virt/cluster/vm_cloning/test_vm_cloning.py\` (gating)
- \`tests/virt/cluster/migration_and_maintenance/test_evictionstrategy.py\` (gating)
- \`tests/virt/node/general/test_machinetype.py\` (gating)
**Notes:**
- No \`utilities/\` or \`libs/\` files were modified — transitive fixture impact is nil.
- \`Images\` is intentionally kept at \`utilities.constants\` (computed alias in \`__init__.py\`); no action needed.
- \`--collect-only\` passed (535/560); collection errors from bad import paths would have surfaced there." \
2>&1Length of output: 4913 |
|
Clean rebase detected — no code changes compared to previous head ( |
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:running
|
Clean rebase detected — no code changes compared to previous head ( |
|
/retest tox |
|
/approve |
|
/lgtm |
|
/retest tox |
|
/retest tox |
|
/approve |
|
/retest tox |
|
New container for quay.io/openshift-cnv/openshift-virtualization-tests:latest published |
10 participants
What this PR does / why we need it:
Replace all
from utilities.constants import Xwithfrom utilities.constants.<submodule> import Xacross 76 files in tests/virt/. Imports ofImagesare intentionally kept viafrom utilities.constants import Images(computed alias in init.py; pending circular-import cleanup in utilities/architecture.py).Generated-by: Claude Sonnet 4.6
Which issue(s) this PR fixes:
Special notes for reviewer:
This is a next PR in a series.
https://redhat.atlassian.net/browse/CNV-80952 and a follow up of #5188
jira-ticket:
https://redhat.atlassian.net/browse/CNV-89564
Summary by CodeRabbit
Release Notes