From d506b3f73a90e71b4c657af514183adaf1573dc1 Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Sat, 19 Sep 2026 15:34:36 +0000 Subject: [PATCH 1/3] test(metrics): match process assertions to collector platforms Darwin does not register the Linux ProcessCollector, so the repository-counting test must not require its CPU and resident-memory samples. Keep those assertions inside the existing Linux block while retaining repository metrics coverage on every platform. This changes only platform-specific test expectations; production metrics and Linux coverage remain unchanged. Validation: inspected the collector registration guard and checked the diff. Darwin execution is still required on a Darwin worker. Assisted-by: Codex (GPT-6) --- src/metrics/mod.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/metrics/mod.rs b/src/metrics/mod.rs index b9644a6..dc39ce2 100644 --- a/src/metrics/mod.rs +++ b/src/metrics/mod.rs @@ -1216,10 +1216,12 @@ mod tests { // render() recounts from disk) let output = metrics.render(); assert!(output.contains("ngit_repositories_total 0")); - assert!(output.contains("process_cpu_seconds_total")); - assert!(output.contains("process_resident_memory_bytes")); #[cfg(target_os = "linux")] { + // ProcessCollector is registered only on Linux, just like the + // cgroup collectors below. Repository metrics are portable. + assert!(output.contains("process_cpu_seconds_total")); + assert!(output.contains("process_resident_memory_bytes")); if let Some(directory) = cgroup_v2_directory() { if cgroup_cpu_seconds(&directory).is_some() { assert!(output.contains("ngit_cgroup_cpu_seconds_total")); From d1a06e5ed401175fbb3fdbac506944dbffe093ac Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Sat, 19 Sep 2026 15:36:50 +0000 Subject: [PATCH 2/3] test(common): retain relay diagnostics when a test panics Sandboxed package checks delete the relay logs in /tmp when they fail. The ARM invitation-processing timeout therefore reported only a failed readiness assertion, hiding the state of all three relay subprocesses. After reaping each fixture, emit at most its final 64 KiB when unwinding a panic. Normal cleanup stays silent and diagnostic I/O errors are ignored to avoid a second panic. This adds evidence without changing deadlines, test ordering, or production synchronization; it does not claim to fix the intermittent timeout. Validation: rustfmt and diff checks; a standalone harness compiled the exact Drop implementation and verified silent normal cleanup, bounded panic output, and missing-log handling. One invocation of the existing invitation-test binary passed in 7.9 seconds; full Cargo and Darwin runs remain for host/CI. Assisted-by: Codex (GPT-6) --- tests/common/relay.rs | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/tests/common/relay.rs b/tests/common/relay.rs index dbe4d12..83ac8a0 100644 --- a/tests/common/relay.rs +++ b/tests/common/relay.rs @@ -1095,6 +1095,37 @@ impl Drop for TestRelay { // Ensure process is killed when TestRelay is dropped let _ = self.process.kill(); let _ = self.process.wait(); + if std::thread::panicking() { + // Sandboxed builders discard /tmp after failure. Preserve a bounded + // tail from every participating relay in libtest's failure output. + // Diagnostic I/O must never cause a second panic during unwinding. + use std::io::{Read, Seek, SeekFrom, Write}; + let path = self.log_path(); + let tail = (|| -> std::io::Result> { + let mut file = std::fs::File::open(&path)?; + let start = file.metadata()?.len().saturating_sub(64 * 1024); + file.seek(SeekFrom::Start(start))?; + let mut bytes = Vec::new(); + file.take(64 * 1024).read_to_end(&mut bytes)?; + Ok(bytes) + })(); + let mut stderr = std::io::stderr().lock(); + let _ = writeln!( + stderr, + "Relay {} failure log ({})", + self.port, + path.display() + ); + match tail { + Ok(bytes) => { + let _ = stderr.write_all(&bytes); + let _ = writeln!(stderr); + } + Err(error) => { + let _ = writeln!(stderr, "Could not read relay log: {error}"); + } + } + } } } From 86325a3ecb68ced967b947864f80a3e3926f458d Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Sat, 19 Sep 2026 20:06:37 +0000 Subject: [PATCH 3/3] fix(test-listener): inspect listening TCP sockets on Darwin Darwin exposes the SO_ACCEPTCONN constant but rejects that getsockopt query with ENOPROTOOPT. Every inherited fixture listener therefore failed adoption before integration tests could start. On Apple targets inspect TCP_CONNECTION_INFO and require the TCPS_LISTEN state instead. Retain SO_ACCEPTCONN elsewhere and the loopback check on all targets. Inspection must neither accept a queued connection nor call listen again, so listener ownership and backlog remain intact. This does not broaden the private test protocol or change production listener startup. Add regressions rejecting UDP and connected stream descriptors and preserving a connection queued before adoption. Validation: three focused Linux tests pass using cached dependencies, rustfmt and diff checks pass; XNU implementation and headers confirm the Apple query and state value. Darwin execution remains pending. Assisted-by: Codex (GPT-6) --- src/test_listener.rs | 62 ++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 57 insertions(+), 5 deletions(-) diff --git a/src/test_listener.rs b/src/test_listener.rs index 744999e..c72e86b 100644 --- a/src/test_listener.rs +++ b/src/test_listener.rs @@ -38,12 +38,21 @@ fn listener_from_fd(fd: RawFd) -> Result { return Err(std::io::Error::last_os_error()).context("protect test listener from exec"); } } + if !is_listening(owned)? || !listener.local_addr()?.ip().is_loopback() { + bail!("test listener must be a listening loopback TCP socket"); + } + listener.set_nonblocking(true)?; + TcpListener::from_std(listener).context("adopt test listener") +} + +#[cfg(not(target_vendor = "apple"))] +fn is_listening(fd: RawFd) -> Result { let mut accepting: libc::c_int = 0; let mut length = std::mem::size_of_val(&accepting) as libc::socklen_t; // SAFETY: both pointers reference live, correctly sized writable values. let result = unsafe { libc::getsockopt( - owned, + fd, libc::SOL_SOCKET, libc::SO_ACCEPTCONN, (&mut accepting as *mut libc::c_int).cast(), @@ -53,11 +62,32 @@ fn listener_from_fd(fd: RawFd) -> Result { if result != 0 { return Err(std::io::Error::last_os_error()).context("inspect test listener"); } - if accepting == 0 || !listener.local_addr()?.ip().is_loopback() { - bail!("test listener must be a listening loopback TCP socket"); + Ok(accepting != 0) +} + +#[cfg(target_vendor = "apple")] +fn is_listening(fd: RawFd) -> Result { + // XNU defines SO_ACCEPTCONN but does not support querying it with + // getsockopt. TCP_CONNECTION_INFO exposes the TCP state without accepting + // a queued connection or changing the inherited listener's backlog. + // SAFETY: tcp_connection_info consists entirely of integer fields. + let mut info: libc::tcp_connection_info = unsafe { std::mem::zeroed() }; + let mut length = std::mem::size_of_val(&info) as libc::socklen_t; + // SAFETY: info and length are live, correctly sized writable values. + let result = unsafe { + libc::getsockopt( + fd, + libc::IPPROTO_TCP, + libc::TCP_CONNECTION_INFO, + (&mut info as *mut libc::tcp_connection_info).cast(), + &mut length, + ) + }; + if result != 0 { + return Err(std::io::Error::last_os_error()).context("inspect test listener"); } - listener.set_nonblocking(true)?; - TcpListener::from_std(listener).context("adopt test listener") + // TCPS_LISTEN from XNU's netinet/tcp_fsm.h (not exported by libc). + Ok(info.tcpi_state == 1) } #[cfg(test)] @@ -89,5 +119,27 @@ mod tests { let file = std::fs::File::open("/dev/null").unwrap(); assert!(listener_from_fd(file.as_raw_fd()).is_err()); assert!(listener_from_fd(-1).is_err()); + let datagram = std::net::UdpSocket::bind("127.0.0.1:0").unwrap(); + assert!(listener_from_fd(datagram.as_raw_fd()).is_err()); + } + + #[tokio::test] + async fn adoption_preserves_queued_connections_and_rejects_connected_streams() { + let reserved = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let client = tokio::time::timeout( + std::time::Duration::from_secs(5), + tokio::net::TcpStream::connect(reserved.local_addr().unwrap()), + ) + .await + .unwrap() + .unwrap(); + assert!(listener_from_fd(client.as_raw_fd()).is_err()); + let listener = listener_from_fd(reserved.as_raw_fd()).unwrap(); + drop(reserved); + let (_, peer) = tokio::time::timeout(std::time::Duration::from_secs(5), listener.accept()) + .await + .unwrap() + .unwrap(); + assert_eq!(peer, client.local_addr().unwrap()); } }