Repository navigation
nvx-rs: build the NVXS unit tests' epoch identities with one function - #678
Merged
Pedro Henrique Penna (ppenna) merged 1 commit intoOct 21, 2026
Merged
Conversation
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>
Contributor
There was a problem hiding this comment.
🟢 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
This was referenced Oct 21, 2026
Pedro Henrique Penna (ppenna)
deleted the
nvx-rs-quality/nvxs-test-epochs
branch
October 21, 2026 11:30
This was referenced Oct 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The unit tests of the NVXS session,
nvx-rs/src/protocol/nvxs/tests.rsand itsguestandhostmodules, spelled out the identity of an epoch of their instance as a struct literal,Identity { epoch: N, ..IDENTITY }, at 13 sites, and asIdentity { 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 arecord(...)call that takes one over nine, so the epoch that a case varies was hard to see among the fields.epoch(n), next toIDENTITYandNONEintests.rs, now returns the identity of epochnof 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 andguest/sys,host/artifacts.rs,host/filesystem.rs,host/session.rs, the CLI,host/sandbox, the fake OpenVMM, the NVXS integration test, andtests/guest_tools.rs. This run looked at the parts that they left alone:protocol/nvxsand its unit tests,guest/nvxs.rs,host/nvxs.rs,guest/time_probe,guest/ingress, the guest programs' entry points andguest/status.rs,host/python,host/network,net, andhost/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 insrcandtests, exactly and with names and literals abstracted. It skipped every file that an open pull request todevchanged when it started: those of #652, #653, #655, #656, #662, #672, and #673 undernvx-rs/(among themCargo.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, andtests/{cli,managed_lifecycle,supervisor}.rs), theaci_edge_sandboxes/files of #672, thetests/runtime-adapter/files of #671, anddoc/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 oflowering::port_selectorsabout 8 in 20, and a shared acknowledgement ofpingandstopinhost/cli/session.rsabout 6 in 20. Follow-ups lists them and the others.Where the choice was open, I took the conservative option:
epochis a plain function in the tests' shared module, beside the constants and helpers thatguest.rsandhost.rsalready import withuse super::*.IDENTITYstays the literal that it was.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_streamused itsnextbinding once, so it now passesepoch(2)in its place. The tests that usenextseveral times keep the binding, nowlet next = epoch(2);.the_host_checks_the_sessions_identityandthe_guest_checks_the_identity_of_each_reset; sharing them would be a change of its own.tests/nvxs, is a crate of its own with its ownIDENTITY, and keeps its oneIdentity { 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, afterNONE;host_streampassesepoch(2)to its RESET.tests/host.rs: the RESET out of sequence inthe_host_checks_the_brokers_sequence_numbers; inthe_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; andnextina_reset_ends_the_hosts_attachment;tests/guest.rs: inthe_guest_checks_the_identity_of_each_reset, the list's epoch-0 identity and thelaterandearlierRESETs, of epochs 9 and 8; the DATA of epoch 2 inthe_guest_checks_the_sequence_and_identity_of_data; andnextina_reset_in_the_middle_of_a_message_abandons_itandthe_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)returnsIdentity { epoch: n, ..IDENTITY }, which is the value of each literal that it replaces: those with..IDENTITY, andIdentity { instance_id: INSTANCE, epoch: 0 }, sinceIDENTITY's instance isINSTANCE. Every test therefore encodes the same records, feeds them to the same sessions, and expects the same events, states, and errors.protocol::nvxsstill has 32 unit tests, 10 for each role and 12 for the records.#[cfg(test)]modules change. The library, thenvx-rsCLI, 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::nvxsunit tests, among them the 9 that build these identities, directly or throughhost_stream: 4 of the host's, 4 of the guest's, andthe_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.
nvx-console-pending,nvx-ingress,nvx-time, andnvx-time-probeshare the same seven-linemain, andnvx-mmio-write,nvx-reseed,nvx-device-io, andnvx-exitdiffer from it only in the arguments ofrun. Aguest/status.rsfunction that restoresSIGPIPE, runs a closure onsys::args(), and returns itsExitStatuswould 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 ofnvx-time,nvx-time-probe,nvx-exit, andnvx-device-ioeach define anargshelper that puts the program's name before the arguments.GuestSession::receiveandHostSession::receiveshare the same nine lines after their state check, andsenddiffers only in the state that it requires.guest/nvxs.rs'sattach,acknowledge, andcreditstill repeat the same four steps around a session call.guest.rsandhost.rslist the same two identities that lack an instance or an epoch, andtests.rsandprotocol/tests.rsdefine the samehex.host/cli/session.rs.pingandstopdiffer only in their request, reply, and message, andfeaturesandexecrepeat 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::addanddiag::note_wildcard_tcprepeat a bounded find-or-insert merge, and the tests ofaddress.rsandcollect.rsrepeatkernel_address.netandhost/network/mod.rs.net::is_valid_guest_network,guest_subnet, and the host-loopback check ofopenvmm_argumentsparse a guest network'sADDRESS/PREFIXthree times, with different limits; the last reads only the prefix, so a shared parser must keep that.parse_legacy_rule'sportbranch;host/python/fs.rs's Windowsstatandlstat;host/python/dumps.rs's list and dict loops; thepolicyandnativetest helpers ofhost/sandbox/tests.rsandhost/sandbox/mount/tests.rs;tests/nvxs/broker.rs's parse loops;tests/guest_tools.rs'snvx-ingresshook command lines; and those that wait on files that open pull requests change, such ashost/instance.rs,guest/time/test_mode.rs, andtests/cli.rs.Validation
BASE:
916a61f0297900db13d7d9d04d170f01ef9ed71f,origin/devwhen 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 todevchanges the files of this one.TREE:
e975922dc785d96e76c54c25e2c1a613795139fc, the tree of this commit. Every host applied the patch to BASE and checked thatgit write-treeequals 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.downloadinstalled the release'snvx-rs, whose provenance names BASE'snvx-rssources; TREE differs from them only in#[cfg(test)]modules. Each host built theopenvmm_e2eandmanaged_lifecycletest 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;build-initramfswithNVX_GUEST_BINARIES.initramfs.cpio.gz(SHA-2562d12a797f23e00d7363ba9290f7efdb86b0b401227a380f6bc13c8e58f690f4b), 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:
-D warnings, with all targets and features, for Windows and forx86_64-unknown-linux-gnu, and on the library without default features;protocol::nvxsunit tests;nvx,aci_edge_sandboxes, and the runtime adapter's protocol-only and host-only configurations; the macOS (aarch64-apple-darwin) checks ofnvxandaci_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 itsrustup 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.test-aci-edge-sandboxestest-microvm, default suitecheck-guest-binaries(384 tests), 736nvxtests with all features (642 of the library, among them the 32 ofprotocol::nvxs, and 34guest_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 85aci_edge_sandboxestests, and agent bundling (4). The macOS checks and the guest-only MSRV check ran locallynvxtests with all features (380 of the library, among them the 32 ofprotocol::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 85aci_edge_sandboxestests, and agent bundling (4). The MSRV checks ran locallyEvery 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'smanaged-lifecyclescenario ran themanaged_lifecycletest with Cargo, which passed. OpenVMM warned of a host CPU-profile fallback only in thecpu-profile-fallbackscenario, 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:
test-openvmm-unitandtest-openvmm;smp-lapic,managed-exec-config, andsandbox-lifecycle;