Skip to content

nvx-rs: build the NVXS unit tests' epoch identities with one function - #678

Merged
Pedro Henrique Penna (ppenna) merged 1 commit into
devfrom
nvx-rs-quality/nvxs-test-epochs
Oct 21, 2026
Merged

Pedro Henrique Penna (ppenna) merged 1 commit into
devfrom
nvx-rs-quality/nvxs-test-epochs

Conversation

@ppenna

Copy link
Copy Markdown
Contributor

Summary

The unit tests of the NVXS session, nvx-rs/src/protocol/nvxs/tests.rs and its guest and host modules, spelled out the identity of an epoch of their instance as a struct literal, Identity { epoch: N, ..IDENTITY }, at 13 sites, and as Identity { instance_id: INSTANCE, epoch: 0 }, the same kind of value, in the two lists of identities that lack an instance or an epoch. rustfmt spreads each literal over four lines, and a record(...) call that takes one over nine, so the epoch that a case varies was hard to see among the fields.

epoch(n), next to IDENTITY and NONE in tests.rs, now returns the identity of epoch n of the tests' instance, and all 15 sites call it. Each record that a case encodes now fits on one line, and the tests lose 71 lines in a change of 113.

This is the twentieth nvx-rs-quality/ pull request. #603, #605, #606, #607, #608, #609, #611, #613, #614, #615, #616, #617, #618, #632, #633, #634, #635, #636, and #637 merged, each after a Copilot overview review with no comment 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, the fake OpenVMM, the NVXS integration test, and tests/guest_tools.rs. This run looked at the parts that they left alone: protocol/nvxs and its unit tests, guest/nvxs.rs, host/nvxs.rs, guest/time_probe, guest/ingress, the guest programs' entry points and guest/status.rs, host/python, host/network, net, and host/cli/session.rs. It surveyed them by reading, with Clippy's pedantic lints as hints, and 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. It skipped every file that an open pull request to dev changed when it started: those of #652, #653, #655, #656, #662, #672, and #673 under nvx-rs/ (among them Cargo.toml, README.md, src/lib.rs, the managed agent, guest/mod.rs, guest/sys/{fd,mod,port,process}.rs, guest/time/{checks,commands,daemon,system,test_mode}.rs, host/cli/sandbox/{lifecycle,mod,state}.rs, host/filesystem.rs, host/instance.rs, host/mod.rs, host/platform, host/process.rs, host/sandbox/{distro,mod,options}, host/store/{mod,tests}.rs, host/sys/process.rs, and tests/{cli,managed_lifecycle,supervisor}.rs), the aci_edge_sandboxes/ files of #672, the tests/runtime-adapter/ files of #671, and doc/project-structure.md, which #652, #655, and #671 change. No open pull request changes the three files of this one.

Of the candidates, this one removes the most code per changed line, 71 lines in 113, in a module that no earlier run touched; #637's follow-ups named it. The production-code candidates remove less: a shared entry point for eight guest programs about 16 lines in about 55, parse_legacy_rule's reuse of lowering::port_selectors about 8 in 20, and a shared acknowledgement of ping and stop in host/cli/session.rs about 6 in 20. Follow-ups lists them and the others.

Where the choice was open, I took the conservative option:

  • epoch is a plain function in the tests' shared module, beside the constants and helpers that guest.rs and host.rs already import with use super::*. IDENTITY stays the literal that it was.
  • Only identities of the tests' instance at another epoch call it. Identities of another instance, or without one, such as Identity { instance_id: other, ..IDENTITY }, other_instance, the restored broker's, and the first entry of each list, keep their literals: they vary the instance, not the epoch.
  • host_stream used its next binding once, so it now passes epoch(2) in its place. The tests that use next several times keep the binding, now let next = epoch(2);.
  • The two lists of identities without an instance or an epoch stay where they are, in the_host_checks_the_sessions_identity and the_guest_checks_the_identity_of_each_reset; sharing them would be a change of its own.
  • The NVXS integration test, tests/nvxs, is a crate of its own with its own IDENTITY, and keeps its one Identity { epoch: 2, ..IDENTITY }.

Changes

All changes are in nvx-rs/src/protocol/nvxs/: 21 insertions and 92 deletions in three files.

  • tests.rs: epoch(epoch: u64) -> Identity, with a doc comment, after NONE; host_stream passes epoch(2) to its RESET.
  • The sites that now share it:
    • tests/host.rs: the RESET out of sequence in the_host_checks_the_brokers_sequence_numbers; in the_host_checks_the_sessions_identity, the list's epoch-0 identity, the DATA of epochs 2 and 0, the RESET of epoch 0, the attachment at epoch 3, and its stale RESET of epoch 2; and next in a_reset_ends_the_hosts_attachment;
    • tests/guest.rs: in the_guest_checks_the_identity_of_each_reset, the list's epoch-0 identity and the later and earlier RESETs, of epochs 9 and 8; the DATA of epoch 2 in the_guest_checks_the_sequence_and_identity_of_data; and next in a_reset_in_the_middle_of_a_message_abandons_it and the_guest_reports_the_same_events_for_every_split_of_its_stream.

No library item, binary, message, error kind, protocol byte, golden vector, fixture, or file outside these three changes.

Behavior

  • epoch(n) returns Identity { epoch: n, ..IDENTITY }, which is the value of each literal that it replaces: those with ..IDENTITY, and Identity { instance_id: INSTANCE, epoch: 0 }, since IDENTITY's instance is INSTANCE. Every test therefore encodes the same records, feeds them to the same sessions, and expects the same events, states, and errors.
  • No assertion changes, and no test is added, removed, skipped, or weakened: protocol::nvxs still has 32 unit tests, 10 for each role and 12 for the records.
  • Only #[cfg(test)] modules change. The library, the nvx-rs CLI, and the guest binaries build from unchanged sources; the hypervisor tests' binaries, which link the library, were already current on every host.

Tests that cover the change: the 32 protocol::nvxs unit tests, among them the 9 that build these identities, directly or through host_stream: 4 of the host's, 4 of the guest's, and the_parser_returns_the_same_records_for_every_split_of_a_stream. Every run of the library's unit tests below ran all 32, and all passed.

Follow-ups

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

  • The guest programs' entry points. nvx-console-pending, nvx-ingress, nvx-time, and nvx-time-probe share the same seven-line main, and nvx-mmio-write, nvx-reseed, nvx-device-io, and nvx-exit differ from it only in the arguments of run. A guest/status.rs function that restores SIGPIPE, runs a closure on sys::args(), and returns its ExitStatus would serve all eight; nvx-managed-agent, whose module nvx-rs: add caller-owned guest cgroup and child-status APIs #655 changes, could follow. The tests of nvx-time, nvx-time-probe, nvx-exit, and nvx-device-io each define an args helper that puts the program's name before the arguments.
  • The NVXS sessions. GuestSession::receive and HostSession::receive share the same nine lines after their state check, and send differs only in the state that it requires. guest/nvxs.rs's attach, acknowledge, and credit still repeat the same four steps around a session call.
  • The NVXS unit tests. guest.rs and host.rs list the same two identities that lack an instance or an epoch, and tests.rs and protocol/tests.rs define the same hex.
  • host/cli/session.rs. ping and stop differ only in their request, reply, and message, and features and exec repeat the mismatched-request-ID check.
  • guest/time_probe/clock.rs. run's two kernel-message sections repeat the same nine-line match, and its kernel section writes a label and a line three times.
  • guest/ingress. ListenerTable::add and diag::note_wildcard_tcp repeat a bounded find-or-insert merge, and the tests of address.rs and collect.rs repeat kernel_address.
  • net and host/network/mod.rs. net::is_valid_guest_network, guest_subnet, and the host-loopback check of openvmm_arguments parse a guest network's ADDRESS/PREFIX three times, with different limits; the last reads only the prefix, so a shared parser must keep that.
  • Follow-ups of earlier runs that remain: parse_legacy_rule's port branch; host/python/fs.rs's Windows stat and lstat; host/python/dumps.rs's list and dict loops; the policy and native test helpers of host/sandbox/tests.rs and host/sandbox/mount/tests.rs; tests/nvxs/broker.rs's parse loops; tests/guest_tools.rs's nvx-ingress hook command lines; and those that wait on files that open pull requests change, such as host/instance.rs, guest/time/test_mode.rs, and tests/cli.rs.

Validation

  • BASE: 916a61f0297900db13d7d9d04d170f01ef9ed71f, origin/dev when the run started and still when this pull request was opened: a performance-baseline update after the merge of nvx-rs: run the control session on the shared NVXS session's host role #641. No open pull request to dev changes the files of this one.

  • TREE: e975922dc785d96e76c54c25e2c1a613795139fc, 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.dad3e41d7f05, from the merge of nvx-rs: run the control session on the shared NVXS session's host role #641, the nearest first-parent ancestor of BASE that had packages, two commits earlier. Its OpenVMM, d0bdbed8f6e3b86271277dc22658109e2e633b2d, is the pinned revision.

  • download installed the release's nvx-rs, whose provenance names BASE's nvx-rs sources; TREE differs from them only in #[cfg(test)] modules. Each host built the openvmm_e2e and managed_lifecycle test binaries after its other Cargo work, found them current, and its hypervisor test logs show no compilation.

  • 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 2d12a797f23e00d7363ba9290f7efdb86b0b401227a380f6bc13c8e58f690f4b), 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, and on the library without default features;
    • the 32 protocol::nvxs unit tests;
    • the steps that no host could run, because the hosts lack the toolchain or target: the Windows MSRV (1.89) checks of nvx, aci_edge_sandboxes, and the runtime adapter's protocol-only and host-only configurations; the macOS (aarch64-apple-darwin) checks of nvx and aci_edge_sandboxes; and the MSRV check of the runtime adapter's guest-only configuration. That step joined check-nvx-rs after the last of these runs began, and no host's 1.89 has the musl target, so this machine ran it as the action defines it, starting with its rustup target add, which gave the local 1.89 the musl target; I removed the target again afterwards.

    The change adds, moves, and deletes no file and changes no Markdown, so the CLI tests of validate-nvx and the link check had nothing to cover.

  • One deviation, now undone: my first probe of prometheus28 listed the targets of 1.89, which that host did not have, and rustup installed it to answer. I uninstalled it at once, before any gate ran, so the host keeps only its own toolchains, and its gates skipped the MSRV steps, which ran on this machine.

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 (384 tests), 736 nvx tests with all features (642 of the library, among them the 32 of protocol::nvxs, and 34 guest_tools) and 359 with the default ones, artifact bundling (15), the runtime adapter's protocol-only, host-only, and musl guest-only configurations (13, 14, and 21 tests), 158 and 85 aci_edge_sandboxes tests, and agent bundling (4). The macOS checks and the guest-only MSRV check ran locally Copied in, digests match Pass (18/18) Pass (41 scenario runs)
prometheus30 mshv baremetal None; prometheus32 and prometheus28 ran the gates Built and tested here Pass (18/18) Pass (41 scenario runs)
prometheus28 whp baremetal Pass: every Windows step, including 429 nvx tests with all features (380 of the library, among them the 32 of protocol::nvxs) and 358 with the default ones, artifact bundling (15), the runtime adapter's protocol-only and host-only configurations (13 and 14 tests), 158 and 85 aci_edge_sandboxes tests, and agent bundling (4). The MSRV checks ran locally Copied in, digests match Pass (18/18) Pass (41 scenario runs)

Every gate and test passed at its first attempt. The Linux gates ran in the foreground of their SSH sessions with no signal ignored (SigIgn: 0), so the CLI's Ctrl-C tests ran as intended. 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, and the time-ABI evidence lines counted no boot, capture, or restore over its CPU-time budget on any host.

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.

The unit tests of the NVXS session, protocol/nvxs/tests.rs and its
guest and host modules, spelled out the identity of an epoch of their
instance as a struct literal, Identity { epoch: N, ..IDENTITY }, at 13
sites, and as Identity { instance_id: INSTANCE, epoch: 0 } in the two
lists of identities that lack an instance or an epoch. rustfmt spreads
each literal over four lines, and a record() call that takes one over
nine, so the epoch that a case varies hid in its layout.

epoch(n), next to IDENTITY in tests.rs, now returns that identity, and
all 15 sites call it: the RESET of host_stream; in the host's tests,
the RESET out of sequence, the DATA of epochs 2 and 0, the RESET of
epoch 0, the attachment at epoch 3 and its stale RESET, and the epoch
that a reset starts; and in the guest's tests, the later and earlier
RESETs, the DATA of epoch 2, and the epochs that a reset starts in the
middle of a message and in a split stream. The two lists take epoch(0)
for their second identity. Each record that a case encodes now fits on
one line, and the tests lose 71 lines.

Behavior is unchanged: epoch(n) returns the value of each literal that
it replaces, so every test encodes the same records and expects the
same events and errors. Only test code changes; the library, the CLI,
and the guest binaries build from unchanged sources.

The 32 protocol::nvxs unit tests cover the change, among them the 9
that build these identities, directly or through host_stream.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 21, 2026 04:47

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 test-only replacements preserve identity values and assertions, with no unresolved issues.

No open issues 🎉

What changed in this PR

Simplifies NVXS unit-test setup with a shared epoch identity helper, preserving test behavior and leaving production code unchanged.

Changes:

  • Adds epoch(n) beside the shared test constants.
  • Replaces 15 identity literals across the shared, host, and guest tests without changing assertions.
File Description
nvx-rs/​src/​protocol/​nvxs/​tests/​host.rs Uses the helper for host sequence, identity, and reset cases.
nvx-rs/​src/​protocol/​nvxs/​tests/​guest.rs Uses the helper for guest identity and reset cases.
nvx-rs/​src/​protocol/​nvxs/​tests.rs Defines the helper and simplifies the shared host stream.

Review effort: Balanced

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.

2 participants