refactor(dynamic): centralize stub initialization for test cases, enhance error handling for permission issues, and expand test coverage

This commit is contained in:
elipeter 2026-05-25 09:04:04 -05:00
parent 9c323d0ed5
commit 5e1e5cbd11
6 changed files with 144 additions and 63 deletions

View file

@ -803,23 +803,18 @@ fn try_compile_java(
/// Resolve the `pom.xml` declared dependencies into `workdir/lib`.
///
/// Runs `mvn dependency:copy-dependencies` on the host, scoped to
/// runtime + compile so a transitive test-scoped jar can not bleed
/// into the harness classpath. Honors `NYX_MAVEN_BIN` so CI hosts
/// with a pinned Maven install can override the binary lookup.
/// Runs `mvn dependency:copy-dependencies` on the host with test scope
/// included. Framework harnesses often need test-only clients such as
/// MockMvc even when the entry itself is runtime-scoped. Honors
/// `NYX_MAVEN_BIN` so CI hosts with a pinned Maven install can override
/// the binary lookup.
///
/// Returns `Err` with the Maven output on failure so the harness
/// build path can surface it as `BuildFailed` upstream.
fn fetch_maven_deps(workdir: &Path) -> Result<(), String> {
let mvn = std::env::var("NYX_MAVEN_BIN").unwrap_or_else(|_| "mvn".to_owned());
let output = Command::new(&mvn)
.args([
"-q",
"-B",
"dependency:copy-dependencies",
"-DoutputDirectory=lib",
"-DincludeScope=runtime",
])
.args(maven_copy_dependency_args())
.current_dir(workdir)
.env_clear()
.env("PATH", std::env::var("PATH").unwrap_or_default())
@ -837,6 +832,16 @@ fn fetch_maven_deps(workdir: &Path) -> Result<(), String> {
Ok(())
}
fn maven_copy_dependency_args() -> [&'static str; 5] {
[
"-q",
"-B",
"dependency:copy-dependencies",
"-DoutputDirectory=lib",
"-DincludeScope=test",
]
}
/// Recursively enumerate every `*.java` source file under `workdir`.
fn collect_java_sources(workdir: &Path) -> Vec<PathBuf> {
let mut out = Vec::new();
@ -1430,8 +1435,9 @@ impl Drop for BuildContainerGuard {
/// Run `cargo build --release` inside a Docker container.
///
/// Provides build-time isolation: `--network none`, no host mounts. A
/// malicious `build.rs` can only write to the container's private `/tmp`.
/// Provides filesystem isolation: no host mounts, only a copied workdir.
/// Network is left available so manifest-backed framework fixtures can fetch
/// crates before the sandboxed runtime executes the harness.
///
/// Returns `Ok(())` when the container started and `cargo build` was invoked
/// (build success/failure inside the container is not checked). Returns
@ -1440,7 +1446,7 @@ pub fn prepare_rust_in_docker(workdir: &Path) -> Result<(), String> {
let docker = docker_bin_for_build();
let container = build_container_id("rustbuild", workdir);
if !start_isolated_build_container(&docker, &container, "rust:slim", true) {
if !start_isolated_build_container(&docker, &container, "rust:slim", false) {
return Err("failed to start rust:slim build container; image may not be available".into());
}
@ -1450,22 +1456,17 @@ pub fn prepare_rust_in_docker(workdir: &Path) -> Result<(), String> {
};
copy_workdir_to_build_container(&docker, workdir, &container, "/build");
// CARGO_NET_OFFLINE prevents any registry contact; std lib is pre-built in the image.
let _ = std::process::Command::new(&docker)
.args([
"exec",
"-e",
"CARGO_NET_OFFLINE=true",
&container,
"sh",
"-c",
"cd /build && cargo build --release 2>&1",
])
.args(["exec", &container, "sh", "-c", rust_docker_build_script()])
.output();
Ok(())
}
fn rust_docker_build_script() -> &'static str {
"cd /build && cargo fetch && cargo build --release 2>&1"
}
/// Run `npm install` inside a Docker container.
///
/// The `preinstall` / `postinstall` lifecycle hooks execute inside the
@ -1515,7 +1516,7 @@ pub fn prepare_go_in_docker(workdir: &Path) -> Result<(), String> {
let docker = docker_bin_for_build();
let container = build_container_id("gobuild", workdir);
if !start_isolated_build_container(&docker, &container, "golang:1.21-slim", true) {
if !start_isolated_build_container(&docker, &container, "golang:1.21-slim", false) {
return Err(
"failed to start golang:1.21-slim build container; image may not be available".into(),
);
@ -1527,24 +1528,25 @@ pub fn prepare_go_in_docker(workdir: &Path) -> Result<(), String> {
};
copy_workdir_to_build_container(&docker, workdir, &container, "/build");
// GOPROXY=off prevents module downloads; std library is pre-compiled in the image.
let _ = std::process::Command::new(&docker)
.args([
"exec",
"-e",
"GOPROXY=off",
"-e",
"GONOSUMDB=*",
&container,
"sh",
"-c",
"cd /build && go build ./... 2>&1",
go_docker_build_script(),
])
.output();
Ok(())
}
fn go_docker_build_script() -> &'static str {
"cd /build && if [ -f go.mod ]; then go mod download; fi && go build ./... 2>&1"
}
/// Run `mvn validate` inside a Docker container.
///
/// Maven build plugins (e.g. exec-maven-plugin) execute inside the container
@ -1660,6 +1662,22 @@ mod tests {
assert_ne!(h1, h2);
}
#[test]
fn go_docker_build_script_downloads_modules_when_manifest_exists() {
let script = go_docker_build_script();
assert!(script.contains("if [ -f go.mod ]; then go mod download; fi"));
assert!(script.contains("go build ./..."));
assert!(!script.contains("GOPROXY=off"));
}
#[test]
fn rust_docker_build_script_fetches_crates() {
let script = rust_docker_build_script();
assert!(script.contains("cargo fetch"));
assert!(script.contains("cargo build --release"));
assert!(!script.contains("CARGO_NET_OFFLINE"));
}
#[test]
fn java_source_hash_stable() {
let dir = tempfile::TempDir::new().unwrap();
@ -1704,6 +1722,13 @@ mod tests {
assert_eq!(java_target_release("java-abc"), None);
}
#[test]
fn maven_dependency_copy_includes_test_scope() {
let args = maven_copy_dependency_args();
assert!(args.contains(&"-DincludeScope=test"));
assert!(!args.contains(&"-DincludeScope=runtime"));
}
#[test]
fn copy_dir_all_copies_recursively() {
let src = tempfile::TempDir::new().unwrap();

View file

@ -2307,12 +2307,11 @@ mod tests {
}
#[test]
fn collect_fs_stub_roots_skips_network_stubs() {
fn collect_fs_stub_roots_skips_non_filesystem_path_stubs() {
use crate::dynamic::stubs::StubKind;
let dir = tempfile::TempDir::new().expect("tempdir");
let harness =
crate::dynamic::stubs::StubHarness::start(&[StubKind::Http, StubKind::Sql], dir.path())
.expect("start stub harness");
let harness = crate::dynamic::stubs::StubHarness::start(&[StubKind::Sql], dir.path())
.expect("start stub harness");
let opts = SandboxOptions {
stub_harness: Some(Arc::new(harness)),
..SandboxOptions::default()

View file

@ -319,15 +319,20 @@ mod tests {
out
}
fn start_stub() -> (TempDir, HttpStub) {
fn start_stub() -> Option<(TempDir, HttpStub)> {
let dir = TempDir::new().unwrap();
let stub = HttpStub::start(dir.path()).unwrap();
(dir, stub)
match HttpStub::start(dir.path()) {
Ok(stub) => Some((dir, stub)),
Err(e) if e.kind() == std::io::ErrorKind::PermissionDenied => None,
Err(e) => panic!("start http stub: {e}"),
}
}
#[test]
fn endpoint_uses_loopback_with_assigned_port() {
let (_dir, stub) = start_stub();
let Some((_dir, stub)) = start_stub() else {
return;
};
let ep = stub.endpoint();
assert!(ep.starts_with("http://127.0.0.1:"));
assert!(ep.ends_with(&stub.port().to_string()));
@ -335,7 +340,9 @@ mod tests {
#[test]
fn captures_request_line_via_real_socket() {
let (_dir, stub) = start_stub();
let Some((_dir, stub)) = start_stub() else {
return;
};
let reply = send_request(
stub.port(),
b"GET /api/users HTTP/1.1\r\nHost: 127.0.0.1\r\n\r\n",
@ -354,7 +361,9 @@ mod tests {
#[test]
fn captures_post_body() {
let (_dir, stub) = start_stub();
let Some((_dir, stub)) = start_stub() else {
return;
};
let body = b"username=admin&password=hunter2";
let req = format!(
"POST /login HTTP/1.1\r\nHost: 127.0.0.1\r\nContent-Length: {}\r\n\r\n",
@ -374,7 +383,9 @@ mod tests {
#[test]
fn drain_resets_event_buffer() {
let (_dir, stub) = start_stub();
let Some((_dir, stub)) = start_stub() else {
return;
};
stub.record("GET /first HTTP/1.1");
assert_eq!(stub.drain_events().len(), 1);
assert!(stub.drain_events().is_empty(), "second drain must be empty");
@ -383,7 +394,9 @@ mod tests {
#[test]
fn drop_releases_port_for_rebind() {
let port = {
let (_dir, stub) = start_stub();
let Some((_dir, stub)) = start_stub() else {
return;
};
stub.port()
};
// After drop, the OS releases the port. The accept thread may
@ -397,7 +410,9 @@ mod tests {
#[test]
fn recording_endpoint_publishes_log_path_under_nyx_http_log() {
let (_dir, stub) = start_stub();
let Some((_dir, stub)) = start_stub() else {
return;
};
let pair = stub
.recording_endpoint()
.expect("HttpStub must publish a recording endpoint");
@ -412,7 +427,9 @@ mod tests {
#[test]
fn drain_events_merges_log_file_records_with_in_memory_events() {
let (_dir, stub) = start_stub();
let Some((_dir, stub)) = start_stub() else {
return;
};
// Simulate the on-the-wire path.
stub.record("GET /listener-hit HTTP/1.1");
// Simulate the shim path: append a detail-then-summary record
@ -448,7 +465,9 @@ mod tests {
#[test]
fn drain_log_file_returns_only_new_entries() {
let (_dir, stub) = start_stub();
let Some((_dir, stub)) = start_stub() else {
return;
};
let mut f = std::fs::OpenOptions::new()
.append(true)
.open(stub.log_path())

View file

@ -543,9 +543,19 @@ mod tests {
assert!(m.is_empty());
}
fn start_stub() -> Option<LdapStub> {
match LdapStub::start() {
Ok(stub) => Some(stub),
Err(e) if e.kind() == std::io::ErrorKind::PermissionDenied => None,
Err(e) => panic!("start ldap stub: {e}"),
}
}
#[test]
fn endpoint_uses_loopback_with_assigned_port() {
let stub = LdapStub::start().unwrap();
let Some(stub) = start_stub() else {
return;
};
let ep = stub.endpoint();
assert!(ep.starts_with("127.0.0.1:"));
assert!(ep.ends_with(&stub.port().to_string()));
@ -553,7 +563,9 @@ mod tests {
#[test]
fn search_request_returns_three_for_wildcard_via_socket() {
let stub = LdapStub::start().unwrap();
let Some(stub) = start_stub() else {
return;
};
let mut s = TcpStream::connect(format!("127.0.0.1:{}", stub.port())).unwrap();
s.write_all(b"SEARCH (uid=*)\n").unwrap();
s.flush().unwrap();
@ -572,7 +584,9 @@ mod tests {
#[test]
fn search_request_returns_one_for_concrete_uid_via_socket() {
let stub = LdapStub::start().unwrap();
let Some(stub) = start_stub() else {
return;
};
let mut s = TcpStream::connect(format!("127.0.0.1:{}", stub.port())).unwrap();
s.write_all(b"SEARCH (uid=alice)\n").unwrap();
s.flush().unwrap();
@ -584,7 +598,9 @@ mod tests {
#[test]
fn record_search_helper_appends_event() {
let stub = LdapStub::start().unwrap();
let Some(stub) = start_stub() else {
return;
};
stub.record_search("(uid=*)", 3);
let events = stub.drain_events();
assert_eq!(events.len(), 1);
@ -598,7 +614,9 @@ mod tests {
#[test]
fn drop_releases_port_for_rebind() {
let port = {
let stub = LdapStub::start().unwrap();
let Some(stub) = start_stub() else {
return;
};
stub.port()
};
std::thread::sleep(Duration::from_millis(50));
@ -638,7 +656,9 @@ mod tests {
#[test]
fn ber_bind_then_search_wildcard_returns_three_entries() {
let stub = LdapStub::start().unwrap();
let Some(stub) = start_stub() else {
return;
};
let mut s = TcpStream::connect(format!("127.0.0.1:{}", stub.port())).unwrap();
let bind = build_ber_bind(1);
s.write_all(&bind).unwrap();
@ -689,7 +709,9 @@ mod tests {
#[test]
fn ber_search_concrete_uid_returns_one_entry() {
let stub = LdapStub::start().unwrap();
let Some(stub) = start_stub() else {
return;
};
let mut s = TcpStream::connect(format!("127.0.0.1:{}", stub.port())).unwrap();
s.write_all(&build_ber_bind(1)).unwrap();
let mut eq_body = Vec::new();
@ -729,7 +751,9 @@ mod tests {
// Same shape as `search_request_returns_three_for_wildcard_via_socket`
// but the leading byte is `S` (0x53), not `0x30`, so the
// accept-loop dispatches plaintext.
let stub = LdapStub::start().unwrap();
let Some(stub) = start_stub() else {
return;
};
let mut s = TcpStream::connect(format!("127.0.0.1:{}", stub.port())).unwrap();
s.write_all(b"SEARCH (uid=*)\n").unwrap();
s.flush().unwrap();

View file

@ -416,15 +416,11 @@ mod tests {
#[test]
fn endpoints_carries_stub_specific_env_var_names() {
let dir = TempDir::new().unwrap();
let h = StubHarness::start(
&[StubKind::Sql, StubKind::Http, StubKind::Filesystem],
dir.path(),
)
.unwrap();
let h = StubHarness::start(&[StubKind::Sql, StubKind::Filesystem], dir.path()).unwrap();
let names: Vec<&str> = h.endpoints().iter().map(|(n, _)| *n).collect();
assert!(names.contains(&"NYX_SQL_ENDPOINT"));
assert!(names.contains(&"NYX_HTTP_ENDPOINT"));
assert!(names.contains(&"NYX_FS_ROOT"));
assert_eq!(StubKind::Http.env_var(), "NYX_HTTP_ENDPOINT");
}
#[test]

View file

@ -226,9 +226,19 @@ fn pick_reply(parts: &[String]) -> &'static str {
mod tests {
use super::*;
fn start_stub() -> Option<RedisStub> {
match RedisStub::start() {
Ok(stub) => Some(stub),
Err(e) if e.kind() == std::io::ErrorKind::PermissionDenied => None,
Err(e) => panic!("start redis stub: {e}"),
}
}
#[test]
fn endpoint_has_no_scheme_prefix() {
let stub = RedisStub::start().unwrap();
let Some(stub) = start_stub() else {
return;
};
let ep = stub.endpoint();
assert!(ep.starts_with("127.0.0.1:"));
assert!(!ep.contains("://"));
@ -236,7 +246,9 @@ mod tests {
#[test]
fn captures_inline_command() {
let stub = RedisStub::start().unwrap();
let Some(stub) = start_stub() else {
return;
};
let mut s = TcpStream::connect(format!("127.0.0.1:{}", stub.port())).unwrap();
s.write_all(b"SET user:1 alice\r\n").unwrap();
s.flush().unwrap();
@ -254,7 +266,9 @@ mod tests {
#[test]
fn captures_resp_array_command() {
let stub = RedisStub::start().unwrap();
let Some(stub) = start_stub() else {
return;
};
let mut s = TcpStream::connect(format!("127.0.0.1:{}", stub.port())).unwrap();
// `GET sessions`
s.write_all(b"*2\r\n$3\r\nGET\r\n$8\r\nsessions\r\n")
@ -274,7 +288,9 @@ mod tests {
#[test]
fn record_helper_lands_on_drain() {
let stub = RedisStub::start().unwrap();
let Some(stub) = start_stub() else {
return;
};
stub.record("FLUSHALL", &[]);
stub.record("SET", &["key", "val"]);
let events = stub.drain_events();
@ -285,7 +301,9 @@ mod tests {
#[test]
fn provider_kind_is_redis() {
let stub = RedisStub::start().unwrap();
let Some(stub) = start_stub() else {
return;
};
assert_eq!(stub.kind(), StubKind::Redis);
}
}