Repository navigation
nvx-rs: share the managed agent's cgroup and export test fixtures - #618
Merged
Pedro Henrique Penna (ppenna) merged 1 commit intoOct 20, 2026
Merged
Conversation
The managed agent's unit tests built the same fake trees in five places. cgroup.rs's setup and dispatch.rs's direct_system were the same function: a cgroup root with empty cgroup.procs and cgroup.subtree_control files, and an nvx-exec cgroup whose pids.current reads 0 and whose cgroup.kill and cgroup.procs are empty. The loopback test wrote the same five files inline. dispatch.rs's mapping_system and the setup of mappings.rs built the same host-filesystem export: child 0 with the directory src and the file notes.txt, and the empty child 1. TestSystem, the seam that all of these tests construct, now builds both trees: TestSystem::with_cgroup and TestSystem::with_host_export return a system whose paths point into a new temporary directory, with the directory that keeps it alive. The five copies become calls, so a change to the layout that the agent expects has one place to go, and the tests that share a tree can no longer drift apart. Behavior is unchanged: only test code changes, and each fixture creates the same directories and files, with the same contents, at the same paths below its temporary directory, and leaves the other paths at their defaults. The loopback test still adds its own export, root/0/source, and its unprivileged spawner to the cgroup system. On Linux, cargo test --all-features runs every test that uses the fixtures, and check-guest-binaries runs the cgroup, dispatch, and mappings tests in the guest profile. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The test-only refactor preserves fixture contents and lifetimes, with no unresolved findings; tests were not rerun during review.
No open issues 🎉
What changed in this PR
Consolidates managed-agent test fixtures in TestSystem, reducing duplication without changing production code or test behavior.
Changes:
- Adds shared cgroup and host-export fixture constructors.
- Updates existing tests to use them while preserving temporary-directory lifetimes and test-specific setup.
| File | Description |
|---|---|
nvx-rs/src/guest/managed_agent/seam.rs |
Adds both shared fixture constructors. |
nvx-rs/src/guest/managed_agent/mappings.rs |
Reuses the host-export fixture. |
nvx-rs/src/guest/managed_agent/loopback.rs |
Reuses the cgroup fixture, retaining custom setup. |
nvx-rs/src/guest/managed_agent/dispatch.rs |
Replaces duplicated helpers with shared fixtures. |
nvx-rs/src/guest/managed_agent/cgroup.rs |
Replaces local setup with the shared cgroup fixture. |
Review effort: Balanced
Pedro Henrique Penna (ppenna)
deleted the
nvx-rs-quality/agent-test-fixtures
branch
October 20, 2026 18:37
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 managed agent's unit tests (
nvx::guest::managed_agent) built the same two fake trees in five places: a cgroup root with an emptynvx-execcgroup, incgroup.rs, indispatch.rs, and inline inloopback.rs, and a host-filesystem export with the children0and1, indispatch.rsandmappings.rs. This change builds each tree once, in a constructor ofTestSystem, the test seam that all of these tests already construct, so a change to the layout that the agent expects has one place to go and the tests that share a tree can no longer drift apart. Only test code changes.Changes
All in
nvx-rs/src/guest/managed_agent/:seam.rs:TestSystem::with_cgroup()returns a system whosecgroup_rootis in a new temporary directory, with emptycgroup.procsandcgroup.subtree_controlfiles and annvx-execcgroup whosepids.currentreads0and whosecgroup.killandcgroup.procsare empty.TestSystem::with_host_export()returns a system whosehostfs_diris in a new temporary directory and holds the exportroot, with the child0, which holds the directorysrcand the filenotes.txt, and the empty child1.TempDirthat keeps its tree alive, and leaves the otherPathsat their defaults.with_cgroup:cgroup::tests::setup, removed, for its six tests;dispatch::tests::direct_system, removed, for its three tests; andloopback::tests::control_session_broker_and_real_agent_loop_back, which wrote the same five files inline and now adds its own export and spawner to the system thatwith_cgroupreturns.with_host_export:dispatch::tests::mapping_system, removed, for its four tests; andmappings::tests::setup, which still builds each test'sHostMappingsaround it.Pathsimports that only the copies used are gone.The diff is 57 insertions and 96 deletions.
Behavior
Nothing observable changes. The change is confined to
#[cfg(test)]code: the library, the guest binaries, and every public API are untouched.cgroup_root,hostfs_dir, andspawnerwith struct-update syntax, and now setshostfs_dirandspawneron the system thatwith_cgroupreturns. It still creates its own export,root/0/source, and its mapping target.The guest module is Linux-only, so only Linux builds compile these tests. On Linux,
cargo test --all-featuresruns every test that uses the fixtures, the loopback test included, andcheck-guest-binariesruns the cgroup, dispatch, and mappings tests in the guest profile (--no-default-features --features guest, forx86_64-unknown-linux-musl). No new test is needed: the fixtures are test code that these tests exercise.Follow-ups
The survey turned these up, and this change leaves them alone. I found no suspected bug.
dispatch::tests::maps_payloadandmappings::tests::payloadare the sameMAPSrequest encoder. Sharing it beside these fixtures would finish the managed agent's test fixtures, butmappings.rscalls it 19 times, so it wants a change of its own to stay small.guest/device_io.rs,run_fileandrun_networkboth measure, format the result line, and write it with the same error, whichruncould do once; and thedevice_iohelper oftests/guest_tools.rsrepeats the spawn and output collection ofrun.host/sandbox/mount.rs,MountAccess,MountOwner, andMountKindrepeatfrom_namewith the same message form, andparse_mount_valuerepeats the match ofMountAccess::from_namewith a message of its own;sandbox/mount/tests.rsandsandbox/tests.rsdefine the samepolicytest helper.option_valuesinhost/filesystem.rs,rulesinhost/network/mod.rs, andvaluesinhost/sandbox/options/tests.rs;Instance::runtimeandInstance::launchinhost/instance.rs, which read a record and refuse anotherSTATE_FORMATin two shapes;host/process.rsandhost/cli/sandbox/identity.rs;argstest helpers and themainentry convention of the guest programs, and the time tools' watcher-violation sites and port-window lock failures.Validation
3ddd1ea9e603be3a435b973c874adc474f08e904,origin/devwhen the run started.71c4256a1da3b24b47b7e4fa67ac71a318f4d42e, the tree of this commit, applied as a patch on BASE on each host.v0.1.0-dev.3ddd1ea9e603, whichnvx.py downloadinstalled on each host, built from BASE, with OpenVMMd0bdbed8f6e3b86271277dc22658109e2e633b2d, the pinned revision.build-guest-binaries, rantest-guest-binarieson them (34 passed), and built the Alpine initramfs from them withNVX_GUEST_BINARIES. All three hosts ran the sameinitramfs.cpio.gz(SHA-25654364e7301c11f7048f15c67f10c679f75d5ab5fc3cc1abd9025df6cef4b297e), package manifest, and provenance. Each host also rebuilt thenvx-rsCLI from TREE withcargo build --releaseand recorded its provenance before its hypervisor tests.cargo fmt --check; Clippy with-D warningsfor all targets and features and for the library without default features, and, because the guest module is Linux-only, cross-checked forx86_64-unknown-linux-gnuwith all features and forx86_64-unknown-linux-muslwith--no-default-features --features guest;cargo testwith all features and with the defaults; and rustdoc with-D warnings. This machine also ran the steps for which a host lacks a toolchain or target: the Rust 1.89 MSRV check of both crates on Windows, and theaarch64-apple-darwincheck of both crates, which CI runs on Linux.check-guest-binaries; the macOS check ran locallybuild-guest-binaries,test-guest-binaries, andbuild-initramfspassOn prometheus32, the library tests of
cargo test --all-features(548 passed) ran every test that uses the fixtures, the loopback test included, and those ofcheck-guest-binaries(279 passed) the cgroup, dispatch, and mappings tests in the guest profile.directories_on_another_volume_are_mappedpasses by skipping on hosts without a second volume, as on all three. No step failed or needed a rerun.Left to CI: the
test-microvmscenarios that run only when named (smp-lapic,managed-exec-config, andsandbox-lifecycle), the Ubuntu and Azure Linux guests, the OpenVMM suites, guest determinism, benchmarks, and check-quality and validate-nvx, which this change doesn't affect: it changes no Python, shell, or Markdown file, and adds, moves, or deletes none.