util: remove fields with the same default values in infrastructure and utilities#5219
Merged
Conversation
Signed-off-by: rkishner <rkishner@redhat.com>
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? |
Contributor
|
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 (3)
💤 Files with no reviewable changes (2)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (18)
🧰 Additional context used📓 Path-based instructions (4)**/*.py📄 CodeRabbit inference engine (AGENTS.md)
Files:
utilities/unittests/**/*.py📄 CodeRabbit inference engine (Custom checks)
Files:
utilities/unittests/**⚙️ CodeRabbit configuration file
Files:
**⚙️ CodeRabbit configuration file
Files:
⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (34)📚 Learning: 2026-01-07T11:51:57.253ZApplied to files:
📚 Learning: 2026-01-12T11:24:13.825ZApplied to files:
📚 Learning: 2026-01-12T14:25:05.723ZApplied to files:
📚 Learning: 2026-01-20T01:03:13.139ZApplied to files:
📚 Learning: 2026-01-21T21:26:41.805ZApplied to files:
📚 Learning: 2026-01-25T13:18:21.675ZApplied to files:
📚 Learning: 2026-02-18T06:35:39.536ZApplied to files:
📚 Learning: 2026-02-23T16:33:22.070ZApplied to files:
📚 Learning: 2026-03-17T01:32:02.617ZApplied to files:
📚 Learning: 2026-03-17T01:32:02.617ZApplied to files:
📚 Learning: 2026-05-04T13:45:29.122ZApplied to files:
📚 Learning: 2026-05-04T13:45:33.892ZApplied to files:
📚 Learning: 2026-05-05T17:01:15.294ZApplied to files:
📚 Learning: 2026-05-08T12:49:20.694ZApplied to files:
📚 Learning: 2026-05-12T05:10:24.601ZApplied to files:
📚 Learning: 2026-05-13T19:23:09.603ZApplied to files:
📚 Learning: 2026-05-18T06:30:56.781ZApplied to files:
📚 Learning: 2026-05-18T06:31:12.015ZApplied to files:
📚 Learning: 2026-05-18T06:31:15.083ZApplied to files:
📚 Learning: 2026-05-18T06:31:20.848ZApplied to files:
📚 Learning: 2026-05-18T09:09:09.479ZApplied to files:
📚 Learning: 2026-05-19T07:48:17.119ZApplied to files:
📚 Learning: 2026-05-19T07:48:17.119ZApplied to files:
📚 Learning: 2026-05-26T15:52:31.613ZApplied to files:
📚 Learning: 2026-01-19T07:53:55.780ZApplied to files:
📚 Learning: 2026-02-18T06:34:38.042ZApplied to files:
📚 Learning: 2026-02-23T21:16:51.920ZApplied to files:
📚 Learning: 2026-05-03T15:38:22.954ZApplied to files:
📚 Learning: 2026-05-18T06:31:30.830ZApplied to files:
📚 Learning: 2026-05-03T15:38:09.624ZApplied to files:
📚 Learning: 2026-05-15T18:42:02.504ZApplied to files:
📚 Learning: 2026-05-18T10:38:33.820ZApplied to files:
📚 Learning: 2026-06-01T06:43:07.271ZApplied to files:
📚 Learning: 2026-06-15T10:56:21.758ZApplied to files:
🔇 Additional comments (1)
📝 WalkthroughWalkthroughSix call sites across ChangesDefault Argument Removal
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested reviewers
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (4 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 |
Contributor
|
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
the
tox:verify-tc-requirement-polarion:running
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:passed
Signed-off-by: rkishner <rkishner@redhat.com>
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:running
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5219 +/- ##
==========================================
- Coverage 98.67% 98.65% -0.02%
==========================================
Files 25 25
Lines 2487 2461 -26
==========================================
- Hits 2454 2428 -26
Misses 33 33
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:passed
Contributor
Author
|
/verified |
Contributor
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}
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== Step 6a: Modified symbols ==="
echo "--- utilities/bitwarden.py ---"
rg -n "def _run_bws_cli" utilities/bitwarden.py || true
echo "--- utilities/cluster.py ---"
rg -n "def get_oc_whoami_username\|def _whoami" utilities/cluster.py || true
echo "--- utilities/network.py ---"
rg -n "def enable_hyperconverged_ovs_annotations\|def wait_for_ovs_status" utilities/network.py || true
echo "--- utilities/oadp.py ---"
rg -n "def create_rhel_vm" utilities/oadp.py || true
echo "--- utilities/virt.py ---"
rg -n "def pause_unpause_vm_and_check_connectivity" utilities/virt.py || true
echo ""
echo "=== Step 6b: Find callers in tests/ ==="
echo "--- callers of get_oc_whoami_username ---"
rg -rn "get_oc_whoami_username" tests/ || echo "none"
echo "--- callers of enable_hyperconverged_ovs_annotations ---"
rg -rn "enable_hyperconverged_ovs_annotations" tests/ || echo "none"
echo "--- callers of create_rhel_vm ---"
rg -rn "create_rhel_vm" tests/ || echo "none"
echo "--- callers of pause_unpause_vm_and_check_connectivity ---"
rg -rn "pause_unpause_vm_and_check_connectivity" tests/ || echo "none"
echo "--- callers of _run_bws_cli ---"
rg -rn "_run_bws_cli" tests/ || echo "none"
echo "--- callers of wait_for_final_status_pipelinerun (changed in conftest) ---"
rg -rn "wait_for_final_status_pipelinerun\|final_status_pipelinerun" tests/ || echo "none"
echo ""
echo "=== Step 6d: Same-file callers ==="
echo "--- callers in utilities/network.py ---"
rg -n "enable_hyperconverged_ovs_annotations" utilities/ || echo "none"
echo "--- callers in utilities/oadp.py ---"
rg -n "create_rhel_vm" utilities/ || echo "none"
echo "--- callers in utilities/virt.py ---"
rg -n "pause_unpause_vm_and_check_connectivity" utilities/ || echo "none"
echo ""
echo "=== Step 7: Smoke tests ==="
rg -l "`@pytest.mark.smoke`" tests/ | head -50 || echo "none"
echo ""
echo "=== Step 8: Gating tests ==="
rg -l "`@pytest.mark.gating`" tests/ | head -50 || echo "none"Length of output: 9053 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== Check root conftest.py for autouse fixtures using modified symbols ==="
rg -n "autouse\|enable_hyperconverged_ovs_annotations\|get_oc_whoami_username" tests/conftest.py | head -60
echo ""
echo "=== Check tests/virt/utils.py callers in smoke/gating tests ==="
# Get the function name from virt/utils.py that calls pause_unpause_vm_and_check_connectivity
rg -n "pause_unpause_vm_and_check_connectivity" tests/virt/utils.py
echo ""
echo "=== Check if virt/utils.py functions are used in smoke tests ==="
# Get the function in virt/utils.py that wraps pause_unpause_vm_and_check_connectivity
VIRT_WRAPPER=$(rg -B5 "pause_unpause_vm_and_check_connectivity" tests/virt/utils.py | grep "^def\|^ def" | head -1 | awk '{print $2}' | cut -d'(' -f1)
echo "Wrapper function: $VIRT_WRAPPER"
if [ -n "$VIRT_WRAPPER" ]; then
rg -rn "$VIRT_WRAPPER" tests/ | grep -v "virt/utils.py" | head -20 || echo "no callers found"
fi
echo ""
echo "=== Check if GPU tests are smoke or gating marked ==="
rg -n "`@pytest.mark.smoke`\|`@pytest.mark.gating`" tests/virt/node/gpu/vgpu/test_rhel_vm_with_vgpu.py | head -5 || echo "not smoke/gating"
rg -n "`@pytest.mark.smoke`\|`@pytest.mark.gating`" tests/virt/node/gpu/gpu_pci_passthrough/test_rhel_vm_with_gpu_pci_passthrough.py | head -5 || echo "not smoke/gating"
echo ""
echo "=== Check tekton tests using final_status_pipelinerun fixture ==="
fd "test_" tests/infrastructure/tekton/ --extension py | xargs rg -l "final_status_pipelinerun" 2>/dev/null || echo "none"
echo ""
echo "=== Check if data_protection/oadp or chaos/oadp have smoke/gating tests ==="
rg -rn "`@pytest.mark.smoke`\|`@pytest.mark.gating`" tests/data_protection/oadp/ | head -10 || echo "none"
rg -rn "`@pytest.mark.smoke`\|`@pytest.mark.gating`" tests/chaos/oadp/ | head -10 || echo "none"
echo ""
echo "=== Check the enable_hyperconverged_ovs_annotations fixture context in conftest ==="
rg -n -B2 -A5 "enable_hyperconverged_ovs_annotations" tests/conftest.py
echo ""
echo "=== Check the get_oc_whoami_username fixture context in conftest ==="
rg -n -B2 -A5 "get_oc_whoami_username\|whoami" tests/conftest.py | head -40Length of output: 2472 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== Check tests/conftest.py for autouse and fixture definitions ==="
grep -n "autouse\|`@pytest.fixture`" tests/conftest.py | head -60
echo ""
echo "=== Lines around enable_hyperconverged_ovs_annotations import/usage ==="
grep -n "ovs\|hyperconverged\|network" tests/conftest.py | head -30
echo ""
echo "=== Lines around whoami/cluster import/usage ==="
grep -n "whoami\|cluster\|username" tests/conftest.py | head -30
echo ""
echo "=== Check tekton conftest for final_status_pipelinerun fixture definition and usage ==="
grep -n "final_status_pipelinerun\|autouse\|`@pytest.fixture`\|wait_timeout\|sleep_interval" tests/infrastructure/tekton/conftest.py | head -30
echo ""
echo "=== List tekton test files ==="
fd "test_" tests/infrastructure/tekton/ --extension py
echo ""
echo "=== Check virt/utils.py around pause_unpause ==="
grep -n -B3 -A3 "pause_unpause_vm_and_check_connectivity" tests/virt/utils.py | head -30
echo ""
echo "=== Check if pause_unpause_vm_and_check_connectivity callers in tests/virt/utils.py are in smoke/gating tests ==="
# Find the wrapper function name first
grep -n "def.*pause\|def.*vm_connectivity\|def.*pause_unpause" tests/virt/utils.py | head -10
echo ""
echo "=== Check data_protection conftest ==="
grep -n "create_rhel_vm\|autouse\|`@pytest.fixture`" tests/data_protection/oadp/conftest.py | head -20
echo ""
echo "=== Check chaos/oadp conftest ==="
grep -n "create_rhel_vm\|autouse\|`@pytest.fixture`" tests/chaos/oadp/conftest.py | head -20Length of output: 3933 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== Get PR files and their first changed lines ==="
gh api repos/RedHatQE/openshift-virtualization-tests/pulls/5219/files \
--jq '.[] | {filename: .filename, patch: (.patch // "no patch"), sha: .sha}' | head -100
echo ""
echo "=== Get PR commit SHA for inline comment ==="
gh api repos/RedHatQE/openshift-virtualization-tests/pulls/5219 \
--jq '{head_sha: .head.sha, base_sha: .base.sha}'
echo ""
echo "=== Get the diff for the first file to find the first changed line ==="
gh api repos/RedHatQE/openshift-virtualization-tests/pulls/5219/files \
--jq '.[0] | {filename: .filename, patch: .patch}'Length of output: 6754 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Post inline review comment on the first changed line of the first file
# First file: tests/infrastructure/tekton/conftest.py
# First changed line: line 31 (TIMEOUT_1MIN removal, LEFT side)
# Head SHA: c7d8ed1a2d9bd8f4864f4c92c73873cb44061c3e
COMMENT_BODY="## Test Execution Plan
**Run smoke tests: False**
No concrete path traced from any smoke-marked test to any modified symbol.
**Run gating tests: False**
No concrete path traced from any gating-marked test to any modified symbol.
---
### Affected tests to run
**Tekton pipeline tests** (fixture \`final_status_pipelinerun\` no longer overrides \`wait_timeout\`/\`sleep_interval\` — now uses defaults from \`wait_for_final_status_pipelinerun\`):
\`\`\`
tests/infrastructure/tekton/
\`\`\`
**OADP / data protection tests** (\`create_rhel_vm\` → \`running_vm\` no longer passes \`wait_for_interfaces=True\`; behavioral change if default ≠ True):
\`\`\`
tests/data_protection/oadp/
tests/chaos/oadp/
\`\`\`
**GPU tests — hardware required** (\`pause_unpause_vm_and_check_connectivity\` → \`wait_for_ssh_connectivity\` now uses its default timeout instead of \`TIMEOUT_2MIN\`):
\`\`\`
tests/virt/node/gpu/vgpu/test_rhel_vm_with_vgpu.py
tests/virt/node/gpu/gpu_pci_passthrough/test_rhel_vm_with_gpu_pci_passthrough.py
\`\`\`
Hardware note: verify collection succeeds on clusters **without** GPU hardware (fixtures must tolerate absence).
**Unit tests** (already updated in this PR — run to confirm):
\`\`\`
utilities/unittests/test_bitwarden.py
utilities/unittests/test_cluster.py
utilities/unittests/test_oadp.py
\`\`\`
---
### Real tests (cluster required)
**Happy path — OADP \`create_rhel_vm\` with \`wait_running=True\`** (verify \`running_vm\` default for \`wait_for_interfaces\` is acceptable):
\`\`\`
pytest tests/data_protection/oadp/ -k \"rhel\" -x
\`\`\`
**Happy path — Tekton \`final_status_pipelinerun\` default timeouts are sufficient:**
\`\`\`
pytest tests/infrastructure/tekton/ -x
\`\`\`
**Happy path — SSH connectivity after pause/unpause within default timeout:**
\`\`\`
pytest tests/virt/node/gpu/vgpu/test_rhel_vm_with_vgpu.py -x
\`\`\`
(requires cluster with vGPU hardware)"
gh api \
repos/RedHatQE/openshift-virtualization-tests/pulls/5219/reviews \
--method POST \
--field commit_id="c7d8ed1a2d9bd8f4864f4c92c73873cb44061c3e" \
--field event="COMMENT" \
--field body="" \
--field "comments[][path]=tests/infrastructure/tekton/conftest.py" \
--field "comments[][line]=31" \
--field "comments[][side]=LEFT" \
--field "comments[][body]=$COMMENT_BODY"Length of output: 4372 |
Contributor
|
/lgtm |
Contributor
|
/retest all Auto-triggered: Files in this PR were modified by merged PR #4873. Overlapping filesutilities/network.py |
vsibirsk
approved these changes
Jun 17, 2026
Collaborator
|
/approve |
This was referenced Jun 17, 2026
|
New container for quay.io/openshift-cnv/openshift-virtualization-tests:latest published |
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
10 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:
Remove fields with the same value as the default one when calling functions in tests/infrastructure and utilities folders
Which issue(s) this PR fixes:
Special notes for reviewer:
This change only apply to the infrastructure and the utilities folders
jira-ticket:
Summary by CodeRabbit
Refactor
Tests