Add vnic_info metric test for NAD reference change#5224
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
Important Review skippedIgnore keyword(s) in the title. ⛔ Ignored keywords (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
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-2
requested review from
azhivovk and
frenzyfriday
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:passed
openshift-virtualization-qe-bot-2
requested review from
Anatw,
EdDev,
dshchedr,
nirdothan,
orelmisan,
rlobillo,
rnetser,
servolkov,
vsibirsk and
yossisegev
|
/build-and-push-container |
|
New container for quay.io/openshift-cnv/openshift-virtualization-tests:pr-5224 published |
|
/verified All tests passed for PR #5224. Execution details |
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: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
# Get current head SHA
HEAD_SHA=$(gh pr view 5224 --repo RedHatQE/openshift-virtualization-tests --json headRefOid --jq '.headRefOid')
echo "HEAD SHA: $HEAD_SHA"
# Get files changed and first changed line
gh pr diff 5224 --repo RedHatQE/openshift-virtualization-tests | head -100
echo "=== Files changed ==="
gh pr view 5224 --repo RedHatQE/openshift-virtualization-tests --json files --jq '.files[].path'Length of output: 4385 🏁 Script executed: #!/bin/bash
set -euo pipefail
HEAD_SHA="1693929620674473d56d3961e744bb996a1dbf89"
echo "=== FULL DIFF ==="
gh pr diff 5224 --repo RedHatQE/openshift-virtualization-tests
echo ""
echo "=== SMOKE MARKED FILES ==="
rg -l '`@pytest.mark.smoke`' tests/ || true
echo ""
echo "=== GATING MARKED FILES ==="
rg -l '`@pytest.mark.gating`' tests/ || true
echo ""
echo "=== update_nad_references callers ==="
rg -n 'update_nad_references' tests/ libs/ || true
echo ""
echo "=== binding_name_and_type_from_vm_or_vmi callers ==="
rg -n 'binding_name_and_type_from_vm_or_vmi' tests/ libs/ || true
echo ""
echo "=== bridge_nad_a and bridge_nad_b fixture definitions ==="
rg -rn 'def bridge_nad_a\|def bridge_nad_b' tests/ || true
echo ""
echo "=== First changed line number (line 18 area in conftest.py) ==="
# Verify the line number for the inline comment
sed -n '15,25p' tests/observability/metrics/conftest.py || trueLength of output: 12817 🏁 Script executed: #!/bin/bash
set -euo pipefail
HEAD_SHA="1693929620674473d56d3961e744bb996a1dbf89"
python3 << 'PYEOF'
import subprocess
import json
comment_body = """## Test Execution Plan
**Run smoke tests: False**
No traced path from any smoke-marked file to the modified symbols (`validate_vnic_info`, `update_nad_references`, `binding_name_and_type_from_vm_or_vmi`). Smoke-marked files are in `tests/storage/`, `tests/virt/`, `tests/install_upgrade_operators/`, and `tests/infrastructure/` — none import or use changed symbols, and no conftest in their hierarchy references them.
**Run gating tests: False**
No gating-marked file is in `tests/observability/`. None of the gating-marked files call `validate_vnic_info` (modified in `utils.py`) or use the new fixtures (`vm_for_nad_swap_test`, `post_nad_swap_vm`, `expected_vnic_info_after_swap`).
---
**Affected tests to run**
New/fixed test (previously stub with `__test__ = False`, now enabled with proper fixtures):
- `tests/observability/metrics/test_vms_metrics.py::TestVmVnicInfo::test_metric_kubevirt_vm_vnic_info_after_nad_swap` (tier2)
- Fixture chain: `post_nad_swap_vm` → `vm_for_nad_swap_test` → `bridge_nad_a` + `update_nad_references()` from `libs.net.vmspec`
- `expected_vnic_info_after_swap` → `binding_name_and_type_from_vm_or_vmi()` from `utils.py`
`validate_vnic_info` in `utils.py` was refactored (comparison moved into retry loop, early return on match). All callers of this function should be re-verified:
- `tests/observability/metrics/test_vms_metrics.py` — all tests that call `validate_vnic_info` are within `TestVmVnicInfo`
Full class to verify the `validate_vnic_info` refactor does not regress existing tests:
- `tests/observability/metrics/test_vms_metrics.py::TestVmVnicInfo`
"""
payload = {
"body": comment_body,
"commit_id": "1693929620674473d56d3961e744bb996a1dbf89",
"path": "tests/observability/metrics/conftest.py",
"line": 18,
"side": "RIGHT"
}
result = subprocess.run(
[
"gh", "api",
"--method", "POST",
"-H", "Accept: application/vnd.github+json",
"/repos/RedHatQE/openshift-virtualization-tests/pulls/5224/comments",
"--input", "-"
],
input=json.dumps(payload),
capture_output=True,
text=True
)
print("STDOUT:", result.stdout[:1000] if result.stdout else "")
print("STDERR:", result.stderr[:500] if result.stderr else "")
print("Return code:", result.returncode)
PYEOFLength of output: 3613 |
|
/retest all Auto-triggered: Files in this PR were modified by merged PR #5316. Overlapping filestests/observability/metrics/conftest.py |
openshift-virtualization-qe-bot-2
added
lgtm-hmeir
and removed
changes-requested-hmeir
labels
|
/approve |
|
Successfully removed PR tag: quay.io/openshift-cnv/openshift-virtualization-tests:pr-5224. |
|
New container for quay.io/openshift-cnv/openshift-virtualization-tests:latest published |
11 participants
What this PR does / why we need it:
Verify that vnic_info metric labels update correctly after swapping
a VM secondary network NAD reference and completing the live migration.
Which issue(s) this PR fixes:
Special notes for reviewer:
jira-ticket:
https://redhat.atlassian.net/browse/CNV-85094