Skip to content

microvm tests: stream OpenVMM transcripts to disk as they are read - #588

Open
Jay (jay23606) wants to merge 4 commits into
microsoft:devfrom
jay23606:feat/stream-openvmm-transcript
Open

Jay (jay23606) wants to merge 4 commits into
microsoft:devfrom
jay23606:feat/stream-openvmm-transcript

Conversation

@jay23606

Copy link
Copy Markdown
Contributor

Fixes #176.

Summary

  • OpenvmmProcess kept the transcript in memory and wrote it only from wait or close. A run that was terminated before cleanup left no transcript, or a truncated one, even though its output had been read.
  • The transcript is now opened before OpenVMM starts. Each consumed chunk is appended and flushed as it is read, and wait and close only close the file. Bytes are written in the order received, with no prefixes and no rewrite at cleanup, so nothing is duplicated.
  • A transcript that cannot be created stops the run before OpenVMM starts. Output still queued at close is written too.
  • In-memory output, marker search and timeout behavior are unchanged.
  • Scope: OpenvmmProcess only. TcpConsole has no transcript path, and the benchmark helpers in benchmark.py are not changed. Those are follow-ups.

Test

  • Three new tests: growth while the process runs (fails before the change, since the file did not exist until wait), a transcript that cannot be created (fails before the change), and output delivered after the last wait.
  • A real Windows child process printed BOOT, then LATE three seconds later. The transcript contained BOOT while the child was still running, and the final transcript matched the output.
  • test_microvm_tests: 193 tests. The dev baseline (190 tests) has the same nine failures and errors in host-loopback, private network ingress and SMP timer tests. They are unrelated to the transcript code and present on untouched dev.
  • ruff check and format are clean.
  • Not run: the PTY path on Linux, which needs a Linux host.

🤖 Generated with Claude Code

Copilot AI balanced review requested due to automatic review settings October 16, 2026 20:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Cleanup ordering can lose final reader output and skip resource closure when transcript writes fail.

1 open finding
What changed in this PR

Streams consumed OpenVMM output to disk incrementally, improving microVM test diagnostics when runs end before cleanup.

Changes:

  • Opens the transcript before starting OpenVMM.
  • Writes and flushes each consumed chunk while preserving in-memory output.
  • Adds tests for transcript growth, creation failures, and queued cleanup output.
File Description
scripts/​test_microvm_tests.py Adds three transcript persistence tests.
scripts/​nvx_tools/​openvmm_process.py Replaces cleanup-time transcript rewrites with incremental writes.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread scripts/nvx_tools/openvmm_process.py Outdated
Copilot AI balanced review requested due to automatic review settings October 16, 2026 20:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Interaction shutdown errors skip queue draining, losing already-queued diagnostics.

1 open finding
1 resolved since last review

🧠 Review effort: Balanced

Comment thread scripts/nvx_tools/openvmm_process.py
Copilot AI balanced review requested due to automatic review settings October 16, 2026 20:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The focused changes address the prior cleanup issues and include regression coverage, with no unresolved findings.

0 open findings

2 resolved since last review

🧠 Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 19, 2026 00:07
@jay23606
Jay (jay23606) force-pushed the feat/stream-openvmm-transcript branch from 74fb53b to 7aee64c Compare October 19, 2026 00:07

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The scoped changes and regression tests have no identified blocking issues; runtime validation was not performed during this review.

No open issues 🎉

Review effort: Balanced

Jay (jay23606) and others added 3 commits October 19, 2026 07:26
OpenvmmProcess kept the transcript in memory and wrote it only from wait or
close, so a run terminated before cleanup left no transcript, or a truncated
one, even though output had been read.

Open the transcript before OpenVMM starts, append each consumed chunk and flush
it, and close the file at wait or close. Bytes are written in the order received,
with no prefixes and no duplication at cleanup. An error creating the transcript
stops the run before OpenVMM starts. Output still queued at close is written
too. The in-memory output and marker search are unchanged.

Adds tests for growth while the process runs, a transcript that cannot be
created, and output delivered after the last wait.

Fixes microsoft#176

Co-Authored-By: Claude Haiku 5.5 <noreply@anthropic.com>
Signed-off-by: jay23606 <4602056+jay23606@users.noreply.github.com>
… release the transcript

close drained the reader's queue before the interaction had stopped, so a final
chunk queued during shutdown was lost from both output and transcript. A write
failure during that drain also skipped closing the interaction and the transcript.

Stop the interaction first, then drain, and release the transcript in a finally
block.

Adds regression tests for output queued during interaction close and for a
transcript write failure.

Co-Authored-By: Claude Haiku 5.5 <noreply@anthropic.com>
Signed-off-by: jay23606 <4602056+jay23606@users.noreply.github.com>
If InteractiveProcess.close raised, the drain was skipped and chunks still queued
were missing from both output and the transcript. Drain in a finally around the
interaction shutdown, so the transcript is complete whichever step fails.

Adds a regression test with queued output and a failing interaction shutdown.

Co-Authored-By: Claude Haiku 5.5 <noreply@anthropic.com>
Signed-off-by: jay23606 <4602056+jay23606@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 19, 2026 11:26
@jay23606
Jay (jay23606) force-pushed the feat/stream-openvmm-transcript branch from 7aee64c to bd17cf2 Compare October 19, 2026 11:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Transcript I/O failures can prevent remaining queued bytes from reaching in-memory output.

1 open issue

Review effort: Balanced

Comment thread scripts/nvx_tools/openvmm_process.py Outdated
_drain_available stopped at the first failed transcript write, so chunks
still queued behind it never reached process.output. Keep collecting them
in memory and re-raise the first error once the queue is empty.

Co-Authored-By: Claude Haiku 5.5 <noreply@anthropic.com>
Signed-off-by: jay23606 <4602056+jay23606@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 19, 2026 22:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

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.

Stream OpenVMM test transcripts to disk incrementally

2 participants