dumbgoy/soulseek-rs-modded
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

refactor(tests): share one soulfind fixture across the e2e suites

Browse Source 
The TUI suite had grown its own copy of the soulfind spawner the protocol
suite already carried, and its own copy of the CLI suite's in-process
client and settle helpers. All four suites now spawn, log in, settle and
skip through `tests/common`: the CLI suite's own server goes too, and
the fixture checks its child is still alive before calling the port
answered, so a lost port cannot pass another server off as ours.
This commit is contained in:
Michel de Graaf
2026-09-08 13:12:12 +02:00
parent 76f4854169
commit 3ef78c6de1
7 changed files with 189 additions and 332 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.github/workflows/ci.yml
+1 -2
View File
@@ -111,8 +111,7 @@ jobs:
run: cargo test -p soulseek-rs --test cli_e2e -- --nocapture
# The window over a real session: keys in, a screen out, a peer on the
# other end. The unit tests drive it against a stand-in session and the
# CLI suite never opens it, so this is the only place the two meet.
# other end. Nothing else opens the TUI against a server.
- name: Run TUI e2e tests (required against soulfind)
env:
SOULSEEK_E2E_REQUIRED: "1"
README.md
+12 -12
View File
@@ -621,19 +621,19 @@ cargo fmt
### End-to-end tests
Three suites run against [soulfind](https://github.com/soulfind-dev/soulfind),
Four suites run against [soulfind](https://github.com/soulfind-dev/soulfind),
a local Soulseek server: `soulseek-rs-lib/tests/e2e.rs` covers the protocol
library, `soulseek-rs/tests/cli_e2e.rs` drives the binary the way a script
would, and `soulseek-rs/tests/tui_e2e.rs` drives the window with key presses
and reads the screen back. All are **server-optional**, running when a server
is available and skipping otherwise, so `cargo test` stays green everywhere.
would, `soulseek-rs/tests/daemon_protocol_e2e.rs` meets the daemon's control
protocol as a third party would, and `soulseek-rs/tests/tui_e2e.rs` drives the
window with key presses and reads the screen back. All are
**server-optional**, running when a server is available and skipping
otherwise, so `cargo test` stays green everywhere.
They locate a server in this order:
1. `SOULSEEK_TEST_SERVER=host:port`: connect to an already-running server, or
2. `SOULFIND_BIN=/path/to/soulfind` (or a `soulfind/bin/soulfind` checkout in a
parent directory), which spawns soulfind on an ephemeral port with a
throwaway database.
Every suite spawns soulfind from `SOULFIND_BIN=/path/to/soulfind` (or a
`soulfind/bin/soulfind` checkout in a parent directory) on an ephemeral port
with a throwaway database. The library and CLI suites take
`SOULSEEK_TEST_SERVER=host:port` first, to run against a server already up.
```bash
# Build soulfind once (see its BUILDING.md), then:
@@ -659,8 +659,8 @@ soulfind instead of skipping.
`cargo clippy --workspace --all-targets -- -D warnings`.
- **Test**: `cargo test --verbose` on Linux, macOS, and Windows.
- **End-to-end**: builds soulfind from source (LDC + `dub build :server`),
points `SOULFIND_BIN` at it, and runs both the library and CLI e2e suites
with `SOULSEEK_E2E_REQUIRED=1`, so a missing server fails instead of skipping.
points `SOULFIND_BIN` at it, and runs all four e2e suites with
`SOULSEEK_E2E_REQUIRED=1`, so a missing server fails instead of skipping.
## Contributing
soulseek-rs/src/ui/main_tui/search.rs
+8 -19
View File
@@ -213,14 +213,9 @@ impl MainTui {
self.show_search(self.state.searches.len() - 1);
}
/// Run the search at `index` again, in its place.
///
/// This is what a restored list is for: last session's queries come back
/// with nothing under them, and typing one out again to see what the
/// network has now is the work the list was meant to save. The entry
/// keeps its row rather than moving to the end — the user is looking at
/// it — and a run still collecting is stopped first, or two threads would
/// be pouring into one result set.
/// Run the search at `index` again, in its row: a restored query comes
/// back with nothing under it, and this saves typing it out. A run still
/// collecting is stopped first, or two threads would fill one set.
pub(super) fn rerun_search_at_index(&mut self, index: usize) {
let Some(previous) = self.state.searches.get(index) else {
return;
@@ -255,18 +250,12 @@ impl MainTui {
let client = self.client.clone();
let timeout = self.search_timeout;
// Results are polled in update_search_results, not returned here.
thread::spawn(move || {
match client.search_with_cancel(
&query,
timeout,
Some(cancel_flag.clone()),
) {
Ok(_results) => {
// Results will be polled in update_search_results
}
Err(e) => {
soulseek_rs::warn!("Search failed: {e}");
}
if let Err(e) =
client.search_with_cancel(&query, timeout, Some(cancel_flag))
{
soulseek_rs::warn!("Search failed: {e}");
}
});
soulseek-rs/tests/cli_e2e.rs
+11 -73
View File
@@ -11,8 +11,8 @@
mod common;
use common::{CLI_ENV_VARS, free_port, soulfind_binary};
use soulseek_rs::{Client, ClientSettings, PeerAddress};
use common::{CLI_ENV_VARS, Soulfind, free_port, login, settle};
use soulseek_rs::Client;
use std::io::Write;
use std::net::{TcpStream, ToSocketAddrs};
use std::path::{Path, PathBuf};
@@ -508,8 +508,9 @@ static SERVER_GATE: std::sync::Mutex<()> = std::sync::Mutex::new(());
struct TestServer {
host: String,
port: u16,
child: Option<Child>,
db: Option<PathBuf>,
/// The soulfind spawned for this run; `None` against a server named by
/// `SOULSEEK_TEST_SERVER`.
_server: Option<Soulfind>,
_gate: std::sync::MutexGuard<'static, ()>,
/// Daemons started for this server, one per (user, session flags). Dropped
/// with the server, which kills them.
@@ -640,41 +641,17 @@ impl TestServer {
return Some(Self {
host: host.to_string(),
port,
child: None,
db: None,
_server: None,
_gate: gate,
daemons: std::sync::Mutex::new(std::collections::HashMap::new()),
});
}
let bin = soulfind_binary()?;
let port = free_port()?;
let db = std::env::temp_dir().join(format!("soulfind-cli-{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()?;
if wait_until_listening("127.0.0.1", port, Duration::from_secs(5))
.is_none()
{
let _ = child.kill();
let _ = child.wait();
return None;
}
let server = Soulfind::start()?;
Some(Self {
host: "127.0.0.1".to_string(),
port,
child: Some(child),
db: Some(db),
port: server.port(),
_server: Some(server),
_gate: gate,
daemons: std::sync::Mutex::new(std::collections::HashMap::new()),
})
@@ -777,31 +754,7 @@ impl TestServer {
/// An in-process client, used to stand up the other end of a test.
fn client(&self, user: &str, shares: Vec<String>) -> Client {
let port = free_port().expect("peer port");
let mut client = Client::with_settings(ClientSettings {
username: user.to_string(),
password: "pw".to_string(),
server_address: PeerAddress::new(self.host.clone(), self.port),
enable_listen: true,
listen_port: port,
shared_directories: shares,
version: soulseek_rs::ClientVersion::default(),
});
client.connect().expect("peer connect");
assert!(client.login().expect("peer login"), "peer should log in");
client
}
}
impl Drop for TestServer {
fn drop(&mut self) {
if let Some(child) = self.child.as_mut() {
let _ = child.kill();
let _ = child.wait();
}
if let Some(db) = self.db.as_ref() {
let _ = std::fs::remove_file(db);
}
login(&self.host, self.port, user, shares)
}
}
@@ -829,17 +782,7 @@ macro_rules! server_or_skip {
match TestServer::resolve() {
Some(server) => server,
None => {
let required = std::env::var("SOULSEEK_E2E_REQUIRED")
.is_ok_and(|v| v != "0" && !v.is_empty());
assert!(
!required,
"SOULSEEK_E2E_REQUIRED is set but no soulfind server could \
be started (set SOULFIND_BIN or SOULSEEK_TEST_SERVER)"
);
println!(
"cli e2e skipped: no soulfind server (set SOULFIND_BIN or \
SOULSEEK_TEST_SERVER to run)"
);
common::no_server("cli e2e");
return;
}
}
@@ -936,11 +879,6 @@ fn split_session_flags(args: &[&str]) -> (Vec<String>, Vec<String>) {
(session, command)
}
/// Wait out the SetWaitPort registrations so peer lookups resolve.
fn settle() {
std::thread::sleep(Duration::from_secs(1));
}
/// Like [`cli`], but keeping a config file so a command that reads or writes
/// one (the wishlist) sees it.
///
soulseek-rs/tests/common/mod.rs
+117
View File
@@ -3,7 +3,11 @@
#![allow(dead_code)]
use soulseek_rs::{Client, ClientSettings, PeerAddress};
use std::net::TcpStream;
use std::path::{Path, PathBuf};
use std::process::{Child, Command, Stdio};
use std::time::{Duration, Instant};
/// Every environment variable the binary reads, cleared for each run so a
/// developer's shell (or a stray `.env`) cannot change what a test observes —
@@ -44,3 +48,116 @@ pub fn free_port() -> Option<u16> {
.ok()
.map(|addr| addr.port())
}
/// A soulfind spawned for one test, on a port nobody else holds, with a
/// throwaway database that goes with it.
pub struct Soulfind {
child: Child,
db: PathBuf,
port: u16,
}
impl Soulfind {
pub fn start() -> Option<Self> {
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<Child> {
let mut child = Command::new(soulfind_binary()?)
.args(["-d", db.to_str()?, "-p", &port.to_string()])
.stdout(Stdio::null())
.stderr(Stdio::null())
.spawn()
.ok()?;
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.
if child.try_wait().ok().flatten().is_some() {
return None;
}
if TcpStream::connect(("127.0.0.1", port)).is_ok() {
return Some(child);
}
std::thread::sleep(Duration::from_millis(100));
}
let _ = child.kill();
let _ = child.wait();
None
}
pub const fn port(&self) -> u16 {
self.port
}
pub fn address(&self) -> String {
format!("127.0.0.1:{}", self.port)
}
pub fn stop(&mut self) {
let _ = self.child.kill();
let _ = self.child.wait();
}
/// Bring it back on the same port and database.
pub fn restart(&mut self) -> Option<()> {
self.stop();
self.child = Self::spawn(self.port, &self.db)?;
Some(())
}
/// A client logged in to this server as `user`, sharing `shares`.
pub fn client(&self, user: &str, shares: Vec<String>) -> Client {
login("127.0.0.1", self.port, user, shares)
}
}
impl Drop for Soulfind {
fn drop(&mut self) {
self.stop();
let _ = std::fs::remove_file(&self.db);
}
}
/// An in-process client logged in to the server at `host:port` as `user`,
/// sharing `shares`: the other end of a test.
///
/// The listener stays on: a peer delivers search results and transfers over
/// a connection it opens back to us, so a client without one hears nothing.
pub fn login(host: &str, port: u16, user: &str, shares: Vec<String>) -> Client {
let mut client = Client::with_settings(ClientSettings {
username: user.to_string(),
password: "pw".to_string(),
server_address: PeerAddress::new(host.to_string(), port),
enable_listen: true,
listen_port: free_port().expect("peer port"),
shared_directories: shares,
version: soulseek_rs::ClientVersion::default(),
});
client.connect().expect("peer connect");
assert!(client.login().expect("peer login"), "peer should log in");
client
}
/// Wait out the SetWaitPort registrations so peer lookups resolve.
pub fn settle() {
std::thread::sleep(Duration::from_secs(1));
}
/// What a suite does without a server: fail when the run said one is
/// required, otherwise say it skipped. The caller returns afterwards.
pub fn no_server(suite: &str) {
let required = std::env::var("SOULSEEK_E2E_REQUIRED")
.is_ok_and(|v| v != "0" && !v.is_empty());
assert!(
!required,
"SOULSEEK_E2E_REQUIRED is set but no soulfind server could be \
started for the {suite} suite (set SOULFIND_BIN)"
);
println!("{suite} skipped: no soulfind server (set SOULFIND_BIN to run)");
}
soulseek-rs/tests/daemon_protocol_e2e.rs
+2 -72
View File
@@ -13,7 +13,7 @@
mod common;
use common::{CLI_ENV_VARS, free_port, soulfind_binary};
use common::{CLI_ENV_VARS, Soulfind, free_port};
use std::io::{BufRead, BufReader, Write};
use std::net::TcpStream;
use std::path::{Path, PathBuf};
@@ -215,17 +215,7 @@ macro_rules! daemon_or_skip {
match Daemon::start() {
Some(daemon) => daemon,
None => {
let required = std::env::var("SOULSEEK_E2E_REQUIRED")
.is_ok_and(|v| v != "0" && !v.is_empty());
assert!(
!required,
"SOULSEEK_E2E_REQUIRED is set but no daemon could be \
started (set SOULFIND_BIN)"
);
println!(
"daemon protocol e2e skipped: no soulfind server (set \
SOULFIND_BIN to run)"
);
common::no_server("daemon protocol e2e");
return;
}
}
@@ -431,66 +421,6 @@ fn discover_serves_a_contract_whose_parameters_match_what_is_accepted() {
// Fixtures
// ---------------------------------------------------------------------------
struct Soulfind {
child: Child,
db: PathBuf,
port: u16,
}
impl Soulfind {
fn start() -> Option<Self> {
let port = free_port()?;
let db = std::env::temp_dir().join(format!("soulfind-proto-{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<Child> {
let bin = soulfind_binary()?;
let mut child = Command::new(bin)
.args(["-d", db.to_str()?, "-p", &port.to_string()])
.stdout(Stdio::null())
.stderr(Stdio::null())
.spawn()
.ok()?;
let deadline = Instant::now() + Duration::from_secs(10);
while Instant::now() < deadline {
if TcpStream::connect(("127.0.0.1", port)).is_ok() {
return Some(child);
}
std::thread::sleep(Duration::from_millis(100));
}
let _ = child.kill();
let _ = child.wait();
None
}
fn address(&self) -> String {
format!("127.0.0.1:{}", self.port)
}
fn stop(&mut self) {
let _ = self.child.kill();
let _ = self.child.wait();
}
fn restart(&mut self) -> Option<()> {
self.stop();
self.child = Self::spawn(self.port, &self.db)?;
Some(())
}
}
impl Drop for Soulfind {
fn drop(&mut self) {
self.stop();
let _ = std::fs::remove_file(&self.db);
}
}
fn wait_until(timeout: Duration, mut f: impl FnMut() -> bool) -> bool {
let deadline = Instant::now() + timeout;
while Instant::now() < deadline {
soulseek-rs/tests/tui_e2e.rs
+38 -154
View File
@@ -14,17 +14,15 @@
mod common;
use common::{free_port, soulfind_binary};
use common::{Soulfind, settle};
use ratatui::Terminal;
use ratatui::backend::TestBackend;
use ratatui::crossterm::event::{KeyCode, KeyEvent, KeyModifiers};
use soulseek_rs::{Client, ClientSettings, PeerAddress};
use soulseek_rs::Client;
use soulseek_rs_tui::MainTui;
use soulseek_rs_tui::models::{FocusedPane, SearchStatus};
use soulseek_rs_tui::models::SearchStatus;
use soulseek_rs_tui::persist::state::StateStore;
use std::net::TcpStream;
use std::path::Path;
use std::process::{Child, Command, Stdio};
use std::sync::Arc;
use std::time::{Duration, Instant};
@@ -32,98 +30,18 @@ use std::time::{Duration, Instant};
/// it out; long enough for a peer on loopback to answer many times over.
const SEARCH_WINDOW: Duration = Duration::from_secs(3);
/// A soulfind spawned for one test, on a port nobody else holds.
struct Soulfind {
child: Child,
db: std::path::PathBuf,
port: u16,
}
impl Soulfind {
fn start() -> Option<Self> {
let bin = soulfind_binary()?;
let port = free_port()?;
let db = std::env::temp_dir().join(format!("soulfind-tui-{port}.db"));
let _ = std::fs::remove_file(&db);
let mut child = Command::new(bin)
.args(["-d", db.to_str()?, "-p", &port.to_string()])
.stdout(Stdio::null())
.stderr(Stdio::null())
.spawn()
.ok()?;
let deadline = Instant::now() + Duration::from_secs(10);
while Instant::now() < deadline {
if TcpStream::connect(("127.0.0.1", port)).is_ok() {
return Some(Self { child, db, port });
}
std::thread::sleep(Duration::from_millis(100));
}
let _ = child.kill();
let _ = child.wait();
None
}
/// A client logged in to this server as `user`, sharing `shares`.
///
/// The listener stays on: a peer delivers search results over a
/// connection it opens back to the searcher, so a searcher without one
/// hears nothing.
fn client(&self, user: &str, shares: Vec<String>) -> Client {
let mut client = Client::with_settings(ClientSettings {
username: user.to_string(),
password: "pw".to_string(),
server_address: PeerAddress::new(
"127.0.0.1".to_string(),
self.port,
),
enable_listen: true,
listen_port: free_port().expect("peer port"),
shared_directories: shares,
version: soulseek_rs::ClientVersion::default(),
});
client.connect().expect("peer connect");
assert!(client.login().expect("peer login"), "peer should log in");
client
}
}
impl Drop for Soulfind {
fn drop(&mut self) {
let _ = self.child.kill();
let _ = self.child.wait();
let _ = std::fs::remove_file(&self.db);
}
}
macro_rules! server_or_skip {
() => {
match Soulfind::start() {
Some(server) => server,
None => {
let required = std::env::var("SOULSEEK_E2E_REQUIRED")
.is_ok_and(|v| v != "0" && !v.is_empty());
assert!(
!required,
"SOULSEEK_E2E_REQUIRED is set but no soulfind server could \
be started (set SOULFIND_BIN)"
);
println!(
"tui e2e skipped: no soulfind server (set SOULFIND_BIN to \
run)"
);
common::no_server("tui e2e");
return;
}
}
};
}
/// Wait out the SetWaitPort registrations so peer lookups resolve.
fn settle() {
std::thread::sleep(Duration::from_secs(1));
}
/// The window under test: the real `MainTui` over a real session, drawn onto
/// a buffer instead of a terminal.
struct Window {
@@ -162,23 +80,22 @@ impl Window {
self.terminal.backend().to_string()
}
/// Keep the window running until `done` says so, or `timeout` runs out.
/// Returns the last screen either way, so a failing test shows what the
/// user would have been looking at.
/// Keep the window running until `done` says so, and hand back the
/// screen it stopped on. Past `timeout` the test fails showing the last
/// screen, what the user would have been looking at, under `what`.
fn run_until(
&mut self,
timeout: Duration,
what: &str,
mut done: impl FnMut(&MainTui, &str) -> bool,
) -> (bool, String) {
) -> String {
let deadline = Instant::now() + timeout;
loop {
let screen = self.frame();
if done(&self.tui, &screen) {
return (true, screen);
}
if Instant::now() > deadline {
return (false, screen);
return screen;
}
assert!(Instant::now() <= deadline, "{what}:\n{screen}");
std::thread::sleep(Duration::from_millis(100));
}
}
@@ -192,10 +109,6 @@ impl Window {
.map(|file| file.filename.clone())
.collect()
}
fn search_status(&self, index: usize) -> SearchStatus {
self.tui.state().searches[index].status.clone()
}
}
/// Whether the file the sharer offers has reached the Results pane, on
@@ -208,6 +121,11 @@ fn shows_the_shared_file(tui: &MainTui, screen: &str) -> bool {
&& screen.contains("tui_probe_rerun")
}
/// Whether the one search has run its window out.
fn finished(tui: &MainTui, _: &str) -> bool {
tui.state().searches[0].status == SearchStatus::Completed
}
#[test]
fn shift_s_runs_a_restored_query_again_and_the_answers_land_on_screen() {
let server = server_or_skip!();
@@ -215,11 +133,8 @@ fn shift_s_runs_a_restored_query_again_and_the_answers_land_on_screen() {
// The other end of the wire: a peer sharing one file with a name only
// this test searches for.
let share = tempfile::tempdir().expect("share dir");
std::fs::write(
share.path().join("tui_probe_rerun_vexq.bin"),
(0..4096u32).map(|i| (i % 251) as u8).collect::<Vec<u8>>(),
)
.expect("share file");
std::fs::write(share.path().join("tui_probe_rerun_vexq.bin"), [7u8; 64])
.expect("share file");
let _sharer = server
.client("tui_e2e_sharer", vec![share.path().display().to_string()]);
settle();
@@ -240,46 +155,21 @@ fn shift_s_runs_a_restored_query_again_and_the_answers_land_on_screen() {
screen.contains("vexq"),
"the old query is listed:\n{screen}"
);
assert_eq!(window.tui.state().searches.len(), 1);
assert_eq!(window.search_status(0), SearchStatus::Completed);
assert!(
window.result_names().is_empty(),
"and it has nothing under it"
);
assert_eq!(
window.tui.state().focused_pane,
FocusedPane::Searches,
"the window opens on the list, so the query is already highlighted"
);
// One key, no typing.
// One key, no typing: the window opens on the list with the query
// highlighted.
window.press(KeyCode::Char('S'));
assert_eq!(window.tui.state().searches.len(), 1, "no second row");
assert_eq!(window.search_status(0), SearchStatus::Active);
assert_eq!(window.tui.state().focused_pane, FocusedPane::Results);
let (found, screen) =
window.run_until(Duration::from_secs(15), shows_the_shared_file);
assert!(
found,
"the sharer's file should reach the screen:\n{screen}"
window.run_until(
Duration::from_secs(15),
"the sharer's file should reach the screen",
shows_the_shared_file,
);
assert!(
window
.result_names()
.iter()
.all(|name| name.contains("tui_probe_rerun")),
"only what the network answered: {:?}",
window.result_names()
);
// The search runs its window out and settles, keeping what it found.
let (done, screen) = window
.run_until(SEARCH_WINDOW + Duration::from_secs(5), |tui, _| {
tui.state().searches[0].status == SearchStatus::Completed
});
assert!(done, "the search should finish:\n{screen}");
let screen = window.run_until(
SEARCH_WINDOW + Duration::from_secs(5),
"the search should finish",
finished,
);
assert!(shows_the_shared_file(&window.tui, &screen));
let first_run = window.result_names();
@@ -287,23 +177,17 @@ fn shift_s_runs_a_restored_query_again_and_the_answers_land_on_screen() {
// the same peer has to answer the same searcher a second time, and the
// window has to start the display over rather than stack the answers.
window.press(KeyCode::Char('1'));
assert_eq!(window.tui.state().focused_pane, FocusedPane::Searches);
window.press(KeyCode::Char('S'));
assert_eq!(window.search_status(0), SearchStatus::Active);
assert!(
window.result_names().is_empty(),
"the display starts over: {:?}",
window.result_names()
window.run_until(
Duration::from_secs(15),
"the second run should be answered too",
shows_the_shared_file,
);
window.run_until(
SEARCH_WINDOW + Duration::from_secs(5),
"the second run should finish",
finished,
);
let (found, screen) =
window.run_until(Duration::from_secs(15), shows_the_shared_file);
assert!(found, "the second run should be answered too:\n{screen}");
let (done, _) = window
.run_until(SEARCH_WINDOW + Duration::from_secs(5), |tui, _| {
tui.state().searches[0].status == SearchStatus::Completed
});
assert!(done);
assert_eq!(
window.result_names(),
first_run,