utils: add client to create_dv and assert_pvc_snapshot_clone_annotation#5120
utils: add client to create_dv and assert_pvc_snapshot_clone_annotation#5120RoniKishner wants to merge 1 commit into
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? |
|
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 (2)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (4)**/*.py📄 CodeRabbit inference engine (AGENTS.md)
Files:
tests/**/*.py📄 CodeRabbit inference engine (AGENTS.md)
Files:
**⚙️ CodeRabbit configuration file
Files:
⚙️ CodeRabbit configuration file
Files:
utilities/storage.py📄 CodeRabbit inference engine (AGENTS.md)
Files:
🔇 Additional comments (5)
📝 WalkthroughWalkthroughTests and the DataVolume HTTP/HTTPS path now explicitly pass the Kubernetes client (and namespace for Artifactory helpers). Tests construct ChangesClient and namespace parameter passing
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
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. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5120 +/- ##
==========================================
- 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:
|
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/storage/utils.py`:
- Around line 451-453: The assertion comparing pvc.use_populator to
expected_use_populator_value should include a descriptive failure message;
update the assert near StorageClass(name=storage_class,
client=pvc.client).instance.get("provisioner") ... to read assert
pvc.use_populator == expected_use_populator_value,
f"StorageClass={storage_class} (client={pvc.client}): expected
use_populator={expected_use_populator_value}, got {pvc.use_populator}" so the
message shows the storage class, client scope, and actual vs expected values for
easier CI debugging.
🪄 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: c2ce74ec-308b-4b6a-90a9-dc7550ff6671
📒 Files selected for processing (2)
tests/storage/utils.pyutilities/storage.py
|
Clean rebase detected — no code changes compared to previous head ( |
|
/verified |
|
/retest all Auto-triggered: Files in this PR were modified by merged PR #4985. Overlapping filesutilities/storage.py |
|
/retest all Auto-triggered: Files in this PR were modified by merged PR #5099. Overlapping filestests/storage/utils.py |
|
/approve |
Signed-off-by: rkishner <rkishner@redhat.com>
|
/verified |
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
PR_NUMBER=5120
REPO="RedHatQE/openshift-virtualization-tests"
echo "=== Latest commit SHA ==="
gh pr view $PR_NUMBER --repo $REPO --json commits --jq '.commits[-1].oid'
echo ""
echo "=== PR files changed ==="
gh api repos/$REPO/pulls/$PR_NUMBER/files --jq '[.[] | {filename, patch: .patch[0:300]}]'Length of output: 1162 🏁 Script executed: #!/bin/bash
PR_NUMBER=5120
REPO="RedHatQE/openshift-virtualization-tests"
COMMIT_SHA="9618a26f0488e44e1246d3b80e8d64084fdd8e60"
BODY="## Test Execution Plan
**Run smoke tests: True**
- \`tests/storage/cdi_clone/test_clone.py\` (smoke) → calls \`assert_pvc_snapshot_clone_annotation\` + \`assert_use_populator\` → \`StorageProfile(client=pvc.client)\` / \`StorageClass(client=pvc.client)\` in \`tests/storage/utils.py\`
- \`tests/storage/cdi_upload/test_upload_virtctl.py\` (smoke) → calls \`assert_use_populator\` → \`StorageClass(client=pvc.client)\` in \`tests/storage/utils.py\`
**Run gating tests: True**
- \`tests/storage/cdi_clone/test_clone.py\` (gating) → \`assert_pvc_snapshot_clone_annotation\` / \`assert_use_populator\` → \`tests/storage/utils.py\`
- \`tests/storage/cdi_import/test_import_http.py\` (gating) → \`assert_use_populator\` → \`tests/storage/utils.py\`; HTTP path also calls \`create_dv\` → \`get_artifactory_secret(client=...)\` in \`utilities/storage.py\`
- \`tests/storage/cdi_upload/test_upload_virtctl.py\` (gating) → \`assert_use_populator\` → \`tests/storage/utils.py\`
- \`tests/storage/restricted_namespace_cloning/test_restricted_namespace_cloning.py\` (gating) → \`assert_pvc_snapshot_clone_annotation\` → \`tests/storage/utils.py\`
- \`tests/storage/restricted_namespace_cloning/test_restricted_namespace_cloning_vms.py\` (gating) → \`assert_pvc_snapshot_clone_annotation\` → \`tests/storage/utils.py\`
**Affected tests to run:**
- \`tests/storage/cdi_clone/test_clone.py\` (smoke, gating)
- \`tests/storage/cdi_upload/test_upload_virtctl.py\` (smoke, gating)
- \`tests/storage/cdi_import/test_import_http.py\` (gating)
- \`tests/storage/restricted_namespace_cloning/test_restricted_namespace_cloning.py\` (gating)
- \`tests/storage/restricted_namespace_cloning/test_restricted_namespace_cloning_vms.py\` (gating)"
gh api repos/$REPO/pulls/$PR_NUMBER/reviews \
--method POST \
--field commit_id="$COMMIT_SHA" \
--field event="COMMENT" \
--field "comments[][path]=tests/storage/utils.py" \
--field "comments[][line]=333" \
--field "comments[][side]=RIGHT" \
--field "comments[][body]=$BODY"
echo "Exit code: $?"Length of output: 3965 |
What this PR does / why we need it:
Missing client on files causes warning:
This PR fixes the warning for
create_dvandassert_pvc_snapshot_clone_annotationWhich issue(s) this PR fixes:
FutureWarningshowingSpecial notes for reviewer:
jira-ticket:
NONE
Summary by CodeRabbit
Tests
Chores