Skip to content

nvx-rs: run the guest-binary tests' nvx-time hooks with one helper - #637

Open
Pedro Henrique Penna (ppenna) wants to merge 1 commit into
devfrom
nvx-rs-quality/nvx-time-hooks
Open

Pedro Henrique Penna (ppenna) wants to merge 1 commit into
devfrom
nvx-rs-quality/nvx-time-hooks

Conversation

@ppenna

Copy link
Copy Markdown
Contributor

Summary

nvx-rs/tests/guest_tools.rs runs nvx-time's test hooks on fixture files at 18 sites, and each spelled out the hook's command line, nvx-time test HOOK FIXTURE ARGUMENTS..., as an array of OsStr values: OsStr::new("test"), OsStr::new(HOOK), fixture.as_os_str(), and an OsStr::new for each argument, in 4 to 10 lines. One of the sites was already a helper, run_time_packet, which writes a packet fixture and runs the packet hook on it, yet the three packet sites of time_test_mode_decodes_packets_samples_and_arithmetic built the same command line by hand.

time_hook(hook, fixture, arguments), next to time_fixture, which writes the fixtures, now builds that command line once and runs it with run. run_time_packet keeps its signature and calls it, and so does every other site, so each call names only the hook, the fixture, and the arguments that vary. The hook's command line now has one definition, and the file loses 106 lines.

This is the nineteenth nvx-rs-quality/ pull request. #603, #605, #606, #607, #608, #609, #611, #613, #614, and #615 merged, and #616, #617, #618, #632, #633, #634, #635, and #636 are open. Each had a Copilot overview review and 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, and the NVXS integration test. This run looked at the parts that they left alone: host/network, host/python, host/instance.rs, host/launch.rs, host/nvxs.rs, net, protocol/nvxs, guest/time, guest/time_probe, guest/ingress, guest/nvxs.rs, and tests/guest_tools.rs and tests/cli.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, and once more over production code alone. 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; 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 guest/spawn.rs, and tests/store.rs and tests/supervisor.rs); and doc/project-structure.md, which #626, #628, #629, and #630 change.

Of the candidates, this one removes the most code per changed line, 106 lines in 182, by generalizing a helper that the file already had, in a file that no earlier run touched. The production-code candidates remove less: a shared main for the four guest programs whose run takes the arguments and both standard streams would remove about 15 lines in about 45, and parse_legacy_rule's reuse of lowering::port_selectors about 8 in 20. Follow-ups lists them and the others.

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

  • run_time_packet stays, with its signature, because its six callers in time_test_mode_checks_packet_and_sample_boundaries pass it a directory, a name, and packet bytes; only its body changes.
  • The sites keep their statements that write fixtures, so only their command lines change. The three packet sites of time_test_mode_decodes_packets_samples_and_arithmetic therefore call time_hook on the fixtures that they already wrote, rather than run_time_packet.
  • The nvx-time event hook, which takes no fixture and passes an argument that is not UTF-8, and the nvx-ingress test hook, of another program, keep their command lines. A helper for them would be a change of its own.
  • time_hook takes its arguments as &str, as every site passed them, and is #[cfg(target_arch = "x86_64")], as the file's other nvx-time helpers are.

Changes

All changes are in nvx-rs/tests/guest_tools.rs: 38 insertions and 144 deletions.

  • time_hook, after time_fixture, with a doc comment: chains test, the hook, and the fixture's path with the arguments, and runs nvx-time with run.
  • The sites that now share it:
    • time_test_mode_decodes_packets_samples_and_arithmetic: the packet hook on the packet, ranges, and no-ranges fixtures, and the sample hook;
    • time_test_mode_checks_logs_timer_state_and_status: the boot-log, kmsg, and timer-list hooks, the state hook on the valid and the malformed state, and the status hook;
    • time_test_mode_preserves_boot_log_and_syslog_results: the run_log closure, which runs the boot-log and syslog hooks;
    • run_time_packet, which the six packet cases of time_test_mode_checks_packet_and_sample_boundaries call, and that test's two sample sites;
    • time_test_mode_covers_checks_events_records_and_workers: the two cpuinfo sites;
    • time_test_mode_covers_remaining_events_records_and_workers: the two restore-record sites.

No library item, binary, message, error kind, protocol byte, fixture, or file outside tests/guest_tools.rs changes.

Behavior

  • time_hook passes run OsStr::new("test"), OsStr::new(hook), fixture.as_os_str(), and OsStr::new of each argument, in that order: the values that each site passed, in its order. run spawns nvx-time with them as before, so every test runs the same command and compares the same exit status, standard output, and standard error with the same expectations.
  • run and every assertion are unchanged, and no test is added, removed, skipped, or weakened: guest_tools still runs 34 tests.
  • Only test code changes. The library, the nvx-rs CLI, and the guest binaries build from unchanged sources: the Alpine initramfs built here from the guest binaries of TREE has the SHA-256 that nvx-rs: encode the NVXS tests' records with one function #636 reported for its own build, 4ef90a58ebbc2abfaf47474147972eb2d0cfe57d087c4d7af58bcecff4207a00.

Tests that cover the change: the 34 guest_tools tests, which call time_hook at every site above. On Linux, cargo test --all-features, check-guest-binaries, and test-guest-binaries against a guest-profile build of the binaries each ran all 34, and all passed.

Follow-ups

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

  • tests/guest_tools.rs. The five nvx-ingress test DIRECTORY sites spell out their command line as the nvx-time sites did, and device_io repeats run's spawn and collection to keep the ExitStatus for its message.
  • The guest programs' entry points. nvx-console-pending, nvx-ingress, nvx-time, and nvx-time-probe have the same seven-line main, which restores SIGPIPE and turns the result of run(&sys::args(), &mut sys::Stdout::new(), &mut io::stderr()) into an exit code. guest/status.rs could hold it. nvx-device-io and nvx-managed-agent, whose files open pull requests change, start differently, and the tests of nvx-time and nvx-time-probe repeat one args helper.
  • The NVXS unit tests. protocol/nvxs/tests/guest.rs and host.rs write Identity { epoch: N, ..IDENTITY } 13 times, and tests.rs once more, and both list the same two identities that lack an instance or an epoch.
  • host/network/policy/mod.rs. parse_legacy_rule still writes out the port branch of lowering::port_selectors. Reusing it would let port_protocol_selectors fold into port_selectors.
  • host/python/fs.rs. The Windows stat and lstat build the same Stat, and the POSIX status returns a Result that never fails.
  • Follow-ups of earlier runs that remain: those that nvx-rs: encode the NVXS tests' records with one function #636 lists for tests/nvxs/broker.rs, protocol/nvxs/tests.rs, net and host/network/mod.rs, host/instance.rs, guest/nvxs.rs, host/python/dumps.rs, guest/time/test_mode.rs, the sandbox test helpers, host/launch.rs, the time tools, guest/ingress, and tests/cli.rs.

Validation

  • BASE: 6cb1189dec42b247062790623d0ca85b716316f1, which was origin/dev when the run started: the merge of build: reject Ubuntu initramfs source edits made during packing #584. Before this pull request was opened, origin/dev had advanced to aface705d by the merges of guest: match C hexadecimal output for zero clock values #631 and nvx-rs: add verified process references with event-driven exit waits #625 and a performance-baseline update, none of which changes tests/guest_tools.rs, and no open pull request to dev changes it.

  • TREE: 51709d6584a619148322918ac51f13a55a59f312, 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.09d60f9c8ee6, from the merge of nvx-rs: check the guest agent's replies through shared methods #615, 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 sources equal BASE's. Each host built the openvmm_e2e and managed_lifecycle test binaries after its other Cargo work, 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 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/guest_tools.rs, and on the guest_tools test for x86_64-unknown-linux-gnu with only guest;
    • 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.

    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.

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, 34 of them guest_tools), 657 nvx-rs tests with all features (34 of them guest_tools) and 308 with the default ones, artifact bundling (15), 158 and 85 aci_edge_sandboxes tests, and agent bundling. The macOS checks 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 377 nvx-rs tests with all features and 308 with the default ones, artifact bundling (15), 158 and 85 aci_edge_sandboxes tests, and agent bundling. The MSRV checks ran locally Copied in, digests match Pass (18/18) Pass (42 scenario runs)

Every gate and test passed at its first attempt. The Linux gates 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 prometheus28. On prometheus32 and prometheus30, one boot of 58 each went over it (boot_cpu_us up to 17,939 and 10,015 µs). The suite reports that count without gating on it, and this change leaves the time tools and the guest binaries, which built to the bytes of #636's, alone.

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.

tests/guest_tools.rs runs nvx-time's test hooks on fixture files at 18
sites, and each spelled out the hook's command line, `nvx-time test
HOOK FIXTURE ARGUMENTS...`, as an array of OsStr values: "test", the
hook, the fixture's path, and the hook's arguments. One of the sites
was already a helper, run_time_packet, which writes a packet fixture
and runs the packet hook on it, yet the three packet sites of
time_test_mode_decodes_packets_samples_and_arithmetic built the same
command line by hand.

time_hook(hook, fixture, arguments), next to time_fixture, which
writes the fixtures, now builds that command line once and runs it
with run. run_time_packet keeps its signature and calls it, and so do
the other 17 sites: the packet, sample, boot-log, kmsg, timer-list,
state, status, cpuinfo, and restore-record hooks, and the run_log
closure, which runs the boot-log and syslog hooks. Each call names
only what varies, and the file loses 106 lines. The nvx-time event
hook, which takes no fixture, and the nvx-ingress test hook keep their
own command lines.

Behavior is unchanged: through time_hook, each site hands run the
arguments that it passed before, in the same order, so every test
runs the same command and checks the same exit status and output.
Only the test changes; the library, the CLI, and the guest binaries
build from unchanged sources.

The guest_tools tests cover the change. On Linux, the all-features
test run, check-guest-binaries, and test-guest-binaries against a
guest-profile build of the binaries each ran all 34 of them, and all
passed.

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

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 refactor preserves existing behavior, with no unresolved issues identified.

No open issues 🎉

What changed in this PR

Consolidates fixture-based nvx-time test commands into one helper without changing production code or test expectations.

Changes:

  • Adds time_hook and routes 18 command-construction sites through it.
  • Preserves argument order, native fixture paths, and existing assertions.
File Description
nvx-rs/​tests/​guest_tools.rs Shares hook command construction across guest-binary tests.

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