[Storage] Add STD docstrings to CCLM tests#5480
Conversation
Assisted-by: Claude <noreply@anthropic.com> Signed-off-by: Jenia Peimer <jpeimer@redhat.com>
📝 WalkthroughWalkthroughThis PR adds multi-line docstrings to ChangesTest Documentation for Cross-Cluster Live Migration
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
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 |
|
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
Custom Commands
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. |
|
/verified Only added the docstrings, no need to run any tests |
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
gh pr diff 5480 --repo RedHatQE/openshift-virtualization-tests | head -40Length of output: 2145 🏁 Script executed: #!/bin/bash
set -euo pipefail
REPO="RedHatQE/openshift-virtualization-tests"
PR=5480
HEAD_SHA=$(gh pr view $PR --repo "$REPO" --json headRefOid --jq '.headRefOid')
echo "HEAD_SHA=$HEAD_SHA"
BODY='**Test Execution Plan**
- **Run smoke tests: False** — No production/utilities/libs code paths were modified; only docstrings were added to test methods and classes.
- **Run gating tests: False** — No test logic, fixtures, parametrization, or markers were changed; only documentation (docstrings) was added.
- **Affected tests to run**: None required beyond a documentation review. No functional test behavior changed.
Rationale: This PR only inserts docstrings (Preconditions/Steps/Expected sections) into `tests/storage/cross_cluster_live_migration/test_cclm.py`. Test signatures, fixture dependencies, `@pytest.mark.dependency`/`@pytest.mark.polarion` decorators, and helper calls (migration condition wait, boot-id checks, file persistence checks, VM stop/delete assertions, compute live migration verification) remain unchanged. No `utilities/` or `libs/` files were touched, so there is no traceable dependency path to smoke- or gating-marked tests.'
gh api \
--method POST \
-H "Accept: application/vnd.github+json" \
"/repos/${REPO}/pulls/${PR}/comments" \
-f body="$BODY" \
-f commit_id="$HEAD_SHA" \
-f path="tests/storage/cross_cluster_live_migration/test_cclm.py" \
-F line=62 \
-f side="RIGHT"Length of output: 5265 The inline test execution plan has been posted successfully on line 62 of |
Assisted-by: Claude <noreply@anthropic.com> Signed-off-by: Jenia Peimer <jpeimer@redhat.com>
|
/verified Only added the docstrings, no need to run any tests |
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
set -euo pipefail
REPO="RedHatQE/openshift-virtualization-tests"
PR=5480
HEAD_SHA=$(gh pr view $PR --repo "$REPO" --json headRefOid --jq '.headRefOid')
echo "HEAD_SHA=$HEAD_SHA"
echo "---existing review comments by coderabbitai---"
gh api "/repos/${REPO}/pulls/${PR}/comments" --jq '.[] | select(.user.login=="coderabbitai[bot]") | {id, commit_id, path, line, body: .body[0:80]}'Length of output: 869 The test execution plan inline comment already exists on |
rnetser
left a comment
There was a problem hiding this comment.
Code Review
Found 1 issue(s) in this PR:
💡 Suggestions (1)
| File | Line | Issue |
|---|---|---|
tests/storage/cross_cluster_live_migration/test_cclm.py |
3 | [WARNING] The Jira epic: → Jira: rename is an out-of-scope cosmetic change. |
Review generated by pi
|
/lgtm |
|
New container for quay.io/openshift-cnv/openshift-virtualization-tests:latest published |
…5601) ##### What this PR does / why we need it: Manual cherry-pick of #4650 and #5480 1. Cross-storage CCLM support (PR #4650) - conftest.py — Adds `remote_cluster_source_storage_class`, `local_cluster_target_storage_class` fixtures and wires them into `local_cluster_mtv_storage_map` and VM fixtures instead of hardcoding `py_config["default_storage_class"]`. - test_cclm.py — Adds `TestCCLMFromStorageAtoB` class that migrates from `STORAGE_CLASS_A` to `STORAGE_CLASS_B`. Existing test classes updated to parametrize storage classes (both use STORAGE_CLASS_B for same-storage). - Shared code moved — `STORAGE_CLASS_A/B`, `NO_STORAGE_CLASS_FAILURE_MESSAGE`, and `get_storage_class_for_storage_migration` moved from `storage_migration/` to shared `storage/constants.py` and `storage/utils.py` for reuse by CCLM. 2. STD docstrings (PR #5480) - All tests now have proper STD docstrings. 3. Conflict resolution fix (our change) - utils.py — `verify_compute_live_migration_after_cclm` now accepts `client: DynamicClient` and passes it to `migrate_vm_and_verify`. This was needed because the upstream PRs added the client parameter at the call sites but the function signature didn't have it on this branch. - test_cclm.py — Added TYPE_CHECKING imports for DynamicClient and VirtualMachineForTests used in the type hints. ##### Which issue(s) this PR fixes: ##### Special notes for reviewer: ##### jira-ticket: <!-- full-ticket-url needs to be provided. This would add a link to the pull request to the jira and close it when the pull request is merged If the task is not tracked by a Jira ticket, just write "NONE". --> --------- Signed-off-by: Jenia Peimer <jpeimer@redhat.com>
What this PR does / why we need it:
Jira epic:→Jira:Which issue(s) this PR fixes:
Special notes for reviewer:
Assisted-by: Claude noreply@anthropic.com
jira-ticket:
Summary by CodeRabbit