From d23c6d3e7a1d6aab4ad53a760d3a236369585396 Mon Sep 17 00:00:00 2001 From: ajianaz Date: Fri, 16 Oct 2026 16:11:43 +0700 Subject: [PATCH] test(cli): wait for the server's own listening line and retry on a port race --- crates/tole-cli/tests/common/mod.rs | 58 +++++++++++++++++ crates/tole-cli/tests/mcp_http_hardening.rs | 32 +++------- crates/tole-cli/tests/serve_approvals.rs | 33 +++++----- crates/tole-cli/tests/serve_http_hardening.rs | 64 ++++++++++++------- crates/tole-cli/tests/serve_session_cancel.rs | 45 ++++++------- 5 files changed, 147 insertions(+), 85 deletions(-) create mode 100644 crates/tole-cli/tests/common/mod.rs diff --git a/crates/tole-cli/tests/common/mod.rs b/crates/tole-cli/tests/common/mod.rs new file mode 100644 index 0000000..5957882 --- /dev/null +++ b/crates/tole-cli/tests/common/mod.rs @@ -0,0 +1,58 @@ +//! Shared helpers for tests that spawn `tole serve`. +//! +//! Picking a port by binding `127.0.0.1:0` and closing it is a race: before +//! the server binds it, the same ephemeral port can be taken by another +//! test's client socket, the server exits with "Address already in use", +//! and a health poll then times out (or, worse, reaches a different +//! listener). Readiness is therefore taken from the server's own +//! "listening on" line, and a server that exits instead is retried on a +//! fresh port. + +#![allow(dead_code)] + +use std::io::{BufRead, BufReader}; +use std::net::TcpListener; +use std::process::{Child, Command, Stdio}; +use std::sync::mpsc; +use std::time::Duration; + +const ATTEMPTS: usize = 5; +const READY_TIMEOUT: Duration = Duration::from_secs(20); + +fn candidate_port() -> u16 { + TcpListener::bind("127.0.0.1:0") + .unwrap() + .local_addr() + .unwrap() + .port() +} + +/// Spawn the command `make(port)` builds until one instance reports +/// "listening on". Its stderr is forwarded to this test's stderr (so it +/// shows up in a failure report). Returns the running child and its port. +pub fn spawn_listening(mut make: impl FnMut(u16) -> Command) -> (Child, u16) { + for _ in 0..ATTEMPTS { + let port = candidate_port(); + let mut cmd = make(port); + cmd.stderr(Stdio::piped()); + let mut child = cmd.spawn().expect("spawn tole"); + let stderr = child.stderr.take().expect("stderr piped"); + let (tx, rx) = mpsc::channel(); + std::thread::spawn(move || { + for line in BufReader::new(stderr).lines().map_while(Result::ok) { + if line.contains("listening on") { + let _ = tx.send(true); + } + eprintln!("{line}"); + } + // EOF: the process exited (or closed stderr) before listening. + let _ = tx.send(false); + }); + if rx.recv_timeout(READY_TIMEOUT) == Ok(true) { + return (child, port); + } + let _ = child.kill(); + let _ = child.wait(); + } + panic!("tole serve did not start listening after {ATTEMPTS} attempts"); +} diff --git a/crates/tole-cli/tests/mcp_http_hardening.rs b/crates/tole-cli/tests/mcp_http_hardening.rs index d77b2f3..b14cd91 100644 --- a/crates/tole-cli/tests/mcp_http_hardening.rs +++ b/crates/tole-cli/tests/mcp_http_hardening.rs @@ -6,10 +6,12 @@ #![cfg(all(feature = "shell-tools", feature = "mcp-http"))] use std::io::{Read, Write}; -use std::net::{TcpListener, TcpStream}; +use std::net::TcpStream; use std::process::{Child, Command, Stdio}; use std::time::Duration; +mod common; + const TOKEN: &str = "t0ken"; const INIT: &str = r#"{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2024-11-05","capabilities":{},"clientInfo":{"name":"t","version":"1"}}}"#; @@ -20,13 +22,9 @@ struct Server { impl Server { fn spawn() -> Self { - let port = TcpListener::bind("127.0.0.1:0") - .unwrap() - .local_addr() - .unwrap() - .port(); - let child = Command::new(env!("CARGO_BIN_EXE_tole")) - .args([ + let (child, port) = common::spawn_listening(|port| { + let mut cmd = Command::new(env!("CARGO_BIN_EXE_tole")); + cmd.args([ "serve", "--transport", "mcp", @@ -36,20 +34,10 @@ impl Server { TOKEN, ]) .stdin(Stdio::null()) - .stdout(Stdio::null()) - .stderr(Stdio::null()) - .spawn() - .expect("spawn tole serve --transport mcp"); - let me = Self { child, port }; - // Ready = it answers TCP. (A wrong-token probe would burn the - // limiter budget the tests below measure.) - for _ in 0..100 { - if TcpStream::connect(("127.0.0.1", port)).is_ok() { - return me; - } - std::thread::sleep(Duration::from_millis(100)); - } - panic!("mcp server never listened"); + .stdout(Stdio::null()); + cmd + }); + Self { child, port } } fn connect(&self) -> TcpStream { diff --git a/crates/tole-cli/tests/serve_approvals.rs b/crates/tole-cli/tests/serve_approvals.rs index 6b622b9..2163abf 100644 --- a/crates/tole-cli/tests/serve_approvals.rs +++ b/crates/tole-cli/tests/serve_approvals.rs @@ -15,31 +15,28 @@ use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::Arc; use std::time::Duration; +mod common; + struct ServeProcess { child: Child, port: u16, } impl ServeProcess { - fn spawn(env: &[(&str, &str)], token: &str, port: u16) -> Self { - let mut cmd = Command::new(env!("CARGO_BIN_EXE_tole")); - cmd.args(["serve", "--port", &port.to_string(), "--token", token]) - .stdin(Stdio::null()) - .stdout(Stdio::null()) - .stderr(Stdio::inherit()); - for (k, v) in env { - cmd.env(k, v); - } - let child = cmd.spawn().expect("spawn tole serve"); - let me = Self { child, port }; - // Health-poll until the listener is up. - for _ in 0..50 { - if me.http("GET", "/health", None).0 == 200 { - return me; + /// `_port` is ignored: a free port is chosen (and retried) by + /// `common::spawn_listening`, which also waits for the listener. + fn spawn(env: &[(&str, &str)], token: &str, _port: u16) -> Self { + let (child, port) = common::spawn_listening(|port| { + let mut cmd = Command::new(env!("CARGO_BIN_EXE_tole")); + cmd.args(["serve", "--port", &port.to_string(), "--token", token]) + .stdin(Stdio::null()) + .stdout(Stdio::null()); + for (k, v) in env { + cmd.env(k, v); } - std::thread::sleep(Duration::from_millis(100)); - } - panic!("serve never became healthy"); + cmd + }); + Self { child, port } } fn http(&self, method: &str, path: &str, body: Option<&str>) -> (u16, Value) { diff --git a/crates/tole-cli/tests/serve_http_hardening.rs b/crates/tole-cli/tests/serve_http_hardening.rs index 2608a83..804448b 100644 --- a/crates/tole-cli/tests/serve_http_hardening.rs +++ b/crates/tole-cli/tests/serve_http_hardening.rs @@ -8,10 +8,12 @@ #![cfg(feature = "shell-tools")] use std::io::{Read, Write}; -use std::net::{TcpListener, TcpStream}; +use std::net::TcpStream; use std::process::{Child, Command, Stdio}; use std::time::{Duration, Instant}; +mod common; + struct Serve { child: Child, port: u16, @@ -19,28 +21,14 @@ struct Serve { impl Serve { fn spawn() -> Self { - let port = TcpListener::bind("127.0.0.1:0") - .unwrap() - .local_addr() - .unwrap() - .port(); - let child = Command::new(env!("CARGO_BIN_EXE_tole")) - .args(["serve", "--port", &port.to_string(), "--token", "t0ken"]) - .stdin(Stdio::null()) - .stdout(Stdio::null()) - .stderr(Stdio::inherit()) - .spawn() - .expect("spawn tole serve"); - let me = Self { child, port }; - for _ in 0..50 { - if let Ok(r) = me.raw("GET /health HTTP/1.1\r\nConnection: close\r\n\r\n") { - if r.starts_with("HTTP/1.1 200") { - return me; - } - } - std::thread::sleep(Duration::from_millis(100)); - } - panic!("serve never became healthy"); + let (child, port) = common::spawn_listening(|port| { + let mut cmd = Command::new(env!("CARGO_BIN_EXE_tole")); + cmd.args(["serve", "--port", &port.to_string(), "--token", "t0ken"]) + .stdin(Stdio::null()) + .stdout(Stdio::null()); + cmd + }); + Self { child, port } } fn connect(&self) -> std::io::Result { @@ -166,3 +154,33 @@ fn request_line_and_headers_share_one_size_ceiling() { status(&r) ); } + +/// The port-race guard itself: an instance that exits before listening +/// (as `tole serve` does on "Address already in use") is retried on a new +/// port, and readiness comes from the "listening on" line. +#[cfg(unix)] +#[test] +fn spawn_listening_retries_an_instance_that_exits_before_listening() { + let mut attempts = 0; + let (mut child, port) = common::spawn_listening(|port| { + attempts += 1; + let script = if attempts == 1 { + "echo 'binding: Address already in use' >&2; exit 1".to_string() + } else { + format!("echo 'listening on http://127.0.0.1:{port}' >&2; sleep 30") + }; + let mut cmd = Command::new("sh"); + cmd.args(["-c", &script]) + .stdin(Stdio::null()) + .stdout(Stdio::null()); + cmd + }); + assert_eq!(attempts, 2, "the first instance exited and was retried"); + assert!(port > 0); + assert!( + child.try_wait().unwrap().is_none(), + "the second one is running" + ); + let _ = child.kill(); + let _ = child.wait(); +} diff --git a/crates/tole-cli/tests/serve_session_cancel.rs b/crates/tole-cli/tests/serve_session_cancel.rs index 7453d15..35f1c4b 100644 --- a/crates/tole-cli/tests/serve_session_cancel.rs +++ b/crates/tole-cli/tests/serve_session_cancel.rs @@ -16,6 +16,8 @@ use std::process::{Child, Command, Stdio}; use std::sync::Arc; use std::time::Duration; +mod common; + struct ServeProcess { child: Child, addr: String, @@ -24,9 +26,6 @@ struct ServeProcess { impl ServeProcess { fn spawn(base_url: &str) -> Arc { - let port_listener = TcpListener::bind("127.0.0.1:0").unwrap(); - let port = port_listener.local_addr().unwrap().port(); - drop(port_listener); let cwd = std::env::temp_dir().join(format!( "tole-serve-cancel-{}-{:x}", std::time::SystemTime::now() @@ -36,31 +35,33 @@ impl ServeProcess { std::process::id() )); std::fs::create_dir_all(&cwd).unwrap(); - let mut cmd = Command::new(env!("CARGO_BIN_EXE_tole")); - cmd.args([ - "serve", - "--port", - &port.to_string(), - "--workspace", - cwd.to_string_lossy().as_ref(), - ]) - .stdin(Stdio::null()) - .stdout(Stdio::null()) - .stderr(Stdio::null()) - .env_remove("TOLE_MEMORY") - .env_remove("TOLE_MEMORY_NAMESPACE") - .env_remove("TOLE_TRUST"); - cmd.env("TOLE_BASE_URL", base_url); - cmd.env("TOLE_MODEL", "mock-model"); - cmd.env("TOLE_API_KEY", "sk-test"); let token = "test-token-123".to_string(); - cmd.env("TOLE_SERVE_TOKEN", &token); - let child = cmd.spawn().expect("spawn tole serve"); + let (child, port) = common::spawn_listening(|port| { + let mut cmd = Command::new(env!("CARGO_BIN_EXE_tole")); + cmd.args([ + "serve", + "--port", + &port.to_string(), + "--workspace", + cwd.to_string_lossy().as_ref(), + ]) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .env_remove("TOLE_MEMORY") + .env_remove("TOLE_MEMORY_NAMESPACE") + .env_remove("TOLE_TRUST"); + cmd.env("TOLE_BASE_URL", base_url); + cmd.env("TOLE_MODEL", "mock-model"); + cmd.env("TOLE_API_KEY", "sk-test"); + cmd.env("TOLE_SERVE_TOKEN", &token); + cmd + }); let proc = Arc::new(Self { child, addr: format!("127.0.0.1:{port}"), token, }); + // The listener is up; also confirm the HTTP face answers. let deadline = std::time::Instant::now() + Duration::from_secs(20); while std::time::Instant::now() < deadline { if let Some((200, _)) = proc.request("GET", "/health", "") {