Add Windows golden image storage tests (T2)#5126
Conversation
Add T2 tests for Windows VMs using the self-validation golden image DataSource: 1. test_windows_vm_boots_from_golden_image: Verifies a Windows VM can boot from the golden image, guest agent connects, and reports Windows OS. 2. test_windows_multi_disk_snapshot_freeze_within_threshold: Creates a 2-disk Windows VM (boot + data disk), takes an online snapshot, and asserts the freeze window (guest agent freeze/thaw) completes within 20 seconds. Measures actual freeze duration using VMSnapshot status.creationTime vs metadata.creationTimestamp. Tests are gated by: - ACCEPT_WINDOWS_EULA=true environment variable - Windows golden image DataSource existence (skips gracefully if missing) - pytest.mark.windows marker (auto-included by T2 when EULA is accepted) Co-authored-by: Cursor <cursoragent@cursor.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? |
📝 WalkthroughWalkthroughAdds Windows storage test infrastructure and two validation test modules: a golden image boot test that verifies Windows VM provisioning from a pre-created DataSource, and an app-consistent snapshot freeze-duration test on multi-disk VMs that measures guest agent interaction overhead. ChangesWindows Storage Test Suite
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (2 errors)
✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/windows/conftest.py`:
- Line 114: The Windows snapshot fixture is disabling SSH validation by calling
running_vm(vm=vm, wait_for_interfaces=True, check_ssh_connectivity=False);
revert this so SSH connectivity is validated: remove the
check_ssh_connectivity=False override (or set it to True) in the running_vm call
inside the Windows snapshot fixture in tests/storage/windows/conftest.py to
ensure the fixture verifies SSH/WSL2 connectivity as expected.
In `@tests/storage/windows/test_windows_app_consistent_snapshot.py`:
- Around line 4-6: The module docstring stating the freeze limit as "< 10s" is
inconsistent with the test assertion that enforces "< 20s"; update the
module-level docstring at the top of
tests/storage/windows/test_windows_app_consistent_snapshot.py to state "< 20s"
so it matches the enforced threshold (or, if policy is to enforce 10s, instead
change the assertion that checks the freeze duration to use 10 seconds). Ensure
you edit the module docstring and confirm consistency with the
assertion/threshold used in the test (the line that asserts the measured freeze
duration is less than 20 seconds).
In `@tests/storage/windows/test_windows_golden_image.py`:
- Line 51: The test disables SSH verification for the golden-image path by
calling running_vm(vm=vm, wait_for_interfaces=True,
check_ssh_connectivity=False); restore SSH checks by removing the override or
setting check_ssh_connectivity=True so running_vm uses the Windows SSH
validation (matching behavior in
tests/infrastructure/instance_types/supported_os/test_windows_os.py). Update the
call in tests/storage/windows/test_windows_golden_image.py to call running_vm
with the default (or explicit True) for check_ssh_connectivity to ensure
SSH/WSL2 paths are validated.
🪄 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: 19ab8428-a352-4bad-b6f3-7bd75e462f3c
📒 Files selected for processing (4)
tests/storage/windows/__init__.pytests/storage/windows/conftest.pytests/storage/windows/test_windows_app_consistent_snapshot.pytests/storage/windows/test_windows_golden_image.py
| ), | ||
| ) as vm: | ||
| add_dv_to_vm(vm=vm, template_dv=blank_data_disk_template) | ||
| running_vm(vm=vm, wait_for_interfaces=True, check_ssh_connectivity=False) |
There was a problem hiding this comment.
HIGH: Keep SSH validation enabled for the Windows snapshot fixture.
This fixture is the readiness gate for the snapshot scenario. With check_ssh_connectivity=False, a Windows image can pass setup even when SSH/WSL2 is broken, so the snapshot test stops validating the expected Windows image contract.
Suggested fix
- running_vm(vm=vm, wait_for_interfaces=True, check_ssh_connectivity=False)
+ running_vm(vm=vm, wait_for_interfaces=True)Based on learnings: In tests/infrastructure/instance_types/supported_os/test_windows_os.py, Windows guest images are configured with SSH support, so running_vm() should keep the default check_ssh_connectivity=True to verify SSH connectivity is working properly in the Windows guest images.
🤖 Prompt for 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.
In `@tests/storage/windows/conftest.py` at line 114, The Windows snapshot fixture
is disabling SSH validation by calling running_vm(vm=vm,
wait_for_interfaces=True, check_ssh_connectivity=False); revert this so SSH
connectivity is validated: remove the check_ssh_connectivity=False override (or
set it to True) in the running_vm call inside the Windows snapshot fixture in
tests/storage/windows/conftest.py to ensure the fixture verifies SSH/WSL2
connectivity as expected.
| Verifies that an online VirtualMachineSnapshot of a multi-disk Windows VM | ||
| (boot + data disk) freezes for less than 10 seconds, proving guest agent | ||
| freeze/thaw works correctly on the storage provider. |
There was a problem hiding this comment.
LOW: Align the documented freeze limit with the enforced threshold.
The module docstring says < 10s, but the code enforces < 20s. That mismatch will mislead anyone triaging runs around the 10-20 second range.
Suggested fix
- Verifies that an online VirtualMachineSnapshot of a multi-disk Windows VM
- (boot + data disk) freezes for less than 10 seconds, proving guest agent
+ Verifies that an online VirtualMachineSnapshot of a multi-disk Windows VM
+ (boot + data disk) freezes for less than 20 seconds, proving guest agent
freeze/thaw works correctly on the storage provider.Also applies to: 21-21
🤖 Prompt for 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.
In `@tests/storage/windows/test_windows_app_consistent_snapshot.py` around lines 4
- 6, The module docstring stating the freeze limit as "< 10s" is inconsistent
with the test assertion that enforces "< 20s"; update the module-level docstring
at the top of tests/storage/windows/test_windows_app_consistent_snapshot.py to
state "< 20s" so it matches the enforced threshold (or, if policy is to enforce
10s, instead change the assertion that checks the freeze duration to use 10
seconds). Ensure you edit the module docstring and confirm consistency with the
assertion/threshold used in the test (the line that asserts the measured freeze
duration is less than 20 seconds).
| vm = windows_vm_from_golden_image | ||
|
|
||
| LOGGER.info(f"Starting Windows VM {vm.name} from golden image...") | ||
| running_vm(vm=vm, wait_for_interfaces=True, check_ssh_connectivity=False) |
There was a problem hiding this comment.
HIGH: Re-enable SSH checks in the golden-image validation path.
This is the test that proves the golden image is usable. Disabling SSH verification means the test can still pass when the Windows image boots and reports guest-agent info but its SSH/WSL2 path is broken.
Suggested fix
- running_vm(vm=vm, wait_for_interfaces=True, check_ssh_connectivity=False)
+ running_vm(vm=vm, wait_for_interfaces=True)Based on learnings: In tests/infrastructure/instance_types/supported_os/test_windows_os.py, Windows guest images are configured with SSH support, so running_vm() should keep the default check_ssh_connectivity=True to verify SSH connectivity is working properly in the Windows guest images.
🤖 Prompt for 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.
In `@tests/storage/windows/test_windows_golden_image.py` at line 51, The test
disables SSH verification for the golden-image path by calling running_vm(vm=vm,
wait_for_interfaces=True, check_ssh_connectivity=False); restore SSH checks by
removing the override or setting check_ssh_connectivity=True so running_vm uses
the Windows SSH validation (matching behavior in
tests/infrastructure/instance_types/supported_os/test_windows_os.py). Update the
call in tests/storage/windows/test_windows_golden_image.py to call running_vm
with the default (or explicit True) for check_ssh_connectivity to ensure
SSH/WSL2 paths are validated.
2 participants
Summary
status.creationTime - metadata.creationTimestampto isolate freeze duration from backend finalization timeGating
ACCEPT_WINDOWS_EULA=trueenv var (skips if not set)windows11-golden-imageDataSource must exist (skips gracefully if missing)pytest.mark.windowsmarker — auto-included by T2 script when EULA is acceptedTested
sp-balanced-storage), freeze window was 11s, test passed with 20s thresholdTest plan
Made with Cursor
Summary by CodeRabbit