refactor(constants): migrate tests/infrastructure imports to submodules#5335
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (32)
📝 WalkthroughWalkthroughImports across 32 ChangesImport 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)
⚔️ Resolve merge conflicts
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 |
|
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. |
Replace from utilities.constants import with direct submodule imports across 32 infrastructure test files. Images kept on compat import where required. Generated-by: Claude Sonnet 4.6 Signed-off-by: vsibirsk <vsibirsk@redhat.com>
|
/test-plan |
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
# Check for smoke markers in the affected test directory
echo "=== Smoke marked files in tests/infrastructure/ ==="
rg -l '`@pytest.mark.smoke`' tests/infrastructure/ 2>/dev/null || echo "None found"
echo ""
echo "=== Gating marked files in tests/infrastructure/ ==="
rg -l '`@pytest.mark.gating`' tests/infrastructure/ 2>/dev/null || echo "None found"
echo ""
echo "=== Check if any modified conftest files have autouse fixtures ==="
for f in tests/infrastructure/conftest.py \
tests/infrastructure/golden_images/conftest.py \
tests/infrastructure/golden_images/update_boot_source/conftest.py \
tests/infrastructure/instance_types/conftest.py \
tests/infrastructure/instance_types/supported_os/conftest.py \
tests/infrastructure/numa/conftest.py \
tests/infrastructure/tekton/conftest.py \
tests/infrastructure/vm_console_proxy/conftest.py; do
if [ -f "$f" ]; then
echo "--- $f autouse fixtures ---"
rg 'autouse' "$f" || echo " No autouse"
fi
done
echo ""
echo "=== Verify all new submodule imports exist ==="
python3 -c "
import importlib, sys
modules = [
'utilities.constants.hco',
'utilities.constants.images',
'utilities.constants.timeouts',
'utilities.constants.storage',
'utilities.constants.instance_types',
'utilities.constants.os_matrix',
'utilities.constants.components',
'utilities.constants.virt',
'utilities.constants.tekton',
'utilities.constants.cluster',
'utilities.constants.architecture',
'utilities.constants.pytest',
'utilities.constants.networking',
'utilities.virt',
]
sys.path.insert(0, '.')
for m in modules:
try:
importlib.import_module(m)
print(f'OK: {m}')
except ImportError as e:
print(f'MISSING: {m} -> {e}')
" 2>/dev/null || echo "Python import check skipped (no interpreter or path issues)"
echo ""
echo "=== List all files under utilities/constants/ to verify submodule structure ==="
fd . utilities/constants/ --type f -e py 2>/dev/null | sort || ls utilities/constants/ 2>/dev/null || echo "Path not found"Length of output: 5236 🏁 Script executed: #!/bin/bash
# Get PR files to find the first changed file and its first changed line
gh api repos/RedHatQE/openshift-virtualization-tests/pulls/5335/files \
--jq '.[0] | {filename: .filename, patch: .patch}' 2>/dev/null | head -50
echo ""
echo "=== Get commit SHA for inline comment ==="
gh api repos/RedHatQE/openshift-virtualization-tests/pulls/5335 \
--jq '.head.sha' 2>/dev/nullLength of output: 1081 🏁 Script executed: #!/bin/bash
# Post a non-blocking inline review comment on the first changed line of tests/infrastructure/conftest.py
BODY='## Test Execution Plan
**Run smoke tests: True**
Dependency path: `tests/infrastructure/instance_types/supported_os/test_rhel_os.py` (`@pytest.mark.smoke`) is **directly modified** (import refactored from `utilities.constants` → `utilities.constants.instance_types`) **AND** `tests/infrastructure/conftest.py` (session-scoped `autouse=True` fixture) is modified — an `ImportError` here aborts the entire session before any test runs.
**Run gating tests: True**
Dependency path: `test_common_vm_instancetype.py` and `test_common_vm_preference.py` (both `@pytest.mark.gating`) are **directly modified** (import refactored from `utilities.constants` → `utilities.constants.components`). `test_rhel_os.py` (also gating + smoke) is directly modified. `test_update_vm_with_instancetype_preference.py` (gating) is affected via the modified parent `tests/infrastructure/instance_types/conftest.py`.
---
**Affected tests to run**
| File | Marker |
|---|---|
| `tests/infrastructure/instance_types/supported_os/test_rhel_os.py` | smoke + gating |
| `tests/infrastructure/instance_types/test_common_vm_instancetype.py` | gating |
| `tests/infrastructure/instance_types/test_common_vm_preference.py` | gating |
| `tests/infrastructure/instance_types/test_update_vm_with_instancetype_preference.py` | gating (via conftest) |
All remaining files under `tests/infrastructure/` are affected at collection time via the session-scoped `autouse` fixture in `tests/infrastructure/conftest.py`.
---
**Real tests (cluster required)**
The `tests/infrastructure/conftest.py` session-scoped `autouse` fixture runs at collection time. A wrong import path causes an `ImportError` that aborts collection for the full session.
Error path (verify imports resolve correctly):Length of output: 2275 🏁 Script executed: #!/bin/bash
cat > /tmp/plan.md << 'ENDOFPLAN'
## Test Execution Plan
**Run smoke tests: True**
Dependency path: `tests/infrastructure/instance_types/supported_os/test_rhel_os.py` (`@pytest.mark.smoke`) is **directly modified** (import refactored from `utilities.constants` → `utilities.constants.instance_types`) **AND** `tests/infrastructure/conftest.py` (session-scoped `autouse=True` fixture) is modified — an `ImportError` here aborts the entire session before any test runs.
**Run gating tests: True**
Dependency path: `test_common_vm_instancetype.py` and `test_common_vm_preference.py` (both `@pytest.mark.gating`) are **directly modified** (import refactored from `utilities.constants` → `utilities.constants.components`). `test_rhel_os.py` (also gating + smoke) is directly modified. `test_update_vm_with_instancetype_preference.py` (gating) is affected via the modified parent `tests/infrastructure/instance_types/conftest.py`.
---
**Affected tests to run**
| File | Marker |
|---|---|
| `tests/infrastructure/instance_types/supported_os/test_rhel_os.py` | smoke + gating |
| `tests/infrastructure/instance_types/test_common_vm_instancetype.py` | gating |
| `tests/infrastructure/instance_types/test_common_vm_preference.py` | gating |
| `tests/infrastructure/instance_types/test_update_vm_with_instancetype_preference.py` | gating (via conftest) |
All remaining files under `tests/infrastructure/` are affected at collection time via the session-scoped `autouse` fixture in `tests/infrastructure/conftest.py`.
---
**Real tests (cluster required)**
`tests/infrastructure/conftest.py` has a session-scoped `autouse` fixture that runs at collection time. A wrong import path causes an `ImportError` that aborts collection for the full session.
Error path (verify imports resolve correctly):Length of output: 2040 🏁 Script executed: #!/bin/bash
python3 << 'PYEOF'
import subprocess, json, sys
body = """## Test Execution Plan
**Run smoke tests: True**
Dependency path: `tests/infrastructure/instance_types/supported_os/test_rhel_os.py` (`@pytest.mark.smoke`) is **directly modified** (import refactored from `utilities.constants` \u2192 `utilities.constants.instance_types`) **AND** `tests/infrastructure/conftest.py` (session-scoped `autouse=True` fixture) is modified \u2014 an `ImportError` here aborts the entire session before any test runs.
**Run gating tests: True**
Dependency path: `test_common_vm_instancetype.py` and `test_common_vm_preference.py` (both `@pytest.mark.gating`) are **directly modified** (import refactored from `utilities.constants` \u2192 `utilities.constants.components`). `test_rhel_os.py` (also gating + smoke) is directly modified. `test_update_vm_with_instancetype_preference.py` (gating) is affected via the modified parent `tests/infrastructure/instance_types/conftest.py`.
---
**Affected tests to run**
| File | Marker |
|---|---|
| `tests/infrastructure/instance_types/supported_os/test_rhel_os.py` | smoke + gating |
| `tests/infrastructure/instance_types/test_common_vm_instancetype.py` | gating |
| `tests/infrastructure/instance_types/test_common_vm_preference.py` | gating |
| `tests/infrastructure/instance_types/test_update_vm_with_instancetype_preference.py` | gating (via conftest) |
All remaining files under `tests/infrastructure/` are affected at collection time via the session-scoped `autouse` fixture in `tests/infrastructure/conftest.py`.
---
**Real tests (cluster required)**
`tests/infrastructure/conftest.py` has a session-scoped `autouse` fixture that runs at collection time. A wrong import path causes an `ImportError` that aborts collection for the full session.
Error path (verify imports resolve correctly):
pytest tests/infrastructure/ --collect-only -q
Expected: all modules listed with no `ImportError` or `ModuleNotFoundError`
Happy path (smoke + gating regression):
pytest tests/infrastructure/instance_types/ -m "smoke or gating" -v
Expected: all previously-passing tests collect and pass"""
payload = {
"commit_id": "0d59bec95fd6e9f618120a80ea0192004064bfc7",
"body": "",
"event": "COMMENT",
"comments": [
{
"path": "tests/infrastructure/conftest.py",
"line": 15,
"side": "RIGHT",
"body": body
}
]
}
with open('/tmp/payload.json', 'w') as f:
json.dump(payload, f)
result = subprocess.run(
['gh', 'api',
'repos/RedHatQE/openshift-virtualization-tests/pulls/5335/reviews',
'--method', 'POST',
'--input', '/tmp/payload.json',
'--jq', '.id'],
capture_output=True, text=True
)
print("stdout:", result.stdout)
print("stderr:", result.stderr)
print("returncode:", result.returncode)
PYEOFLength of output: 3042 |
0d59bec to
83bf615
Compare
|
Clean rebase detected — no code changes compared to previous head ( |
|
/retest tox |
|
/retest tox |
|
Clean rebase detected — no code changes compared to previous head ( |
|
/check-can-merge |
|
/approve |
|
/lgtm |
|
New container for quay.io/openshift-cnv/openshift-virtualization-tests:latest published |
check now, you should have access to linked repositories |
🧩 Analysis chain⚙️ CodeRabbit configuration inspected Length of output: 11794
Since I can't directly verify quota changes from here, the most reliable way to confirm is to trigger a fresh full review — if the entitlement has been updated on your account/plan, the next run should pick it up and analyze I'll kick off a full review now to re-check: ✅ Action performedFull review finished. |
What this PR does / why we need it:
Replace from utilities.constants import with direct submodule imports across 32 infrastructure test files. Images kept on compat import where required.
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-91055
Summary by CodeRabbit