Skip to content

parity: add LCOW document permutation tests#1

Closed
shreyanshjain7174 wants to merge 1 commit intoparity/lcow-document-testfrom
parity/lcow-permutations
Closed

parity: add LCOW document permutation tests#1
shreyanshjain7174 wants to merge 1 commit intoparity/lcow-document-testfrom
parity/lcow-permutations

Conversation

@shreyanshjain7174
Copy link
Copy Markdown
Owner

This PR extends the LCOW parity test infrastructure from microsoft#2629 with permutation tests that exercise annotation and option combinations triggering different document construction branches in the legacy and v2 pipelines.

Changes

Permutation tests (TestLCOWDocumentParityPermutations)

19 test cases covering:

  • CPU partial combinations — count-only, limit-only, weight-only
  • Memory — overcommit disabled, cold discard hint
  • Boot mode — kernel direct + VHD rootfs
  • Feature flags — scratch encryption + disabled writable shares, writable overlay dirs
  • Device interactions — VPMem disabled (triggers 4 SCSI controllers)
  • Cross-group — physically backed + VPMem disabled + scratch encryption, full CPU + memory + MMIO + QoS + cold discard + VHD boot
  • Shim option overrides — annotation CPU/memory overriding shim option values
  • Kernel args — VPCIEnabled + custom boot options, time sync + process dump + writable overlay, initrd boot whitespace quirk

Gap tests

3 tests documenting known v2 builder differences using inverted assertions (they expect a diff and only fail if the documents unexpectedly match):

Gap Legacy V2
No CPUGroupID set CpuGroup = nil CpuGroup = &{Id:""}
No StorageQoS set StorageQoS = nil StorageQoS = &{}
Initrd boot VPMem controller allocated VirtualPMem = nil

These are tracked for fixes in the v2 builder (see microsoft#2630).

Kernel cmdline whitespace handling

Added normalizeKernelCmdLine and isOnlyKernelCmdLineWhitespaceDiff helpers. The legacy builder produces a leading space in kernel command lines for initrd+KernelDirect boot that the v2 builder correctly omits. Since HCS trims whitespace, this difference is harmless and we warn instead of failing.

Testing

Requires admin PowerShell + Hyper-V enabled (tests call processorinfo.HostProcessorInfo via HCS).

cd test && go test -v -run TestLCOWDocumentParityPermutations ./parity/vm/

Depends on

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6a7f0642fe

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +521 to +524
if diff == "" {
t.Errorf("gap test unexpectedly passed: v2 builder bug may be fixed; remove from gaps")
} else {
t.Logf("expected gap diff (-legacy +v2):\n%s", diff)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Assert concrete gap fields rather than any mismatch

The gap: branch passes whenever cmp.Diff is non-empty, so these cases no longer verify the documented gaps (CpuGroup, StorageQoS, VirtualPMem) and will also pass if a different regression appears in those scenarios. This makes the test both brittle (it fails when parity improves) and blind to unrelated regressions; tightening the assertion to check for the specific expected field differences would preserve the intended signal.

Useful? React with 👍 / 👎.

Add TestLCOWDocumentParityPermutations to exercise annotation and option
combinations that trigger different document construction branches in the
legacy and v2 LCOW pipelines. Each test sets all required fields so
comparisons check real values rather than defaults.

Permutation categories:
- CPU partial combinations (count, limit, weight)
- Memory (overcommit disabled, cold discard hint)
- Boot mode (kernel direct + VHD rootfs)
- Feature flags (scratch encryption, writable overlay)
- Device interactions (VPMem disabled → 4 SCSI controllers)
- Cross-group (physically backed + VPMem + encryption)
- Shim option overrides (annotation CPU/memory priority)
- Kernel args (VPCIEnabled, time sync, process dump, initrd boot)

Gap tests document three known v2 builder differences:
- No CPUGroupID: legacy nil vs v2 empty CpuGroup struct
- No StorageQoS: legacy nil vs v2 empty StorageQoS struct
- Initrd boot: legacy VPMem controller vs v2 nil VirtualPMem

Gap tests use inverted assertions — they expect a diff and only fail if
documents unexpectedly match, signaling the v2 bug was fixed.

Also adds normalizeKernelCmdLine and isOnlyKernelCmdLineWhitespaceDiff
helpers to handle a known legacy quirk where initrd+KernelDirect boot
produces a leading space in kernel command lines that v2 correctly omits.

Signed-off-by: Shreyansh Sancheti <shsancheti@microsoft.com>
@shreyanshjain7174 shreyanshjain7174 force-pushed the parity/lcow-permutations branch from 6a7f064 to 917d728 Compare March 16, 2026 11:15
@shreyanshjain7174
Copy link
Copy Markdown
Owner Author

Closing: duplicate of microsoft#2631

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant