Run the fake FXServer tests on ports the system gives, so that two runs on one host no longer collide

crates/fake-fxserver/tests/archive.rs

@@ -3,15 +3,16 @@
 // The helpers below are test code too, though not inside a `#[test]` function.
 #![allow(clippy::unwrap_used)]
 
+mod common;
+
 use std::io::{BufRead, BufReader};
 use std::net::TcpListener;
 use std::process::{ChildStdout, Command, Stdio};
 use std::sync::mpsc;
 use std::thread;
 use std::time::Duration;
 
-/// The build tests' port: Cargo runs one test binary at a time.
-const PORT: u16 = 41956;
+use common::Port;
 
 /// The first line on `stdout`, unless none comes within 30 seconds.
 fn first_line(stdout: ChildStdout) -> Option<String> {
@@ -28,14 +29,15 @@ fn first_line(stdout: ChildStdout) -> Option<String> {
 /// tells that it is the one listening.
 #[test]
 fn fake_archive_says_it_listens_only_on_a_port_of_its_own() {
-    let other = TcpListener::bind(("127.0.0.1", PORT)).unwrap();
+    let port = Port::free();
+    let other = TcpListener::bind(("127.0.0.1", port.number)).unwrap();
     let refused = Command::new(env!("CARGO_BIN_EXE_fake-archive"))
-        .arg(PORT.to_string())
+        .arg(port.number.to_string())
         .output()
         .unwrap();
     drop(other);
     let mut serving = Command::new(env!("CARGO_BIN_EXE_fake-archive"))
-        .arg(PORT.to_string())
+        .arg(port.number.to_string())
         .stdout(Stdio::piped())
         .spawn()
         .unwrap();
@@ -48,5 +50,5 @@ fn fake_archive_says_it_listens_only_on_a_port_of_its_own() {
         refused.stdout.is_empty(),
         "a port it could not take is never announced"
     );
-    assert_eq!(said.as_deref(), Some("listening on 127.0.0.1:41956\n"));
+    assert_eq!(said, Some(format!("listening on 127.0.0.1:{}\n", port.number)));
 }

crates/fake-fxserver/tests/artifacts.rs

@@ -7,14 +7,17 @@
 // The helpers below are test code too, though not inside a `#[test]` function.
 #![allow(clippy::unwrap_used)]
 
+mod common;
+
 use std::fs;
 use std::io::{BufRead, BufReader};
-use std::net::{IpAddr, Ipv4Addr, SocketAddr};
+use std::net::{IpAddr, Ipv4Addr};
 use std::path::{Path, PathBuf};
 use std::process::{Child, Command, Stdio};
-use std::sync::{LazyLock, Mutex, PoisonError};
+use std::sync::LazyLock;
 use std::time::Duration;
 
+use common::{Port, Ports};
 use jiff::{SignedDuration, Timestamp, Unit};
 use observer_app::{
     Accounts, ArtifactTimings, Artifacts, ArtifactsSetup, ArtifactsTask, AuditLog, Caller, Client, Clock,
@@ -46,8 +49,8 @@ const NEW: u32 = 37900;
 const OTHER: u32 = 37950;
 /// The build the fake's own archive holds.
 const ARCHIVED: u32 = 99001;
-/// Where the fake serves its archive, apart from the browser tests' 41955.
-const ARCHIVE_PORT: u16 = 41956;
+/// Where the fake serves its archive.
+static ARCHIVE: LazyLock<Port> = LazyLock::new(Port::free);
 const COMMIT: &str = "0cfcdb5ead57a4a26fcb6ebad98adbcd0273e607";
 const SELECTION_ROUTE: (&str, &str) = ("PUT", "/api/servers/default/artifacts/selection");
 /// The build on probation crashes late enough that Observer has seen it online and read its
@@ -56,27 +59,8 @@ const CRASH_ON_PROBATION: &str = "set fake_crash_after_ms 8000";
 /// The main loop of the build on probation freezes once it is ready, well before its window ends.
 const HANG_ON_PROBATION: &str = "set fake_hang_after_ms 5000";
 
-/// Ports of their own, apart from the other test binaries'.
-static PORTS: LazyLock<Mutex<Vec<u16>>> = LazyLock::new(|| Mutex::new((41960..=41962).collect()));
-
-struct Port(u16);
-
-impl Port {
-    async fn lease() -> Self {
-        loop {
-            if let Some(port) = PORTS.lock().unwrap_or_else(PoisonError::into_inner).pop() {
-                return Self(port);
-            }
-            tokio::time::sleep(Duration::from_millis(50)).await;
-        }
-    }
-}
-
-impl Drop for Port {
-    fn drop(&mut self) {
-        PORTS.lock().unwrap_or_else(PoisonError::into_inner).push(self.0);
-    }
-}
+/// The labs' ports, lent to three at a time.
+static PORTS: Ports = Ports::new(3);
 
 struct Lab {
     dir: TempDir,
@@ -99,7 +83,7 @@ impl Lab {
     /// healthy, and 37900 installed beside it doing what `new_build` says. Its catalog lists
     /// 99001, downloaded from the fake's own archive.
     async fn new(stable_for: Duration, new_build: &str) -> Self {
-        let port = Port::lease().await;
+        let port = PORTS.lease().await;
         let dir = tempfile::tempdir().unwrap();
         let root = dir.path().join("observer-data/artifacts");
         install_fake(&root, HEALTHY, "");
@@ -110,7 +94,7 @@ impl Lab {
             data.join("server.cfg"),
             format!(
                 "endpoint_add_tcp \"127.0.0.1:{0}\"\nendpoint_add_udp \"127.0.0.1:{0}\"\nsv_maxclients 8\n",
-                port.0
+                port.number
             ),
         )
         .unwrap();
@@ -153,7 +137,7 @@ impl Lab {
                 supervision: supervision.clone(),
                 schedule: schedule.status(),
                 catalog: Catalog::fixed(Ok(listing)),
-                http: Http::local(ARCHIVE_PORT),
+                http: Http::local(ARCHIVE.number),
             },
             |timings: &mut ArtifactTimings| timings.stable_for = stable_for,
         )
@@ -292,7 +276,7 @@ async fn supervise(
             accounts: accounts.clone(),
             servers: servers.clone(),
             data_dir: dir.join("observer-data"),
-            panel: SocketAddr::from(([127, 0, 0, 1], 41975)),
+            panel: common::panel(),
             artifacts: dir.join("observer-data/artifacts"),
             console_logs: dir.join("observer-data/logs/console"),
             log: None,
@@ -395,7 +379,8 @@ fn archived() -> CatalogBuild {
         latest: false,
         availability: Availability::Downloadable {
             url: format!(
-                "http://127.0.0.1:{ARCHIVE_PORT}/{ARCHIVED}-{COMMIT}/{}",
+                "http://127.0.0.1:{}/{ARCHIVED}-{COMMIT}/{}",
+                ARCHIVE.number,
                 kind.file_name()
             )
             .parse()
@@ -411,15 +396,15 @@ fn archived() -> CatalogBuild {
     }
 }
 
-/// `fake-archive` serving the fake on `ARCHIVE_PORT`, until dropped.
+/// `fake-archive` serving the fake on [`ARCHIVE`]'s port, until dropped.
 struct ArchiveServer(Child);
 
 impl ArchiveServer {
     /// Once it says it listens, which it does once its archive is made. A server already on the
     /// port would answer too, so the line is waited for rather than a connection.
     fn start() -> Self {
         let mut server = Command::new(env!("CARGO_BIN_EXE_fake-archive"))
-            .arg(ARCHIVE_PORT.to_string())
+            .arg(ARCHIVE.number.to_string())
             .stdout(Stdio::piped())
             .spawn()
             .unwrap();
@@ -429,7 +414,8 @@ impl ArchiveServer {
             .unwrap();
         assert!(
             !said.is_empty(),
-            "fake-archive could not serve on port {ARCHIVE_PORT}, see its error above"
+            "fake-archive could not serve on port {}, see its error above",
+            ARCHIVE.number
         );
         Self(server)
     }

crates/fake-fxserver/tests/changes.rs

@@ -3,12 +3,14 @@
 // The helpers below are test code too, though not inside a `#[test]` function.
 #![allow(clippy::unwrap_used)]
 
+mod common;
+
 use std::fs;
-use std::net::{IpAddr, Ipv4Addr, SocketAddr};
+use std::net::{IpAddr, Ipv4Addr};
 use std::path::{Path, PathBuf};
-use std::sync::{LazyLock, Mutex, PoisonError};
 use std::time::{Duration, Instant};
 
+use common::{Port, Ports};
 use jiff::Timestamp;
 use observer_app::{
     Accounts, Caller, Changes, ChangesSetup, ChangesTask, Client, Clock, ComparedTo, DiffOptions,
@@ -39,26 +41,8 @@ const CONSOLE_ROUTE: (&str, &str) = ("POST", "/api/servers/default/console");
 /// A webhook planted in a resource, which no stored text may hold.
 const WEBHOOK: &str = "https://discord.com/api/webhooks/123456/PlantedTokenThatMustNeverBeStored";
 
-static PORTS: LazyLock<Mutex<Vec<u16>>> = LazyLock::new(|| Mutex::new((41940..=41942).collect()));
-
-struct Port(u16);
-
-impl Port {
-    async fn lease() -> Self {
-        loop {
-            if let Some(port) = PORTS.lock().unwrap_or_else(PoisonError::into_inner).pop() {
-                return Self(port);
-            }
-            tokio::time::sleep(Duration::from_millis(50)).await;
-        }
-    }
-}
-
-impl Drop for Port {
-    fn drop(&mut self) {
-        PORTS.lock().unwrap_or_else(PoisonError::into_inner).push(self.0);
-    }
-}
+/// The labs' ports, lent to three at a time.
+static PORTS: Ports = Ports::new(3);
 
 struct Lab {
     dir: TempDir,
@@ -74,15 +58,15 @@ struct Lab {
 
 impl Lab {
     async fn new() -> Self {
-        let port = Port::lease().await;
+        let port = PORTS.lease().await;
         let dir = tempfile::tempdir().unwrap();
         let observer_data = dir.path().join("observer-data");
         let build = observer_data.join("artifacts").join(BUILD.to_string());
         let program = fake_program(&build);
         fs::create_dir_all(program.parent().unwrap()).unwrap();
         fs::copy(env!("CARGO_BIN_EXE_fake-fxserver"), &program).unwrap();
         let data = dir.path().join("server-data");
-        plant(&data, port.0);
+        plant(&data, port.number);
         let db = Database::open(dir.path().join("observer.sqlite3"), dir.path().join("backups"))
             .await
             .unwrap();
@@ -95,7 +79,7 @@ impl Lab {
             accounts,
             servers,
             data_dir: observer_data.clone(),
-            panel: SocketAddr::from(([127, 0, 0, 1], 41975)),
+            panel: common::panel(),
             artifacts: observer_data.join("artifacts"),
             console_logs: observer_data.join("logs/console"),
             log: None,

crates/fake-fxserver/tests/common/mod.rs

@@ -0,0 +1,76 @@
+//! Loopback ports for the servers these tests start, each given by the system: another run of the
+//! tests on the same host, from another checkout, gets other ports.
+
+// Each test file uses some of these helpers; all are test code, though not in `#[test]` functions.
+#![allow(dead_code, clippy::unwrap_used)]
+
+use std::io::ErrorKind;
+use std::net::{Ipv4Addr, SocketAddr, TcpListener};
+use std::ops::RangeInclusive;
+use std::sync::LazyLock;
+
+use tokio::sync::{Semaphore, SemaphorePermit};
+
+/// The loopback address a test holds its port on, which leaves 127.0.0.1 to FXServer.
+const CLAIMS: Ipv4Addr = Ipv4Addr::new(127, 0, 0, 2);
+/// txAdmin's ports, among those Linux gives: the cfg scan warns of an endpoint on one of them.
+const TXADMIN: RangeInclusive<u16> = 40120..=40150;
+
+/// A port of 127.0.0.1 the system gave, this test's own until dropped.
+pub struct Port {
+    pub number: u16,
+    /// The system may give a port again as soon as nothing listens on it, and FXServer only
+    /// listens once it has started. Whoever is given this one meanwhile, in this run or another,
+    /// cannot claim it and asks for another.
+    _claim: TcpListener,
+    _turn: Option<SemaphorePermit<'static>>,
+}
+
+impl Port {
+    pub fn free() -> Self {
+        loop {
+            let free = TcpListener::bind((Ipv4Addr::LOCALHOST, 0)).unwrap();
+            let number = free.local_addr().unwrap().port();
+            if TXADMIN.contains(&number) {
+                continue;
+            }
+            match TcpListener::bind((CLAIMS, number)) {
+                Ok(claim) => {
+                    return Self {
+                        number,
+                        _claim: claim,
+                        _turn: None,
+                    };
+                }
+                Err(error) if error.kind() == ErrorKind::AddrInUse => {}
+                Err(error) => panic!("cannot claim port {number} on {CLAIMS}: {error}"),
+            }
+        }
+    }
+}
+
+/// Lends ports to the labs of a test file, to so many of them at a time, which bounds how many
+/// servers the file runs at once.
+pub struct Ports(Semaphore);
+
+impl Ports {
+    pub const fn new(at_a_time: usize) -> Self {
+        Self(Semaphore::const_new(at_a_time))
+    }
+
+    pub async fn lease(&'static self) -> Port {
+        let turn = self.0.acquire().await.unwrap();
+        Port {
+            _turn: Some(turn),
+            ..Port::free()
+        }
+    }
+}
+
+/// The panel's address the labs give Observer, which the cfg checks only compare with: a port held
+/// for the whole run, so that no server is given it.
+pub fn panel() -> SocketAddr {
+    static PANEL: LazyLock<TcpListener> =
+        LazyLock::new(|| TcpListener::bind((Ipv4Addr::LOCALHOST, 0)).unwrap());
+    PANEL.local_addr().unwrap()
+}