[Pedro Henrique Penna (ppenna)]

Summary

The NVXS integration test, nvx-rs/tests/nvxs, encoded its records in four places. host.rs and guest.rs each defined the same record() function and the same IDENTITY constant, the identity of the test broker's first epoch, word for word. The broker's queue_guest and queue_host in broker.rs each spelled out the same encode_record call, which differed only in the buffer and the sequence counter it used, and session.rs built the first identity inline once more.

broker.rs already holds the session's INSTANCE and CAPABILITY, and every build of the test compiles it. It now defines IDENTITY and record() once, as pub(crate) items. The other modules import them, and the broker's queue methods encode with record() at self.identity(). The test now has one record encoder and one definition of its first identity, so a change to either, such as a new header field, touches one place. The four files lose 39 lines.

This is the eighteenth nvx-rs-quality/ pull request. #603, #605, #606, #607, #608, #609, #611, #613, #614, and #615 merged, #615 while this run was underway. #616, #617, #618, #632, #633, #634, and #635 are open. Each had a Copilot review that recommended approval with no open issues, and no other feedback to follow, and none was closed without merging. Those runs changed the EXEC and MAPS codecs, the managed agent, the guest tools and guest/sys, host/artifacts.rs, host/filesystem.rs, host/session.rs, the CLI, host/sandbox, and the fake OpenVMM. This run looked at the parts that they left alone. It surveyed the crate by reading, with a throwaway script, kept outside the repository, that reports every repeated window of six or more normalized lines in src and tests, exactly and with names and literals abstracted, and with a function-level similarity pass. It skipped every file that an open pull request to dev changed when it started: those of the open nvx-rs-quality/ pull requests above, and those of #625, #628, #629, and #631 (Cargo.toml, README.md, src/lib.rs, the managed agent, guest/mod.rs, guest/sys/{fd,mod,spawn}.rs, guest/time_probe/clock.rs, host/mod.rs, host/platform, host/process.rs, host/sys, the new host/store, and tests/store.rs and tests/supervisor.rs).

Of the candidates, this cluster, which the NVXS session of #627 introduced, removes the most duplicated code: 39 lines, all of it exact duplication, in a part of the crate that no earlier run touched. It changes only tests. The policy and native helpers that host/sandbox/tests.rs and host/sandbox/mount/tests.rs both define would remove somewhat more per changed line, about 11 lines in about 18. They are two small helpers, rather than four copies of an encoder and three of a constant, in the tests of a module that recent runs changed, so I took the larger consolidation. The candidates in production code remove less: the closest, a shared IPv4 ADDRESS/PREFIX parser for net::is_valid_guest_network and host/network/mod.rs's guest_subnet, would remove about 8 lines in about 40 changed. Follow-ups lists the others.

Changes

All changes are in nvx-rs/tests/nvxs/: 37 insertions and 76 deletions in four files.

• broker.rs defines IDENTITY, next to INSTANCE and CAPABILITY, and record(), moved as it was. Its signature now wraps, since pub(crate) makes the line too long.
• The sites that now share the code:
  • Broker::queue_guest and Broker::queue_host in broker.rs, which encode with record() at self.identity() and append the record to their buffer, instead of calling encode_record with INSTANCE and self.epoch;
  • host.rs, which drops its own IDENTITY and record() and imports the broker's;
  • guest.rs, likewise;
  • session.rs, whose messages_of_an_independent_framing_span_records_and_survive_partial_reads compares the host's attachment with IDENTITY instead of a local identity of INSTANCE at epoch 1.
• host.rs and guest.rs no longer import INSTANCE or encode_record, and guest.rs and session.rs no longer import Identity, which nothing else in them uses.

INSTANCE keeps its pub(crate) visibility, though only broker.rs uses it now. No library item, binary, message, error kind, protocol byte, fixture, or file outside tests/nvxs changes.

Behavior

• record() keeps its body, so every scripted record of the host and guest tests has the same bytes.
• The broker writes the same bytes in the same order. Broker::identity() returns Identity { instance_id: INSTANCE, epoch: self.epoch }, the instance and epoch that the queue methods passed to encode_record, at the same sequence numbers. encode_record validates a record and appends its encoding to its output, so appending the encoded record to the broker's buffer leaves the same buffer as encoding into it did. A record that encode_record refused would still panic the test, now in record(); the broker queues none.
• IDENTITY has the value of the constants that it replaces and of session.rs's local identity.
• Only test code changes, and no test is added, removed, skipped, or weakened. The library, the nvx-rs CLI, and the guest binaries build from unchanged sources, so the release's nvx-rs, whose provenance covers the crate's manifest, lock file, build script, src/, and rust-toolchain.toml but not tests/, still described the checkout on every host.
• The broker's queue methods use record(), and session.rs uses IDENTITY. Every build of the test compiles both modules, so the shared items never become dead code in a build without host or guest.

Tests that cover the change: the nvxs test's host tests and session tests on every platform, and its guest tests on Linux with the guest feature. The broker encodes every record that it relays in those tests with its queue methods, and the host and guest tests encode their scripted records with record(). All passed in the test runs below.

Follow-ups

The survey turned these up, and this change leaves them alone. I found no suspected bug.

• tests/nvxs/broker.rs. accept_guest and accept_host repeat the same parse loop over their parser, and check_guest and the host's DATA branch of host_record repeat the same header check.
• The NVXS unit tests. protocol/nvxs/tests.rs repeats the tests' application framing, framed and Reassembler, of tests/nvxs/main.rs, and its own record(). An integration test cannot share a library's test module, so sharing them would need a testing item.
• net and host/network/mod.rs. net::is_valid_guest_network and guest_subnet parse an IPv4 ADDRESS/PREFIX and derive its network and mask the same way. A shared parser would take their prefix limits, /1 to /30 and /1 to /32.
• host/instance.rs. Instance::runtime and Instance::launch read a record and refuse another STATE_FORMAT in two control-flow shapes.
• guest/nvxs.rs. GuestConnection::attach, acknowledge, and credit repeat the same four steps around a different session call: check, clear the output, encode, and write.
• host/python/dumps.rs. write_value repeats the bracket, comma, and indentation loop for lists and dicts.
• guest/time/test_mode.rs. test_cmdline and test_clocksource, and in part test_cpuinfo, write ok or a check's failure event in the same way.
• host/network/policy/mod.rs. parse_legacy_rule writes out the port branch of lowering::port_selectors, which its own earlier endPort requires port check makes equivalent.
• Follow-ups of earlier runs that remain:
  • the policy and native test helpers of host/sandbox/tests.rs and host/sandbox/mount/tests.rs;
  • the option-values test helpers of host/filesystem.rs, host/network/mod.rs, and host/sandbox/options/tests.rs;
  • host/launch.rs's process_command_line_units, which measures the quoting of python::shell::list2cmdline. It models the platform's quoting rather than Python's, so a shared writer wants a decision of its own;
  • the guest programs' main entry convention and args test helpers, which wait on open pull requests;
  • the time tools' watcher-violation sites and port-window lock failures;
  • guest/ingress's bounded find-or-insert merges;
  • the stop-and-deprovision loop of tests/cli.rs's Ctrl-C tests.

Validation

• BASE: 28ae9449e45274aff4cd450e245d0ae05fe9c3bf, which was origin/dev when the run started: a performance-baseline update on top of the merge of nvx-rs: add an application-neutral NVXS session #627. Before this pull request was opened, origin/dev had advanced by the merges of nvx-rs: check the guest agent's replies through shared methods #615 and build: reject Ubuntu initramfs source edits made during packing #584 and another baseline update, none of which changes the four files, and no open pull request to dev changes them.

• TREE: 8454093be411f026e0c067da256f27c865bbbd61, the tree of this commit. Every host applied the patch to BASE and checked that git write-tree equals TREE.

• Release on every host: v0.1.0-dev.f9b6b142aec4, from the merge of nvx-rs: add an application-neutral NVXS session #627, the nearest first-parent ancestor of BASE that had packages, three commits earlier. Its OpenVMM, d0bdbed8f6e3b86271277dc22658109e2e633b2d, is the pinned revision.

• download installed the release's nvx-rs, since its provenance describes the checkout's nvx-rs sources, which this change leaves alone. Each host built the openvmm_e2e and managed_lifecycle test binaries after its other Cargo work, so that no Cargo build overlapped its hypervisor tests.

• Guest artifacts were rebuilt from TREE on prometheus30, whose user namespaces let Alpine's apk run its scripts, with these steps:

  • build-guest-binaries --output;
  • test-guest-binaries --binaries, where 34 tests passed;
  • the Alpine build-initramfs with NVX_GUEST_BINARIES.

  initramfs.cpio.gz (SHA-256 4ef90a58ebbc2abfaf47474147972eb2d0cfe57d087c4d7af58bcecff4207a00), its packages manifest, and its provenance went through this machine to the other two hosts, where the SHA-256 digests matched.

• Local checks on Windows with Rust 1.95.0:

  • rustfmt;
  • Clippy with -D warnings, with all targets and features, for Windows and for x86_64-unknown-linux-gnu, which compiles tests/nvxs/guest.rs;
  • Clippy with -D warnings on the nvxs test for x86_64-unknown-linux-gnu without default features, and with only guest;
  • the nvxs test with all features and with the default ones, 9 tests each;
  • the steps that no host could run, because the hosts lack the toolchain or target: the Windows MSRV (1.89) checks and the macOS (aarch64-apple-darwin) checks, for both nvx-rs and aci_edge_sandboxes.

Host profile	Backend	Host type	check-nvx-rs and check-aci-edge-sandboxes without a hypervisor	Guest artifacts	test-aci-edge-sandboxes	test-microvm, default suite
prometheus32	kvm	baremetal	Pass: every Linux step, including MSRV 1.89, check-guest-binaries (361 tests), 659 nvx-rs tests with all features and 306 with the default ones, artifact bundling, 158 and 85 aci_edge_sandboxes tests, and agent bundling. The nvxs test ran 13 of them with all features, 6 host, 3 session, and 4 guest tests, and 9 with the default ones. The macOS checks ran locally	Copied in, digests match	Pass (18/18)	Pass (31 scenarios)
prometheus30	mshv	baremetal	None; prometheus32 and prometheus28 ran the gates	Built and tested here	Pass (18/18)	Pass (31 scenarios)
prometheus28	whp	baremetal	Pass: every Windows step, including 375 nvx-rs tests with all features and 306 with the default ones, 9 of each from nvxs, artifact bundling, 158 and 85 aci_edge_sandboxes tests, and agent bundling. The MSRV checks ran locally	Copied in, digests match	Pass (18/18)	Pass (31 scenarios)

Every gate and test passed at its first attempt. The Linux gates and tests ran in the foreground of their SSH sessions with SIGINT at its default disposition, so the CLI's Ctrl-C tests ran as intended; the sessions' shells ignored only SIGQUIT (SigIgn: 0x4). Each suite's managed-lifecycle scenario ran the managed_lifecycle test with Cargo, which passed. OpenVMM warned of a host CPU-profile fallback only in the cpu-profile-fallback scenario, which forces one. The time-ABI evidence lines counted no capture or restore over its CPU-time budget, and no boot over it on prometheus32 and prometheus28. On prometheus30, one boot of 58, that of the time-abi-conformance scenario at 8 vCPUs, spent 10,035 µs in nvx-time's boot check against MSHV's 10,000 µs budget. The suite reports that count without gating on it, and this change does not touch the time tools.

Left to CI:

• the OpenVMM unit and VMM suites, test-openvmm-unit and test-openvmm;
• the microVM scenarios that run only when named: smp-lapic, managed-exec-config, and sandbox-lifecycle;
• the debug-kernel scenarios, the Ubuntu and Azure Linux guests, and guest determinism;
• the benchmarks;
• the virtual-machine runners;
• check-quality and validate-nvx, whose inputs this change does not touch.

[Copilot]

🟢 Approval recommended

The focused test-only refactor preserves record bytes, identities, and sequencing, with no blocking issues identified.

No open issues 🎉

What changed in this PR

Consolidates record encoding and the first-epoch identity in the NVXS integration tests without changing production code.

Changes:

• Defines shared record() and IDENTITY helpers in the test broker.
• Reuses them across broker queues and host, guest, and session tests, removing 39 lines overall.

File	Description
nvx-rs/​tests/​nvxs/​session.rs	Uses the shared identity in the attachment assertion.
nvx-rs/​tests/​nvxs/​host.rs	Replaces duplicate helpers with broker imports.
nvx-rs/​tests/​nvxs/​guest.rs	Replaces duplicate helpers with broker imports.
nvx-rs/​tests/​nvxs/​broker.rs	Defines shared helpers and uses the encoder in both queues.

_{Review effort: Balanced}

[Enrique Saurez (esaurez)]

lgtm