diff --git a/crates/mcp/src/tests.rs b/crates/mcp/src/tests.rs index 092d5855b..e3368663e 100644 --- a/crates/mcp/src/tests.rs +++ b/crates/mcp/src/tests.rs @@ -822,13 +822,18 @@ fn answer_dialog_needs_the_desktop_app() { assert!(text_of(&r).contains("needs the desktop app"), "{r}"); } +/// How long a fake app waits for [`Remote`] to connect or to send, and how long a test waits for +/// a connection to end or a line to arrive. The wait only ends a test whose client or app never +/// does. A machine busy with other builds can hold a thread for seconds. +const WAIT: std::time::Duration = std::time::Duration::from_secs(20); + #[test] fn remote_does_not_replay_when_edit_reply_is_lost() { let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); listener.set_nonblocking(true).unwrap(); let addr = listener.local_addr().unwrap().to_string(); let h = std::thread::spawn(move || { - let timeout = std::time::Duration::from_secs(5); + let timeout = WAIT; let mut backend = Headless::with_document(); let base = backend.call("document.inspect", json!({})).unwrap()["objects"].as_u64().unwrap(); for connection in 0..2 { @@ -988,7 +993,7 @@ fn remote_rejects_uncorrelated_replies_without_replaying() { listener.set_nonblocking(true).unwrap(); let addr = listener.local_addr().unwrap().to_string(); let h = std::thread::spawn(move || { - let timeout = std::time::Duration::from_secs(5); + let timeout = WAIT; let mut seen = vec![]; for connection in 0..2 { let deadline = std::time::Instant::now() + timeout; @@ -1043,7 +1048,7 @@ fn remote_rejects_a_reply_with_a_mismatched_id_and_reconnects() { let h = std::thread::spawn(move || { let mut seen = vec![]; for stream in listener.incoming().flatten().take(2) { - stream.set_read_timeout(Some(std::time::Duration::from_secs(5))).unwrap(); + stream.set_read_timeout(Some(WAIT)).unwrap(); let mut out = stream.try_clone().unwrap(); let mut reader = BufReader::new(stream); let mut line = String::new(); @@ -1079,12 +1084,12 @@ fn remote_reports_the_reason_of_a_refusal_without_an_id() { let h = std::thread::spawn(move || { let (mut refused, _) = listener.accept().unwrap(); // The request is read first, so the refusal answers it and the close is orderly. - refused.set_read_timeout(Some(std::time::Duration::from_secs(5))).unwrap(); + refused.set_read_timeout(Some(WAIT)).unwrap(); let _ = BufReader::new(refused.try_clone().unwrap()).read_line(&mut String::new()); writeln!(refused, "{}", json!({"ok": false, "error": "too many control connections"})).unwrap(); drop(refused); let (stream, _) = listener.accept().unwrap(); - stream.set_read_timeout(Some(std::time::Duration::from_secs(5))).unwrap(); + stream.set_read_timeout(Some(WAIT)).unwrap(); let mut out = stream.try_clone().unwrap(); let mut line = String::new(); BufReader::new(stream).read_line(&mut line).unwrap(); @@ -1099,6 +1104,26 @@ fn remote_reports_the_reason_of_a_refusal_without_an_id() { h.join().unwrap(); } +/// How long a refused connect to the app may take: longer than the 800 ms connect timeout of +/// [`Remote`], and shorter than the 2 s a refused local connect takes on Windows without one. +const FAILED_CONNECT: std::time::Duration = std::time::Duration::from_millis(1500); + +/// Runs `attempt` until one run takes less than `bound`, at most five times, and returns how long +/// each run took. A machine busy with other builds can hold the test's thread for seconds in one +/// run; a connect that waits longer than its own timeout is slow in every run. +fn runs_until_faster_than(bound: std::time::Duration, mut attempt: impl FnMut()) -> Vec { + let mut took = Vec::new(); + while took.len() < 5 { + let start = std::time::Instant::now(); + attempt(); + took.push(start.elapsed()); + if took.last() < Some(&bound) { + break; + } + } + took +} + #[test] fn remote_connect_fails_fast() { // A port this test holds, bound but not listening, so connecting to it is refused. (A port @@ -1106,9 +1131,8 @@ fn remote_connect_fails_fast() { let held = socket2::Socket::new(socket2::Domain::IPV4, socket2::Type::STREAM, None).unwrap(); held.bind(&std::net::SocketAddr::from(([127, 0, 0, 1], 0)).into()).unwrap(); let port = held.local_addr().unwrap().as_socket().unwrap().port(); - let start = std::time::Instant::now(); - assert!(Remote::connect(&format!("127.0.0.1:{port}")).is_err()); - assert!(start.elapsed() < std::time::Duration::from_secs(5), "took {:?}", start.elapsed()); + let took = runs_until_faster_than(FAILED_CONNECT, || assert!(Remote::connect(&format!("127.0.0.1:{port}")).is_err())); + assert!(took.last() < Some(&FAILED_CONNECT), "took {took:?}"); // Nothing else could listen there meanwhile. assert!(std::net::TcpListener::bind(format!("127.0.0.1:{port}")).is_err()); drop(held); @@ -1124,7 +1148,7 @@ fn reusable_listener(addr: std::net::SocketAddr) -> std::net::TcpListener { fn wait_for_remote_eof(remote: &Remote) { let probe = remote.connection_probe(); - probe.set_read_timeout(Some(std::time::Duration::from_secs(2))).unwrap(); + probe.set_read_timeout(Some(WAIT)).unwrap(); assert_eq!(probe.peek(&mut [0]).unwrap(), 0); } @@ -1135,7 +1159,7 @@ fn remote_edit_succeeds_once_when_app_restarts() { let mut remote = Remote::connect(&addr.to_string()).unwrap(); let (old_connection, _) = listener.accept().unwrap(); let old_handle = std::thread::spawn(move || { - let timeout = std::time::Duration::from_secs(2); + let timeout = WAIT; old_connection.set_read_timeout(Some(timeout)).unwrap(); old_connection.set_write_timeout(Some(timeout)).unwrap(); let mut reader = BufReader::new(old_connection.try_clone().unwrap()); @@ -1157,7 +1181,7 @@ fn remote_edit_succeeds_once_when_app_restarts() { let listener = reusable_listener(addr); listener.set_nonblocking(true).unwrap(); let handle = std::thread::spawn(move || { - let timeout = std::time::Duration::from_secs(2); + let timeout = WAIT; let deadline = std::time::Instant::now() + timeout; let stream = loop { match listener.accept() { @@ -1204,15 +1228,15 @@ fn remote_does_not_send_on_a_connection_with_a_stray_line() { let addr = listener.local_addr().unwrap().to_string(); let mut remote = Remote::connect(&addr).unwrap(); let (mut stale, _) = listener.accept().unwrap(); - stale.set_read_timeout(Some(std::time::Duration::from_secs(2))).unwrap(); + stale.set_read_timeout(Some(WAIT)).unwrap(); writeln!(stale, "{}", json!({"ok": false, "error": "idle"})).unwrap(); let probe = remote.connection_probe(); - probe.set_read_timeout(Some(std::time::Duration::from_secs(2))).unwrap(); + probe.set_read_timeout(Some(WAIT)).unwrap(); assert_eq!(probe.peek(&mut [0]).unwrap(), 1, "the stray line arrived"); drop(probe); let h = std::thread::spawn(move || { let (stream, _) = listener.accept().unwrap(); - stream.set_read_timeout(Some(std::time::Duration::from_secs(5))).unwrap(); + stream.set_read_timeout(Some(WAIT)).unwrap(); let mut out = stream.try_clone().unwrap(); let mut line = String::new(); BufReader::new(stream).read_line(&mut line).unwrap(); @@ -1239,13 +1263,18 @@ fn remote_call_reports_nothing_sent_when_reconnect_is_refused() { held.set_reuse_address(true).unwrap(); held.bind(&addr.into()).unwrap(); - let start = std::time::Instant::now(); - let error = remote.call("engine.execute", json!({"command": "shape.rectangle", "params": {"width": 20, "height": 20}})).unwrap_err(); + // Every call connects again: the first after finding the old connection closed, a later one because the failed call left none. + let mut errors = Vec::new(); + let took = runs_until_faster_than(FAILED_CONNECT, || { + errors.push(remote.call("engine.execute", json!({"command": "shape.rectangle", "params": {"width": 20, "height": 20}})).unwrap_err()); + }); - assert!(start.elapsed() < std::time::Duration::from_secs(2)); - assert!(error.contains("not reachable"), "{error}"); - assert!(error.contains("nothing was sent"), "{error}"); - assert!(!error.contains("operation may have completed"), "{error}"); + assert!(took.last() < Some(&FAILED_CONNECT), "took {took:?}"); + for error in errors { + assert!(error.contains("not reachable"), "{error}"); + assert!(error.contains("nothing was sent"), "{error}"); + assert!(!error.contains("operation may have completed"), "{error}"); + } } #[test]