Repository navigation
nvx-rs: share the sandbox tests' policy and native helpers - #755
Merged
Pedro Henrique Penna (ppenna) merged 1 commit intoOct 22, 2026
Merged
Conversation
The launch tests in host/sandbox/tests.rs and the share tests in host/sandbox/mount/tests.rs each defined the same two helpers, word for word: policy, which builds the MountPolicy of a share from the paths that it hides, exposes again, and lets the workload modify, and native, which renders a path as str(pathlib.Path(path)) does on the host. So a change to MountPolicy, or to how the tests render host paths, had two copies to keep alike. The share tests now import both helpers from the launch tests, whose module they can reach from below host::sandbox, so each helper has one definition. The launch tests' copies become pub(super), with doc comments that say what they return and that the share tests use them. Neither host/sandbox/mod.rs nor host/sandbox/mount.rs changes. Behavior is unchanged: only test code changes, and every test calls the same function bodies with the same arguments, so it builds the same policies and paths and makes the same assertions. No library item changes, and no test is added, removed, or weakened. The helpers' callers, the launch tests' mount helper, three launch tests, and six share tests, run in the crate's tests with the default features and with all of them, on Linux and on Windows. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The narrow test-only refactor preserves helper behavior and callers, with compatible visibility and test configuration.
No open issues 🎉
What changed in this PR
This PR removes duplicated sandbox test helpers in nvx-rs, giving the launch-profile and mount tests one shared implementation without changing production code or test behavior.
Changes:
- Makes the existing
policyandnativehelpers visible within the sandbox module and documents their shared use. - Replaces the mount tests’ identical helper definitions with imports from the launch-profile test module.
| File | Description |
|---|---|
nvx-rs/src/host/sandbox/tests.rs |
Exposes and documents the shared test helpers, preserving their bodies. |
nvx-rs/src/host/sandbox/mount/tests.rs |
Imports the shared helpers and removes duplicate definitions. |
Review effort: Balanced
Pedro Henrique Penna (ppenna)
deleted the
nvx-rs-quality/sandbox-test-helpers
branch
October 22, 2026 19:59
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 launch-profile tests in
nvx-rs/src/host/sandbox/tests.rsand the share tests innvx-rs/src/host/sandbox/mount/tests.rseach defined the same two helpers, word for word:So a change to
MountPolicy, or to how the tests render host paths, had two copies to keep alike. This pull request keeps the launch tests' copies, and the share tests import them, so each helper has one definition. #683, #684, #689, and #705 listed this duplication among their follow-ups.Changes
nvx-rs/src/host/sandbox/tests.rs:policyandnativebecomepub(super), which makes them visible inhost::sandboxand the modules below it, with doc comments that say what they return and that the share tests use them.nvx-rs/src/host/sandbox/mount/tests.rs: its copies are gone, anduse crate::host::sandbox::tests::{native, policy};imports the shared ones. The share tests reach the launch tests' module from belowhost::sandbox, so neitherhost/sandbox/mod.rsnorhost/sandbox/mount.rs, which open pull requests change, changes.policy: in the launch tests, theirmounthelper,launch_contract_attaches_a_file_as_an_aggregate_child, andlaunch_validation_rejects_overlapping_or_foreign_host_paths; in the share tests,mount_parser_defaults_to_read_only_and_accepts_rw,mount_parser_accepts_host_paths_with_commas,mount_owner_defaults_to_vmm_and_forwards_caller,mount_access_policy_is_validated_and_forwarded, andmount_absolute_path_keeps_symbolic_link_components;native:launch_validation_requires_the_layers_and_the_scratch_imagein the launch tests, andmount_parser_rejects_malformed_specificationsandmount_parser_accepts_host_paths_with_commasin the share tests.6 insertions and 15 deletions.
Behavior
Unchanged:
MountPolicyvalues and paths and makes the same assertions. No test is added, removed, skipped, or weakened: the launch tests and the share tests still run 15 and 10 tests in each feature set.pub(super)widens the helpers' visibility only inside the crate's test build, tohost::sandboxand the modules below it.Follow-ups
Not in this pull request, to keep it to one change. I found no suspected bug.
net::is_valid_guest_networkandguest_subnetinhost/network/mod.rseach parse an IPv4ADDRESS/PREFIXand derive its network, with prefix limits of /1 to /30 and /1 to /32. This run first prepared a change that shared that parsing, and dropped it when nvx-rs: lower sandbox networks through one typed policy with a proxy-only mode #753, opened during the run, began to change both files. nvx-rs: lower sandbox networks through one typed policy with a proxy-only mode #753 moves the first parse into aguest_addresshelper that also checks the host part; a parser that both could share once nvx-rs: lower sandbox networks through one typed policy with a proxy-only mode #753 lands would take the prefix limit and leave that check tois_valid_guest_network.host/python/dumps.rs:write_value's list and dict arms repeat the bracket, comma, and indentation loop, and their empty forms. This run also drafted one container writer for both, which passed the crate's tests but added 16 lines net, so it chose this smaller change.guest/time_probe/clock.rs:runprints its two kernel-log sections aroundprint_kmsgwith the same ten-line match;print_kmsgcould print its whole section and return its counters. No test checks those sections' text.messagetest helpers ofhost/sandbox/mount/tests.rsandhost/sandbox/options/tests.rsdiffer only in the message of their error-kind assertion, but the launch tests'message, which asserts no error kind, has the same name, so sharing them needs a name of its own.kernel_addresstest helper ofguest/ingress/address.rsandcollect.rs, the bounded merges ofListenerTable::addandnote_wildcard_tcp, the time probe's TSC-rate checks, and thehextest helpers ofprotocol/tests.rsandprotocol/nvxs/tests.rs.Validation
BASE:
2817f4eb79911ef8380dc2b580b9d5c77e6b5b21,origin/devwhen the run started and when it committed. No commit since then and no open pull request todevchanges either file.TREE:
e0435a8ed2b1ee79d0fe15fe919c312a57c0ca77, the tree of this commit. Every host applied the patch to BASE withgit apply --indexand checked thatgit write-treeequals TREE.Release on every host:
v0.1.0-dev.e714acbd47bf, whichdownloadchose 2 first-parent commits before BASE, since BASE, a[skip ci]baseline update, has no release. Its OpenVMM,9f04d05c5441a9b31f7f81effa0ce7c4fc66a0d3, is the pinned revision, as each host checked inbuild/openvmm.provenance.json.nvx-rs: each host deleted thenvx-rsexecutable thatnvx.pyruns, whichdownloadmay install from the release, rebuilt it from TREE with the Cargo command with whichnvx.pybuilds it, and recorded its provenance withrecord-nvx-rs-provenance. Each host checked that the record names TREE's sources (source_sha2566f7b3859b4d07785b4164287aba8c6c57bf16a246442c65bb6a97c2c88b59d1c, which this machine also computes for TREE) and the new executable, and then built theopenvmm_e2eandmanaged_lifecycletest binaries, after its other Cargo work and before its hypervisor tests.Guest artifacts were rebuilt from TREE on prometheus30, whose user namespaces let Alpine's apk run its scripts (prometheus32 denies them):
build-guest-binaries --output, nine static executables;test-guest-binaries --binaries, where 34 tests passed;build-initramfswithNVX_GUEST_BINARIES.initramfs.cpio.gz(SHA-25651fd870d670a1f58b54c76d7efb95f9b5756b499b0323682c8469f566fa37ae1), its packages manifest (1ab07b6c71dccafa627732ed95d6c58717e9dd161eb583469862183ffa0ba572), and its provenance (15d5df20422a7a86c3cb6c3b2252b92bf582432c18633eddf3d4bc9ff8147444) went through this machine to the other two hosts, where the SHA-256 digests matched. The 126 source files that the provenance records match TREE's; the two changed test files are not among the guest's sources.Local checks with Rust 1.95.0 on Windows: rustfmt; Clippy with
-D warningson all targets with all features and on the library without default features; andcargo testwith all features (507 tests in 12 test targets) and with the default ones (430 in 10).This machine also ran the steps for which the hosts lack the toolchain or target: on Windows, the MSRV (1.89) checks of
nvx,aci_edge_sandboxes, and the runtime adapter's protocol-only and host-only configurations; and, on WSL Ubuntu, the macOS (aarch64-apple-darwin) checks ofnvxandaci_edge_sandboxesand the MSRV check of the runtime adapter's guest-only configuration, which ran as the action defines it, starting with itsrustup target add, which gave the local 1.89 the musl target. I removed that 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.
The hosts ran each action's steps with the toolchains and targets that they have, with
RUSTUP_AUTO_INSTALL=0and without the actions' installation commands. On prometheus28, the artifact-bundling step staged its synthetic release with PowerShell instead of the step's bash script, with the same files and the same digests and paths inSHA256SUMS.Earlier in the run, the hosts went through these steps for the change that this run dropped (see Follow-ups). Every host was then prepared again at BASE, and every result below is TREE's.
test-aci-edge-sandboxestest-microvm, default suitenvx,aci_edge_sandboxes, and the runtime adapter's protocol-only and host-only configurations;nvxtests: 946 in 39 test targets with all features and 431 in 13 with the default ones; artifact bundling (15);check-guest-binaries(530 in 32 test targets); the runtime adapter's protocol-only, host-only, and musl guest-only configurations (13, 14, and 21 tests); 158 and 85aci_edge_sandboxestests over the two feature sets; agent bundling (4). The macOS checks and the guest-only MSRV check ran locallynvxtests: 507 in 12 test targets with all features and 430 in 10 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 over the two feature sets; agent bundling (4). The MSRV checks ran locallyEvery gate and hypervisor test passed at its first run on TREE. Each suite's
managed-lifecyclescenario ran themanaged_lifecycletest with Cargo, which passed. The time-ABI evidence lines counted no boot, capture, or restore over its CPU-time budget on any host, and the exhaustive check reportedok/8/0on each. On every host, oneopenvmm_e2etest printedskipped: the host has no second volumefor a check that needs one, and on the Linux hosts the filesystem caller-ownership scenario reported that OpenVMM cannot assume other host identities, so it checked that guest root and foreign guest callers fail withEPERM; both are properties of the hosts, not of this change.Left to CI:
test-openvmm-unitandtest-openvmm, since OpenVMM and the guest's sources are unchanged;test-microvmscenarios that run only when named:smp-lapic, andmanaged-exec-configandsandbox-lifecycle, which need artifacts that packages omit;verify-guest-determinism;