Repository navigation
cli: start a capture test's fake VMM before its deadline (SL3.7b follow-up) - #1495
Conversation
This pull request changes existing testsIt modifies or removes 3 existing test asset(s), listed below. Review each with care: a change to a test can hide a regression that the test caught. See doc/contribute.md#changes-to-existing-tests. Test code
Relocated unchanged: 0; added: 4. Compared the merge base |
…ow-up) The feature branch's push CI failed on Windows at 66fd431 (run 43071545182) in the capture test a_capture_ack_and_successful_output_without_timely_exit_are_stopped_but_never_finished: the fake VMM's ack.json was missing after the capture timed out. Four slice agents saw it fail on their Windows machines too, more often under CPU load; Linux hasn't shown it. The test captures with a 1 s timeout and a fake VMM that acknowledges the capture, writes successful output and never exits, to check that the command stops the VM and never finishes the capture. --timeout bounds launch, capture and exit, as the command's help says, so the deadline also covers the fake's startup, a new process that runs the test binary again. On a loaded Windows host that startup alone can outlast the deadline: the capture then timed out while it connected to the fake's control endpoint, before the fake had acknowledged anything, and the test checked a phase other than the one it means to check. The production code applies the deadline as documented. The test now starts its fake VMM before the capture, as an owned child, as the interrupted-capture tests start theirs. The fake waits for its capability on its standard input, which the capture's spawn step hands over, as spawn_vm does, and then enters its VM directory, so its startup never runs against the deadline. The child's guard ends the fake only after the test has checked that the capture stopped it. The test also checks that the error is the exit wait's own timeout, "capture VM did not exit before its deadline", so that a timeout in another phase fails with its cause, and it gives the two processes' handshake 5 s, which took up to 2.4 s under heavy CPU contention. The other capture tests keep spawning their fake through spawn_vm. Evidence, on a Windows 11 host (Intel i9-12900H, 20 logical processors) at 3340bbd, with 16 CPU-bound processes pinned to 2 logical processors and each run pinned to the same 2, interleaving four builds for 25 rounds: - the base test, 1 s: 23 of 25 runs failed, with the CI symptom; - the fake started first, 1 s: 2 of 25 failed, connecting; - the base test, 5 s: 4 of 25 failed, with the CI symptom; - the fake started first, 5 s: none of 25 failed. A second interleaving of the base test and this commit failed 6 and 0 of 25 runs. With a fake VMM that takes 6 s to start, the base test with a 5 s timeout failed 6 of 6 runs with the CI symptom, and this change passed 6 of 6. A capture that no longer stops its VM fails the test. On nvx-kvm-test (Linux, KVM), the test passed 20 of 20 runs, and 20 of 20 with 8 busy loops on its 2 processors, as the base test did, and the crate's unit tests fail the same 28 guest tests as at the base. Tests: the test keeps its assertions and also checks which phase timed out. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
841cf77 to
9b816f7
Compare
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues block the test-only update aimed at reducing Windows snapshot-capture timeout flakiness.
No open issues 🎉
What changed in this PR
This test-only follow-up to SL3.7b reduces Windows startup-related flakiness in the CLI’s snapshot-capture timeout test. Production capture behavior is unchanged.
Changes:
- Starts the fake VMM and waits for its startup receipt before beginning the capture deadline.
- Increases the timeout from one to five seconds and checks that failure comes specifically from waiting for the VMM to exit.
- Shares recipe-writing and process-recording helpers, while retaining the child guard until the test verifies that capture stopped the VMM.
| File | Description |
|---|---|
nvx-rs/src/host/cli/snapshots/capture/tests.rs |
Adds the prestarted fake-VMM path and strengthens the exit-timeout test without changing other tests’ spawn behavior. |
Review effort: Balanced
|
Rerun of failed jobs, for a GitHub infrastructure failure. Attempt 1 of CI run 43094763088 at this head (
The artifact API returned HTML error pages instead of JSON. That came before GitHub incident Before the outage, the jobs that run the changed test passed: I reran the failed jobs once ( |
ac3bbef
into
feature/nvx-refactor
Part of #1076. Follow-up to #1464 (SL3.7b).
What and why
Symptom. The push CI of
feature/nvx-refactorfailed on Windows at66fd43145ce(run 43071545182). The jobnvx-rs / windows-latestfailed in a capture test that SL3.7b (#1464) added:The job's other failure, in the restore driver's 60/30 s test, is the one that #1483 fixed. Agents working on other slices saw this test fail intermittently on their Windows workstations too, more often under CPU load. Linux hasn't shown it.
Root cause. The test captures with a 1 s
--timeoutand a fake VMM in itshangmode, which acknowledges the capture, writes successful output and never exits. It means to check that a capture whose VMM misses the exit deadline is stopped and never finished. Butcapture()takes one deadline when it starts, and its locks, the connection, the attach, the capture request and the exit wait all share it. So the deadline also covered the fake VMM's startup:spawn_vmstarts a new process that runs the test binary again (--exact capture_helper --ignored), which has to load, start the test harness, read its capability and create its control pipe. On a loaded Windows host that startup alone can take more than 1 s. The capture then timed out intransport::connect_verified, withthe control connection timed out: The system cannot find the file specified. (os error 2), before the fake had created its pipe, let alone acknowledged anything. The test thus checked the connection instead of the exit wait, andack.jsonwas missing. Temporary timing probes under CPU load showed it: in the runs that passed, the fake reached its first line 0.13 to 0.74 s after the test started its capture, and in those that failed, it hadn't reached it when the capture gave up and stopped it.Why the production code is unchanged. It applies the deadline as documented.
snapshot capture --helpsays that--timeoutis "positive whole seconds for launch, capture and exit, and each lock wait". Thesnapshot capturesection ofdoc/usage.mdsays that only a capture whose own VMM exits within--timeoutcan publish, and that a timed-out VM is stopped and its capture never finished. #1114's SL3.7b Scope says that a missed wait deadline stops the VM and never finishes the capture. A timeout in any phase stops the VM and finishes nothing, so the command is right; only the test assumed that its deadline started after the launch.Fix. Only the test changes:
Fixture::capture_startedstarts the fake VMM before the capture, as an owned child, as the interrupted-capture tests start theirs, and waits for itsstartedreceipt. The fake then blocks on its standard input until the capture's spawn step hands it the capability, asspawn_vmdoes, and enters its VM directory. Its startup never runs against the deadline.capture VM did not exit before its deadline, so a timeout in another phase now fails with its cause rather than with a missing file.The other capture tests keep spawning their fake VMM through
spawn_vm.Before and after, on a Windows 11 workstation (Intel i9-12900H, 20 logical processors) at
3340bbd7ee4, with 16 CPU-bound processes pinned to 2 logical processors and each run of the test pinned to the same 2:The first four rows ran interleaved, one run of each per round, and so did the two builds of the later series. Without busy processes pinned to its processors, the base test passed 60 of 60 runs in three series: alone, beside 20 busy processes, and as 10 concurrent copies. With 8 busy processes pinned to its 4 processors, it failed 1 of 20.
On nvx-kvm-test (Linux, KVM), the test passed 20 of 20 runs, and 20 of 20 interleaved with the base test, which also passed 20 of 20, with 8 busy loops on the test's 2 processors.
Size
Measured against
7bc238eb3d0: 0 core lines in 0 counted files. The change is innvx-rs/src/host/cli/snapshots/capture/tests.rsalone, with 75 lines added and 8 removed.Changes to existing tests
python scripts\check_test_changes.py --base origin/feature/nvx-refactorlists 3 changes to existing tests, hencetests-modified, and 4 added items:STARTED_VM_DIR,Fixture::capture_started,write_recipeandrecord.a_capture_ack_and_successful_output_without_timely_exit_are_stopped_but_never_finishedstarts its fake VMM before the capture, has a 5 s deadline instead of 1 s, and also checks that the error is the exit wait's timeout. It keeps every assertion that it had.capture_helper, the ignored fake VMM: when the fixture starts it ahead (NVX_CAPTURE_STARTED_VM_DIR), it writes astartedreceipt before it reads its capability, and then enters its VM directory. Spawned throughspawn_vm, it runs as before.fake_spawn: its checks of the launch barrier and its recipe moved intowrite_recipe, and its record of the process intorecord, whichcapture_startedshares. It spawns and returns the fake as before.Not in this PR
Production code, and the other capture tests, whose deadlines are 20 s or more.
Validation
On the local Windows 11 workstation, in
nvx-rs/withNVX_BUNDLE=skip. The changed module builds only in tests with theclifeature, so the--all-featurestest and Clippy runs are the ones that compile it. The head moved only by a rebase onto7bc238eb3d0with no change to the patch (git range-diff), and by a review fix that keeps the fake's guard until the test's last check.cargo fmt --all --check841cf773931cargo test --locked --all-features --lib host::cli::snapshots::capture841cf773931cargo clippy --locked --all-targets --all-features -- -D warnings254f97c99e6cargo test --locked --all-features --lib254f97c99e6cargo test --locked --all-featureswith the integration tests and doctests,cargo test --locked, the--lib --no-default-featuresbuild and tests (283), the other four Clippy configurations ofcheck-nvx-rs, andcargo +1.89 check --locked --all-targets --all-featuresa88deb1a499, before the review fixpython scripts\check_test_layout.pya88deb1a499On nvx-kvm-test (Azure Linux 3 VM with KVM), at
904965d576d, the tree of254f97c99e6:cargo test --locked --all-featuresfailed the same 28guest::library tests that fail at the base on that host. In that run, 12 tests of thecliintegration suite also failed while other jobs loaded the shared host (load average 12.75 on 8 processors). They passed at the base and ata88deb1a499, and rerun on this head once the load fell, the suite passed 102 of 102; it runs thenvx-rsbinary, which a test-only change doesn't alter. Ata88deb1a499,cargo test --lockedpassed.Left to CI: the full Linux and Windows suites at this head, the microVM scenarios, and the gates that a test-only change can't affect (artifact bundling, rustdoc, the runtime adapter,
nvx-cliand Markdown links).