From 76f48541693ae454e9ee85a1478462f9b40a6e42 Mon Sep 17 00:00:00 2001 From: Michel de Graaf Date: Tue, 8 Sep 2026 05:21:21 +0000 Subject: [PATCH 1/2] feat(tui): run the highlighted search again with S MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The search list survives a restart, but a restored query was only ever a reminder: seeing what the network has now meant typing it out again. In the Searches pane, `S` now re-runs the highlighted query in its place — no duplicate row, no reordering — cancelling a run still collecting first and starting the Results display over for what comes back. The key is listed in the shortcut bar, the `?` overlay, the README and the website. `start_search` is split into launching a search and showing it, so both paths share one implementation. A new `tui_e2e` suite drives the real window over a real session against soulfind: keys in, a screen out, a sharing peer on the other end. It restores a query from a previous run's state directory, presses `S`, and checks the peer's file lands on screen — then again once the query has been on the wire, proving a repeat is answered and the display does not stack. To make that possible `MainTui` is exported with `handle_key_event`, `render`, `poll_session` and a `state()` accessor, and CI runs the suite with `SOULSEEK_E2E_REQUIRED=1`. Closes #85 --- .github/workflows/ci.yml | 8 + README.md | 14 +- soulseek-rs/src/lib.rs | 6 + soulseek-rs/src/ui/main_tui/input.rs | 18 +- soulseek-rs/src/ui/main_tui/mod.rs | 166 ++++++++++++-- soulseek-rs/src/ui/main_tui/render.rs | 5 +- soulseek-rs/src/ui/main_tui/search.rs | 80 +++++-- soulseek-rs/src/ui/mod.rs | 2 +- soulseek-rs/tests/tui_e2e.rs | 313 ++++++++++++++++++++++++++ website/src/pages/docs.tsx | 1 + 10 files changed, 550 insertions(+), 63 deletions(-) create mode 100644 soulseek-rs/tests/tui_e2e.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6241a96..f8913f0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -110,6 +110,14 @@ jobs: SOULSEEK_E2E_REQUIRED: "1" 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. + - name: Run TUI e2e tests (required against soulfind) + env: + SOULSEEK_E2E_REQUIRED: "1" + run: cargo test -p soulseek-rs --test tui_e2e -- --nocapture + # The control protocol as a third party meets it: the TCP transport, the # token, and the gate. None of it is reachable through the command # surface, so the CLI suite cannot cover it in either mode. diff --git a/README.md b/README.md index ac4d221..fca9053 100644 --- a/README.md +++ b/README.md @@ -107,7 +107,8 @@ zooms the focused pane to the whole window, `w` hides it so the others widen takes `↑`/`↓`, `Home`/`End`, `PgUp`/`PgDn` and the vim keys, and `h`/`l` or `←`/`→` scroll a long file name, folder or query sideways, `0` and `$` to either end. `PgUp`/`PgDn` scroll a chat log back through its history, and `?` -shows every key for where you are. +shows every key for where you are. `s` starts a search, and `S` runs the +highlighted one again. ### Daemon mode @@ -620,11 +621,12 @@ cargo fmt ### End-to-end tests -Two suites run against [soulfind](https://github.com/soulfind-dev/soulfind), a -local Soulseek server: `soulseek-rs-lib/tests/e2e.rs` covers the protocol -library, and `soulseek-rs/tests/cli_e2e.rs` drives the binary the way a script -would. Both are **server-optional**, running when a server is available and -skipping otherwise, so `cargo test` stays green everywhere. +Three 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. They locate a server in this order: diff --git a/soulseek-rs/src/lib.rs b/soulseek-rs/src/lib.rs index bda54b4..f09c0c3 100644 --- a/soulseek-rs/src/lib.rs +++ b/soulseek-rs/src/lib.rs @@ -14,3 +14,9 @@ pub(crate) mod port_mapping; pub mod remote; pub mod run; pub(crate) mod ui; + +/// The interactive window, reachable so an integration test can drive it +/// with key events, advance it a frame at a time and read the screen back. +/// Hidden from the documented API: it is a harness door, not a contract. +#[doc(hidden)] +pub use ui::MainTui; diff --git a/soulseek-rs/src/ui/main_tui/input.rs b/soulseek-rs/src/ui/main_tui/input.rs index c9c20e8..abefc80 100644 --- a/soulseek-rs/src/ui/main_tui/input.rs +++ b/soulseek-rs/src/ui/main_tui/input.rs @@ -14,7 +14,8 @@ use ratatui::widgets::TableState; const NAME_SCROLL_STEP: isize = 8; impl MainTui { - pub(super) fn handle_key_event(&mut self, key: KeyEvent) { + /// One key press, routed to whatever owns the keyboard right now. + pub fn handle_key_event(&mut self, key: KeyEvent) { // Command bar takes priority if self.state.command_bar_active { return self.handle_command_bar_input(key); @@ -400,11 +401,10 @@ impl MainTui { if self.navigate_focused_list(key) || self.scroll_focused_name(key) { return; } + let selected = self.state.searches_table_state.selected(); match key.code { KeyCode::Enter => { - if let Some(selected) = - self.state.searches_table_state.selected() - { + if let Some(selected) = selected { self.state.selected_search_index = Some(selected); if let Some(search) = self.state.searches.get(selected) { self.state.results_items = search.results.clone(); @@ -420,12 +420,16 @@ impl MainTui { } } KeyCode::Char('d') => { - if let Some(selected) = - self.state.searches_table_state.selected() - { + if let Some(selected) = selected { self.remove_search_at_index(selected); } } + // `s` asks for a query; `S` reuses the highlighted one. + KeyCode::Char('S') => { + if let Some(selected) = selected { + self.rerun_search_at_index(selected); + } + } KeyCode::Char('C') => { self.clear_all_searches(); } diff --git a/soulseek-rs/src/ui/main_tui/mod.rs b/soulseek-rs/src/ui/main_tui/mod.rs index fa39086..7623a7e 100644 --- a/soulseek-rs/src/ui/main_tui/mod.rs +++ b/soulseek-rs/src/ui/main_tui/mod.rs @@ -196,31 +196,30 @@ impl MainTui { result } + /// What the window holds, for a test to read back after driving it. + #[must_use] + pub const fn state(&self) -> &AppState { + &self.state + } + + /// One frame's worth of catching up with the session: everything the + /// event loop does between drawing and reading the keyboard. + pub fn poll_session(&mut self) { + self.update_search_results(); + self.update_downloads(); + self.poll_private_messages(); + self.poll_browse_result(); + self.poll_room_events(); + self.state.uploads = self.client.uploads(); + self.spinner_state = (self.spinner_state + 1) % 10; + self.save_persisted_state(); + } + fn run_event_loop(&mut self, terminal: &mut DefaultTerminal) -> Result<()> { while !self.state.should_exit { terminal.draw(|frame| self.render(frame))?; - // Poll for search results updates - self.update_search_results(); - - // Poll for download updates - self.update_downloads(); - - // Poll for incoming private messages - self.poll_private_messages(); - - // Poll for a browse (shared-file listing) response - self.poll_browse_result(); - - // Poll for chat-room events - self.poll_room_events(); - - // Refresh the uploads we are serving to peers - self.state.uploads = self.client.uploads(); - - self.spinner_state = (self.spinner_state + 1) % 10; - - self.save_persisted_state(); + self.poll_session(); // Drain every queued input event before the next draw: key // autorepeat outpaces the frame time, and handling one event per @@ -279,6 +278,8 @@ mod tests { searches: std::sync::Mutex>, /// What `try_get_search_results` hands back to the window. results: std::sync::Mutex>, + /// Every query a window put on the wire, in order. + searched: std::sync::Mutex>, /// How many times a window asked for the search list. all_searches_calls: std::sync::atomic::AtomicUsize, /// How many times a window pulled a full result set across. Against a @@ -347,10 +348,14 @@ mod tests { } fn search_with_cancel( &self, - _query: &str, + query: &str, _timeout: Duration, _cancel: Option>, ) -> soulseek_rs::Result> { + self.searched + .lock() + .expect("not poisoned") + .push(query.to_string()); Ok(Vec::new()) } fn get_search_results( @@ -1954,11 +1959,15 @@ mod tests { } #[test] - fn the_searches_pane_lists_its_clear_all_key() { + fn the_searches_pane_lists_its_clear_all_and_rerun_keys() { let mut tui = furnished_tui(); tui.state.focused_pane = FocusedPane::Searches; let screen = screen_sized(&mut tui, 300, 40); assert!(screen.contains("[C → clear all]"), "{screen}"); + assert!(screen.contains("[S → search again]"), "{screen}"); + press(&mut tui, KeyCode::Char('?')); + let screen = screen_of(&mut tui); + assert!(screen.contains("run the search again"), "{screen}"); } #[test] @@ -2051,4 +2060,115 @@ mod tests { let screen = screen_of(&mut tui); assert!(!screen.contains("@@x/Music/Album"), "{screen}"); } + + /// The queries the window has sent so far, once at least `count` of them + /// are out: a search goes to the session from a worker thread, so a test + /// has to give it a moment. + fn searched(session: &TalkativeSession, count: usize) -> Vec { + let deadline = std::time::Instant::now() + Duration::from_secs(5); + loop { + let seen = session.searched.lock().expect("not poisoned").clone(); + if seen.len() >= count || std::time::Instant::now() > deadline { + return seen; + } + std::thread::sleep(Duration::from_millis(10)); + } + } + + /// A window over last session's search list: the queries came back from + /// disk, with nothing under them. + fn restored(session: Arc, queries: &[&str]) -> MainTui { + let mut tui = attach(session); + let queries: Vec = + queries.iter().map(ToString::to_string).collect(); + restore_searches(&mut tui.state, &queries); + tui.state.focused_pane = FocusedPane::Searches; + tui + } + + #[test] + fn shift_s_runs_the_highlighted_search_again_in_its_place() { + // The list survives a restart, but a query on it was only ever a + // reminder: seeing what the network has now meant typing it again. + let session = Arc::new(TalkativeSession::default()); + let mut tui = + restored(session.clone(), &["boards of canada", "aphex twin"]); + tui.state.searches_table_state.select(Some(1)); + + press(&mut tui, KeyCode::Char('S')); + + assert_eq!( + searched(&session, 1), + vec!["aphex twin".to_string()], + "the highlighted query went out again, untyped" + ); + let queries: Vec<&str> = tui + .state + .searches + .iter() + .map(|s| s.query.as_str()) + .collect(); + assert_eq!( + queries, + ["boards of canada", "aphex twin"], + "it keeps its row: no duplicate, nothing moved" + ); + assert_eq!(tui.state.searches[1].status, SearchStatus::Active); + assert_eq!(tui.state.selected_search_index, Some(1)); + assert_eq!( + tui.state.focused_pane, + FocusedPane::Results, + "and the window turns to where the results will land" + ); + assert!(tui.state.results_items.is_empty()); + } + + #[test] + fn shift_s_stops_a_run_still_collecting_before_starting_over() { + let mut tui = furnished_tui(); + tui.state.searches[0].status = SearchStatus::Active; + tui.state.searches[0].start_time = std::time::Instant::now() + .checked_sub(Duration::from_secs(2)) + .expect("two seconds ago"); + let previous = tui.state.searches[0].cancel_flag.clone(); + tui.state.focused_pane = FocusedPane::Searches; + tui.state.searches_table_state.select(Some(0)); + + press(&mut tui, KeyCode::Char('S')); + + assert!( + previous.load(std::sync::atomic::Ordering::Relaxed), + "the worker that was collecting is told to stop" + ); + let fresh = &tui.state.searches[0]; + assert!(!fresh.cancel_flag.load(std::sync::atomic::Ordering::Relaxed)); + assert!(fresh.results.is_empty(), "stale rows do not linger"); + assert_eq!(tui.state.searches.len(), 1); + } + + #[test] + fn shift_s_is_only_a_key_over_the_searches_list() { + let session = Arc::new(TalkativeSession::default()); + let mut tui = restored(session.clone(), &["aphex twin"]); + + // Over Results the same key means nothing, so a stray shift does not + // throw a finished search away. + tui.state.focused_pane = FocusedPane::Results; + press(&mut tui, KeyCode::Char('S')); + assert_eq!(tui.state.searches[0].status, SearchStatus::Completed); + std::thread::sleep(Duration::from_millis(50)); + assert!(searched(&session, 0).is_empty(), "nothing went out"); + } + + #[test] + fn a_held_shift_s_is_one_search_not_one_per_repeat() { + let session = Arc::new(TalkativeSession::default()); + let mut tui = restored(session.clone(), &["aphex twin"]); + press(&mut tui, KeyCode::Char('S')); + press(&mut tui, KeyCode::Char('1')); + press(&mut tui, KeyCode::Char('S')); + assert_eq!(searched(&session, 1).len(), 1); + std::thread::sleep(Duration::from_millis(50)); + assert_eq!(searched(&session, 1).len(), 1, "the repeat is ignored"); + } } diff --git a/soulseek-rs/src/ui/main_tui/render.rs b/soulseek-rs/src/ui/main_tui/render.rs index a9389e0..25221e5 100644 --- a/soulseek-rs/src/ui/main_tui/render.rs +++ b/soulseek-rs/src/ui/main_tui/render.rs @@ -63,6 +63,7 @@ const HELP_RIGHT: &[(&str, &[(&str, &str)])] = &[ "Searches", &[ ("Enter", "show its results"), + ("S", "run the search again"), ("d", "remove the search"), ("C", "clear every search"), ("c", "chat rooms"), @@ -126,7 +127,8 @@ fn filter_keys( } impl MainTui { - pub(super) fn render(&mut self, frame: &mut Frame) { + /// Draw the whole window. + pub fn render(&mut self, frame: &mut Frame) { let shortcuts = self.shortcut_rows(frame.area().width); let mut constraints = vec![ Constraint::Length(3), // Status bar @@ -631,6 +633,7 @@ impl MainTui { let mut keys = match self.state.focused_pane { FocusedPane::Searches => vec![ ("s", "search"), + ("S", "search again"), ("Enter", "results"), ("d", "remove"), ("C", "clear all"), diff --git a/soulseek-rs/src/ui/main_tui/search.rs b/soulseek-rs/src/ui/main_tui/search.rs index d574d11..95fafe2 100644 --- a/soulseek-rs/src/ui/main_tui/search.rs +++ b/soulseek-rs/src/ui/main_tui/search.rs @@ -5,11 +5,18 @@ use crate::models::{ }; use chrono::Local; use std::{ - sync::{Arc, atomic::AtomicBool}, + sync::{ + Arc, + atomic::{AtomicBool, Ordering}, + }, thread, time::Instant, }; +/// How long after a search went out a repeat of `S` counts as the same +/// press: key autorepeat is faster than anyone means to search. +const RERUN_HOLD: std::time::Duration = std::time::Duration::from_secs(1); + impl MainTui { pub(super) fn remove_search_at_index(&mut self, index: usize) { if index >= self.state.searches.len() { @@ -18,9 +25,7 @@ impl MainTui { // Cancel the search if it's active if let Some(search) = self.state.searches.get(index) { - search - .cancel_flag - .store(true, std::sync::atomic::Ordering::Relaxed); + search.cancel_flag.store(true, Ordering::Relaxed); } // Check if we're removing the currently active search @@ -66,9 +71,7 @@ impl MainTui { pub(super) fn clear_all_searches(&mut self) { // Cancel all active searches for search in &self.state.searches { - search - .cancel_flag - .store(true, std::sync::atomic::Ordering::Relaxed); + search.cancel_flag.store(true, Ordering::Relaxed); } // And drop them from the session, or the next sync brings every one @@ -205,6 +208,39 @@ impl MainTui { } pub(super) fn start_search(&mut self, query: String) { + let entry = self.launch_search(query); + self.state.searches.push(entry); + 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. + pub(super) fn rerun_search_at_index(&mut self, index: usize) { + let Some(previous) = self.state.searches.get(index) else { + return; + }; + // A held key repeats, and every repeat would put another search on + // the wire: a run that only just went out is left to run. + if previous.status == SearchStatus::Active + && previous.start_time.elapsed() < RERUN_HOLD + { + return; + } + previous.cancel_flag.store(true, Ordering::Relaxed); + self.state.searches[index] = self.launch_search(previous.query.clone()); + self.show_search(index); + } + + /// Put `query` on the wire and hand back the entry that tracks it. The + /// session keys results by query, so a repeat starts its set afresh, and + /// the entry's empty results match that. + fn launch_search(&self, query: String) -> SearchEntry { let cancel_flag = Arc::new(AtomicBool::new(false)); let search_entry = SearchEntry { query: query.clone(), @@ -216,24 +252,6 @@ impl MainTui { cancel_flag: cancel_flag.clone(), }; - self.state.searches.push(search_entry); - let search_index = self.state.searches.len() - 1; - self.state.searches_table_state.select(Some(search_index)); - - // Make this search the active one - self.state.selected_search_index = Some(search_index); - - // Initialize results display (empty at first) - self.state.results_items.clear(); - self.state.results_filtered_items.clear(); - self.state.results_filtered_indices.clear(); - self.state.results_selected_indices.clear(); - self.state.results_table_state.select(Some(0)); - self.state.results_name_offset = 0; - - // Switch focus to Results pane, bringing it back if it was hidden - self.state.focus_pane(FocusedPane::Results); - let client = self.client.clone(); let timeout = self.search_timeout; @@ -251,6 +269,18 @@ impl MainTui { } } }); + + search_entry + } + + /// Make the search at `index` the one the Results pane shows, starting + /// from an empty display, filter and all, for what is about to arrive. + fn show_search(&mut self, index: usize) { + self.clear_results_pane(); + self.state.searches_table_state.select(Some(index)); + self.state.selected_search_index = Some(index); + self.state.results_table_state.select(Some(0)); + self.state.focus_pane(FocusedPane::Results); } /// Show the searches the session knows about, not just this window's. diff --git a/soulseek-rs/src/ui/mod.rs b/soulseek-rs/src/ui/mod.rs index d84b850..fd9542f 100644 --- a/soulseek-rs/src/ui/mod.rs +++ b/soulseek-rs/src/ui/mod.rs @@ -7,7 +7,7 @@ mod styles; mod utils; pub use downloads::render_download_stats; -pub use main_tui::launch_main_tui; +pub use main_tui::{MainTui, launch_main_tui}; pub use paging::*; pub use styles::*; pub use utils::*; diff --git a/soulseek-rs/tests/tui_e2e.rs b/soulseek-rs/tests/tui_e2e.rs new file mode 100644 index 0000000..b192362 --- /dev/null +++ b/soulseek-rs/tests/tui_e2e.rs @@ -0,0 +1,313 @@ +//! End-to-end tests of the window itself against a real Soulseek server: +//! key presses in, a screen out, with soulfind and a real sharing peer on the +//! other end of the wire. +//! +//! The CLI suite proves the command surface; the unit tests under `ui` prove +//! the window's keys against a stand-in session. What neither covers is the +//! two together — a key that is supposed to put a search on the network and +//! bring its answers back onto the screen. The window is driven in-process +//! through the same handlers the terminal feeds, rendered onto a test +//! backend, and the session is a real client logged in to a real server. +//! +//! Like the rest of the e2e suite this needs a soulfind server and skips +//! without one; `SOULSEEK_E2E_REQUIRED=1` turns that skip into a failure. + +mod common; + +use common::{free_port, soulfind_binary}; +use ratatui::Terminal; +use ratatui::backend::TestBackend; +use ratatui::crossterm::event::{KeyCode, KeyEvent, KeyModifiers}; +use soulseek_rs::{Client, ClientSettings, PeerAddress}; +use soulseek_rs_tui::MainTui; +use soulseek_rs_tui::models::{FocusedPane, 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}; + +/// How long a search collects for. Short, because every run here has to wait +/// 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 { + 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) -> 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)" + ); + 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 { + tui: MainTui, + terminal: Terminal, +} + +impl Window { + /// Open the window as the terminal would on start-up, over `state` — the + /// directory a previous run left its search list in. + fn open(client: Arc, state: &Path, downloads: &Path) -> Self { + let tui = MainTui::new( + client, + downloads.display().to_string(), + SEARCH_WINDOW, + Some(StateStore::new(state.to_path_buf())), + None, + ); + let terminal = Terminal::new(TestBackend::new(160, 40)) + .expect("a test backend always opens"); + Self { tui, terminal } + } + + fn press(&mut self, code: KeyCode) { + self.tui + .handle_key_event(KeyEvent::new(code, KeyModifiers::NONE)); + } + + /// One turn of the event loop: catch up with the session, then draw. + /// Returns the screen as the user would see it. + fn frame(&mut self) -> String { + self.tui.poll_session(); + self.terminal + .draw(|frame| self.tui.render(frame)) + .expect("draw"); + 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. + fn run_until( + &mut self, + timeout: Duration, + mut done: impl FnMut(&MainTui, &str) -> bool, + ) -> (bool, 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); + } + std::thread::sleep(Duration::from_millis(100)); + } + } + + /// File names in the Results pane right now. + fn result_names(&self) -> Vec { + self.tui + .state() + .results_items + .iter() + .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 +/// screen as well as in the state behind it. +fn shows_the_shared_file(tui: &MainTui, screen: &str) -> bool { + tui.state() + .results_items + .iter() + .any(|file| file.filename.contains("tui_probe_rerun")) + && screen.contains("tui_probe_rerun") +} + +#[test] +fn shift_s_runs_a_restored_query_again_and_the_answers_land_on_screen() { + let server = server_or_skip!(); + + // 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::>(), + ) + .expect("share file"); + let _sharer = server + .client("tui_e2e_sharer", vec![share.path().display().to_string()]); + settle(); + + // Last time this user ran the window they searched for "vexq" and quit. + // The list came back with them; the results did not. + let state = tempfile::tempdir().expect("state dir"); + let downloads = tempfile::tempdir().expect("download dir"); + StateStore::new(state.path().to_path_buf()) + .save_search_queries(&["vexq".to_string()]) + .expect("a previous run's search list"); + + let seeker = Arc::new(server.client("tui_e2e_seeker", Vec::new())); + let mut window = Window::open(seeker, state.path(), downloads.path()); + + let screen = window.frame(); + assert!( + 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. + 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}" + ); + 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}"); + assert!(shows_the_shared_file(&window.tui, &screen)); + let first_run = window.result_names(); + + // Again, now that the query has been on the wire once in this session: + // 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() + ); + + 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, + "the same answer, once, not the old set with the new one on top" + ); + assert_eq!(window.tui.state().searches.len(), 1, "still one row"); +} diff --git a/website/src/pages/docs.tsx b/website/src/pages/docs.tsx index 0ab7b42..63322aa 100644 --- a/website/src/pages/docs.tsx +++ b/website/src/pages/docs.tsx @@ -17,6 +17,7 @@ import { LINKS } from '@/lib/links' const GLOBAL_KEYS: readonly (readonly [string, string])[] = [ ['s', 'search the network'], + ['S', 'run the highlighted search again'], ['Space', 'select a result'], ['Enter', 'download / send'], ['b', "browse the owner's files"], From 3ef78c6de1d68129519c104ab1ffbd580d000db8 Mon Sep 17 00:00:00 2001 From: Michel de Graaf Date: Tue, 8 Sep 2026 06:03:31 +0000 Subject: [PATCH 2/2] refactor(tests): share one soulfind fixture across the e2e suites 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. --- .github/workflows/ci.yml | 3 +- README.md | 24 +-- soulseek-rs/src/ui/main_tui/search.rs | 27 +--- soulseek-rs/tests/cli_e2e.rs | 84 ++-------- soulseek-rs/tests/common/mod.rs | 117 ++++++++++++++ soulseek-rs/tests/daemon_protocol_e2e.rs | 74 +-------- soulseek-rs/tests/tui_e2e.rs | 192 +++++------------------ 7 files changed, 189 insertions(+), 332 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f8913f0..bcfc68c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -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" diff --git a/README.md b/README.md index fca9053..c8d031d 100644 --- a/README.md +++ b/README.md @@ -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 diff --git a/soulseek-rs/src/ui/main_tui/search.rs b/soulseek-rs/src/ui/main_tui/search.rs index 95fafe2..e2892cf 100644 --- a/soulseek-rs/src/ui/main_tui/search.rs +++ b/soulseek-rs/src/ui/main_tui/search.rs @@ -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}"); } }); diff --git a/soulseek-rs/tests/cli_e2e.rs b/soulseek-rs/tests/cli_e2e.rs index b6b0cb9..b045e31 100644 --- a/soulseek-rs/tests/cli_e2e.rs +++ b/soulseek-rs/tests/cli_e2e.rs @@ -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, - db: Option, + /// The soulfind spawned for this run; `None` against a server named by + /// `SOULSEEK_TEST_SERVER`. + _server: Option, _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) -> 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, Vec) { (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. /// diff --git a/soulseek-rs/tests/common/mod.rs b/soulseek-rs/tests/common/mod.rs index 0ecb1f5..223131c 100644 --- a/soulseek-rs/tests/common/mod.rs +++ b/soulseek-rs/tests/common/mod.rs @@ -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 { .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 { + 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 { + 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) -> 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) -> 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)"); +} diff --git a/soulseek-rs/tests/daemon_protocol_e2e.rs b/soulseek-rs/tests/daemon_protocol_e2e.rs index 88e7bca..60fd48f 100644 --- a/soulseek-rs/tests/daemon_protocol_e2e.rs +++ b/soulseek-rs/tests/daemon_protocol_e2e.rs @@ -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 { - 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 { - 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 { diff --git a/soulseek-rs/tests/tui_e2e.rs b/soulseek-rs/tests/tui_e2e.rs index b192362..6436c57 100644 --- a/soulseek-rs/tests/tui_e2e.rs +++ b/soulseek-rs/tests/tui_e2e.rs @@ -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 { - 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) -> 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::>(), - ) - .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,