Skip to content

[Storage] Refactor test_clone tests to remove DV wait usage#4605

Open
ema-aka-young wants to merge 1 commit intoRedHatQE:mainfrom
ema-aka-young:cnv-73917-remove-dvwait-usage
Open

[Storage] Refactor test_clone tests to remove DV wait usage#4605
ema-aka-young wants to merge 1 commit intoRedHatQE:mainfrom
ema-aka-young:cnv-73917-remove-dvwait-usage

Conversation

@ema-aka-young
Copy link
Copy Markdown
Contributor

@ema-aka-young ema-aka-young commented Apr 27, 2026

Short description:

Removing DataVolume wait usage to allow us to remove it from the wrapper. Using wait_for_dv_success() instead.

More details:

Part of this task's work https://redhat.atlassian.net/browse/CNV-73197

What this PR does / why we need it:

To abandon DataVolume wait(), which will be deprecated.

Which issue(s) this PR fixes:
Special notes for reviewer:
jira-ticket:

Summary by CodeRabbit

  • Tests
    • Improved test efficiency and reliability for data volume operations during VM restart scenarios.

…sage and utilize create_dv for DataVolume creation

Signed-off-by: Emanuele Prella <eprella@redhat.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 27, 2026

📝 Walkthrough

Walkthrough

Test simplification in test_successful_vm_restart_with_cloned_dv refactors data volume creation to use the create_dv helper method, consolidates separate wait operations into a single wait_for_dv_success() call, and removes an unused import.

Changes

Cohort / File(s) Summary
Test Refactoring
tests/storage/cdi_clone/test_clone.py
Replaced direct DataVolume instantiation with create_dv helper (removing explicit api_name="storage" parameter). Consolidated two-step wait logic (checking DV existence, then PVC) into single cdv.wait_for_dv_success() call. Removed unused TIMEOUT_1MIN import.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title accurately describes the main change: refactoring test_clone tests to remove DV wait usage and use the create_dv helper instead.
Description check ✅ Passed The pull request description follows the required template with all key sections completed: short description, more details with task reference, purpose statement, and appropriate handling of issue/ticket fields.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@ema-aka-young ema-aka-young changed the title Refactor test_successful_vm_restart_with_cloned_dv to remove dvwait usage and utilize create_dv for DataVolume creation Refactor test_clone to remove DV wait usage Apr 27, 2026
@ema-aka-young ema-aka-young changed the title Refactor test_clone to remove DV wait usage [Storage] Refactor test_clone to remove DV wait usage Apr 28, 2026
@ema-aka-young ema-aka-young changed the title [Storage] Refactor test_clone to remove DV wait usage [Storage] Refactor test_clone tests to remove DV wait usage and use create_dv Apr 28, 2026
@ema-aka-young ema-aka-young marked this pull request as ready for review April 28, 2026 08:36
@openshift-virtualization-qe-bot-4
Copy link
Copy Markdown

Report bugs in Issues

Welcome! 🎉

This pull request will be automatically processed with the following features:

🔄 Automatic Actions

  • Reviewer Assignment: Reviewers are automatically assigned based on the OWNERS file in the repository root
  • Size Labeling: PR size labels (XS, S, M, L, XL, XXL) are automatically applied based on changes
  • Issue Creation: A tracking issue is created for this PR and will be closed when the PR is merged or closed
  • Branch Labeling: Branch-specific labels are applied to track the target branch
  • Auto-verification: Auto-verified users have their PRs automatically marked as verified
  • Labels: Enabled categories: branch, can-be-merged, cherry-pick, has-conflicts, hold, needs-rebase, size, verified, wip

📋 Available Commands

PR Status Management

  • /wip - Mark PR as work in progress (adds WIP: prefix to title)
  • /wip cancel - Remove work in progress status
  • /hold - Block PR merging (approvers only)
  • /hold cancel - Unblock PR merging
  • /verified - Mark PR as verified
  • /verified cancel - Remove verification status
  • /reprocess - Trigger complete PR workflow reprocessing (useful if webhook failed or configuration changed)
  • /regenerate-welcome - Regenerate this welcome message

Review & Approval

  • /lgtm - Approve changes (looks good to me)
  • /approve - Approve PR (approvers only)
  • /assign-reviewers - Assign reviewers based on OWNERS file
  • /assign-reviewer @username - Assign specific reviewer
  • /check-can-merge - Check if PR meets merge requirements

Testing & Validation

  • /retest tox - Run Python test suite with tox
  • /retest build-container - Rebuild and test container image
  • /retest verify-bugs-are-open - verify-bugs-are-open
  • /retest all - Run all available tests

Container Operations

  • /build-and-push-container - Build and push container image (tagged with PR number)
    • Supports additional build arguments: /build-and-push-container --build-arg KEY=value

Cherry-pick Operations

  • /cherry-pick <branch> - Schedule cherry-pick to target branch when PR is merged
    • Multiple branches: /cherry-pick branch1 branch2 branch3

Label Management

  • /<label-name> - Add a label to the PR
  • /<label-name> cancel - Remove a label from the PR

✅ Merge Requirements

This PR will be automatically approved when the following conditions are met:

  1. Approval: /approve from at least one approver
  2. LGTM Count: Minimum 2 /lgtm from reviewers
  3. Status Checks: All required status checks must pass
  4. No Blockers: No wip, hold, has-conflicts labels and PR must be mergeable (no conflicts)
  5. Verified: PR must be marked as verified

📊 Review Process

Approvers and Reviewers

Approvers:

  • jpeimer

Reviewers:

  • Ahmad-Hafe
  • acinko-rh
  • dalia-frank
  • ema-aka-young
  • josemacassan
  • jpeimer
  • kgoldbla
  • kshvaika
Available Labels
  • hold
  • verified
  • wip
  • lgtm
  • approve
AI Features
  • Cherry-Pick Conflict Resolution: Enabled (claude/claude-opus-4-6[1m])

💡 Tips

  • WIP Status: Use /wip when your PR is not ready for review
  • Verification: The verified label is removed on new commits unless the push is detected as a clean rebase
  • Cherry-picking: Cherry-pick labels are processed when the PR is merged
  • Container Builds: Container images are automatically tagged with the PR number
  • Permission Levels: Some commands require approver permissions
  • Auto-verified Users: Certain users have automatic verification and merge privileges

For more information, please refer to the project documentation or contact the maintainers.


with DataVolume(
name="dv-target",
with create_dv(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There may be an issue with WFFC storage.

create_dv() creates a first consumer pod, which may put a PVC on a certain node, and if this node cannot accommodate our future VM, VM will never be scheduled.
The expected WFFC flow is to create a DV, leave it without the consumer, and then VM that wants to use it becomes its first consumer.

Copy link
Copy Markdown
Contributor Author

@ema-aka-young ema-aka-young Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, thank you. I'll revert this change.

@ema-aka-young ema-aka-young changed the title [Storage] Refactor test_clone tests to remove DV wait usage and use create_dv [Storage] Refactor test_clone tests to remove DV wait usage Apr 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants