Skip to content

docs: add schema-conforming product test plans to all test classes - #765

Merged
myakove merged 3 commits into
mainfrom
docs/issue-761-test-plan-retrofit
Oct 20, 2026
Merged

myakove merged 3 commits into
mainfrom
docs/issue-761-test-plan-retrofit

Conversation

@myakove

@myakove myakove commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator

Closes #761

Second step of the coverage-discoverability effort (#760 format-enforcing hooks -> this retrofit -> #762 CLI): every test class carries its product test plan in the uniform machine-parseable docstring format.

What

Rewrite/extend 56 class docstrings (52 linter-required + 4 CopyoffloadSnapshotBase subclasses that inherit their test methods) to the full test_plan_schema.md format: one-sentence summary, Purpose/Regression, Prerequisites, numbered Test plan, Expected result — across 20 test files.

  • Customer-readable: repeatable without reading Python or repo configuration; repository VM names appear as examples only
  • Accurate: verified against each class's methods, parametrization in tests/tests_config/config.py, and the utilities called. Corrections made where old docstrings overclaimed: dropped the MTV-5622 note from the warm RDM virtual-disk class (test expects success) and the Hyper-V SMB max-channels claim from TestColdMigrationComprehensive (not asserted)
  • Docstring-only change: AST-equivalence modulo docstrings verified on all 20 files; no code, config, or import changes

This branch does not contain the enforcing hooks (PR #763) — it is independent and can merge first.


Assisted-by: PI (rits/zai-org/glm-5-3)

Closes #761

Rewrite/extend 56 class docstrings (52 linter-required + 4
CopyoffloadSnapshotBase subclasses that inherit their test methods)
to the full test_plan_schema.md format: one-sentence summary,
Purpose/Regression, Prerequisites, numbered Test plan, Expected
result.

Plans are customer-readable (repeatable without reading Python or
repo configuration; repository VM names appear as examples only) and
verified accurate against each class's methods, parametrization in
tests/tests_config/config.py, and the utilities called. Accuracy
corrections: dropped the MTV-5622 note from the warm RDM virtual-disk
class (the test expects success) and dropped the Hyper-V SMB
max-channels claim from TestColdMigrationComprehensive (not asserted
by this test).

Docstring-only change: AST-equivalence modulo docstrings verified on
all files; no code, config, or import changes.

When the enforcing hooks (PR #763) merge, the schema baseline is
deleted in the same integration - all plans conform, nothing is
grandfathered.

Assisted-by: PI (litellm/claude-opus-4-6) <noreply@pi.dev>
Signed-off-by: Meni Yakove <myakove@gmail.com>
@coderabbitai

coderabbitai Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 3 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository: RedHatQE/mtv-api-tests/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: dae7e380-d03c-46e0-8e68-c9f593aa7903
📥 Commits

Reviewing files that changed from the base of the PR and between 9601a36 and e96f596.

📒 Files selected for processing: 21
  • docs/copy-offload-migrations.md
  • tests/cold/test_ca_crt_migration.py
  • tests/cold/test_cold_migration_comprehensive.py
  • tests/cold/test_cold_migration_xfs.py
  • tests/cold/test_dual_nic_cold_migration.py
  • tests/cold/test_insecure_skip_verify_migration.py
  • tests/cold/test_mtv_cold_migration.py
  • tests/cold/test_ova_cold_migration.py
  • tests/copyoffload/test_copyoffload_migration.py
  • tests/deep_inspection/test_plan_driven_di.py
  • tests/deep_inspection/test_standalone_di.py
  • tests/hooks/test_aap_hook_migration.py
  • tests/hooks/test_post_hook_retain_failed_vm.py
  • tests/luks/test_luks_cold_migration.py
  • tests/shared_disk/test_shared_disk_rhel_migration.py
  • tests/shared_disk/test_shared_disk_windows_migration.py
  • tests/upgrade/test_upgrade_migration.py
  • tests/warm/test_mtv_warm_clusterrole_migration.py
  • tests/warm/test_mtv_warm_migration.py
  • tests/warm/test_warm_migration_comprehensive.py
  • tests/warm/test_warm_migration_xfs.py
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@redhat-qe-bot1

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: Disabled for this repository
  • 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: All label categories are enabled (default configuration)

📋 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 (PR author or approvers)
  • /hold cancel - Unblock PR merging (PR author or approvers)
  • /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
  • /security-override - Set security check runs to pass (maintainers only)
  • /security-override cancel - Re-run security checks

Review & Approval

  • /lgtm - Approve changes (looks good to me)
  • /approve - Approve PR (approvers only)
  • /automerge - Enable automatic merging when all requirements are met (maintainers and approvers only)
  • /automerge cancel - Remove automerge label (maintainers and 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 conventional-title - Validate commit message format
  • /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
  • /cherry-pick-retry <branch> - Retry a failed cherry-pick (merged PRs only)

Branch Management

  • /rebase - Rebase this PR branch onto its base branch

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. Status Checks: All required status checks must pass
  3. No Blockers: No wip, hold, has-conflicts labels and PR must be mergeable (no conflicts)
  4. Verified: PR must be marked as verified

📊 Review Process

Approvers and Reviewers

Approvers:

  • krcmarik
  • myakove
  • solenoci

Reviewers:

  • krcmarik
  • myakove
  • solenoci
Available Labels
  • hold
  • verified
  • wip
  • lgtm
  • approve
  • automerge
AI Features
  • Conventional Title: Mode: fix (EnMaaS/rits/zai-org/glm-5-3)
  • Cherry-Pick Conflict Resolution: Enabled (EnMaaS/rits/zai-org/glm-5-3)
Security Checks
  • Suspicious Path Detection: Monitors paths: .claude/, .vscode/, .cursor/, .devcontainer/, .pi/, .github/workflows/, .github/actions/
  • Committer Identity Check: Verifies last committer matches PR author
  • Mandatory: Security checks block merge (use /security-override to bypass — maintainers only)

💡 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.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Document product test plans across migration test classes

📝 Documentation 🕐 40+ Minutes

Grey Divider

AI Description

• Add schema-conforming, customer-readable product test plans to 56 classes across 20 test files.
• Cover prerequisites, ordered actions, expected outcomes, and failure diagnostics without changing
 test behavior.
• Correct two existing claims that did not match the tests' assertions.
Diagram

graph TD
  S["Test plan schema"] --> M["Cold and warm plans"] --> R["Product reviewers"]
  S --> C["Copy-offload plans"] --> R
  S --> X["Specialized scenario plans"] --> R
Loading
High-Level Assessment

Keeping plans beside their test classes makes each scenario discoverable where it is maintained, including subclasses with inherited test methods. Separate plan files would add a synchronization burden without improving this documentation-only retrofit.

Files changed (20) +2568 / -206

Documentation (20) +2568 / -206
test_ca_crt_migration.pyDocument ca.crt provider connectivity validation +38/-5

Document ca.crt provider connectivity validation

• Explains the provider Secret setup, map creation, and Ready Plan check that establish support for the ca.crt certificate field. Explicitly states that no migration runs.

tests/cold/test_ca_crt_migration.py

test_cold_migration_comprehensive.pyDocument comprehensive cold-migration checks +58/-12

Document comprehensive cold-migration checks

• Adds a reproducible Windows migration plan covering networking, PVC naming, scheduling, labels, affinity, and destination validation. Removes the prior Hyper-V SMB max-channels claim because this test does not assert it.

tests/cold/test_cold_migration_comprehensive.py

test_cold_migration_xfs.pyDocument cold migration of XFS v4 disks +42/-3

Document cold migration of XFS v4 disks

• Specifies the non-CRC XFS source disk, migration sequence, and post-migration filesystem and VM checks.

tests/cold/test_cold_migration_xfs.py

test_dual_nic_cold_migration.pyDocument dual-NIC Plan admission regression +38/-16

Document dual-NIC Plan admission regression

• Details the per-NIC mappings from one source network to two destinations and the expected Ready Plan without a duplicate-mapping error. Clarifies that the scenario does not execute a migration.

tests/cold/test_dual_nic_cold_migration.py

test_insecure_skip_verify_migration.pyDocument TLS skip-verification Plan checks +63/-26

Document TLS skip-verification Plan checks

• Adds separate plans for the general and RHV provider variants, including Secret verification, inventory prerequisites, and Plan readiness. Both plans distinguish connectivity validation from migration execution.

tests/cold/test_insecure_skip_verify_migration.py

test_mtv_cold_migration.pyDocument cold-migration sanity and remote-destination scenarios +79/-2

Document cold-migration sanity and remote-destination scenarios

• Expands the basic Linux migration and remote OpenShift migration class docstrings into reproducible plans with destination VM validation.

tests/cold/test_mtv_cold_migration.py

test_ova_cold_migration.pyDocument OVA migration destination checks +40/-1

Document OVA migration destination checks

• Describes OVA provider preparation, migration, and verification of destination connectivity, labels, and affinity. Notes why source-side equivalence checks do not apply.

tests/cold/test_ova_cold_migration.py

test_copyoffload_migration.pyDocument copy-offload migration scenarios +1424/-89

Document copy-offload migration scenarios

• Adds full plans for disk provisioning, snapshots and fallback, RDM, datastore placement, scale, concurrency, and throttling scenarios, including four snapshot subclasses that inherit test methods. Replaces the warm virtual-RDM warning with the successful outcome the test expects.

tests/copyoffload/test_copyoffload_migration.py

test_plan_driven_di.pyDocument critical-concern preflight failure +42/-4

Document critical-concern preflight failure

• Explains how a by-path fstab entry induces a Critical Deep Inspection concern and where to confirm that warm migration stops at PreflightInspection.

tests/deep_inspection/test_plan_driven_di.py

test_standalone_di.pyDocument standalone Deep Inspection outcomes +109/-17

Document standalone Deep Inspection outcomes

• Adds plans for successful standalone inspection, cancellation and rerun, and validation of a missing VDDK image. Each identifies the Conversion status and cleanup evidence relevant to its outcome.

tests/deep_inspection/test_standalone_di.py

test_aap_hook_migration.pyDocument AAP pre- and post-hook migration +45/-5

Document AAP pre- and post-hook migration

• Specifies AWX prerequisites, hook placement in a cold-migration Plan, and the job and VM outcomes to inspect.

tests/hooks/test_aap_hook_migration.py

test_post_hook_retain_failed_vm.pyDocument VM retention after PostHook failure +44/-1

Document VM retention after PostHook failure

• Describes the deliberately failing PostHook, the expected failed pipeline step, and validation of the retained destination VM.

tests/hooks/test_post_hook_retain_failed_vm.py

test_luks_cold_migration.pyDocument correct- and wrong-key LUKS migrations +80/-5

Document correct- and wrong-key LUKS migrations

• Adds plans for successful encrypted-disk migration and for failure at ImageConversion with an incorrect passphrase. The plans identify the passphrase Secret and observable disk or failure evidence.

tests/luks/test_luks_cold_migration.py

test_shared_disk_rhel_migration.pyDocument Linux shared-disk migration +48/-1

Document Linux shared-disk migration

• Explains owner and consumer shared-disk settings and the bidirectional read/write check between two migrated Linux VMs.

tests/shared_disk/test_shared_disk_rhel_migration.py

test_shared_disk_windows_migration.pyDocument Windows shared-NTFS-disk migration +53/-1

Document Windows shared-NTFS-disk migration

• Details volume labeling, owner and consumer migration settings, and bidirectional data checks on the migrated Windows VMs.

tests/shared_disk/test_shared_disk_windows_migration.py

test_upgrade_migration.pyDocument migration across an MTV operator upgrade +48/-1

Document migration across an MTV operator upgrade

• Describes creating resources before an operator upgrade, confirming Plan readiness afterwards, and validating the resulting migration.

tests/upgrade/test_upgrade_migration.py

test_mtv_warm_clusterrole_migration.pyDocument remote-cluster SCC permission cases +89/-2

Document remote-cluster SCC permission cases

• Adds paired plans for the expected warm-migration failure without an SCC binding and success after granting it.

tests/warm/test_mtv_warm_clusterrole_migration.py

test_mtv_warm_migration.pyDocument warm-migration sanity and topology cases +127/-3

Document warm-migration sanity and topology cases

• Expands plans for static-IP and inspection checks, a two-disk/two-NIC VM, and migration to a remote OpenShift destination.

tests/warm/test_mtv_warm_migration.py

test_warm_migration_comprehensive.pyDocument comprehensive warm-migration checks +55/-9

Document comprehensive warm-migration checks

• Details the Windows warm-migration prerequisites, cutover, and checks for static networking, generated PVC names, labels, affinity, namespace, and CPU features.

tests/warm/test_warm_migration_comprehensive.py

test_warm_migration_xfs.pyDocument warm migration of XFS v4 disks +46/-3

Document warm migration of XFS v4 disks

• Specifies the non-CRC XFS disk setup, incremental migration and cutover, and post-migration filesystem and VM validation.

tests/warm/test_warm_migration_xfs.py

… plan

Step 3 of TestCopyoffloadDedicatedHostSharedPopulatorLimit contained a
nested "1." line, which breaks the strictly consecutive step numbering
the schema (and its linter) requires. Make it a plain continuation line.

Assisted-by: PI (rits/zai-org/glm-5-3) <noreply@pi.dev>
Signed-off-by: Meni Yakove <myakove@gmail.com>
@qodo-code-review

qodo-code-review Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Warm disk coverage reports an untested LUN ✓ Resolved
Description
The warm virtual-RDM plan says rdmAsLun is enabled and the destination disk is checked as a SCSI
LUN, but test_create_plan does not pass that setting and test_check_vms does not inspect its
bus. A successful run therefore cannot establish the LUN behavior that the plan advertises.
Code

tests/copyoffload/test_copyoffload_migration.py[R2452-2453]

+        3. Create a warm Plan for the VM with copy-offload and rdmAsLun
+           enabled. Check the Plan's Ready condition is True.
Relevance

●●● Strong

Recent copy-offload precedent accepted requiring explicit rdm_as_lun configuration and plan
propagation.

PR-#627

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The configuration has no rdm_as_lun entry, the Plan call omits that argument, and the only
post-migration checks here are generic VM validation and disk count.

tests/tests_config/config.py[226-241]
tests/copyoffload/test_copyoffload_migration.py[2544-2572]
tests/copyoffload/test_copyoffload_migration.py[2598-2633]
utilities/mtv_migration.py[328-329]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The warm virtual-RDM plan claims LUN presentation and verification that its test does not perform.
## Fix Focus Areas
- tests/copyoffload/test_copyoffload_migration.py[2421-2470]
- tests/copyoffload/test_copyoffload_migration.py[2544-2633]
## Recommended Fix
Remove the rdmAsLun and SCSI-LUN claims from this plan, or explicitly configure rdmAsLun and add a destination bus assertion if LUN behavior is intended to be covered.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. RHV runs overstate network checks ✓ Resolved
Description
The sanity and comprehensive warm plans promise static-address verification on both supported
sources, and the comprehensive plan also promises interface-name checks, but check_vms excludes
RHV from the static-IP check and limits name checks to vSphere. Both classes can run with an RHV
source, where those validations are skipped even when the Plans request static-IP preservation.
Code

tests/warm/test_mtv_warm_migration.py[R70-72]

+        5. Validate the migrated VM: power state, guest-agent presence,
+           connectivity, static IP/subnet/gateway preservation, NIC name
+           preservation, and CPU, memory, network, and storage equivalence
Relevance

●●● Strong

The warm plans overstate RHV validation coverage when provider-specific checks explicitly skip RHV.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The verification provider tuple contains vSphere and Hyper-V, and the interface-name branch is
vSphere-only; the warm plans describe vSphere or RHV runs without that qualification.

utilities/post_migration.py[47-48]
utilities/post_migration.py[1973-2005]
tests/warm/test_mtv_warm_migration.py[38-84]
tests/warm/test_warm_migration_comprehensive.py[49-101]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Two warm migration plans describe network assertions that their RHV runs do not execute.
## Fix Focus Areas
- tests/warm/test_mtv_warm_migration.py[40-84]
- tests/warm/test_warm_migration_comprehensive.py[49-101]
## Recommended Fix
Qualify static-IP and interface-name validation by the providers for which those checks actually run, or add the missing RHV assertions before advertising that coverage.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Concurrent copy-offload test lacks naming ✓ Resolved
Description
TestConcurrentXcopyVddkMigration documents a scenario that runs an XCOPY copy-offload migration
alongside a VDDK migration, but its class name does not contain the required Copyoffload
substring. Because this class lives in the copy-offload test module and exercises copy-offload
behavior, naming-based discovery cannot identify it as a copy-offload test.
Code

tests/copyoffload/test_copyoffload_migration.py[6705]

+    """An XCOPY copy-offload Plan and a standard VDDK Plan must run concurrently and both succeed.
Relevance

●●● Strong

Naming-based copy-offload discoverability is the stated PR goal, making the missing class substring
directly relevant.

PR-#550

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The changed class docstring explicitly describes an XCOPY copy-offload Plan running concurrently
with a VDDK Plan, while the surrounding class declaration is TestConcurrentXcopyVddkMigration and
omits the required Copyoffload substring.

Rule 70450: Enforce naming for copy-offload snapshot test classes
tests/copyoffload/test_copyoffload_migration.py[6704-6713]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`TestConcurrentXcopyVddkMigration` exercises copy-offload behavior but does not include the required `Copyoffload` substring in its class name, so naming-based discovery and compliance checks cannot classify it consistently.

## Fix Focus Areas
- tests/copyoffload/test_copyoffload_migration.py[6704-6705]

## Recommended Fix
Rename the class to include the exact case-sensitive `Copyoffload` substring, such as `TestCopyoffloadConcurrentXcopyVddkMigration`, and update any references to the class.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. The warm XFS plan checks the wrong disk ✓ Resolved
Description
The warm XFS plan tells readers to prepare and inspect an additional data disk, but
test_verify_xfs_version takes its check path from a configuration value of /. If a reader
prepares only the described data disk as XFS v4, the automated test instead inspects the root
filesystem and does not verify that disk.
Code

tests/warm/test_warm_migration_xfs.py[R77-79]

+        5. Verify the filesystem on the migrated VM: run ``xfs_info`` on the
+           data-disk mount point and confirm the output still reports
+           ``crc=0`` (XFS v4).
Relevance

●●● Strong

The documented data-disk check conflicts directly with configuration and code checking the root
filesystem.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The warm scenario configures / as its mount point, and the test passes that value to the command
check; /sdb1 belongs to the separate cold scenario.

tests/tests_config/config.py[897-919]
tests/warm/test_warm_migration_xfs.py[244-273]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The warm XFS docstring describes a data-disk check, while the test runs xfs_info on `/`.
## Fix Focus Areas
- tests/warm/test_warm_migration_xfs.py[55-61]
- tests/warm/test_warm_migration_xfs.py[77-88]
- tests/tests_config/config.py[897-904]
## Recommended Fix
Describe an XFS v4 root filesystem and its `/` check, or change the test configuration and setup to check the additional data disk.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View medium (4)
5. The throttling plan loses its limit ✓ Resolved
Description
Removing the leading 1. from the dedicated-host plan also removes the value after “active-pod peak
never exceeds,” leaving that verification sentence unfinished. A reader following step 3 has no
stated upper bound to compare against the observed peak, despite the scenario configuring a limit of
one.
Code

tests/copyoffload/test_copyoffload_migration.py[5165]

+           During one poll, require a still-Pending PVC with a structured
Relevance

●●● Strong

The documentation sentence is objectively malformed and omits the configured numeric limit.

PR-#550

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The changed text reads “never exceeds During one poll”; the preceding step sets the per-host
populator limit to one.

tests/copyoffload/test_copyoffload_migration.py[5157-5167]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The numbering edit removed the numeric bound from the active-pod verification sentence.
## Fix Focus Areas
- tests/copyoffload/test_copyoffload_migration.py[5163-5166]
## Recommended Fix
Complete the sentence with the configured bound, such as “never exceeds 1. During one poll, require …”, while keeping the numbered plan step at 3.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


6. AWX job records are not checked ✓ Resolved
Description
Step 5 of the AAP hook plan says to confirm in AWX that both job templates completed successfully,
but the class only waits for migration success and validates the destination VM. Its automated
result contains no AWX job-record query or assertion, so the plan attributes a direct job-completion
check to a run that does not perform one.
Code

tests/hooks/test_aap_hook_migration.py[R85-87]

+        5. Validate the retained destination VM (power state and resource
+           equivalence with the source), and in AWX confirm both job
+           templates ran to successful completion. After the checks, delete
Relevance

●●● Strong

The product plan promises AWX completion checks that the automated test does not perform.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The migration method invokes execute_migration, and the following method invokes check_vms;
neither queries AWX job records.

tests/hooks/test_aap_hook_migration.py[215-265]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The plan advertises direct AWX job verification that the class never performs.
## Fix Focus Areas
- tests/hooks/test_aap_hook_migration.py[82-97]
- tests/hooks/test_aap_hook_migration.py[215-265]
## Recommended Fix
Either add assertions against both AWX job records or describe job success as inferred from the successful migration rather than checked in AWX.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


7. Snapshot cleanup appears covered but is not ✓ Resolved
Description
The standalone inspection plan instructs readers to confirm snapshot removal and lists no leftover
snapshot as an expected result, but test_verify_di_results checks only the Conversion's inspection
results. A successful standalone inspection test can leave snapshot cleanup unverified; the
snapshot-removal assertion is in the separate cancellation class.
Code

tests/deep_inspection/test_standalone_di.py[R73-74]

+           all present. Confirm the inspection snapshot was removed from the
+           source VM. The Conversion CR is removed with the run.
Relevance

●●● Strong

The plan explicitly promises snapshot cleanup verification, but the test only verifies inspection
results.

PR-#624

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The class ends by calling verify_di_results, which reads inspectionResult; its methods do not
inspect source snapshots.

tests/deep_inspection/test_standalone_di.py[84-101]
utilities/deep_inspection.py[366-383]
tests/deep_inspection/test_standalone_di.py[118-159]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The standalone success plan presents snapshot removal as a checked outcome without an assertion in that class.
## Fix Focus Areas
- tests/deep_inspection/test_standalone_di.py[69-79]
- tests/deep_inspection/test_standalone_di.py[89-101]
## Recommended Fix
Add a source snapshot-removal check to the standalone success class, or remove its explicit cleanup verification and pass-criterion claims.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


8. Inspection phases are not tracked ✓ Resolved
Description
The invalid-inspection plan says to check that the Conversion never reaches Running or Succeeded,
but test_verify_validation_error asserts the phase returned when the first Critical condition is
found. An earlier or later transition into either phase is outside that single sampled assertion.
Code

tests/deep_inspection/test_standalone_di.py[R270-272]

+        3. Wait for the controller's validation and check that the
+           Conversion has a Critical VDDKImageNotSet condition and that its
+           phase never reaches Running or Succeeded. The Conversion CR is
Relevance

●● Moderate

The finding identifies a real temporal-coverage gap, but historical evidence supports stronger
assertions more than phase-history tracking.

PR-#629

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
wait_for_critical_conditions breaks on its first Critical-condition sample and returns one phase,
which the test checks once.

utilities/deep_inspection.py[443-463]
tests/deep_inspection/test_standalone_di.py[289-306]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The validation plan promises a phase-history guarantee while the test checks one sampled phase.
## Fix Focus Areas
- tests/deep_inspection/test_standalone_di.py[265-277]
- tests/deep_inspection/test_standalone_di.py[289-306]
- utilities/deep_inspection.py[443-463]
## Recommended Fix
Describe the phase check as occurring when the Critical condition appears, or capture and assert the phase history across the required observation window.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
❌ Tickets: failed to load — Ticket collection could not establish complete evidence
✅ Compliance rules (platform): 104 rules
✅ Skills: remote-debugging
✅ Cross-repo context — repo relationships
Review mode: Auto: 🧠 Deep: Dozens of independent plans create many easy-to-miss accuracy defects.
✅ Fix-loop round: fix loop

Grey Divider

Tip of the day
💡 Did you know, you can open the rule or relationship behind a finding in one click

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Previous reviews

Review updated until commit e96f596

Results up to commit 43f80a9 ⚖️ Balanced


🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)


Action required
1. Warm disk coverage reports an untested LUN ✓ Resolved
Description
The warm virtual-RDM plan says rdmAsLun is enabled and the destination disk is checked as a SCSI
LUN, but test_create_plan does not pass that setting and test_check_vms does not inspect its
bus. A successful run therefore cannot establish the LUN behavior that the plan advertises.
Code

tests/copyoffload/test_copyoffload_migration.py[R2452-2453]

+        3. Create a warm Plan for the VM with copy-offload and rdmAsLun
+           enabled. Check the Plan's Ready condition is True.
Relevance

●●● Strong

Recent copy-offload precedent accepted requiring explicit rdm_as_lun configuration and plan
propagation.

PR-#627

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The configuration has no rdm_as_lun entry, the Plan call omits that argument, and the only
post-migration checks here are generic VM validation and disk count.

tests/tests_config/config.py[226-241]
tests/copyoffload/test_copyoffload_migration.py[2544-2572]
tests/copyoffload/test_copyoffload_migration.py[2598-2633]
utilities/mtv_migration.py[328-329]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The warm virtual-RDM plan claims LUN presentation and verification that its test does not perform.
## Fix Focus Areas
- tests/copyoffload/test_copyoffload_migration.py[2421-2470]
- tests/copyoffload/test_copyoffload_migration.py[2544-2633]
## Recommended Fix
Remove the rdmAsLun and SCSI-LUN claims from this plan, or explicitly configure rdmAsLun and add a destination bus assertion if LUN behavior is intended to be covered.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended
2. RHV runs overstate network checks ✓ Resolved
Description
The sanity and comprehensive warm plans promise static-address verification on both supported
sources, and the comprehensive plan also promises interface-name checks, but check_vms excludes
RHV from the static-IP check and limits name checks to vSphere. Both classes can run with an RHV
source, where those validations are skipped even when the Plans request static-IP preservation.
Code

tests/warm/test_mtv_warm_migration.py[R70-72]

+        5. Validate the migrated VM: power state, guest-agent presence,
+           connectivity, static IP/subnet/gateway preservation, NIC name
+           preservation, and CPU, memory, network, and storage equivalence
Relevance

●●● Strong

The warm plans overstate RHV validation coverage when provider-specific checks explicitly skip RHV.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The verification provider tuple contains vSphere and Hyper-V, and the interface-name branch is
vSphere-only; the warm plans describe vSphere or RHV runs without that qualification.

utilities/post_migration.py[47-48]
utilities/post_migration.py[1973-2005]
tests/warm/test_mtv_warm_migration.py[38-84]
tests/warm/test_warm_migration_comprehensive.py[49-101]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Two warm migration plans describe network assertions that their RHV runs do not execute.
## Fix Focus Areas
- tests/warm/test_mtv_warm_migration.py[40-84]
- tests/warm/test_warm_migration_comprehensive.py[49-101]
## Recommended Fix
Qualify static-IP and interface-name validation by the providers for which those checks actually run, or add the missing RHV assertions before advertising that coverage.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Concurrent copy-offload test lacks naming ✓ Resolved
Description
TestConcurrentXcopyVddkMigration documents a scenario that runs an XCOPY copy-offload migration
alongside a VDDK migration, but its class name does not contain the required Copyoffload
substring. Because this class lives in the copy-offload test module and exercises copy-offload
behavior, naming-based discovery cannot identify it as a copy-offload test.
Code

tests/copyoffload/test_copyoffload_migration.py[6705]

+    """An XCOPY copy-offload Plan and a standard VDDK Plan must run concurrently and both succeed.
Relevance

●●● Strong

Naming-based copy-offload discoverability is the stated PR goal, making the missing class substring
directly relevant.

PR-#550

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The changed class docstring explicitly describes an XCOPY copy-offload Plan running concurrently
with a VDDK Plan, while the surrounding class declaration is TestConcurrentXcopyVddkMigration and
omits the required Copyoffload substring.

Rule 70450: Enforce naming for copy-offload snapshot test classes
tests/copyoffload/test_copyoffload_migration.py[6704-6713]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`TestConcurrentXcopyVddkMigration` exercises copy-offload behavior but does not include the required `Copyoffload` substring in its class name, so naming-based discovery and compliance checks cannot classify it consistently.

## Fix Focus Areas
- tests/copyoffload/test_copyoffload_migration.py[6704-6705]

## Recommended Fix
Rename the class to include the exact case-sensitive `Copyoffload` substring, such as `TestCopyoffloadConcurrentXcopyVddkMigration`, and update any references to the class.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. The warm XFS plan checks the wrong disk ✓ Resolved
Description
The warm XFS plan tells readers to prepare and inspect an additional data disk, but
test_verify_xfs_version takes its check path from a configuration value of /. If a reader
prepares only the described data disk as XFS v4, the automated test instead inspects the root
filesystem and does not verify that disk.
Code

tests/warm/test_warm_migration_xfs.py[R77-79]

+        5. Verify the filesystem on the migrated VM: run ``xfs_info`` on the
+           data-disk mount point and confirm the output still reports
+           ``crc=0`` (XFS v4).
Relevance

●●● Strong

The documented data-disk check conflicts directly with configuration and code checking the root
filesystem.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The warm scenario configures / as its mount point, and the test passes that value to the command
check; /sdb1 belongs to the separate cold scenario.

tests/tests_config/config.py[897-919]
tests/warm/test_warm_migration_xfs.py[244-273]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The warm XFS docstring describes a data-disk check, while the test runs xfs_info on `/`.
## Fix Focus Areas
- tests/warm/test_warm_migration_xfs.py[55-61]
- tests/warm/test_warm_migration_xfs.py[77-88]
- tests/tests_config/config.py[897-904]
## Recommended Fix
Describe an XFS v4 root filesystem and its `/` check, or change the test configuration and setup to check the additional data disk.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View medium (4)
5. The throttling plan loses its limit ✓ Resolved
Description
Removing the leading 1. from the dedicated-host plan also removes the value after “active-pod peak
never exceeds,” leaving that verification sentence unfinished. A reader following step 3 has no
stated upper bound to compare against the observed peak, despite the scenario configuring a limit of
one.
Code

tests/copyoffload/test_copyoffload_migration.py[5165]

+           During one poll, require a still-Pending PVC with a structured
Relevance

●●● Strong

The documentation sentence is objectively malformed and omits the configured numeric limit.

PR-#550

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The changed text reads “never exceeds During one poll”; the preceding step sets the per-host
populator limit to one.

tests/copyoffload/test_copyoffload_migration.py[5157-5167]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The numbering edit removed the numeric bound from the active-pod verification sentence.
## Fix Focus Areas
- tests/copyoffload/test_copyoffload_migration.py[5163-5166]
## Recommended Fix
Complete the sentence with the configured bound, such as “never exceeds 1. During one poll, require …”, while keeping the numbered plan step at 3.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


6. AWX job records are not checked ✓ Resolved
Description
Step 5 of the AAP hook plan says to confirm in AWX that both job templates completed successfully,
but the class only waits for migration success and validates the destination VM. Its automated
result contains no AWX job-record query or assertion, so the plan attributes a direct job-completion
check to a run that does not perform one.
Code

tests/hooks/test_aap_hook_migration.py[R85-87]

+        5. Validate the retained destination VM (power state and resource
+           equivalence with the source), and in AWX confirm both job
+           templates ran to successful completion. After the checks, delete
Relevance

●●● Strong

The product plan promises AWX completion checks that the automated test does not perform.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The migration method invokes execute_migration, and the following method invokes check_vms;
neither queries AWX job records.

tests/hooks/test_aap_hook_migration.py[215-265]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The plan advertises direct AWX job verification that the class never performs.
## Fix Focus Areas
- tests/hooks/test_aap_hook_migration.py[82-97]
- tests/hooks/test_aap_hook_migration.py[215-265]
## Recommended Fix
Either add assertions against both AWX job records or describe job success as inferred from the successful migration rather than checked in AWX.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


7. Snapshot cleanup appears covered but is not ✓ Resolved
Description
The standalone inspection plan instructs readers to confirm snapshot removal and lists no leftover
snapshot as an expected result, but test_verify_di_results checks only the Conversion's inspection
results. A successful standalone inspection test can leave snapshot cleanup unverified; the
snapshot-removal assertion is in the separate cancellation class.
Code

tests/deep_inspection/test_standalone_di.py[R73-74]

+           all present. Confirm the inspection snapshot was removed from the
+           source VM. The Conversion CR is removed with the run.
Relevance

●●● Strong

The plan explicitly promises snapshot cleanup verification, but the test only verifies inspection
results.

PR-#624

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The class ends by calling verify_di_results, which reads inspectionResult; its methods do not
inspect source snapshots.

tests/deep_inspection/test_standalone_di.py[84-101]
utilities/deep_inspection.py[366-383]
tests/deep_inspection/test_standalone_di.py[118-159]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The standalone success plan presents snapshot removal as a checked outcome without an assertion in that class.
## Fix Focus Areas
- tests/deep_inspection/test_standalone_di.py[69-79]
- tests/deep_inspection/test_standalone_di.py[89-101]
## Recommended Fix
Add a source snapshot-removal check to the standalone success class, or remove its explicit cleanup verification and pass-criterion claims.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


8. Inspection phases are not tracked ✓ Resolved
Description
The invalid-inspection plan says to check that the Conversion never reaches Running or Succeeded,
but test_verify_validation_error asserts the phase returned when the first Critical condition is
found. An earlier or later transition into either phase is outside that single sampled assertion.
Code

tests/deep_inspection/test_standalone_di.py[R270-272]

+        3. Wait for the controller's validation and check that the
+           Conversion has a Critical VDDKImageNotSet condition and that its
+           phase never reaches Running or Succeeded. The Conversion CR is
Relevance

●● Moderate

The finding identifies a real temporal-coverage gap, but historical evidence supports stronger
assertions more than phase-history tracking.

PR-#629

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
wait_for_critical_conditions breaks on its first Critical-condition sample and returns one phase,
which the test checks once.

utilities/deep_inspection.py[443-463]
tests/deep_inspection/test_standalone_di.py[289-306]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The validation plan promises a phase-history guarantee while the test checks one sampled phase.
## Fix Focus Areas
- tests/deep_inspection/test_standalone_di.py[265-277]
- tests/deep_inspection/test_standalone_di.py[289-306]
- utilities/deep_inspection.py[443-463]
## Recommended Fix
Describe the phase check as occurring when the Critical condition appears, or capture and assert the phase history across the required observation window.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread tests/warm/test_warm_migration_xfs.py Resolved
Comment thread tests/warm/test_mtv_warm_migration.py Resolved
Comment thread tests/hooks/test_aap_hook_migration.py Resolved
- rename TestConcurrentXcopyVddkMigration to
  TestCopyoffloadConcurrentXcopyVddkMigration (copy-offload naming rule),
  update docs references
- warm virtual-RDM plan: drop rdmAsLun/SCSI-LUN claims (not configured
  in this scenario)
- dedicated-host shared populator limit plan: restore the "never exceeds
  1." bound lost in the earlier numbering fix
- warm XFS plan: describe the root-filesystem (/, crc=0) check actually
  configured, not a data disk
- warm sanity/comprehensive plans: qualify static-IP (vSphere/Hyper-V)
  and NIC-name (vSphere) assertions by provider
- standalone DI success plan: snapshot removal is pipeline behavior,
  not a checked outcome
- standalone DI invalid plan: phase is sampled when Critical appears
- AAP hook plan: AWX job success inferred from migration Succeeded

Assisted-by: PI (litellm/claude-opus-4-6) <noreply@pi.dev>
Signed-off-by: Meni Yakove <myakove@gmail.com>
@myakove

myakove commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator Author

/qodo

The following review comments were reviewed and a decision was made:

tests/copyoffload/test_copyoffload_migration.py:2452 (qodo bug) — Warm disk coverage reports an untested LUN

Addressed: Addressed in e96f596: removed the rdmAsLun/SCSI-LUN claims from the warm virtual-RDM plan — the scenario's config (test_copyoffload_warm_rdm_virtual_disk_migration) does not set rdm_as_lun (unlike the cold RDM scenarios), and the class validates via check_vms + verify_vm_disk_count only. The plan now claims exactly what runs: warm copy-offload transfer of the RDM disk with XCOPY, disk-count and equivalence validation.

tests/copyoffload/test_copyoffload_migration.py:6705 (qodo rule violation) — Concurrent copy-offload test lacks naming

Addressed: Addressed in e96f596: class renamed to TestCopyoffloadConcurrentXcopyVddkMigration, satisfying the repo's copy-offload naming rule (AGENTS.md: copy-offload class names must contain Copyoffload). The docs table row and prose in docs/copy-offload-migrations.md were updated, including removing the now-obsolete paragraph justifying the non-conforming name. No code references the old class name (parametrize ids and config keys are unchanged and don't reference the class name).

tests/copyoffload/test_copyoffload_migration.py:5165 (qodo bug) — The throttling plan loses its limit

Addressed: Addressed in e96f596: the "1." deleted by the earlier numbering fix was the populator limit value, not a step number. The sentence is restored inline ("Verify the observed active-pod peak never exceeds 1.") with "During one poll, ..." continuing on the next line, so the numbered plan step stays at 3 and the linter no longer sees a line-start numbered item.

tests/warm/test_warm_migration_xfs.py:77 (qodo bug) — The warm XFS plan checks the wrong disk

Addressed: Addressed in e96f596: the plan now describes the root-filesystem check actually configured — the scenario's xfs_check runs xfs_info on / (mount_point: "/"), not on an added data disk. Summary, Purpose, Prerequisites, step 1, step 5, and Expected result all describe the XFS v4 root filesystem and its / check.

tests/warm/test_mtv_warm_migration.py:70 (qodo bug) — RHV runs overstate network checks

Addressed: Addressed in e96f596: the sanity warm plan's step 5 and Expected result now qualify the network assertions by provider: static IP/subnet/gateway preservation is verified for vSphere and Hyper-V sources (_STATIC_IP_PROVIDERS in utilities/post_migration.py), NIC-name preservation only for vSphere (guest-operations NIC detection is vSphere-only in conftest.py). Matches check_vms gating in utilities/post_migration.py.

tests/deep_inspection/test_standalone_di.py:73 (qodo bug) — Snapshot cleanup appears covered but is not

Addressed: Addressed in e96f596: the success plan no longer presents snapshot removal as a checked outcome — the class asserts only the inspectionResult contents (verify_di_results). Step 4 now describes snapshot removal as part of the pipeline ("removed as part of the pipeline"), and the Expected result and failure diagnostics no longer claim a leftover-snapshot check.

tests/hooks/test_aap_hook_migration.py:85 (qodo bug) — AWX job records are not checked

Addressed: Addressed in e96f596: step 5 and the Expected result now state that both AWX jobs completing is inferred from the migration reaching Succeeded (Forklift gates the pipeline on hook completion) rather than checked in AWX — the class's test_check_vms runs only the standard check_vms validation and never queries AWX job records.

tests/deep_inspection/test_standalone_di.py:270 (qodo bug) — Inspection phases are not tracked

Addressed: Addressed in e96f596: the invalid-inspection plan now describes the phase check as it runs: wait_for_critical_conditions samples the phase at the moment the Critical condition appears, and the assertion verifies that sampled phase is neither Running nor Succeeded — not a phase-history guarantee across the observation window.


Assisted-by: PI (rits/zai-org/glm-5-3)

@qodo-code-review

Copy link
Copy Markdown

The following review comments were reviewed and a decision was made:

tests/copyoffload/test_copyoffload_migration.py:2452 (qodo bug) — Warm disk coverage reports an untested LUN

Addressed: Addressed in e96f596: removed the rdmAsLun/SCSI-LUN claims from the warm virtual-RDM plan — the scenario's config (test_copyoffload_warm_rdm_virtual_disk_migration) does not set rdm_as_lun (unlike the cold RDM scenarios), and the class validates via check_vms + verify_vm_disk_count only. The plan now claims exactly what runs: warm copy-offload transfer of the RDM disk with XCOPY, disk-count and equivalence validation.

tests/copyoffload/test_copyoffload_migration.py:6705 (qodo rule violation) — Concurrent copy-offload test lacks naming

Addressed: Addressed in e96f596: class renamed to TestCopyoffloadConcurrentXcopyVddkMigration, satisfying the repo's copy-offload naming rule (AGENTS.md: copy-offload class names must contain Copyoffload). The docs table row and prose in docs/copy-offload-migrations.md were updated, including removing the now-obsolete paragraph justifying the non-conforming name. No code references the old class name (parametrize ids and config keys are unchanged and don't reference the class name).

tests/copyoffload/test_copyoffload_migration.py:5165 (qodo bug) — The throttling plan loses its limit

Addressed: Addressed in e96f596: the "1." deleted by the earlier numbering fix was the populator limit value, not a step number. The sentence is restored inline ("Verify the observed active-pod peak never exceeds 1.") with "During one poll, ..." continuing on the next line, so the numbered plan step stays at 3 and the linter no longer sees a line-start numbered item.

tests/warm/test_warm_migration_xfs.py:77 (qodo bug) — The warm XFS plan checks the wrong disk

Addressed: Addressed in e96f596: the plan now describes the root-filesystem check actually configured — the scenario's xfs_check runs xfs_info on / (mount_point: "/"), not on an added data disk. Summary, Purpose, Prerequisites, step 1, step 5, and Expected result all describe the XFS v4 root filesystem and its / check.

tests/warm/test_mtv_warm_migration.py:70 (qodo bug) — RHV runs overstate network checks

Addressed: Addressed in e96f596: the sanity warm plan's step 5 and Expected result now qualify the network assertions by provider: static IP/subnet/gateway preservation is verified for vSphere and Hyper-V sources (_STATIC_IP_PROVIDERS in utilities/post_migration.py), NIC-name preservation only for vSphere (guest-operations NIC detection is vSphere-only in conftest.py). Matches check_vms gating in utilities/post_migration.py.

tests/deep_inspection/test_standalone_di.py:73 (qodo bug) — Snapshot cleanup appears covered but is not

Addressed: Addressed in e96f596: the success plan no longer presents snapshot removal as a checked outcome — the class asserts only the inspectionResult contents (verify_di_results). Step 4 now describes snapshot removal as part of the pipeline ("removed as part of the pipeline"), and the Expected result and failure diagnostics no longer claim a leftover-snapshot check.

tests/hooks/test_aap_hook_migration.py:85 (qodo bug) — AWX job records are not checked

Addressed: Addressed in e96f596: step 5 and the Expected result now state that both AWX jobs completing is inferred from the migration reaching Succeeded (Forklift gates the pipeline on hook completion) rather than checked in AWX — the class's test_check_vms runs only the standard check_vms validation and never queries AWX job records.

tests/deep_inspection/test_standalone_di.py:270 (qodo bug) — Inspection phases are not tracked

Addressed: Addressed in e96f596: the invalid-inspection plan now describes the phase check as it runs: wait_for_critical_conditions samples the phase at the moment the Critical condition appears, and the assertion verifies that sampled phase is neither Running nor Succeeded — not a phase-history guarantee across the observation window.


Assisted-by: PI (rits/zai-org/glm-5-3)

Thanks for the detailed resolution notes. The current review shows all eight findings as implemented, with the documentation now aligned to the behavior actually exercised by each test.

@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit e96f596

@myakove

myakove commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator Author

/lgtm
/approve

@myakove

myakove commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator Author

/verified

@myakove
myakove merged commit a282991 into main Oct 20, 2026
9 checks passed
@myakove
myakove deleted the docs/issue-761-test-plan-retrofit branch October 20, 2026 10:57
@redhat-qe-bot2

Copy link
Copy Markdown

New container for ghcr.io/redhatqe/mtv-api-tests:latest published

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.

Add product test plans (schema docstrings) to all existing test classes

5 participants