From 36819d4c427a68bf0fc1797400666a78112700af Mon Sep 17 00:00:00 2001 From: michel Date: Mon, 20 Jul 2026 20:20:54 +0200 Subject: [PATCH] test: persistence e2e and registration-relogin e2e; lint clean Adds a lib target (soulseek_rs_tui) exposing cli/models/persist so integration tests can drive the persistence pipeline: state and config survive a simulated restart through the real files, with the incomplete download flagged for auto-resume. The lib e2e suite gains a register-then-relogin test against soulfind proving the stored-credential flow's server assumptions. cargo clippy --all-targets and cargo fmt are clean. --- soulseek-rs-lib/tests/e2e.rs | 24 +++++++ soulseek-rs/Cargo.toml | 7 ++ soulseek-rs/src/lib.rs | 6 ++ soulseek-rs/src/main.rs | 16 ++--- soulseek-rs/src/models/app_state.rs | 2 + soulseek-rs/src/persist/config.rs | 14 ++-- soulseek-rs/src/persist/paths.rs | 2 + soulseek-rs/src/persist/secret.rs | 7 +- soulseek-rs/src/persist/snapshot.rs | 27 ++++---- soulseek-rs/src/persist/state.rs | 15 ++-- soulseek-rs/src/ui/login.rs | 41 +++++------ soulseek-rs/src/ui/main_tui/mod.rs | 3 +- soulseek-rs/src/ui/mod.rs | 2 +- soulseek-rs/tests/persist_e2e.rs | 103 ++++++++++++++++++++++++++++ 14 files changed, 203 insertions(+), 66 deletions(-) create mode 100644 soulseek-rs/src/lib.rs create mode 100644 soulseek-rs/tests/persist_e2e.rs diff --git a/soulseek-rs-lib/tests/e2e.rs b/soulseek-rs-lib/tests/e2e.rs index 912ebf4..63c89ce 100644 --- a/soulseek-rs-lib/tests/e2e.rs +++ b/soulseek-rs-lib/tests/e2e.rs @@ -417,6 +417,30 @@ fn wrong_password_is_rejected() { ); } +#[test] +fn registered_username_can_relogin_with_same_password() { + let server = server_or_skip!(); + + // The TUI's stored-credentials flow depends on this pair of server + // behaviors: a fresh username is registered by simply logging in, and a + // later session (a "restart") with the same credentials is accepted. + let user = "e2e_relogin_user"; + let mut first = Client::with_settings(server.settings(user, "pw-123")); + first.connect().expect("connect (registering login)"); + assert!( + first.login().expect("registering login"), + "a fresh username should be auto-registered" + ); + drop(first); + + let mut second = Client::with_settings(server.settings(user, "pw-123")); + second.connect().expect("connect (relogin)"); + assert!( + second.login().expect("relogin"), + "the same credentials must log in again after a restart" + ); +} + #[test] fn two_clients_can_be_logged_in_together() { let server = server_or_skip!(); diff --git a/soulseek-rs/Cargo.toml b/soulseek-rs/Cargo.toml index d480321..d8ae575 100644 --- a/soulseek-rs/Cargo.toml +++ b/soulseek-rs/Cargo.toml @@ -13,6 +13,13 @@ categories = ["command-line-utilities"] name = "soulseek-rs" path = "src/main.rs" +# Small library target so integration tests can exercise the persistence +# pipeline; the dependency soulseek-rs-lib already owns the `soulseek_rs` +# crate name, hence the distinct one. +[lib] +name = "soulseek_rs_tui" +path = "src/lib.rs" + [lints] workspace = true diff --git a/soulseek-rs/src/lib.rs b/soulseek-rs/src/lib.rs new file mode 100644 index 0000000..0a2714b --- /dev/null +++ b/soulseek-rs/src/lib.rs @@ -0,0 +1,6 @@ +//! Library surface of the TUI binary, exposed for integration tests. +//! The binary itself compiles these modules directly (see `main.rs`). + +pub mod cli; +pub mod models; +pub mod persist; diff --git a/soulseek-rs/src/main.rs b/soulseek-rs/src/main.rs index e9794e3..6b3aa3f 100644 --- a/soulseek-rs/src/main.rs +++ b/soulseek-rs/src/main.rs @@ -200,19 +200,15 @@ fn run_default_tui( let enable_listen = !resolved.disable_listener; let listen_port = resolved.listener_port; - let make_settings = move |username: String, password: String| { - ClientSettings { + let make_settings = + move |username: String, password: String| ClientSettings { username, password, - server_address: PeerAddress::new( - server_host.clone(), - server_port, - ), + server_address: PeerAddress::new(server_host.clone(), server_port), enable_listen, listen_port, shared_directory: shared_directory.clone(), - } - }; + }; // Clear screen and enable mouse capture before initializing TUI let _ = @@ -243,8 +239,8 @@ fn run_default_tui( persist_credentials(&outcome, config_path, file_config, &secret_store); - let store = persist::paths::state_dir() - .map(persist::state::StateStore::new); + let store = + persist::paths::state_dir().map(persist::state::StateStore::new); launch_main_tui( terminal, diff --git a/soulseek-rs/src/models/app_state.rs b/soulseek-rs/src/models/app_state.rs index 8eec704..027e12a 100644 --- a/soulseek-rs/src/models/app_state.rs +++ b/soulseek-rs/src/models/app_state.rs @@ -111,6 +111,7 @@ pub struct AppState { } impl AppState { + #[must_use] pub fn new() -> Self { let mut searches_table_state = TableState::default(); searches_table_state.select(Some(0)); @@ -166,6 +167,7 @@ impl AppState { } #[allow(dead_code)] + #[must_use] pub fn get_selected_search(&self) -> Option<&SearchEntry> { self.selected_search_index .and_then(|idx| self.searches.get(idx)) diff --git a/soulseek-rs/src/persist/config.rs b/soulseek-rs/src/persist/config.rs index e93a6c7..42875ce 100644 --- a/soulseek-rs/src/persist/config.rs +++ b/soulseek-rs/src/persist/config.rs @@ -5,7 +5,7 @@ use std::path::Path; /// Optional settings read from `config.toml`. Every field is optional so a /// partial file (or none at all) is valid; unknown keys are ignored so newer /// configs still load in older builds. -#[derive(Debug, Default, Clone, PartialEq, Serialize, Deserialize)] +#[derive(Debug, Default, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(default)] pub struct FileConfig { pub username: Option, @@ -54,7 +54,7 @@ impl FileConfig { /// Fully-resolved settings after layering CLI (which already includes env via /// clap) over the config file over built-in defaults. -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone, PartialEq, Eq)] pub struct Resolved { pub username: Option, pub server: String, @@ -73,10 +73,12 @@ pub const DEFAULT_DOWNLOAD_DIR: &str = "~/Downloads"; pub const DEFAULT_MAX_CONCURRENT_DOWNLOADS: usize = 5; pub const DEFAULT_SEARCH_TIMEOUT: u64 = 10; -/// Layer CLI/env values over the config file over defaults. The -/// `--disable-listener` flag can only enable the disable (a bare flag has no -/// "explicitly off" form), so file `disable_listener = true` wins unless the -/// flag is passed. +/// Layer CLI/env values over the config file over defaults. +/// +/// The `--disable-listener` flag can only enable the disable (a bare flag +/// has no "explicitly off" form), so file `disable_listener = true` wins +/// unless the flag is passed. +#[must_use] pub fn resolve(cli: &crate::cli::Cli, file: &FileConfig) -> Resolved { Resolved { username: cli.username.clone().or_else(|| file.username.clone()), diff --git a/soulseek-rs/src/persist/paths.rs b/soulseek-rs/src/persist/paths.rs index 53b655b..cc2c23c 100644 --- a/soulseek-rs/src/persist/paths.rs +++ b/soulseek-rs/src/persist/paths.rs @@ -4,6 +4,7 @@ use std::path::PathBuf; /// Platform-appropriate locations for the config file and state directory. /// Overridable via `SOULSEEK_CONFIG_DIR` / `SOULSEEK_STATE_DIR` (used by the /// e2e tests, and handy for portable installs). +#[must_use] pub fn config_file() -> Option { if let Ok(dir) = std::env::var("SOULSEEK_CONFIG_DIR") { return Some(PathBuf::from(dir).join("config.toml")); @@ -11,6 +12,7 @@ pub fn config_file() -> Option { project_dirs().map(|d| d.config_dir().join("config.toml")) } +#[must_use] pub fn state_dir() -> Option { if let Ok(dir) = std::env::var("SOULSEEK_STATE_DIR") { return Some(PathBuf::from(dir)); diff --git a/soulseek-rs/src/persist/secret.rs b/soulseek-rs/src/persist/secret.rs index 9b40b39..034fc90 100644 --- a/soulseek-rs/src/persist/secret.rs +++ b/soulseek-rs/src/persist/secret.rs @@ -32,9 +32,10 @@ impl SecretStore for KeyringStore { } } -/// Resolve the password with precedence: CLI/env (`cli_password`, already -/// merged by clap) > OS keychain > `password_cmd` from config.toml. Returns -/// `None` when no source has one — the caller shows the login screen. +/// Resolve the password with precedence: CLI/env > keychain > `password_cmd`. +/// +/// `cli_password` already has env merged in by clap. Returns `None` when no +/// source has one — the caller shows the login screen. /// /// Keychain errors are downgraded to "not found": on headless boxes without /// a secret service the fallbacks must still work. diff --git a/soulseek-rs/src/persist/snapshot.rs b/soulseek-rs/src/persist/snapshot.rs index 99b5a46..2a0e0ac 100644 --- a/soulseek-rs/src/persist/snapshot.rs +++ b/soulseek-rs/src/persist/snapshot.rs @@ -17,6 +17,7 @@ impl Snapshot { /// Extract what should survive a restart. Downloads that are not yet /// `Completed` (including failed/timed-out ones) are marked incomplete /// so the next start can re-enqueue them. + #[must_use] pub fn capture(state: &AppState) -> Self { let downloads = state .downloads @@ -25,10 +26,7 @@ impl Snapshot { username: entry.download.username.clone(), filename: entry.download.filename.clone(), size: entry.download.size, - download_directory: entry - .download - .download_directory - .clone(), + download_directory: entry.download.download_directory.clone(), completed: matches!( entry.download.status, DownloadStatus::Completed @@ -43,8 +41,12 @@ impl Snapshot { } } - let rooms = - state.rooms.open.iter().map(|room| room.name.clone()).collect(); + let rooms = state + .rooms + .open + .iter() + .map(|room| room.name.clone()) + .collect(); Self { downloads, @@ -80,10 +82,7 @@ mod tests { use std::sync::{Arc, mpsc}; use std::time::Instant; - fn download( - filename: &str, - status: DownloadStatus, - ) -> DownloadEntry { + fn download(filename: &str, status: DownloadStatus) -> DownloadEntry { let (sender, _receiver) = mpsc::channel(); DownloadEntry { download: Download { @@ -114,8 +113,12 @@ mod tests { #[test] fn capture_maps_download_status_to_completed_flag() { let mut state = AppState::new(); - state.downloads.push(download("done.mp3", DownloadStatus::Completed)); - state.downloads.push(download("queued.mp3", DownloadStatus::Queued)); + state + .downloads + .push(download("done.mp3", DownloadStatus::Completed)); + state + .downloads + .push(download("queued.mp3", DownloadStatus::Queued)); state.downloads.push(download( "failed.mp3", DownloadStatus::Failed(Some("nope".into())), diff --git a/soulseek-rs/src/persist/state.rs b/soulseek-rs/src/persist/state.rs index 8258f83..f895f0d 100644 --- a/soulseek-rs/src/persist/state.rs +++ b/soulseek-rs/src/persist/state.rs @@ -29,7 +29,8 @@ pub struct StateStore { } impl StateStore { - pub fn new(dir: PathBuf) -> Self { + #[must_use] + pub const fn new(dir: PathBuf) -> Self { Self { dir } } @@ -198,9 +199,7 @@ mod tests { fn migrations_upgrade_old_data() { // Machinery test with a synthetic chain: v0 stored plain strings, // v1 wraps each in an object {"name": ...}. - #[derive( - Debug, PartialEq, Default, Serialize, serde::Deserialize, - )] + #[derive(Debug, PartialEq, Default, Serialize, serde::Deserialize)] struct Named { name: String, } @@ -216,15 +215,11 @@ mod tests { }]; let tmp = tempfile::tempdir().unwrap(); let path = tmp.path().join("things.json"); - std::fs::write(&path, r#"{"version": 0, "data": ["a", "b"]}"#) - .unwrap(); + std::fs::write(&path, r#"{"version": 0, "data": ["a", "b"]}"#).unwrap(); let loaded: Vec = load(&path, chain); assert_eq!( loaded, - vec![ - Named { name: "a".into() }, - Named { name: "b".into() } - ] + vec![Named { name: "a".into() }, Named { name: "b".into() }] ); } diff --git a/soulseek-rs/src/ui/login.rs b/soulseek-rs/src/ui/login.rs index c1f295a..2d9b7c7 100644 --- a/soulseek-rs/src/ui/login.rs +++ b/soulseek-rs/src/ui/login.rs @@ -119,7 +119,7 @@ impl LoginForm { } } - fn focused_field_mut(&mut self) -> &mut String { + const fn focused_field_mut(&mut self) -> &mut String { match self.focused { LoginField::Username => &mut self.username, LoginField::Password => &mut self.password, @@ -148,18 +148,18 @@ pub fn run_login_flow( ) -> Result> { let mut form = LoginForm::new(initial_username); let mut entered_via_form = false; - let mut attempt: Option>> = None; - - if let Some(password) = initial_password - && !form.username.is_empty() - { - form.password = password; - form.phase = LoginPhase::Connecting; - attempt = Some(spawn_attempt(make_settings( - form.username.clone(), - form.password.clone(), - ))); - } + let mut attempt: Option>> = + match initial_password { + Some(password) if !form.username.is_empty() => { + form.password = password; + form.phase = LoginPhase::Connecting; + Some(spawn_attempt(make_settings( + form.username.clone(), + form.password.clone(), + ))) + } + _ => None, + }; loop { terminal.draw(|frame| render(frame, &form))?; @@ -207,9 +207,7 @@ pub fn run_login_flow( } /// Connect and log in on a background thread so the UI stays responsive. -fn spawn_attempt( - settings: ClientSettings, -) -> Receiver> { +fn spawn_attempt(settings: ClientSettings) -> Receiver> { let (tx, rx) = channel(); std::thread::spawn(move || { let mut client = Client::with_settings(settings); @@ -288,13 +286,10 @@ fn render(frame: &mut Frame, form: &LoginForm) { "Connecting…", Style::default().fg(Color::Yellow), ))), - LoginPhase::Failed(message) => { - Paragraph::new(Line::from(Span::styled( - message.clone(), - Style::default().fg(Color::Red), - ))) - .wrap(ratatui::widgets::Wrap { trim: true }) - } + LoginPhase::Failed(message) => Paragraph::new(Line::from( + Span::styled(message.clone(), Style::default().fg(Color::Red)), + )) + .wrap(ratatui::widgets::Wrap { trim: true }), }; frame.render_widget(status, rows[3]); diff --git a/soulseek-rs/src/ui/main_tui/mod.rs b/soulseek-rs/src/ui/main_tui/mod.rs index 38d2b1b..47728bc 100644 --- a/soulseek-rs/src/ui/main_tui/mod.rs +++ b/soulseek-rs/src/ui/main_tui/mod.rs @@ -88,7 +88,8 @@ impl MainTui { status: soulseek_rs::DownloadStatus::Completed, sender: std::sync::mpsc::channel().0, queue_position: None, - metadata: Default::default(), + metadata: soulseek_rs::types::DownloadMetadata::default( + ), }, receiver: None, }); diff --git a/soulseek-rs/src/ui/mod.rs b/soulseek-rs/src/ui/mod.rs index 351ebff..fc07415 100644 --- a/soulseek-rs/src/ui/mod.rs +++ b/soulseek-rs/src/ui/mod.rs @@ -1,6 +1,6 @@ mod download_selector; -pub mod login; mod downloads; +pub mod login; mod main_tui; mod panes; mod styles; diff --git a/soulseek-rs/tests/persist_e2e.rs b/soulseek-rs/tests/persist_e2e.rs new file mode 100644 index 0000000..c678b23 --- /dev/null +++ b/soulseek-rs/tests/persist_e2e.rs @@ -0,0 +1,103 @@ +//! End-to-end test of the persistence pipeline across a simulated restart: +//! capture live-ish app state, write it through the real state files, load +//! it back in a fresh store (a "second session"), and restore it. + +use soulseek_rs::DownloadStatus; +use soulseek_rs::types::{Download, DownloadMetadata}; +use soulseek_rs_tui::models::{AppState, DownloadEntry}; +use soulseek_rs_tui::persist::config::FileConfig; +use soulseek_rs_tui::persist::snapshot::{Snapshot, restore_searches}; +use soulseek_rs_tui::persist::state::StateStore; + +fn download_entry(filename: &str, status: DownloadStatus) -> DownloadEntry { + DownloadEntry { + download: Download { + username: "peer".into(), + filename: filename.into(), + token: 1, + size: 1000, + download_directory: "/music".into(), + status, + sender: std::sync::mpsc::channel().0, + queue_position: None, + metadata: DownloadMetadata::default(), + }, + receiver: None, + } +} + +#[test] +fn state_survives_a_restart() { + let tmp = tempfile::tempdir().unwrap(); + let state_dir = tmp.path().join("state"); + + // Session 1: user searched, joined rooms, and has one finished and one + // in-flight download when the app exits. + { + let mut state = AppState::new(); + state + .downloads + .push(download_entry("done.flac", DownloadStatus::Completed)); + state.downloads.push(download_entry( + "half.flac", + DownloadStatus::InProgress { + bytes_downloaded: 500, + total_bytes: 1000, + speed_bytes_per_sec: 1.0, + }, + )); + restore_searches(&mut state, &["beatles".to_string()]); + state.rooms.focus_or_open("indie"); + + let store = StateStore::new(state_dir.clone()); + let snapshot = Snapshot::capture(&state); + store.save_downloads(&snapshot.downloads).unwrap(); + store.save_search_queries(&snapshot.queries).unwrap(); + store.save_rooms(&snapshot.rooms).unwrap(); + } + + // The files on disk are versioned envelopes. + let raw = + std::fs::read_to_string(state_dir.join("downloads.json")).unwrap(); + let parsed: serde_json::Value = serde_json::from_str(&raw).unwrap(); + assert!(parsed.get("version").is_some(), "missing version envelope"); + + // Session 2: a fresh store (new process) sees the same state; the + // incomplete download is flagged for auto-resume. + let store = StateStore::new(state_dir); + let downloads = store.load_downloads(); + assert_eq!(downloads.len(), 2); + assert!( + downloads + .iter() + .any(|d| d.filename == "done.flac" && d.completed) + ); + assert!( + downloads + .iter() + .any(|d| d.filename == "half.flac" && !d.completed) + ); + assert_eq!(store.load_search_queries(), vec!["beatles".to_string()]); + assert_eq!(store.load_rooms(), vec!["indie".to_string()]); + + let mut state = AppState::new(); + restore_searches(&mut state, &store.load_search_queries()); + assert_eq!(state.searches[0].query, "beatles"); +} + +#[test] +fn config_survives_a_restart() { + let tmp = tempfile::tempdir().unwrap(); + let path = tmp.path().join("config").join("config.toml"); + + let config = FileConfig { + username: Some("alice".into()), + server: Some("localhost:2242".into()), + ..FileConfig::default() + }; + config.save(&path).unwrap(); + + // "Second session": reload from disk. + let loaded = FileConfig::load(&path).unwrap(); + assert_eq!(loaded, config); +}