From 7060d3521a06c516bffa1a9b37d44fe8e3a0631b Mon Sep 17 00:00:00 2001
From: Michel de Graaf
Date: Thu, 10 Sep 2026 13:59:04 +0200
Subject: [PATCH 01/23] fix(rooms): start a room's roster from the member list,
not the join echo
soulfind tells a room about a new member, the new member included, before
it sends them the room's member list. The roster was started from that
echo, so for a moment after joining a room read as holding nobody but us,
and `room users` returns on the first roster it sees: it could print only
the asking user. That is the flake in room_users_lists_who_is_in_the_room.
A join or leave now only updates a roster the server has sent in full,
which is also what room_members already promised for a room we have not
joined. The e2e test waits for the resident's own member list instead of a
fixed second, so it no longer races the resident's join either.
---
soulseek-rs-lib/src/client/context.rs | 8 +++++---
soulseek-rs-lib/src/client/tests/rooms.rs | 19 +++++++++++++++++++
soulseek-rs/tests/cli_e2e.rs | 7 ++++++-
3 files changed, 30 insertions(+), 4 deletions(-)
diff --git a/soulseek-rs-lib/src/client/context.rs b/soulseek-rs-lib/src/client/context.rs
index b51a58a..8606abf 100644
--- a/soulseek-rs-lib/src/client/context.rs
+++ b/soulseek-rs-lib/src/client/context.rs
@@ -100,10 +100,12 @@ impl ClientContext {
// the whole board again on the next join.
self.room_tickers.remove(room);
}
+ // Only a roster the server has sent in full is kept current: the
+ // joiner hears of their own arrival before the member list.
RoomEvent::UserJoined { room, username } => {
- let members =
- self.room_members.entry(room.clone()).or_default();
- if let Err(at) = members.binary_search(username) {
+ if let Some(members) = self.room_members.get_mut(room)
+ && let Err(at) = members.binary_search(username)
+ {
members.insert(at, username.clone());
}
}
diff --git a/soulseek-rs-lib/src/client/tests/rooms.rs b/soulseek-rs-lib/src/client/tests/rooms.rs
index 0af6784..d504839 100644
--- a/soulseek-rs-lib/src/client/tests/rooms.rs
+++ b/soulseek-rs-lib/src/client/tests/rooms.rs
@@ -39,6 +39,25 @@ fn the_room_roster_follows_the_joins_and_leaves_the_server_reports() {
assert!(context.room_members("lobby").is_empty());
}
+#[test]
+fn a_join_announced_before_the_member_list_does_not_start_a_roster() {
+ // soulfind tells a room about a joiner, the joiner included, before it
+ // sends them the member list. A roster started from that echo reads as a
+ // room holding nobody else.
+ let mut context = ClientContext::new();
+ context.apply_room_event(RoomEvent::UserJoined {
+ room: "lobby".to_string(),
+ username: "me".to_string(),
+ });
+ assert!(context.room_members("lobby").is_empty());
+
+ context.apply_room_event(RoomEvent::Joined {
+ room: "lobby".to_string(),
+ users: vec!["me".to_string(), "alice".to_string()],
+ });
+ assert_eq!(context.room_members("lobby"), ["alice", "me"]);
+}
+
#[test]
fn a_user_joining_twice_is_listed_once() {
let mut context = ClientContext::new();
diff --git a/soulseek-rs/tests/cli_e2e.rs b/soulseek-rs/tests/cli_e2e.rs
index 040e663..a65e3e4 100644
--- a/soulseek-rs/tests/cli_e2e.rs
+++ b/soulseek-rs/tests/cli_e2e.rs
@@ -2418,7 +2418,12 @@ fn room_users_lists_who_is_in_the_room() {
let server = server_or_skip!();
let resident = server.client("cli_e2e_resident", Vec::new());
resident.join_room("cli_e2e_roster").expect("join");
- settle();
+ // The member list reaches the resident once the server has them in.
+ let deadline = Instant::now() + Duration::from_secs(10);
+ while resident.room_members("cli_e2e_roster").is_empty() {
+ assert!(Instant::now() < deadline, "the resident never got in");
+ std::thread::sleep(Duration::from_millis(50));
+ }
let output = cli(
&server,
From 7c9f561d2236b430f143e4502036ba066ca852e6 Mon Sep 17 00:00:00 2001
From: Michel de Graaf
Date: Thu, 10 Sep 2026 14:28:04 +0200
Subject: [PATCH 02/23] fix(client): record a watch before the server can
answer it
watch_user sent the request and only then recorded the watch. The server's
answer is applied on the ops thread, so a reply that got there first saw
nothing to drop for a user who does not exist, and the add that followed
left that user watched for good. The lock is now taken before the send and
held until the watch is recorded, so the answer always lands on it; a send
that fails still records nothing.
---
soulseek-rs-lib/src/client/rooms.rs | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/soulseek-rs-lib/src/client/rooms.rs b/soulseek-rs-lib/src/client/rooms.rs
index d11da0c..671ff53 100644
--- a/soulseek-rs-lib/src/client/rooms.rs
+++ b/soulseek-rs-lib/src/client/rooms.rs
@@ -150,8 +150,11 @@ impl Client {
/// Returns [`SoulseekRs::NotConnected`] if the client is not connected.
pub fn watch_user(&self, username: &str) -> Result<()> {
use crate::message::server::MessageFactory;
+ // Held across the send: the reply is applied under this lock, so it
+ // cannot land first and have its "does not exist" undone by the add.
+ let mut context = self.context.write_safe()?;
self.send_server_message(MessageFactory::build_watch_user(username))?;
- self.context.write_safe()?.add_watched_user(username);
+ context.add_watched_user(username);
Ok(())
}
From eeae8e293419bcda48b941f9f270c397e025ea4d Mon Sep 17 00:00:00 2001
From: Michel de Graaf
Date: Thu, 10 Sep 2026 14:29:36 +0200
Subject: [PATCH 03/23] test(resume): bound what a paused transfer may still
take, not when
The pause test slept 1.2 s so a read already in flight could time out
before bytes were queued "while paused", then required that none of them
were taken. The reader's read timeout is 1 s, so any scheduling delay past
200 ms let the in-flight read take the probe and failed the test.
The guarantee is that the pause is checked before every read, so after it
lands at most the one read already in flight completes. The test now
queues more than a read buffer while paused and asserts no more than one
buffer was taken, which holds however the reader is scheduled and still
fails a transfer that ignores the pause. It also drops the 1.2 s sleep.
---
soulseek-rs-lib/tests/resume_e2e.rs | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/soulseek-rs-lib/tests/resume_e2e.rs b/soulseek-rs-lib/tests/resume_e2e.rs
index 7271c25..89ef307 100644
--- a/soulseek-rs-lib/tests/resume_e2e.rs
+++ b/soulseek-rs-lib/tests/resume_e2e.rs
@@ -379,18 +379,21 @@ fn a_live_download_pauses_and_resumes_on_the_same_connection() {
matches!(status, DownloadStatus::Paused { .. })
});
- // Give a read already in flight one timeout cycle to observe the pause.
- std::thread::sleep(Duration::from_millis(1_200));
+ // The pause is checked before every read, so once it lands only the read
+ // already in flight can finish: one 64 KiB buffer at most. Queue more
+ // than that, which a transfer ignoring the pause would take all of.
let stable_len = fs::metadata(&part_path).expect("paused partial").len();
- let queued_while_paused = 8 * 1024;
+ let in_flight = 64 * 1024;
+ let queued_while_paused = in_flight + 8 * 1024;
peer.write_all(&content[first_chunk..first_chunk + queued_while_paused])
.expect("queue bytes while paused");
peer.flush().expect("flush bytes while paused");
std::thread::sleep(Duration::from_millis(300));
- assert_eq!(
- fs::metadata(&part_path).expect("paused partial").len(),
- stable_len,
- "a paused transfer must stop consuming its socket"
+ let paused_len = fs::metadata(&part_path).expect("paused partial").len();
+ assert!(
+ paused_len <= stable_len + in_flight as u64,
+ "a paused transfer must stop consuming its socket: {paused_len} \
+ bytes after pausing at {stable_len}"
);
assert!(subject.client.resume_download(&control.username, filename));
From 8bee7745e1360375cd6ad30d7a43a68a8b7a75ed Mon Sep 17 00:00:00 2001
From: Michel de Graaf
Date: Thu, 10 Sep 2026 14:33:23 +0200
Subject: [PATCH 04/23] test(cli): hand a config command's session flags to the
daemon
cli_with_config passed every flag to the client, so in daemon mode the
wishlist serve tests started their daemons with no --shared-dir. Those
daemons fell back to sharing ~/Downloads/Soulseek, and the tests only
passed because an earlier download test happened to create that folder;
run alone, shuffled, or on a machine without it, both failed with
"nothing to share". Session flags now go to the daemon the way cli()
already routes them.
---
soulseek-rs/tests/cli_e2e.rs | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/soulseek-rs/tests/cli_e2e.rs b/soulseek-rs/tests/cli_e2e.rs
index a65e3e4..6bdce12 100644
--- a/soulseek-rs/tests/cli_e2e.rs
+++ b/soulseek-rs/tests/cli_e2e.rs
@@ -884,21 +884,28 @@ fn split_session_flags(args: &[&str]) -> (Vec, Vec) {
///
/// `--no-config` is dropped for the obvious reason, and `--quiet` with it:
/// these commands report the schedule the *server* chose on stderr, which is
-/// the only place a test can see it.
+/// the only place a test can see it. Session flags go to the daemon, as in
+/// [`cli`].
fn cli_with_config(
server: &TestServer,
user: &str,
config: &Path,
args: &[&str],
) -> Output {
+ let (session_flags, command_args) = match mode() {
+ Mode::Local => {
+ (Vec::new(), args.iter().map(|a| (*a).to_string()).collect())
+ }
+ Mode::Daemon => split_session_flags(args),
+ };
let mut all: Vec = server
- .args(user)
+ .args_with(user, &session_flags)
.into_iter()
.filter(|arg| arg != "--no-config" && arg != "--quiet")
.collect();
all.push("--config".to_string());
all.push(config.to_str().expect("utf-8 path").to_string());
- all.extend(args.iter().map(|a| (*a).to_string()));
+ all.extend(command_args);
let refs: Vec<&str> = all.iter().map(String::as_str).collect();
run(&refs)
}
From 8854f8d005423ea8fd3c9729970d8757ac53efa0 Mon Sep 17 00:00:00 2001
From: Michel de Graaf
Date: Thu, 10 Sep 2026 14:35:37 +0200
Subject: [PATCH 05/23] test(cli): wait for the listener to be in the room, and
keep an early PM
room_listen said its line three seconds after starting the listener and
hoped it had joined by then; a line said before the join never reaches
it. The speaker now waits for the room to announce the listener, which
soulfind only does once it has them in.
message_read sent its message three seconds after starting the reader.
A reader not yet logged in is a name soulfind has never seen, and it
drops a message to such a name. The reader is now registered first, so
a message that beats its login is kept and delivered when it arrives.
---
soulseek-rs/tests/cli_e2e.rs | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/soulseek-rs/tests/cli_e2e.rs b/soulseek-rs/tests/cli_e2e.rs
index 6bdce12..efea975 100644
--- a/soulseek-rs/tests/cli_e2e.rs
+++ b/soulseek-rs/tests/cli_e2e.rs
@@ -1895,8 +1895,19 @@ fn room_listen_streams_what_is_said_in_the_room() {
.spawn()
.expect("the binary should run");
- // Give the listener time to join before saying anything.
- std::thread::sleep(Duration::from_secs(3));
+ // The room announces the listener once the server has it in; a line
+ // said before that never reaches it.
+ let deadline = Instant::now() + Duration::from_secs(30);
+ while !speaker.take_room_events().iter().any(|event| {
+ matches!(
+ event,
+ soulseek_rs::types::RoomEvent::UserJoined { room, username }
+ if room == "cli_e2e_lobby_b" && username == "cli_e2e_room_reader"
+ )
+ }) {
+ assert!(Instant::now() < deadline, "the listener never joined");
+ std::thread::sleep(Duration::from_millis(50));
+ }
speaker
.say_in_room("cli_e2e_lobby_b", "streamed line")
.expect("say");
@@ -1965,6 +1976,11 @@ fn a_private_message_reaches_its_recipient() {
fn message_read_streams_incoming_private_messages() {
let server = server_or_skip!();
let sender = server.client("cli_e2e_pm_sender", Vec::new());
+ // soulfind drops a message to a name it has never seen but keeps one for
+ // a known user until they log in, so however long the reader takes to
+ // get online, the line below reaches it. Registered before the reader's
+ // own session exists: a later login as them would take its place.
+ drop(server.client("cli_e2e_pm_reader", Vec::new()));
settle();
let mut args = server.args("cli_e2e_pm_reader");
From 3c5a3dd64853869a1335cd935cc2db4a6bc9c49a Mon Sep 17 00:00:00 2001
From: Michel de Graaf
Date: Thu, 10 Sep 2026 14:36:32 +0200
Subject: [PATCH 06/23] test(portmap): find the unmap among resent maps
The NAT-PMP client resends a map the gateway has not answered within
250 ms, and the mock gateway records every request, so a slow mock put a
resent map where the test expected the unmap and failed it. The test now
looks for the lifetime-0 request among whatever arrives after the drop.
On Windows the mock's answer to that resend meets an already-closed
socket, and the next receive reports the bounce as a connection reset,
which ended the mock's loop before the unmap. It now carries on past it.
---
soulseek-rs/src/port_mapping/mod.rs | 38 +++++++++++++++++++----------
1 file changed, 25 insertions(+), 13 deletions(-)
diff --git a/soulseek-rs/src/port_mapping/mod.rs b/soulseek-rs/src/port_mapping/mod.rs
index 4787084..f8e67b6 100644
--- a/soulseek-rs/src/port_mapping/mod.rs
+++ b/soulseek-rs/src/port_mapping/mod.rs
@@ -418,7 +418,18 @@ mod tests {
thread::spawn(move || {
let mut buf = [0u8; 64];
// Answer many requests (initial map + cleanup unmap + any renews).
- while let Ok((n, from)) = socket.recv_from(&mut buf) {
+ loop {
+ let (n, from) = match socket.recv_from(&mut buf) {
+ Ok(received) => received,
+ // Windows reports a reply that met an already-closed
+ // client socket on the next receive; the gateway is fine.
+ Err(e)
+ if e.kind() == std::io::ErrorKind::ConnectionReset =>
+ {
+ continue;
+ }
+ Err(_) => break,
+ };
let request = buf[..n].to_vec();
let internal = u16::from_be_bytes([request[4], request[5]]);
let lifetime = u32::from_be_bytes([
@@ -465,20 +476,21 @@ mod tests {
);
// Dropping the mapper must remove the mapping (a lifetime-0 request).
+ // A map the gateway was slow to answer is sent again, and that resend
+ // can arrive first.
drop(mapper);
- let unmap_req = requests
- .recv_timeout(StdDuration::from_secs(3))
- .expect("dropping the mapper should send an unmap request");
- assert_eq!(
+ let unmapped = std::iter::from_fn(|| {
+ requests.recv_timeout(StdDuration::from_secs(3)).ok()
+ })
+ .any(|request| {
u32::from_be_bytes([
- unmap_req[8],
- unmap_req[9],
- unmap_req[10],
- unmap_req[11]
- ]),
- 0,
- "cleanup should request lifetime 0"
- );
+ request[8],
+ request[9],
+ request[10],
+ request[11],
+ ]) == 0
+ });
+ assert!(unmapped, "cleanup should request lifetime 0");
}
/// A minimal in-process UPnP IGD: an HTTP server that answers every SOAP
From 2e23581513c134afe0afe6a12d8e92a3a7799e4f Mon Sep 17 00:00:00 2001
From: Michel de Graaf
Date: Thu, 10 Sep 2026 14:46:11 +0200
Subject: [PATCH 07/23] test(e2e): wait for the join and the password change,
not a sleep
The chat-room test gave both joins 500 ms before speaking; a line said
before bob is in the room never reaches him. It now waits until each
has the member list the server sends once they are in.
The password test slept 500 ms and then disconnected. soulfind stores a
new password only after hashing it, and drops the change if the session
closes first, so a slow hash let the old password keep working. The
session now stays up until a login with the new password is accepted.
---
soulseek-rs-lib/tests/e2e.rs | 37 +++++++++++++++++++++++++-----------
1 file changed, 26 insertions(+), 11 deletions(-)
diff --git a/soulseek-rs-lib/tests/e2e.rs b/soulseek-rs-lib/tests/e2e.rs
index 5727b75..a49db08 100644
--- a/soulseek-rs-lib/tests/e2e.rs
+++ b/soulseek-rs-lib/tests/e2e.rs
@@ -327,8 +327,15 @@ fn a_chat_room_message_is_delivered_between_users() {
alice.join_room(room).expect("alice joins room");
bob.join_room(room).expect("bob joins room");
- // Give both joins time to register on the server before speaking.
- std::thread::sleep(Duration::from_millis(500));
+ // Each is in once the server has sent them the member list; a line said
+ // before bob is in never reaches him.
+ let deadline = Instant::now() + Duration::from_secs(5);
+ while alice.room_members(room).is_empty()
+ || bob.room_members(room).is_empty()
+ {
+ assert!(Instant::now() < deadline, "both should get into the room");
+ std::thread::sleep(Duration::from_millis(50));
+ }
let _ = alice.take_room_events();
let _ = bob.take_room_events();
@@ -4256,7 +4263,23 @@ fn a_changed_password_is_what_the_next_login_needs() {
client
.change_password("second-pw")
.expect("change password");
- std::thread::sleep(Duration::from_millis(500));
+ // soulfind stores the new password once it has hashed it, and forgets the
+ // change if our session closes first. The session stays until a login
+ // with the new password shows it landed; a refused one leaves it alone.
+ let deadline = Instant::now() + Duration::from_secs(10);
+ loop {
+ let mut fresh =
+ Client::with_settings(server.settings(user, "second-pw"));
+ fresh.connect().expect("connect with the new password");
+ if matches!(fresh.login(), Ok(true)) {
+ break;
+ }
+ assert!(
+ Instant::now() < deadline,
+ "the new password must be accepted"
+ );
+ std::thread::sleep(Duration::from_millis(100));
+ }
drop(client);
let mut stale = Client::with_settings(server.settings(user, "first-pw"));
@@ -4265,14 +4288,6 @@ fn a_changed_password_is_what_the_next_login_needs() {
!matches!(stale.login(), Ok(true)),
"the old password must stop working"
);
- drop(stale);
-
- let mut fresh = Client::with_settings(server.settings(user, "second-pw"));
- fresh.connect().expect("connect with the new password");
- assert!(
- fresh.login().expect("login with the new password"),
- "the new password must be accepted"
- );
}
#[test]
From 7ffb85a6ada01b35b8946f663a322be592ca12d2 Mon Sep 17 00:00:00 2001
From: Michel de Graaf
Date: Thu, 10 Sep 2026 14:46:11 +0200
Subject: [PATCH 08/23] test(e2e): start a mock uploader once the client has
registered it
The mock peers signalled ready as soon as their PeerInit was written,
and the tests slept 1.5 s before downloading so the client could
register the connection. A download that got in first dialled a peer
the server has never heard of, and since CantConnectToPeer fails queued
downloads it failed at once instead of waiting for the connection.
A mock now asks for the client's user info before signalling ready. The
client answers through the peer it registered for that connection, so
the answer means the download will queue on it. The six sleeps go.
---
soulseek-rs-lib/tests/e2e.rs | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/soulseek-rs-lib/tests/e2e.rs b/soulseek-rs-lib/tests/e2e.rs
index a49db08..b53c4a3 100644
--- a/soulseek-rs-lib/tests/e2e.rs
+++ b/soulseek-rs-lib/tests/e2e.rs
@@ -533,7 +533,14 @@ fn open_control_and_await_queue(
let mut p = connect_retry(&cfg.listen_addr, Duration::from_secs(5))?;
p.set_read_timeout(Some(Duration::from_secs(10)))?;
p.write_all(&peer_init_bytes(&cfg.peer_username, "P", 0))?;
+ // The client answers a user-info request through the peer it registered
+ // for this connection, so the answer means a download queues on it
+ // rather than dialling a peer the server has never heard of.
+ p.write_all(
+ &soulseek_rs::message::peer::build_user_info_request().get_buffer(),
+ )?;
p.flush()?;
+ expect_code(&mut p, 16, Duration::from_secs(10))?;
let _ = cfg.ready.send(());
loop {
@@ -867,10 +874,6 @@ fn file_downloads_over_p_and_f(
.recv_timeout(Duration::from_secs(5))
.expect("mock uploader P connection");
- // The listener registers an incoming peer under its plain username; give
- // that registration a moment to complete before queuing the download.
- std::thread::sleep(Duration::from_millis(1500));
-
let (_download, status_rx) = client
.download(
filename.to_string(),
@@ -950,7 +953,6 @@ fn cancel_mid_transfer(peer_username: &str, stall_after: Option) {
ready_rx
.recv_timeout(Duration::from_secs(5))
.expect("mock uploader P connection");
- std::thread::sleep(Duration::from_millis(1500));
let (_download, status_rx) = client
.download(
@@ -1050,7 +1052,6 @@ fn a_cancelled_queued_download_declines_the_peers_offer() {
ready_rx
.recv_timeout(Duration::from_secs(5))
.expect("mock uploader P connection");
- std::thread::sleep(Duration::from_millis(1500));
let download_dir = unique_download_dir();
let (_download, status_rx) = client
@@ -1124,7 +1125,6 @@ fn refusal_fails_the_download_quickly(
ready_rx
.recv_timeout(Duration::from_secs(5))
.expect("mock uploader P connection");
- std::thread::sleep(Duration::from_millis(1500));
let download_dir = unique_download_dir();
let (_download, status_rx) = client
@@ -1224,7 +1224,6 @@ fn an_interrupted_download_resumes_from_its_partial_file() {
ready_rx
.recv_timeout(Duration::from_secs(5))
.expect("mock uploader P connection");
- std::thread::sleep(Duration::from_millis(1500));
let (_download, status_rx) = client
.download(
@@ -3359,7 +3358,6 @@ fn download_progress_reports_the_rate_actually_received() {
ready_rx
.recv_timeout(Duration::from_secs(5))
.expect("mock uploader P connection");
- std::thread::sleep(Duration::from_millis(1500));
let (_download, status_rx) = client
.download(
From 2a343dd4b9f82e58151ea463410862491e621ddb Mon Sep 17 00:00:00 2001
From: Michel de Graaf
Date: Thu, 10 Sep 2026 14:46:11 +0200
Subject: [PATCH 09/23] 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 | 73 ++++++++++++++++++++++-----------
soulseek-rs/tests/common/mod.rs | 18 ++++----
2 files changed, 59 insertions(+), 32 deletions(-)
diff --git a/soulseek-rs-lib/tests/e2e.rs b/soulseek-rs-lib/tests/e2e.rs
index b53c4a3..959ea9c 100644
--- a/soulseek-rs-lib/tests/e2e.rs
+++ b/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 {
let bin = soulfind_binary()?;
- let port = free_port()?;
- let db = std::env::temp_dir().join(format!("soulfind-e2e-{port}.db"));
- let _ = std::fs::remove_file(&db);
+ for _ in 0..3 {
+ let port = free_port()?;
+ let db =
+ std::env::temp_dir().join(format!("soulfind-e2e-{port}.db"));
+ let _ = std::fs::remove_file(&db);
- let mut child = Command::new(&bin)
- .arg("-p")
- .arg(port.to_string())
- .arg("-d")
- .arg(&db)
- .stdout(Stdio::null())
- .stderr(Stdio::null())
- .spawn()
- .ok()?;
+ let mut child = Command::new(&bin)
+ .arg("-p")
+ .arg(port.to_string())
+ .arg("-d")
+ .arg(&db)
+ .stdout(Stdio::null())
+ .stderr(Stdio::null())
+ .spawn()
+ .ok()?;
- if wait_until_listening("127.0.0.1", port, Duration::from_secs(5))
- .is_none()
- {
- // Server never came up (e.g. a toolchain/SQLite issue); skip.
+ // A child that has exited is never taken as up: soulfind exits
+ // when its port is gone, and whoever took it may answer instead.
+ let deadline = Instant::now() + Duration::from_secs(10);
+ 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);
}
-
- Some(Self {
- host: "127.0.0.1".to_string(),
- port,
- child: Some(child),
- db: Some(db),
- _gate: gate,
- })
+ None
}
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(),
diff --git a/soulseek-rs/tests/common/mod.rs b/soulseek-rs/tests/common/mod.rs
index d942a9e..82232af 100644
--- a/soulseek-rs/tests/common/mod.rs
+++ b/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 {
- let port = free_port()?;
- let db = std::env::temp_dir().join(format!("soulfind-{port}.db"));
- let _ = std::fs::remove_file(&db);
- let child = Self::spawn(port, &db)?;
- Some(Self { child, db, port })
+ (0..3).find_map(|_| {
+ let port = free_port()?;
+ let db = std::env::temp_dir().join(format!("soulfind-{port}.db"));
+ let _ = std::fs::remove_file(&db);
+ let child = Self::spawn(port, &db)?;
+ Some(Self { child, db, port })
+ })
}
fn spawn(port: u16, db: &Path) -> Option {
@@ -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
- // lost the port and died would otherwise pass as up.
+ // A child that has exited is never taken as up: soulfind exits
+ // when its port is gone, and whoever took it may answer instead.
if child.try_wait().ok().flatten().is_some() {
return None;
}
From a6d4c8b3dfefcbebf6630e2d6c0749b9eacb6d25 Mon Sep 17 00:00:00 2001
From: Michel de Graaf
Date: Thu, 10 Sep 2026 15:14:25 +0200
Subject: [PATCH 10/23] test(e2e): fence on the server where a test slept for
another client
Tests that need one client's request to have landed before another
client acts slept for it: a join before a room message, ticker or room
search, a watcher's global-room join and leave, a guest accepting
invitations, a room created before an outsider asks for it, share
counts and interests before they are queried, a stored message before
its recipient logs in, and an acknowledgement before the session ends.
Several of those requests get no reply to wait on.
soulfind takes each connection's messages in order, so a fence asks it
about a user over the connection that has to be settled and waits for
the answer, which only arrives once everything sent before it has been
handled. Where the event itself is visible - a member list, alice's own
ticker - the tests wait on that instead.
---
soulseek-rs-lib/tests/e2e.rs | 71 ++++++++++++++++++++++++++----------
1 file changed, 52 insertions(+), 19 deletions(-)
diff --git a/soulseek-rs-lib/tests/e2e.rs b/soulseek-rs-lib/tests/e2e.rs
index 959ea9c..d859fa5 100644
--- a/soulseek-rs-lib/tests/e2e.rs
+++ b/soulseek-rs-lib/tests/e2e.rs
@@ -2955,9 +2955,9 @@ fn a_joined_room_reports_its_members_statistics() {
assert!(sharer.login().expect("sharer login"));
sharer.join_room(room).expect("sharer joins room");
- // Let the server register the sharer's membership and share counts, so
- // the observer's own join reply describes a room that already has them.
- std::thread::sleep(Duration::from_millis(750));
+ // The sharer's membership and share counts have to land first, so the
+ // observer's own join reply describes a room that already has them.
+ fence(&sharer, "e2e_stats_observer");
let mut observer =
Client::with_settings(server.settings("e2e_stats_observer", "pw"));
@@ -3038,9 +3038,9 @@ fn watching_a_user_returns_their_status_and_share_counts() {
watcher.connect().expect("watcher connect");
assert!(watcher.login().expect("watcher login"));
- // Let the subject's share counts reach the server before watching, so
- // the reply describes a user it already knows the statistics for.
- std::thread::sleep(Duration::from_millis(750));
+ // The subject's share counts have to reach the server before watching,
+ // so the reply describes a user it already knows the statistics for.
+ fence(&subject, "e2e_watch_watcher");
watcher
.watch_user("e2e_watch_subject")
@@ -3814,6 +3814,18 @@ fn await_room_event(
None
}
+/// Wait until the server has handled everything `client` has sent, including
+/// requests it never answers: soulfind takes a connection's messages in order,
+/// so the answer to a question about `other` asked now comes after them.
+fn fence(client: &Client, other: &str) {
+ client.request_user_info(other).expect("ask about a user");
+ let deadline = Instant::now() + Duration::from_secs(5);
+ while client.user_info(other).is_none() {
+ assert!(Instant::now() < deadline, "the server should answer");
+ std::thread::sleep(Duration::from_millis(50));
+ }
+}
+
#[test]
fn a_ticker_set_in_a_room_reaches_the_other_members() {
use soulseek_rs::types::RoomEvent;
@@ -3830,7 +3842,12 @@ fn a_ticker_set_in_a_room_reaches_the_other_members() {
alice.join_room(room).expect("alice joins");
bob.join_room(room).expect("bob joins");
- std::thread::sleep(Duration::from_millis(500));
+ // A ticker reaches only who is in the room when it is set.
+ let deadline = Instant::now() + Duration::from_secs(5);
+ while bob.room_members(room).is_empty() {
+ assert!(Instant::now() < deadline, "bob should get into the room");
+ std::thread::sleep(Duration::from_millis(50));
+ }
let _ = bob.take_room_events();
alice
@@ -3874,11 +3891,15 @@ fn a_client_joining_later_receives_the_whole_ticker_board() {
alice.connect().expect("alice connect");
assert!(alice.login().expect("alice login"));
alice.join_room(room).expect("alice joins");
- std::thread::sleep(Duration::from_millis(300));
alice
.set_room_ticker(room, "standing message")
.expect("set ticker");
- std::thread::sleep(Duration::from_millis(500));
+ // The room, alice included, is told once the board holds it.
+ await_room_event(&alice, Duration::from_secs(5), |event| {
+ matches!(event, RoomEvent::TickerAdded { room: r, .. } if r == room)
+ .then_some(())
+ })
+ .expect("alice should see her ticker go up");
// Carol joins afterwards and must be handed the board that already exists.
let mut carol =
@@ -3919,7 +3940,7 @@ fn the_global_room_feed_carries_a_room_we_never_joined() {
watcher.join_global_room().expect("join global room");
talker.join_room(room).expect("talker joins a room");
- std::thread::sleep(Duration::from_millis(500));
+ fence(&watcher, "e2e_global_talk");
let _ = watcher.take_room_events();
let body = "spoken where nobody is watching";
@@ -3943,7 +3964,7 @@ fn the_global_room_feed_carries_a_room_we_never_joined() {
// And leaving the feed stops it: the next message must not arrive.
watcher.leave_global_room().expect("leave global room");
- std::thread::sleep(Duration::from_millis(500));
+ fence(&watcher, "e2e_global_talk");
let _ = watcher.take_room_events();
talker
.say_in_room(room, "after leaving")
@@ -4009,7 +4030,16 @@ fn a_room_search_is_answered_by_the_rooms_members() {
);
sharer.join_room(room).expect("sharer joins");
searcher.join_room(room).expect("searcher joins");
- std::thread::sleep(Duration::from_millis(750));
+ // A room search goes to whoever is in the room when it is sent.
+ let deadline = Instant::now() + Duration::from_secs(5);
+ while !searcher
+ .room_members(room)
+ .iter()
+ .any(|member| member == "e2e_rs_sharer")
+ {
+ assert!(Instant::now() < deadline, "both should get into the room");
+ std::thread::sleep(Duration::from_millis(50));
+ }
let query = "relic";
searcher.search_room(room, query).expect("room search");
@@ -4046,7 +4076,7 @@ fn shared_interests_make_two_users_similar() {
alice
.add_dislike("e2e_interest_muzak")
.expect("alice dislikes something");
- std::thread::sleep(Duration::from_millis(500));
+ fence(&bob, "e2e_like_alice");
// Who else likes this item (code 112).
let deadline = Instant::now() + Duration::from_secs(5);
@@ -4140,7 +4170,7 @@ fn recommendations_are_returned_for_our_interests() {
alice.add_interest(shared).expect("alice likes shared");
bob.add_interest(shared).expect("bob likes shared");
bob.add_interest(other).expect("bob likes other");
- std::thread::sleep(Duration::from_millis(500));
+ fence(&bob, "e2e_rec_alice");
let deadline = Instant::now() + Duration::from_secs(8);
let mut recommended = Vec::new();
@@ -4635,7 +4665,8 @@ fn a_private_room_is_owned_granted_and_revoked() {
owner
.join_private_room(room)
.expect("owner creates the room");
- std::thread::sleep(Duration::from_millis(750));
+ // An invite reaches only a user already accepting invitations.
+ fence(&guest, "e2e_priv_owner");
// The guest is invited, and hears about it (code 139).
owner
@@ -4754,7 +4785,7 @@ fn a_private_room_someone_else_owns_cannot_be_taken() {
owner
.join_private_room(room)
.expect("owner creates the room");
- std::thread::sleep(Duration::from_millis(750));
+ fence(&owner, "e2e_taken_outsider");
// An outsider asking for the same name must be refused (code 1003), not
// handed the room.
@@ -4807,7 +4838,7 @@ fn an_acknowledged_offline_message_is_not_delivered_twice() {
sender
.send_private_message("e2e_ack_recipient", body)
.expect("send to an offline user");
- std::thread::sleep(Duration::from_millis(500));
+ fence(&sender, "e2e_ack_recipient");
let received_once = |label: &str| -> bool {
let mut client =
@@ -4825,7 +4856,9 @@ fn an_acknowledged_offline_message_is_not_delivered_twice() {
std::thread::sleep(Duration::from_millis(100));
}
}
- std::thread::sleep(Duration::from_millis(500));
+ // The acknowledgement goes out ahead of the message it answers, so
+ // once the server has handled what followed, it has the ack too.
+ fence(&client, "e2e_ack_sender");
got
};
@@ -5379,7 +5412,7 @@ fn a_member_can_give_up_a_private_room_and_an_owner_can_disband_it() {
owner
.join_private_room(room)
.expect("owner creates the room");
- std::thread::sleep(Duration::from_millis(750));
+ fence(&guest, "e2e_leave_owner");
owner
.add_room_member(room, "e2e_leave_guest")
.expect("owner invites the guest");
From 18b53aa4ba8a8ed0e7f237f8134460ae7e25171f Mon Sep 17 00:00:00 2001
From: Michel de Graaf
Date: Sun, 13 Sep 2026 10:13:38 +0200
Subject: [PATCH 11/23] fix(tui): keep the login screen responsive while
connecting
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The login screen ignored every key while a connect+login attempt ran, so
against an unreachable server it sat on a static "Connecting…" for 20s or
more, with a hint promising that Esc would quit.
While connecting it now shows a spinner: Enter or r restarts the attempt,
Esc stops it and returns to the form, q or Ctrl-C quits. Ctrl-C quits from
every state of the screen instead of typing a "c" into a field.
A failure that got no verdict (unreachable, timed out) keeps the password,
so Enter or r retries it; only a rejection, Ok(false) or
AuthenticationFailed, clears it. Because a retry can now reuse a password
that came from the CLI, env or keychain, entered_via_form compares the
credentials with the ones the flow started with instead of trusting any
submit, so a retried stored password is never written to the keychain.
A restart of an attempt younger than a second is ignored: each attempt
rescans the shares and opens a listener, and a held Enter would otherwise
start one per key repeat. The attempt result is read before drawing, so a
key is never handled against a phase the screen did not show.
A stopped or restarted attempt is abandoned, not cancelled: the server
actor blocks its own mailbox in connect_timeout, so a stop could not reach
it sooner. Its thread runs out its timeouts and drops the client, which
disconnects. Until then it holds the listener port, so a restarted session
can fall back to an ephemeral port, and should a slow server accept the
abandoned login after its replacement got in, the late login displaces the
new session. Fixing either needs a cancellable connect, or a bind after the
login verdict, in the library.
---
soulseek-rs/src/ui/login.rs | 308 ++++++++++++++++++++++++++----------
1 file changed, 228 insertions(+), 80 deletions(-)
diff --git a/soulseek-rs/src/ui/login.rs b/soulseek-rs/src/ui/login.rs
index 4bf2334..911dece 100644
--- a/soulseek-rs/src/ui/login.rs
+++ b/soulseek-rs/src/ui/login.rs
@@ -6,21 +6,25 @@
//! ([`run_login_flow`]) drives it against a real terminal and client.
use crate::ui::{
- GLYPH_CURSOR, accent_style, dimmed_style, error_style, info_style, mask,
- pane_block, primary_style, title_style, warning_style,
+ GLYPH_CURSOR, accent_style, dimmed_style, error_style, get_spinner_char,
+ info_style, mask, pane_block, primary_style, title_style, warning_style,
};
use color_eyre::Result;
use ratatui::{
DefaultTerminal, Frame,
- crossterm::event::{self, Event, KeyCode, KeyEvent, KeyEventKind, poll},
+ crossterm::event::{
+ self, Event, KeyCode, KeyEvent, KeyEventKind, KeyModifiers, poll,
+ },
layout::{Constraint, Flex, Layout, Rect},
style::Modifier,
text::{Line, Span},
widgets::{Clear, Paragraph},
};
-use soulseek_rs::{Client, ClientSettings};
+use soulseek_rs::{Client, ClientSettings, SoulseekRs};
use std::sync::mpsc::{Receiver, channel};
-use std::time::Duration;
+use std::time::{Duration, Instant};
+
+const RESTART_FLOOR: Duration = Duration::from_secs(1);
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum LoginField {
@@ -36,15 +40,17 @@ pub enum LoginPhase {
Connecting,
/// The last attempt failed; the message is shown until the next key.
Failed(String),
+ Rejected(String),
}
/// What the caller should do after feeding a key to the form.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum LoginAction {
None,
- /// Both fields are filled and the user pressed Enter.
+ /// Both fields are filled and the user pressed Enter or r.
Submit,
- /// User pressed Esc — abort the whole program.
+ Stop,
+ /// User pressed Esc, q or Ctrl-C — abort the whole program.
Cancel,
}
@@ -72,9 +78,33 @@ impl LoginForm {
}
pub fn handle_key(&mut self, key: KeyEvent) -> LoginAction {
+ if key.code == KeyCode::Char('c')
+ && key.modifiers.contains(KeyModifiers::CONTROL)
+ {
+ return LoginAction::Cancel;
+ }
+
match self.phase {
- LoginPhase::Connecting => return LoginAction::None,
+ LoginPhase::Connecting => {
+ return match key.code {
+ KeyCode::Enter | KeyCode::Char('r') => LoginAction::Submit,
+ KeyCode::Esc => {
+ self.phase = LoginPhase::Editing;
+ LoginAction::Stop
+ }
+ KeyCode::Char('q') => LoginAction::Cancel,
+ _ => LoginAction::None,
+ };
+ }
LoginPhase::Failed(_) => {
+ self.phase = LoginPhase::Editing;
+ return match key.code {
+ KeyCode::Enter | KeyCode::Char('r') => self.submit(),
+ KeyCode::Esc => LoginAction::Cancel,
+ _ => LoginAction::None,
+ };
+ }
+ LoginPhase::Rejected(_) => {
// Any key acknowledges the error; the password was wrong (or
// the name is taken), so make the user retype it.
self.phase = LoginPhase::Editing;
@@ -97,20 +127,7 @@ impl LoginForm {
};
LoginAction::None
}
- KeyCode::Enter => {
- if !self.username.is_empty() && !self.password.is_empty() {
- self.phase = LoginPhase::Connecting;
- LoginAction::Submit
- } else {
- // Move to the first empty field instead of submitting.
- self.focused = if self.username.is_empty() {
- LoginField::Username
- } else {
- LoginField::Password
- };
- LoginAction::None
- }
- }
+ KeyCode::Enter => self.submit(),
KeyCode::Backspace => {
self.focused_field_mut().pop();
LoginAction::None
@@ -123,6 +140,20 @@ impl LoginForm {
}
}
+ fn submit(&mut self) -> LoginAction {
+ if !self.username.is_empty() && !self.password.is_empty() {
+ self.phase = LoginPhase::Connecting;
+ return LoginAction::Submit;
+ }
+ // Move to the first empty field instead of submitting.
+ self.focused = if self.username.is_empty() {
+ LoginField::Username
+ } else {
+ LoginField::Password
+ };
+ LoginAction::None
+ }
+
const fn focused_field_mut(&mut self) -> &mut String {
match self.focused {
LoginField::Username => &mut self.username,
@@ -142,7 +173,7 @@ pub struct LoginOutcome {
}
/// Drive the login screen until a login succeeds (`Some`) or the user
-/// cancels with Esc (`None`). When both credentials are already known an
+/// quits (`None`). When both credentials are already known an
/// attempt starts immediately and the form is only shown on failure.
pub fn run_login_flow(
terminal: &mut DefaultTerminal,
@@ -150,27 +181,31 @@ pub fn run_login_flow(
initial_username: Option,
initial_password: Option,
) -> Result
- Or pick a route per platform. Cargo needs a Rust toolchain from{' '}
- rustup.rs; the other routes don’t.
+ Or pick a route per platform. Cargo needs Rust 1.91 or newer from{' '}
+ rustup.rs. Nix and Homebrew{' '}
+ --HEAD also build from source, but bring their own Rust.
-
- {INSTALLS.map((route) => (
-
-
-
-
- {route.os}
-
-
-
- {route.best}
-
-
-
- {route.alt}
-
-
- ))}
-
+
+
Every route puts the same binary on your PATH. Run soulseek-rs in
a terminal and the TUI opens, in a script or a pipe it wants a subcommand
@@ -522,11 +442,13 @@ const Config = () => (
)
+// aislop-ignore-file code-quality/duplicate-block -- the two uninstall `lines` arrays repeat the `{ t, text }` shape around different commands. The rule matches punctuation, not logic.
const Uninstall = () => (
Three things exist on disk: the binary, the config, and the state, plus the completion
- scripts and the agent skill if you asked for those. Nothing else, unless you point{' '}
+ scripts and the agent skill if you asked for those. A .deb or .rpm also brings a man page
+ and its own completions, and takes them back when removed. Nothing else, unless you point{' '}
--log-file somewhere. No telemetry to opt out of.
@@ -539,6 +461,8 @@ const Uninstall = () => (
{ t: 'cm', text: '# whichever way you installed it' },
{ t: 'cmd', text: 'brew uninstall soulseek-rs' },
{ t: 'cmd', text: 'cargo uninstall soulseek-rs' },
+ { t: 'cmd', text: 'sudo apt remove soulseek-rs' },
+ { t: 'cmd', text: 'sudo dnf remove soulseek-rs' },
{ t: 'cm', text: '# script install without brew: the binary itself' },
{ t: 'cmd', text: 'rm -f /usr/local/bin/soulseek-rs ~/.local/bin/soulseek-rs' },
{ t: 'cm', text: '# brew only: drop the tap it came from' },
@@ -581,7 +505,7 @@ const Uninstall = () => (
export const Install = () => (
<>
- One command with the install script, Homebrew or cargo. Build from source if
+ One command with the install script, your package manager, Homebrew or cargo. Build from source if
you’d rather. Or depend on the library and write your own client.