test(e2e): give a soulfind that dies on startup a fresh port

free_port() hands out a port that is free when asked, not when soulfind
binds it, and soulfind exits when the port has gone. The library
harness also gave soulfind only five seconds and did not notice a child
that had died, so either read as "no soulfind server" and failed the
test. It now waits ten seconds like the CLI harness, never takes a
child that has exited as up, and retries on a fresh port; the CLI
harness gets the same retry.

A listener that takes the port first can still answer before soulfind
has got as far as binding, so the two library tests that bind listeners
outside the server gate now take it too: while a soulfind starts, no
other test in that binary binds anything.

soulseek-rs-lib/tests/e2e.rs

@@ -76,38 +76,51 @@ impl TestServer {
     }
 
     /// Spawn a local soulfind on an ephemeral port with a throwaway database.
+    ///
+    /// soulfind exits if its port is taken between `free_port()` handing it
+    /// out and soulfind binding it, so a child that dies is retried on a
+    /// fresh port rather than read as a missing server.
     fn spawn(gate: std::sync::MutexGuard<'static, ()>) -> Option<Self> {
         let bin = soulfind_binary()?;
-        let port = free_port()?;
+        for _ in 0..3 {
-        let db = std::env::temp_dir().join(format!("soulfind-e2e-{port}.db"));
+            let port = free_port()?;
-        let _ = std::fs::remove_file(&db);
+            let db =
+                std::env::temp_dir().join(format!("soulfind-e2e-{port}.db"));
+            let _ = std::fs::remove_file(&db);
 
-        let mut child = Command::new(&bin)
+            let mut child = Command::new(&bin)
-            .arg("-p")
+                .arg("-p")
-            .arg(port.to_string())
+                .arg(port.to_string())
-            .arg("-d")
+                .arg("-d")
-            .arg(&db)
+                .arg(&db)
-            .stdout(Stdio::null())
+                .stdout(Stdio::null())
-            .stderr(Stdio::null())
+                .stderr(Stdio::null())
-            .spawn()
+                .spawn()
-            .ok()?;
+                .ok()?;
 
-        if wait_until_listening("127.0.0.1", port, Duration::from_secs(5))
+            // A child that has exited is never taken as up: soulfind exits
-            .is_none()
+            // when its port is gone, and whoever took it may answer instead.
-        {
+            let deadline = Instant::now() + Duration::from_secs(10);
-            // Server never came up (e.g. a toolchain/SQLite issue); skip.
+            while Instant::now() < deadline
+                && child.try_wait().ok().flatten().is_none()
+            {
+                if TcpStream::connect(("127.0.0.1", port)).is_ok() {
+                    return Some(Self {
+                        host: "127.0.0.1".to_string(),
+                        port,
+                        child: Some(child),
+                        db: Some(db),
+                        _gate: gate,
+                    });
+                }
+                std::thread::sleep(Duration::from_millis(100));
+            }
+            // Never came up (e.g. a toolchain/SQLite issue) or died.
             let _ = child.kill();
             let _ = child.wait();
-            return None;
+            let _ = std::fs::remove_file(&db);
         }
-
+        None
-        Some(Self {
-            host: "127.0.0.1".to_string(),
-            port,
-            child: Some(child),
-            db: Some(db),
-            _gate: gate,
-        })
     }
 
     fn settings(&self, username: &str, password: &str) -> ClientSettings {
@@ -4449,6 +4462,11 @@ fn phrases_the_server_excludes_are_kept_for_our_replies() {
     // network (code 160). Nicotine+ applies them to the files it offers in a
     // search reply, not to the searches it sends, and so do we — this pins
     // that the announced list arrives and is kept for that use.
+    // It binds listeners, so it waits for the server tests: a port handed
+    // to a soulfind that is still starting is otherwise up for grabs.
+    let _gate = SERVER_GATE
+        .lock()
+        .unwrap_or_else(std::sync::PoisonError::into_inner);
     let mut phrases = Message::new();
     phrases
         .write_int32(160)
@@ -5472,6 +5490,11 @@ fn a_disconnected_client_gives_its_listener_port_back() {
     // next one has to be able to advertise the port it was configured with,
     // not fall back to an ephemeral one nobody was told about. No server is
     // needed — binding the listener happens in connect(), before the dial.
+    // It binds listeners, so it waits for the server tests: a port handed
+    // to a soulfind that is still starting is otherwise up for grabs.
+    let _gate = SERVER_GATE
+        .lock()
+        .unwrap_or_else(std::sync::PoisonError::into_inner);
     let port = free_port().expect("a port to hold");
     let settings = |port: u16| ClientSettings {
         username: "e2e_port_release".to_string(),

soulseek-rs/tests/common/mod.rs

@@ -60,12 +60,16 @@ pub struct Soulfind {
 }
 
 impl Soulfind {
+    /// soulfind exits if its port is taken between `free_port()` handing it
+    /// out and soulfind binding it, so a start that fails gets a fresh port.
     pub fn start() -> Option<Self> {
-        let port = free_port()?;
+        (0..3).find_map(|_| {
-        let db = std::env::temp_dir().join(format!("soulfind-{port}.db"));
+            let port = free_port()?;
-        let _ = std::fs::remove_file(&db);
+            let db = std::env::temp_dir().join(format!("soulfind-{port}.db"));
-        let child = Self::spawn(port, &db)?;
+            let _ = std::fs::remove_file(&db);
-        Some(Self { child, db, port })
+            let child = Self::spawn(port, &db)?;
+            Some(Self { child, db, port })
+        })
     }
 
     fn spawn(port: u16, db: &Path) -> Option<Child> {
@@ -78,8 +82,8 @@ impl Soulfind {
 
         let deadline = Instant::now() + Duration::from_secs(10);
         while Instant::now() < deadline {
-            // Whoever answers on the port has to be this child: one that
+            // A child that has exited is never taken as up: soulfind exits
-            // lost the port and died would otherwise pass as up.
+            // when its port is gone, and whoever took it may answer instead.
             if child.try_wait().ok().flatten().is_some() {
                 return None;
             }