diff --git a/src/chat/client.rs b/src/chat/client.rs index 5dd71b1..33a94cc 100644 --- a/src/chat/client.rs +++ b/src/chat/client.rs @@ -363,20 +363,15 @@ impl std::error::Error for ChatError {} #[cfg(test)] mod tests { use super::*; - use std::collections::HashMap; - use std::io::{BufRead, Read, Write}; + use crate::chat::testing::{ + CapturedRequest, fake_server as fake_server_serving, http_response, read_request, + sse_response, + }; + use std::io::Write; use std::net::TcpListener; - use std::sync::mpsc::{self, Receiver}; + use std::sync::mpsc::Receiver; use std::thread::JoinHandle; - /// One HTTP request as the fake server saw it. - struct CapturedRequest { - method: String, - path: String, - headers: HashMap, - body: String, - } - /// Serves `response` once to the first connection it accepts, then sends /// the request it received back through the returned channel. /// @@ -386,25 +381,6 @@ mod tests { fake_server_serving(vec![response]) } - /// Serves one response from `responses` per connection, in order, and - /// sends every request it read back through the returned channel. - fn fake_server_serving( - responses: Vec>, - ) -> (String, Receiver, JoinHandle<()>) { - let listener = TcpListener::bind("127.0.0.1:0").unwrap(); - let addr = listener.local_addr().unwrap(); - let (sender, receiver) = mpsc::channel(); - let handle = std::thread::spawn(move || { - for response in responses { - let (mut stream, _) = listener.accept().unwrap(); - let captured = read_request(&stream); - stream.write_all(&response).unwrap(); - sender.send(captured).unwrap(); - } - }); - (format!("http://{addr}"), receiver, handle) - } - /// Accepts one connection, reads the request, writes `sent`, and then /// holds the connection open for `hold` without writing anything more. fn stalling_server(sent: Vec, hold: Duration) -> (String, JoinHandle<()>) { @@ -430,62 +406,6 @@ mod tests { } } - /// Reads one HTTP/1.1 request's method, path, headers, and body from - /// `stream`. - fn read_request(stream: &std::net::TcpStream) -> CapturedRequest { - let mut reader = io::BufReader::new(stream); - let mut request_line = String::new(); - reader.read_line(&mut request_line).unwrap(); - let mut parts = request_line.split_whitespace(); - let method = parts.next().unwrap().to_string(); - let path = parts.next().unwrap().to_string(); - - let mut headers = HashMap::new(); - loop { - let mut line = String::new(); - reader.read_line(&mut line).unwrap(); - let line = line.trim_end(); - if line.is_empty() { - break; - } - let (name, value) = line.split_once(':').unwrap(); - headers.insert(name.trim().to_lowercase(), value.trim().to_string()); - } - - let content_length = headers - .get("content-length") - .and_then(|value| value.parse::().ok()) - .unwrap_or(0); - let mut body = vec![0u8; content_length]; - reader.read_exact(&mut body).unwrap(); - - CapturedRequest { - method, - path, - headers, - body: String::from_utf8(body).unwrap(), - } - } - - /// Builds a canned HTTP/1.1 response with `status`, a body of - /// `content_type`, and `Connection: close`. - fn http_response(status: &str, content_type: &str, body: &str) -> Vec { - format!( - "HTTP/1.1 {status}\r\n\ - Content-Type: {content_type}\r\n\ - Content-Length: {}\r\n\ - Connection: close\r\n\ - \r\n\ - {body}", - body.len() - ) - .into_bytes() - } - - fn sse_response(body: &str) -> Vec { - http_response("200 OK", "text/event-stream", body) - } - fn client_for(api_base: String) -> Client { Client { api_base, diff --git a/src/chat/mod.rs b/src/chat/mod.rs index f193b7d..51b4e0d 100644 --- a/src/chat/mod.rs +++ b/src/chat/mod.rs @@ -1,5 +1,7 @@ pub mod client; pub mod sse; +#[cfg(test)] +pub(crate) mod testing; pub mod wire; pub use client::{ChatError, ChatStream, Client, StreamItem}; diff --git a/src/chat/testing.rs b/src/chat/testing.rs new file mode 100644 index 0000000..a48cf8e --- /dev/null +++ b/src/chat/testing.rs @@ -0,0 +1,100 @@ +//! A fake loopback chat completion server for tests: serves canned +//! HTTP/1.1 responses over a local TCP listener and records each request +//! it received, so a test can assert on both what a client sent and how +//! it handled a scripted response. + +use std::collections::HashMap; +use std::io::{self, BufRead, Read, Write}; +use std::net::TcpListener; +use std::sync::mpsc::{self, Receiver}; +use std::thread::JoinHandle; + +/// One HTTP request as the fake server saw it. +pub(crate) struct CapturedRequest { + pub(crate) method: String, + pub(crate) path: String, + pub(crate) headers: HashMap, + pub(crate) body: String, +} + +/// Serves one response from `responses` per connection, in order, and +/// sends every request it read back through the returned channel. +/// +/// Each entry in `responses` must be a complete HTTP/1.1 response, +/// including status line and headers. A write that fails is ignored: a +/// caller that stops reading before the server finishes writing, such as +/// a client that gives up after its own timeout or a cancelled turn that +/// closes the connection early, must not panic the server thread. +pub(crate) fn fake_server( + responses: Vec>, +) -> (String, Receiver, JoinHandle<()>) { + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + let addr = listener.local_addr().unwrap(); + let (sender, receiver) = mpsc::channel(); + let handle = std::thread::spawn(move || { + for response in responses { + let (mut stream, _) = listener.accept().unwrap(); + let captured = read_request(&stream); + let _ = stream.write_all(&response); + sender.send(captured).unwrap(); + } + }); + (format!("http://{addr}"), receiver, handle) +} + +/// Reads one HTTP/1.1 request's method, path, headers, and body from +/// `stream`. +pub(crate) fn read_request(stream: &std::net::TcpStream) -> CapturedRequest { + let mut reader = io::BufReader::new(stream); + let mut request_line = String::new(); + reader.read_line(&mut request_line).unwrap(); + let mut parts = request_line.split_whitespace(); + let method = parts.next().unwrap().to_string(); + let path = parts.next().unwrap().to_string(); + + let mut headers = HashMap::new(); + loop { + let mut line = String::new(); + reader.read_line(&mut line).unwrap(); + let line = line.trim_end(); + if line.is_empty() { + break; + } + let (name, value) = line.split_once(':').unwrap(); + headers.insert(name.trim().to_lowercase(), value.trim().to_string()); + } + + let content_length = headers + .get("content-length") + .and_then(|value| value.parse::().ok()) + .unwrap_or(0); + let mut body = vec![0u8; content_length]; + reader.read_exact(&mut body).unwrap(); + + CapturedRequest { + method, + path, + headers, + body: String::from_utf8(body).unwrap(), + } +} + +/// Builds a canned HTTP/1.1 response with `status`, a body of +/// `content_type`, and `Connection: close`. +pub(crate) fn http_response(status: &str, content_type: &str, body: &str) -> Vec { + format!( + "HTTP/1.1 {status}\r\n\ + Content-Type: {content_type}\r\n\ + Content-Length: {}\r\n\ + Connection: close\r\n\ + \r\n\ + {body}", + body.len() + ) + .into_bytes() +} + +/// A canned `200 OK` response carrying `body` as an SSE event stream. +pub(crate) fn sse_response(body: &str) -> Vec { + http_response("200 OK", "text/event-stream", body) +} diff --git a/src/cli.rs b/src/cli.rs index d2bc19c..9ef9743 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -88,7 +88,10 @@ pub fn run( cli: Cli, srd_sources: &SrdSources, layer_root: &Path, - session_layers: &[PathBuf], + // `storied sandbox` is the only reader of the layers, and coverage + // builds leave that command out along with the terminal it needs, so + // the parameter has no reader there. + #[cfg_attr(coverage, allow(unused_variables))] session_layers: &[PathBuf], ) -> Result<(), String> { match cli.command { None => { diff --git a/src/dm/dm_tests.rs b/src/dm/dm_tests.rs index 055df46..8b3bb7c 100644 --- a/src/dm/dm_tests.rs +++ b/src/dm/dm_tests.rs @@ -2,98 +2,19 @@ //! keep the production file under the project's file-length guideline. //! The tool-calling round loop has its own sibling test file, which //! shares this file's fake-server harness rather than keeping its own -//! copy. +//! copy. The harness itself lives in `chat::testing`, shared with +//! `chat::client`'s own tests. use super::*; use crate::knowledge::fixtures; -use std::collections::HashMap; use std::fs; -use std::io::{self, BufRead, Read, Write}; -use std::net::TcpListener; use std::sync::Arc; -use std::sync::mpsc::{self, Receiver}; -use std::thread::JoinHandle; use tempfile::TempDir; -/// One HTTP request as the fake server saw it. -pub(super) struct CapturedRequest { - pub(super) body: String, -} - -/// Serves `responses` in order, one per accepted connection, and sends -/// each request it received back through the returned channel in the -/// same order. -/// -/// Each entry in `responses` must be a complete HTTP/1.1 response, -/// including status line and headers. A write that fails is ignored: a -/// cancelled turn can close the connection before the server finishes -/// writing. -pub(super) fn fake_server( - responses: Vec>, -) -> (String, Receiver, JoinHandle<()>) { - let listener = TcpListener::bind("127.0.0.1:0").unwrap(); - let addr = listener.local_addr().unwrap(); - let (sender, receiver) = mpsc::channel(); - let handle = std::thread::spawn(move || { - for response in responses { - let (mut stream, _) = listener.accept().unwrap(); - let captured = read_request(&stream); - let _ = stream.write_all(&response); - sender.send(captured).unwrap(); - } - }); - (format!("http://{addr}"), receiver, handle) -} - -/// Reads one HTTP/1.1 request's body from `stream`, skipping the -/// request line and headers. -fn read_request(stream: &std::net::TcpStream) -> CapturedRequest { - let mut reader = io::BufReader::new(stream); - let mut request_line = String::new(); - reader.read_line(&mut request_line).unwrap(); - - let mut headers = HashMap::new(); - loop { - let mut line = String::new(); - reader.read_line(&mut line).unwrap(); - let line = line.trim_end(); - if line.is_empty() { - break; - } - let (name, value) = line.split_once(':').unwrap(); - headers.insert(name.trim().to_lowercase(), value.trim().to_string()); - } - - let content_length = headers - .get("content-length") - .and_then(|value| value.parse::().ok()) - .unwrap_or(0); - let mut body = vec![0u8; content_length]; - reader.read_exact(&mut body).unwrap(); - - CapturedRequest { - body: String::from_utf8(body).unwrap(), - } -} - -/// Builds a canned HTTP/1.1 response with `status`, a body of -/// `content_type`, and `Connection: close`. -fn http_response(status: &str, content_type: &str, body: &str) -> Vec { - format!( - "HTTP/1.1 {status}\r\n\ - Content-Type: {content_type}\r\n\ - Content-Length: {}\r\n\ - Connection: close\r\n\ - \r\n\ - {body}", - body.len() - ) - .into_bytes() -} - -pub(super) fn sse_response(body: &str) -> Vec { - http_response("200 OK", "text/event-stream", body) -} +use crate::chat::testing::http_response; +// Re-exported for `dm_tool_round_tests.rs`, which shares this harness +// rather than keeping its own copy. +pub(super) use crate::chat::testing::{CapturedRequest, fake_server, sse_response}; fn dm_for(api_base: String) -> Dm { Dm::new( diff --git a/src/play/keys.rs b/src/play/keys.rs index ef84495..92decef 100644 --- a/src/play/keys.rs +++ b/src/play/keys.rs @@ -56,6 +56,9 @@ pub enum Key { Quit, /// Stop the reply streaming in, if one is running. Cancel, + /// Stop the reply streaming in while a turn runs, and leave the game + /// when none does. Ctrl+C means this, the way a REPL reads it. + Interrupt, /// Put the viewport back on the last row of the screen and paint /// every row of it again. A terminal that changed size means this, /// and so does Ctrl+L. @@ -93,7 +96,7 @@ fn decode_key(key: &KeyEvent) -> Option { let alt = key.modifiers.contains(KeyModifiers::ALT); let shift = key.modifiers.contains(KeyModifiers::SHIFT); match key.code { - KeyCode::Char('c') if control => Some(Key::Quit), + KeyCode::Char('c') if control => Some(Key::Interrupt), KeyCode::Char('a') if control => Some(Key::Home), KeyCode::Char('e') if control => Some(Key::End), KeyCode::Char('k') if control => Some(Key::KillToEnd), @@ -173,8 +176,8 @@ mod tests { } #[test] - fn control_c_quits() { - assert_eq!(decode(&control(KeyCode::Char('c'))), Some(Key::Quit)); + fn control_c_interrupts() { + assert_eq!(decode(&control(KeyCode::Char('c'))), Some(Key::Interrupt)); } #[test] diff --git a/src/play/mod.rs b/src/play/mod.rs index 9c0e1e7..86a456b 100644 --- a/src/play/mod.rs +++ b/src/play/mod.rs @@ -10,7 +10,8 @@ //! terminal's own scrollback, so the transcript is still there after the //! game ends. //! -//! Coverage builds leave the `terminal` module out. Everything else here +//! Coverage builds leave the `terminal` module out, because every line of +//! it needs a terminal the test suite does not have. Everything else here //! runs under the test suite. mod editor; @@ -18,7 +19,6 @@ mod history; pub mod keys; pub mod panics; mod prompt; -#[cfg(not(coverage))] pub mod sandbox; pub mod screen; pub mod sync; diff --git a/src/play/sandbox.rs b/src/play/sandbox.rs index 907e4b4..2e894d4 100644 --- a/src/play/sandbox.rs +++ b/src/play/sandbox.rs @@ -6,7 +6,9 @@ //! ends. Nothing persists from one sandbox to the next. use std::fs; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; + +use tempfile::TempDir; /// A throwaway world and player directory for one sandbox session. /// @@ -15,24 +17,42 @@ use std::path::PathBuf; /// are what the session mounts into the DM's knowledge and writes its /// campaign to. pub struct Sandbox { - root: tempfile::TempDir, + root: TempDir, } impl Sandbox { /// Creates a fresh sandbox with empty `world` and `player` - /// directories. + /// directories, under the directory the system keeps temporary files + /// in. pub fn new() -> Result { + Self::inside(&std::env::temp_dir()) + } + + /// Creates the sandbox's own directory under `parent`. + /// + /// The parent is a parameter so that a caller can name a directory + /// the sandbox cannot live in, which is what the failure this reports + /// means: no room, no permission, or nowhere to put the tree. + fn inside(parent: &Path) -> Result { let root = tempfile::Builder::new() .prefix("storied-sandbox-") - .tempdir() + .tempdir_in(parent) .map_err(|error| format!("could not create a sandbox directory: {error}"))?; + Self::under(root) + } + + /// Creates the `world` and `player` directories inside `root`, and + /// takes ownership of the tree. + /// + /// The root is a parameter so that a caller can hand over a root that + /// already holds a file of one of those names, which is what the + /// failure this reports means: the name is taken by something that is + /// not a directory. + fn under(root: TempDir) -> Result { for name in ["world", "player"] { - fs::create_dir_all(root.path().join(name)).map_err(|error| { - format!( - "could not create the sandbox {} directory: {error}", - root.path().join(name).display() - ) - })?; + let directory = root.path().join(name); + fs::create_dir_all(&directory) + .map_err(|error| format!("could not create {}: {error}", directory.display()))?; } Ok(Self { root }) } @@ -62,4 +82,26 @@ mod tests { assert_eq!(std::fs::read_dir(sandbox.player_dir()).unwrap().count(), 0); assert_ne!(sandbox.world_dir(), sandbox.player_dir()); } + + #[test] + fn a_sandbox_with_nowhere_to_live_says_so() { + let parent = TempDir::new().unwrap(); + + let error = Sandbox::inside(&parent.path().join("not-here")) + .err() + .unwrap(); + + assert!(error.contains("could not create a sandbox directory")); + } + + #[test] + fn a_sandbox_whose_world_name_is_taken_names_the_directory() { + let root = TempDir::new().unwrap(); + fs::write(root.path().join("world"), "x").unwrap(); + + let error = Sandbox::under(root).err().unwrap(); + + assert!(error.contains("world")); + assert!(error.contains("could not create")); + } } diff --git a/src/play/screen.rs b/src/play/screen.rs index 5f7c3e4..54c432b 100644 --- a/src/play/screen.rs +++ b/src/play/screen.rs @@ -121,7 +121,9 @@ pub struct SessionText<'a> { /// viewport as they stream, into the terminal's own scrollback, so they /// survive the game. `history` holds the prompts the player can recall /// with Up and Down. Escape while a turn is running sets `worker.cancel`, -/// which the worker checks to stop the turn. +/// which the worker checks to stop the turn. Ctrl+C does the same while a +/// turn runs, so a reply the player no longer wants costs the turn and +/// nothing else; with no turn running it ends the game. /// /// `guard` brackets every render pass in a synchronized update, so the /// terminal paints each pass in one go instead of painting whatever has @@ -193,8 +195,11 @@ pub fn play>( Some(Key::Newline) => screen.input.insert('\n'), Some(Key::Paste(text)) => screen.input.insert_str(&text), Some(Key::Enter) => screen.submit(terminal, worker, text, history, guard, viewport)?, - Some(Key::Cancel) if screen.busy => worker.cancel.store(true, Ordering::Relaxed), + Some(Key::Cancel | Key::Interrupt) if screen.busy => { + worker.cancel.store(true, Ordering::Relaxed); + } Some(Key::Cancel) => {} + Some(Key::Interrupt) => return Ok(()), Some(Key::Tab) => tab_complete(&mut screen), Some(Key::Redraw) => redraw(&screen, terminal, guard, viewport)?, } @@ -210,7 +215,7 @@ fn fit>( ) -> Result<(), B::Error> { let size = terminal.size()?; let forming = screen.transcript.forming_rows(size.width).len(); - let floor = floor(screen.rows, screen.transcript.waiting(), size); + let floor = floor(screen.rows, screen.transcript.waiting(), size, screen.busy); let wanted = wanted_rows(&screen.input, size, forming).max(floor); if wanted == screen.rows { return Ok(()); @@ -250,18 +255,33 @@ fn redraw>( Ok(()) } -/// The fewest rows the viewport may hold now, with `rows` of them now and -/// `waiting` rows of the transcript about to go out. +/// The fewest rows the viewport may hold now, with `rows` of them now, +/// `waiting` rows of the transcript about to go out, and a turn running or +/// not. /// /// A viewport gives its rows back below itself, and the rows the /// transcript puts out above it are what push it back down to the bottom -/// of the screen. It may give back no more of them than those rows fill, -/// so a shrink with nothing to write waits: the viewport keeps the rows, -/// blank, until rows go out that fill them. The cap comes first, though. -/// A screen with no room for the rows the viewport is holding takes them -/// back whether or not anything fills them, because a viewport past the -/// cap leaves the transcript nothing. -fn floor(rows: u16, waiting: u16, size: Size) -> u16 { +/// of the screen. While a turn runs it may give back no more of them than +/// those rows fill, so a shrink with nothing to write waits: the viewport +/// keeps the rows, blank, until rows go out that fill them. More rows are +/// coming, and they arrive within a turn. +/// +/// Between turns nothing is coming. A floor there would hold the rows a +/// prompt gave up, blank, until the next reply: a ten-row paste the +/// player then deletes would keep half a small screen for as long as it +/// took to type the next line. The viewport gives every row back at once +/// instead, and the resize clears them, so nothing of the prompt is left +/// under it. The prompt stops short of the last row until the next rows +/// the transcript writes land in those rows and push it back down, which +/// is the price of giving them back early. +/// +/// The cap comes first either way. A screen with no room for the rows the +/// viewport is holding takes them back whether or not anything fills +/// them, because a viewport past the cap leaves the transcript nothing. +fn floor(rows: u16, waiting: u16, size: Size, busy: bool) -> u16 { + if !busy && waiting == 0 { + return 0; + } rows.saturating_sub(waiting).min(cap(size)) } @@ -379,10 +399,16 @@ impl Screen { guard: &mut G, viewport: &mut V, ) -> Result<(), B::Error> { - if self.busy || self.input.text().trim().is_empty() { + // One trim answers every question the rest of this asks: whether + // the line says anything, whether it names a slash command, and + // what the DM reads. A command the player ended with a space is + // still that command, not a turn. The take that follows clears + // the prompt and gives nothing back that this does not have. + let input = self.input.text().trim().to_string(); + if self.busy || input.is_empty() { return Ok(()); } - let input = self.input.take(); + self.input.take(); // Slash commands stay local. /context shows the DM's system prompt. if input == "/context" { @@ -415,11 +441,15 @@ impl Screen { } } -/// If the input starts with `/`, cycles through matching slash commands. -/// Otherwise does nothing. +/// If the input is a single line starting with `/`, cycles through +/// matching slash commands. Otherwise does nothing. +/// +/// A completion replaces the whole input, so an input of several rows +/// whose first character happens to be `/` is not a command name and Tab +/// leaves it alone rather than throwing the rest of it away. fn tab_complete(screen: &mut Screen) { let text = screen.input.text(); - if !text.starts_with('/') { + if !text.starts_with('/') || text.contains('\n') { return; } // Find the next command whose prefix matches what's been typed so far. diff --git a/src/play/screen_pinned_tests.rs b/src/play/screen_pinned_tests.rs index 22f9302..9868436 100644 --- a/src/play/screen_pinned_tests.rs +++ b/src/play/screen_pinned_tests.rs @@ -30,11 +30,12 @@ use super::*; /// terminal opens on the screen and the scrollback the old one left, the /// way stdout leaves them for the real one. /// -/// The row arithmetic is [`crate::play::viewport`]'s, the same functions -/// the real one calls, so a screen this leaves behind is the screen a -/// terminal is left holding. Recording the heights and the resizes the -/// way `RecordingViewport` does lets the harness report both the same -/// way, whichever fake a test runs on. +/// The rows every write goes to come from [`viewport::Move`], which is +/// where the real one gets them too, so a screen this leaves behind is +/// the screen a terminal is left holding and neither can drift from the +/// other. Recording the heights and the resizes the way +/// `RecordingViewport` does lets the harness report both the same way, +/// whichever fake a test runs on. pub(super) struct PinnedViewport { pub(super) rows: Rc>>, pub(super) timeline: Rc>>, @@ -51,8 +52,7 @@ impl ViewportRows for PinnedViewport { self.timeline.borrow_mut().push(Recorded::Resize); let old_top = terminal.get_frame().area().y; let height = terminal.size()?.height; - let target = viewport::next_top(height, rows, old_top); - move_viewport(terminal, rows, old_top, target) + move_viewport(terminal, viewport::Move::resize(height, rows, old_top)) } /// Changes the size of the screen first, when a `Resize` step asked @@ -70,8 +70,7 @@ impl ViewportRows for PinnedViewport { self.timeline.borrow_mut().push(Recorded::Resize); let old_top = terminal.get_frame().area().y; let height = terminal.size()?.height; - let target = viewport::pinned_top(height, rows); - move_viewport(terminal, rows, old_top, target) + move_viewport(terminal, viewport::Move::repin(height, rows, old_top)) } } @@ -79,29 +78,25 @@ impl ViewportRows for PinnedViewport { /// keys fake, which is where a test schedules one. pub(super) type Resized = Rc>>; -/// Puts a viewport of `rows` rows on `target`, with its old top on -/// `old_top`, the way `terminal::move_viewport` does on a real screen. +/// Writes `plan` to a `TestBackend`, in order, the way +/// `terminal::move_viewport` writes it to a real screen. fn move_viewport( terminal: &mut Terminal, - rows: u16, - old_top: u16, - target: u16, + plan: viewport::Move, ) -> Result<(), Infallible> { let height = terminal.size()?.height; - let scroll = viewport::scroll_up_needed(old_top, target); let backend = terminal.backend_mut(); - if scroll > 0 { - backend.scroll_region_up(0..height, scroll)?; + if plan.scroll > 0 { + backend.scroll_region_up(0..height, plan.scroll)?; } - let clear = viewport::clear_from(old_top.saturating_sub(scroll), target); - backend.set_cursor_position(Position::new(0, clear))?; + backend.set_cursor_position(Position::new(0, plan.clear))?; backend.clear_region(ClearType::AfterCursor)?; - backend.set_cursor_position(Position::new(0, target))?; + backend.set_cursor_position(Position::new(0, plan.top))?; let backend = backend.clone(); *terminal = Terminal::with_options( backend, TerminalOptions { - viewport: Viewport::Inline(rows), + viewport: Viewport::Inline(plan.rows), }, )?; Ok(()) @@ -226,13 +221,38 @@ fn the_transcript_reaches_the_viewport_while_a_block_is_still_forming() { } #[test] -fn the_prompt_stays_on_the_last_row_when_the_input_gives_a_row_back() { +fn the_row_the_input_gives_back_leaves_nothing_behind_it() { let mut steps = typing(&"ab".repeat(20)); steps.push(press(Key::Backspace)); steps.push(press(Key::Backspace)); let mut played = play_pinned(ROOMY, steps); - assert_eq!(played.viewport_bottom(), ROOMY); + // The viewport keeps its top and ends one row short of the last row, + // and the row it gave back holds none of the prompt it used to. + assert_eq!(played.viewport_bottom(), ROOMY - 1); + assert_eq!(played.row(ROOMY - 1), (String::new(), false)); assert_eq!(played.printed(), ["a banner"]); } + +#[test] +fn the_line_the_player_submits_takes_back_the_row_the_prompt_gave_up() { + let mut steps = typing(&"ab".repeat(20)); + steps.push(press(Key::Backspace)); + steps.push(press(Key::Backspace)); + steps.push(press(Key::Enter)); + + let mut played = play_pinned(ROOMY, steps); + + // The blank row is the one the transcript puts between two blocks of + // different kinds, the banner and what the player said. + assert_eq!(played.viewport_bottom(), ROOMY); + assert_eq!( + played.printed(), + [ + "a banner".to_string(), + String::new(), + format!("> {}", "ab".repeat(19)), + ] + ); +} diff --git a/src/play/screen_prompt_tests.rs b/src/play/screen_prompt_tests.rs index dd67b40..c42a947 100644 --- a/src/play/screen_prompt_tests.rs +++ b/src/play/screen_prompt_tests.rs @@ -82,30 +82,33 @@ fn an_input_that_wraps_to_two_rows_asks_for_one_taller_viewport() { } #[test] -fn deleting_back_to_one_row_keeps_the_taller_viewport_until_a_row_goes_out() { +fn deleting_back_to_one_row_gives_the_row_back_between_turns() { let mut steps = typing(&"ab".repeat(20)); steps.push(press(Key::Backspace)); steps.push(press(Key::Backspace)); let played = play_script_on(12, steps); - // The row the prompt no longer needs goes back below the viewport, - // where a row of the transcript has to land to push the prompt down - // to the bottom of the screen again. Nothing is waiting to go out - // here, so the viewport holds the row, blank, instead. - assert_eq!(played.requested(), vec![6]); + // No turn is running and nothing waits to go out, so no rows are + // coming to fill the row the prompt no longer needs. The viewport + // gives it back now rather than holding it blank until the next + // reply. + assert_eq!(played.requested(), vec![6, 5]); } #[test] -fn the_line_the_player_submits_takes_back_the_row_the_prompt_gave_up() { - let mut steps = typing(&"ab".repeat(20)); +fn deleting_back_to_one_row_keeps_the_taller_viewport_while_a_turn_runs() { + let mut steps = typing("hi"); + steps.push(press(Key::Enter)); + steps.extend(typing(&"ab".repeat(20))); steps.push(press(Key::Backspace)); steps.push(press(Key::Backspace)); - steps.push(press(Key::Enter)); let played = play_script_on(12, steps); - assert_eq!(played.requested(), vec![6, 5]); + // The reply is still arriving, so the rows it writes are what the + // row the prompt gave up waits for. + assert_eq!(played.requested(), vec![6]); } #[test] diff --git a/src/play/screen_slash_tests.rs b/src/play/screen_slash_tests.rs index ce29ed3..8d07ebe 100644 --- a/src/play/screen_slash_tests.rs +++ b/src/play/screen_slash_tests.rs @@ -2,7 +2,7 @@ //! instead of going to the worker, and Tab completing command names on //! the prompt. The harness lives in `screen_tests.rs`. -use super::tests::{SLASH_CONTEXT, play_script, press, typing}; +use super::tests::{SLASH_CONTEXT, play_script, play_script_on, press, typing}; use super::*; #[test] @@ -36,6 +36,17 @@ fn slash_context_clears_the_prompt_for_the_next_input() { assert_eq!(played.prompt(), ">"); } +#[test] +fn a_trailing_space_still_names_the_command() { + let mut steps = typing("/context "); + steps.push(press(Key::Enter)); + + let played = play_script(steps); + + assert!(played.transcript().contains(SLASH_CONTEXT)); + assert!(played.nothing_submitted()); +} + #[test] fn tab_completes_a_slash_command_prefix() { let mut steps = typing("/c"); @@ -75,3 +86,15 @@ fn tab_leaves_an_unknown_slash_prefix_alone() { assert_eq!(played.prompt(), "> /x"); } + +#[test] +fn tab_leaves_an_input_of_several_rows_alone() { + let steps = vec![ + press(Key::Paste("/c\nthe rest of it".to_string())), + press(Key::Tab), + ]; + + let played = play_script_on(10, steps); + + assert_eq!(played.prompt(), " the rest of it"); +} diff --git a/src/play/screen_tail_tests.rs b/src/play/screen_tail_tests.rs index c6dbe2e..d6b2906 100644 --- a/src/play/screen_tail_tests.rs +++ b/src/play/screen_tail_tests.rs @@ -75,7 +75,7 @@ fn a_finished_reply_empties_the_tail() { /// Whether `c` is one of the sparkle glyphs the thinking indicator /// pulses through. fn is_sparkle(c: char) -> bool { - c == '\u{B7}' || ('\u{2726}'..='\u{2739}').contains(&c) + thinking::SPARKS.contains(&c) } #[test] diff --git a/src/play/screen_turn_tests.rs b/src/play/screen_turn_tests.rs index 75caaa4..fef8a87 100644 --- a/src/play/screen_turn_tests.rs +++ b/src/play/screen_turn_tests.rs @@ -321,6 +321,46 @@ fn escape_while_idle_does_nothing() { assert!(!played.cancel.load(Ordering::Relaxed)); } +#[test] +fn control_c_mid_turn_sets_the_cancel_flag() { + let mut steps = typing("hi"); + steps.push(press(Key::Enter)); + steps.push(press(Key::Interrupt)); + + let played = play_script(steps); + + assert!(played.cancel.load(Ordering::Relaxed)); +} + +#[test] +fn control_c_mid_turn_leaves_the_game_running() { + let mut steps = typing("hi"); + steps.push(press(Key::Enter)); + steps.push(press(Key::Interrupt)); + steps.push(Step::Turn(TurnEvent::Cancelled("You wa".to_string()))); + steps.extend(typing("again")); + steps.push(press(Key::Enter)); + + let played = play_script(steps); + + assert_eq!(played.submitted(), "hi"); + assert_eq!(played.submitted(), "again"); +} + +#[test] +fn control_c_while_idle_ends_the_game() { + let mut steps = typing("hi"); + steps.push(press(Key::Interrupt)); + steps.extend(typing("more")); + + let played = play_script(steps); + + // The loop draws before it reads a key, so the prompt shows what the + // player had typed when Ctrl+C arrived, and nothing after it. + assert_eq!(played.prompt(), "> hi"); + assert!(!played.cancel.load(Ordering::Relaxed)); +} + #[test] fn a_cancelled_turn_shows_the_partial_text_then_the_interrupted_aside() { let mut steps = typing("hi"); diff --git a/src/play/terminal.rs b/src/play/terminal.rs index 82e259e..793e110 100644 --- a/src/play/terminal.rs +++ b/src/play/terminal.rs @@ -206,8 +206,10 @@ impl ViewportRows> for CrosstermViewport { ) -> io::Result<()> { let old_top = terminal.get_frame().area().y; let screen_height = terminal.size()?.height; - let target = viewport::next_top(screen_height, rows, old_top); - move_viewport(terminal, rows, old_top, target) + move_viewport( + terminal, + viewport::Move::resize(screen_height, rows, old_top), + ) } /// Puts the viewport back on the last row of the screen, wherever a @@ -225,19 +227,20 @@ impl ViewportRows> for CrosstermViewport { ) -> io::Result<()> { let old_top = terminal.get_frame().area().y; let screen_height = terminal.size()?.height; - let target = viewport::pinned_top(screen_height, rows); - move_viewport(terminal, rows, old_top, target) + move_viewport( + terminal, + viewport::Move::repin(screen_height, rows, old_top), + ) } } -/// Puts a viewport of `rows` rows on `target`, with its old top on -/// `old_top`, and builds the terminal that draws it. +/// Writes `plan` to stdout, in order, and builds the terminal that draws +/// the viewport it leaves behind. /// -/// Rows above `target` that content still owns scroll up out of the way -/// first, the same as a program whose output ran past the last row, so -/// they go into the scrollback intact rather than getting overwritten. -/// The clear then takes every row the viewport holds now, and every row -/// it held before, off the screen. +/// The scroll is crossterm's `ScrollUp`, which the terminal treats the +/// same as a program whose output ran past the last row, so the rows it +/// takes off the top go into the scrollback intact rather than getting +/// overwritten. /// /// `Terminal::with_options` reads the cursor back from the row this just /// set and reserves the new viewport's height below it; since that row @@ -259,26 +262,22 @@ impl ViewportRows> for CrosstermViewport { /// as any other I/O failure here. fn move_viewport( terminal: &mut Terminal>, - rows: u16, - old_top: u16, - target: u16, + plan: viewport::Move, ) -> io::Result<()> { - let scroll = viewport::scroll_up_needed(old_top, target); let mut stdout = io::stdout(); - if scroll > 0 { - execute!(stdout, ScrollUp(scroll))?; + if plan.scroll > 0 { + execute!(stdout, ScrollUp(plan.scroll))?; } - let clear = viewport::clear_from(old_top.saturating_sub(scroll), target); execute!( stdout, - MoveTo(0, clear), + MoveTo(0, plan.clear), Clear(ClearType::FromCursorDown), - MoveTo(0, target) + MoveTo(0, plan.top) )?; *terminal = Terminal::with_options( CrosstermBackend::new(io::stdout()), TerminalOptions { - viewport: Viewport::Inline(rows), + viewport: Viewport::Inline(plan.rows), }, )?; Ok(()) diff --git a/src/play/thinking.rs b/src/play/thinking.rs index 2f5578e..277a426 100644 --- a/src/play/thinking.rs +++ b/src/play/thinking.rs @@ -8,7 +8,7 @@ /// The sparkle pulse, one glyph per animation frame, dim to bright and /// back. Dice glyphs are reserved for real rolls when the dice tool /// arrives. -const SPARKS: [char; 8] = ['·', '✧', '✦', '✶', '✷', '✶', '✦', '✧']; +pub const SPARKS: [char; 8] = ['·', '✧', '✦', '✶', '✷', '✶', '✦', '✧']; /// The phrase shown while the DM is narrating. pub const STORYTELLING: &str = "storytelling"; diff --git a/src/play/viewport.rs b/src/play/viewport.rs index b3b5010..2ea7ae7 100644 --- a/src/play/viewport.rs +++ b/src/play/viewport.rs @@ -60,6 +60,69 @@ pub fn clear_from(current_top: u16, pinned_top: u16) -> u16 { current_top.min(pinned_top) } +/// The writes one change of the viewport's height takes, in the order +/// they go out. +/// +/// An implementation of [`ViewportRows`] does these three writes and then +/// builds the new terminal, and nothing else decides how far to scroll, +/// where to clear from, or where the new top belongs. The test double and +/// the real terminal both take their rows from here, so a screen a test +/// leaves behind is the screen a terminal is left holding. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct Move { + /// How many rows to scroll the screen up first. Zero means the screen + /// stays where it is. + pub scroll: u16, + /// The row to clear from, down to the last row of the screen. + pub clear: u16, + /// The row the new viewport's top lands on. + pub top: u16, + /// How many rows the new viewport holds. + pub rows: u16, +} + +impl Move { + /// The move that gives the viewport `rows` rows on a screen + /// `screen_height` rows tall, with its top on `current_top` now. + /// + /// The top follows [`next_top`], so a viewport that grows takes rows + /// from the transcript above and one that shrinks gives its rows back + /// below itself. + pub fn resize(screen_height: u16, rows: u16, current_top: u16) -> Self { + Self::to( + next_top(screen_height, rows, current_top), + rows, + current_top, + ) + } + + /// The move that puts a viewport of `rows` rows back on the last row + /// of a screen `screen_height` rows tall, with its top on + /// `current_top` now. + /// + /// The top follows [`pinned_top`], so the viewport lands flush with + /// the last row whether that moves it up or down. + pub fn repin(screen_height: u16, rows: u16, current_top: u16) -> Self { + Self::to(pinned_top(screen_height, rows), rows, current_top) + } + + /// The move from `current_top` to `target`, with the scroll and the + /// clear that take it there. + /// + /// The scroll comes first, and it carries the old top up with the + /// rest of the screen, so the clear starts from where that row sits + /// after the scroll rather than from where it sat before. + fn to(target: u16, rows: u16, current_top: u16) -> Self { + let scroll = scroll_up_needed(current_top, target); + Self { + scroll, + clear: clear_from(current_top.saturating_sub(scroll), target), + top: target, + rows, + } + } +} + /// Changes the height of the inline viewport pinned to the bottom of the /// screen. /// @@ -81,10 +144,14 @@ pub fn clear_from(current_top: u16, pinned_top: u16) -> u16 { /// ran past the last row. Shrinking leaves that row where it is and /// ends the viewport short of the last row of the screen, so clear from /// it down to take the rows the viewport gave back off the screen. +/// [`Move`] works out all three rows, so an implementation does its +/// writes in order and decides none of them itself. /// - A viewport that shrank stops short of the bottom of the screen until /// the rows the transcript puts above it push it back down. Ask for a -/// shrink only with rows waiting to go out, and put them out before the -/// next draw, or the prompt sits off the bottom of the screen. +/// shrink with rows waiting to go out, and put them out before the next +/// draw, and the prompt stays on the last row. Ask for one with nothing +/// waiting and the prompt sits off the bottom of the screen until rows +/// go out that fill the rows the viewport gave back. /// - Every resize costs exactly one cursor-position query, `ESC [ 6 n`, /// and crossterm blocks up to two seconds for the reply. Call this only /// when the number of rows really changes, never on every keystroke, diff --git a/src/play/viewport_tests.rs b/src/play/viewport_tests.rs index a5882a8..8a5df30 100644 --- a/src/play/viewport_tests.rs +++ b/src/play/viewport_tests.rs @@ -2,7 +2,7 @@ //! bottom of the screen: where its top belongs, and how much of the //! screen above has to scroll to get there without overwriting anything. -use super::{clear_from, next_top, pinned_top, scroll_up_needed}; +use super::{Move, clear_from, next_top, pinned_top, scroll_up_needed}; #[test] fn a_viewport_shorter_than_the_screen_sits_flush_with_the_last_row() { @@ -58,3 +58,55 @@ fn a_viewport_that_moves_down_clears_from_the_row_it_came_from() { fn a_viewport_that_moves_up_clears_from_the_row_it_lands_on() { assert_eq!(clear_from(15, 5), 5); } + +#[test] +fn a_resize_that_grows_scrolls_the_screen_and_clears_from_the_row_it_lands_on() { + assert_eq!( + Move::resize(24, 6, 20), + Move { + scroll: 2, + clear: 18, + top: 18, + rows: 6 + } + ); +} + +#[test] +fn a_resize_that_shrinks_keeps_its_top_and_clears_the_rows_below_it() { + assert_eq!( + Move::resize(24, 4, 18), + Move { + scroll: 0, + clear: 18, + top: 18, + rows: 4 + } + ); +} + +#[test] +fn a_repin_that_moves_down_clears_from_the_row_it_came_from() { + assert_eq!( + Move::repin(24, 4, 18), + Move { + scroll: 0, + clear: 18, + top: 20, + rows: 4 + } + ); +} + +#[test] +fn a_repin_that_moves_up_scrolls_the_screen_by_as_much_as_the_top_moves() { + assert_eq!( + Move::repin(24, 8, 20), + Move { + scroll: 4, + clear: 16, + top: 16, + rows: 8 + } + ); +} diff --git a/src/srd/fetch.rs b/src/srd/fetch.rs index f13f5f5..b8ad286 100644 --- a/src/srd/fetch.rs +++ b/src/srd/fetch.rs @@ -476,7 +476,7 @@ fn download(url: &str) -> Result, FetchError> { .map_err(|error| FetchError::Download(error.to_string())) } -fn sha256_hex(bytes: &[u8]) -> String { +pub(crate) fn sha256_hex(bytes: &[u8]) -> String { let mut hasher = Sha256::new(); hasher.update(bytes); hasher @@ -489,94 +489,10 @@ fn sha256_hex(bytes: &[u8]) -> String { #[cfg(test)] mod tests { use super::*; - use std::io::{Read, Write}; - use std::net::TcpListener; - use std::thread::JoinHandle; + use crate::srd::testing::{append_entry, fixture_meta, serve_once, tiny_tar_gz}; + use std::io::Write; use tempfile::TempDir; - /// Serves `body` once to the first connection it accepts, as a complete - /// HTTP/1.1 response, then returns the URL to fetch it from. - fn serve_once(body: Vec) -> (String, JoinHandle<()>) { - let listener = TcpListener::bind("127.0.0.1:0").unwrap(); - let addr = listener.local_addr().unwrap(); - let handle = std::thread::spawn(move || { - let (mut stream, _) = listener.accept().unwrap(); - let mut request = [0u8; 1024]; - let _ = stream.read(&mut request).unwrap(); - let header = format!( - "HTTP/1.1 200 OK\r\nContent-Length: {}\r\nConnection: close\r\n\r\n", - body.len() - ); - stream.write_all(header.as_bytes()).unwrap(); - stream.write_all(&body).unwrap(); - }); - (format!("http://{addr}/fixture.pdf"), handle) - } - - /// Appends a tar entry for `path` to `builder`, as a directory when - /// `contents` is `None` or as a file with that content otherwise. - fn append_entry(builder: &mut tar::Builder>, path: &str, contents: Option<&[u8]>) { - let mut header = tar::Header::new_gnu(); - header.set_path(path).unwrap(); - header.set_mode(0o644); - match contents { - Some(bytes) => { - header.set_size(bytes.len() as u64); - header.set_cksum(); - builder.append(&header, bytes).unwrap(); - } - None => { - header.set_size(0); - header.set_entry_type(tar::EntryType::Directory); - header.set_cksum(); - builder.append(&header, io::empty()).unwrap(); - } - } - } - - /// Builds a tiny gzipped tarball with one top-level directory (`top_dir`, - /// itself stored as a bare directory entry), shaped like the real - /// dnd.srd.5.2.1 repo: a numbered content directory and a top-level - /// licensing file that `should_vendor` keeps, alongside `.github/`, - /// `docs_original/`, and an unlisted top-level file that it drops. - fn tiny_tar_gz(top_dir: &str) -> Vec { - let mut builder = tar::Builder::new(Vec::new()); - append_entry(&mut builder, &format!("{top_dir}/"), None); - append_entry(&mut builder, &format!("{top_dir}/07_Spells/"), None); - append_entry( - &mut builder, - &format!("{top_dir}/07_Spells/Fireball.md"), - Some(b"# Fireball\n"), - ); - append_entry( - &mut builder, - &format!("{top_dir}/Legal.md"), - Some(b"CC-BY-4.0\n"), - ); - append_entry(&mut builder, &format!("{top_dir}/.github/"), None); - append_entry( - &mut builder, - &format!("{top_dir}/.github/FUNDING.yml"), - Some(b"github: []\n"), - ); - append_entry(&mut builder, &format!("{top_dir}/docs_original/"), None); - append_entry( - &mut builder, - &format!("{top_dir}/docs_original/dnd_srd_5.2.1_cc.pdf"), - Some(b"%PDF-fixture"), - ); - append_entry( - &mut builder, - &format!("{top_dir}/SRD-reForged.png"), - Some(b"\x89PNG-fixture"), - ); - let tar_bytes = builder.into_inner().unwrap(); - - let mut encoder = flate2::write::GzEncoder::new(Vec::new(), flate2::Compression::fast()); - encoder.write_all(&tar_bytes).unwrap(); - encoder.finish().unwrap() - } - /// Builds a tarball where a file entry (`07_Spells`) is later reused as /// a directory prefix (`07_Spells/b.md`), so unpacking it hits a real /// filesystem collision when it tries to create `07_Spells` as a @@ -659,11 +575,11 @@ mod tests { } /// Builds a tarball with a symlink entry (`07_Spells/x`, targeting - /// `/tmp`) followed by a regular file entry whose path walks through - /// that symlink (`07_Spells/x/evil.md`), the way a malicious upstream - /// commit could try to write outside the unpack destination via a - /// symlinked directory rather than a `..` path component. - fn tar_gz_with_a_symlink_escape(top_dir: &str) -> Vec { + /// `symlink_target`) followed by a regular file entry whose path walks + /// through that symlink (`07_Spells/x/evil.md`), the way a malicious + /// upstream commit could try to write outside the unpack destination + /// via a symlinked directory rather than a `..` path component. + fn tar_gz_with_a_symlink_escape(top_dir: &str, symlink_target: &Path) -> Vec { let mut builder = tar::Builder::new(Vec::new()); append_entry(&mut builder, &format!("{top_dir}/"), None); append_entry(&mut builder, &format!("{top_dir}/07_Spells/"), None); @@ -673,7 +589,7 @@ mod tests { .set_path(format!("{top_dir}/07_Spells/x")) .unwrap(); symlink_header.set_entry_type(tar::EntryType::Symlink); - symlink_header.set_link_name("/tmp").unwrap(); + symlink_header.set_link_name(symlink_target).unwrap(); symlink_header.set_mode(0o777); symlink_header.set_size(0); symlink_header.set_cksum(); @@ -691,17 +607,18 @@ mod tests { encoder.finish().unwrap() } - fn fixture_meta() -> SrdMeta { - SrdMeta { - version: "5.2.1".to_string(), - pdf_path: "sources/SRD_CC_v5.2.1.pdf".to_string(), - pdf_url: "https://example.test/srd.pdf".to_string(), - pdf_sha256: "a".repeat(64), - text_path: "sources/dnd.srd.5.2.1/".to_string(), - text_url: "https://example.test/dnd.srd.5.2.1".to_string(), - text_commit: "deadbeef".to_string(), - text_tarball_sha256: "b".repeat(64), - } + #[test] + fn sha256_hex_matches_the_published_fips_180_vector_for_abc() { + // Every other test in this module computes its expected hash by + // calling `sha256_hex` itself, so a bug that truncates or + // otherwise corrupts the hex encoding would pass all of them. + // This pins one published test vector as a literal, so the + // function's actual output is checked against a value nothing in + // this file derived. + assert_eq!( + sha256_hex(b"abc"), + "ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad" + ); } #[test] @@ -1279,7 +1196,13 @@ mod tests { #[test] fn fetch_text_rejects_a_symlink_entry() { - let body = tar_gz_with_a_symlink_escape("dnd.srd.5.2.1-deadbeef"); + // The symlink targets a directory under this test's own temp root, + // rather than a fixed path like `/tmp`, so the assertion below + // checks the actual destination of a real escape instead of a + // path an unrelated file could already occupy, or a real escape + // could land outside of. + let escape_dir = TempDir::new().unwrap(); + let body = tar_gz_with_a_symlink_escape("dnd.srd.5.2.1-deadbeef", escape_dir.path()); let expected = sha256_hex(&body); let (url, server) = serve_once(body); let destination_dir = TempDir::new().unwrap(); @@ -1297,7 +1220,7 @@ mod tests { assert!(matches!(error, FetchError::UnsafeEntry(_))); assert!(error.to_string().contains("07_Spells/x")); assert!(error.to_string().contains("Symlink")); - assert!(!Path::new("/tmp/evil.md").exists()); + assert!(!escape_dir.path().join("evil.md").exists()); server.join().unwrap(); } diff --git a/src/srd/mod.rs b/src/srd/mod.rs index 65c2944..1f6dac2 100644 --- a/src/srd/mod.rs +++ b/src/srd/mod.rs @@ -1,4 +1,6 @@ pub mod fetch; +#[cfg(test)] +pub(crate) mod testing; pub mod verify; /// Where the SRD 5.2 corpus layer lives, relative to the repository root. diff --git a/src/srd/testing.rs b/src/srd/testing.rs new file mode 100644 index 0000000..e680f8c --- /dev/null +++ b/src/srd/testing.rs @@ -0,0 +1,112 @@ +//! Test-support fixtures shared by srd's own tests and by anything else +//! in the crate that exercises `fetch`: a loopback HTTP server that +//! serves one canned response, tarball builders for that server to +//! serve, and a fixture `SrdMeta`. + +use std::io::{Read, Write}; +use std::net::TcpListener; +use std::thread::JoinHandle; + +use super::fetch::SrdMeta; + +/// Serves `body` once to the first connection it accepts, as a complete +/// HTTP/1.1 response, then returns the URL to fetch it from. +pub(crate) fn serve_once(body: Vec) -> (String, JoinHandle<()>) { + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + let addr = listener.local_addr().unwrap(); + let handle = std::thread::spawn(move || { + let (mut stream, _) = listener.accept().unwrap(); + let mut request = [0u8; 1024]; + let _ = stream.read(&mut request).unwrap(); + let header = format!( + "HTTP/1.1 200 OK\r\nContent-Length: {}\r\nConnection: close\r\n\r\n", + body.len() + ); + stream.write_all(header.as_bytes()).unwrap(); + stream.write_all(&body).unwrap(); + }); + (format!("http://{addr}/fixture.pdf"), handle) +} + +/// Appends a tar entry for `path` to `builder`, as a directory when +/// `contents` is `None` or as a file with that content otherwise. +pub(crate) fn append_entry( + builder: &mut tar::Builder>, + path: &str, + contents: Option<&[u8]>, +) { + let mut header = tar::Header::new_gnu(); + header.set_path(path).unwrap(); + header.set_mode(0o644); + match contents { + Some(bytes) => { + header.set_size(bytes.len() as u64); + header.set_cksum(); + builder.append(&header, bytes).unwrap(); + } + None => { + header.set_size(0); + header.set_entry_type(tar::EntryType::Directory); + header.set_cksum(); + builder.append(&header, std::io::empty()).unwrap(); + } + } +} + +/// Builds a tiny gzipped tarball with one top-level directory (`top_dir`, +/// itself stored as a bare directory entry), shaped like the real +/// dnd.srd.5.2.1 repo: a numbered content directory and a top-level +/// licensing file that `should_vendor` keeps, alongside `.github/`, +/// `docs_original/`, and an unlisted top-level file that it drops. +pub(crate) fn tiny_tar_gz(top_dir: &str) -> Vec { + let mut builder = tar::Builder::new(Vec::new()); + append_entry(&mut builder, &format!("{top_dir}/"), None); + append_entry(&mut builder, &format!("{top_dir}/07_Spells/"), None); + append_entry( + &mut builder, + &format!("{top_dir}/07_Spells/Fireball.md"), + Some(b"# Fireball\n"), + ); + append_entry( + &mut builder, + &format!("{top_dir}/Legal.md"), + Some(b"CC-BY-4.0\n"), + ); + append_entry(&mut builder, &format!("{top_dir}/.github/"), None); + append_entry( + &mut builder, + &format!("{top_dir}/.github/FUNDING.yml"), + Some(b"github: []\n"), + ); + append_entry(&mut builder, &format!("{top_dir}/docs_original/"), None); + append_entry( + &mut builder, + &format!("{top_dir}/docs_original/dnd_srd_5.2.1_cc.pdf"), + Some(b"%PDF-fixture"), + ); + append_entry( + &mut builder, + &format!("{top_dir}/SRD-reForged.png"), + Some(b"\x89PNG-fixture"), + ); + let tar_bytes = builder.into_inner().unwrap(); + + let mut encoder = flate2::write::GzEncoder::new(Vec::new(), flate2::Compression::fast()); + encoder.write_all(&tar_bytes).unwrap(); + encoder.finish().unwrap() +} + +/// A fixture `SrdMeta`, standing in for the pinned SRD 5.2.1 record with +/// values a test can distinguish from the real thing at a glance. +pub(crate) fn fixture_meta() -> SrdMeta { + SrdMeta { + version: "5.2.1".to_string(), + pdf_path: "sources/SRD_CC_v5.2.1.pdf".to_string(), + pdf_url: "https://example.test/srd.pdf".to_string(), + pdf_sha256: "a".repeat(64), + text_path: "sources/dnd.srd.5.2.1/".to_string(), + text_url: "https://example.test/dnd.srd.5.2.1".to_string(), + text_commit: "deadbeef".to_string(), + text_tarball_sha256: "b".repeat(64), + } +} diff --git a/src/srd/verify/fidelity.rs b/src/srd/verify/fidelity.rs index 629f2fd..7beb3dc 100644 --- a/src/srd/verify/fidelity.rs +++ b/src/srd/verify/fidelity.rs @@ -387,8 +387,13 @@ mod tests { let source = PathBuf::from("sources/dnd.srd.5.2.1/11_Monsters/Monsters_Each/Rat.md"); let source_text = "# Rat\n\n*Small Beast, Unaligned*\n\n**AC** 10\n\n**HP** 1 (1d4)\n\n**Speed** 5 ft.\n\n| | MOD | SAVE | | MOD | SAVE | | MOD | SAVE |\n| :- | :- | :- | :- | :- | :- | :- | :- | :- |\n| **Str 1** | +0 | +0 | **Dex 1** | +0 | +0 | **Con 1** | +0 | +0 |\n| **Int 1** | +0 | +0 | **Wis 1** | +0 | +0 | **Cha 1** | +0 | +0 |\n\n**CR** 0\n\n## Actions\n\nBite.\n"; write_file(&layer_root.path().join(&source), source_text); - let body = stat_block::source_outside_region(source_text); - let file = corpus_file(Kind::Monster, &body); + // `source_text` with its stat-block region cut, frozen as a literal + // rather than computed by calling `stat_block::source_outside_region` + // itself, so a region-boundary bug in that function shows up as a + // fidelity mismatch here instead of shifting the fixture and the + // check together. + let body = "# Rat\n\n*Small Beast, Unaligned*\n\n## Actions\n\nBite.\n"; + let file = corpus_file(Kind::Monster, body); fn ability() -> crate::srd::verify::monster::Ability { crate::srd::verify::monster::Ability { score: 1, diff --git a/src/srd/verify/fixtures.rs b/src/srd/verify/fixtures.rs index ee41afa..1bfa0ce 100644 --- a/src/srd/verify/fixtures.rs +++ b/src/srd/verify/fixtures.rs @@ -43,6 +43,12 @@ pub const ANIMAL_SOURCE: &str = "sources/dnd.srd.5.2.1/12_Animals/Animals_Each/W const FIREBALL_BODY: &str = "# Fireball\n\n*Level 3 Evocation (Sorcerer, Wizard)*\n\n**Casting Time:** Action\n\n**Range:** 150 feet\n\n**Components:** V, S, M\n\n**Duration:** Instantaneous\n\nA bright streak flashes from you.\n"; const GOBLIN_SOURCE_TEXT: &str = "# Goblin Warrior\n\n*Small Fey (Goblinoid), Chaotic Neutral*\n\n**AC** 15\n\n**HP** 10 (3d6)\n\n**Speed** 30 ft.\n\n| | MOD | SAVE | | MOD | SAVE | | MOD | SAVE |\n| :- | :- | :- | :- | :- | :- | :- | :- | :- |\n| **Str 8** | -1 | -1 | **Dex 15** | +2 | +2 | **Con 10** | +0 | +0 |\n| **Int 10** | +0 | +0 | **Wis 8** | -1 | -1 | **Cha 8** | -1 | -1 |\n\n**CR** 1/4\n\n## Actions\n\n***Scimitar.*** *Melee Attack Roll:* +4, reach 5 ft.\n"; +/// `GOBLIN_SOURCE_TEXT` with its stat-block region cut, the same way +/// `stat_block::source_outside_region` cuts it. Frozen as a literal +/// rather than computed at fixture-build time, so a region-boundary bug +/// in that function shows up as a fidelity mismatch here instead of +/// shifting the fixture and the check together. +const GOBLIN_BODY: &str = "# Goblin Warrior\n\n*Small Fey (Goblinoid), Chaotic Neutral*\n\n## Actions\n\n***Scimitar.*** *Melee Attack Roll:* +4, reach 5 ft.\n"; const AMULET_SOURCE_TEXT: &str = "# Amulet of Health\n\n*Wondrous Item, Rare (Requires Attunement)*\n\nYour Constitution is 19 while you wear this amulet.\n"; const AMULET_FIELD_LINES: &str = @@ -57,6 +63,9 @@ const SPECIES_BODY: &str = "# Elf\n\nElves are a magical people.\n"; const BACKGROUND_BODY: &str = "# Acolyte\n\nYou spent your life in service to a temple.\n"; const ANIMAL_SOURCE_TEXT: &str = "# Wolf\n\n*Medium Beast, Unaligned*\n\n**AC** 12\n\n**HP** 11 (2d8 + 2)\n\n**Speed** 40 ft.\n\n| | MOD | SAVE | | MOD | SAVE | | MOD | SAVE |\n| :- | :- | :- | :- | :- | :- | :- | :- | :- |\n| **Str 12** | +1 | +1 | **Dex 15** | +2 | +2 | **Con 12** | +1 | +1 |\n| **Int 3** | -4 | -4 | **Wis 12** | +1 | +1 | **Cha 6** | -2 | -2 |\n\n**Skills** Perception +3, Stealth +4\n\n**CR** 1/4\n\n## Actions\n\n***Bite.*** *Melee Attack Roll:* +3, reach 5 ft.\n"; +/// `ANIMAL_SOURCE_TEXT` with its stat-block region cut. See +/// `GOBLIN_BODY` for why this is a frozen literal rather than computed. +const ANIMAL_BODY: &str = "# Wolf\n\n*Medium Beast, Unaligned*\n\n## Actions\n\n***Bite.*** *Melee Attack Roll:* +3, reach 5 ft.\n"; /// A small valid corpus layer: one entry per kind, its matching fake /// vendored source, and a README and meta.yaml that satisfy the @@ -84,11 +93,10 @@ impl Layer { ); layer.write(GOBLIN_SOURCE, GOBLIN_SOURCE_TEXT); - let goblin_body = super::stat_block::source_outside_region(GOBLIN_SOURCE_TEXT); layer.write( "rules/monsters/goblin-warrior.md", &format!( - "---\nname: Goblin Warrior\ntype: monster\nsource: {GOBLIN_SOURCE}\nsize: Small\ncreature_type: Fey (Goblinoid)\nalignment: Chaotic Neutral\nac: '15'\nhp: 10 (3d6)\nspeed: 30 ft.\ncr: 1/4\nabilities:\n str: {{score: 8, mod: -1, save: -1}}\n dex: {{score: 15, mod: 2, save: 2}}\n con: {{score: 10, mod: 0, save: 0}}\n int: {{score: 10, mod: 0, save: 0}}\n wis: {{score: 8, mod: -1, save: -1}}\n cha: {{score: 8, mod: -1, save: -1}}\n---\n{goblin_body}" + "---\nname: Goblin Warrior\ntype: monster\nsource: {GOBLIN_SOURCE}\nsize: Small\ncreature_type: Fey (Goblinoid)\nalignment: Chaotic Neutral\nac: '15'\nhp: 10 (3d6)\nspeed: 30 ft.\ncr: 1/4\nabilities:\n str: {{score: 8, mod: -1, save: -1}}\n dex: {{score: 15, mod: 2, save: 2}}\n con: {{score: 10, mod: 0, save: 0}}\n int: {{score: 10, mod: 0, save: 0}}\n wis: {{score: 8, mod: -1, save: -1}}\n cha: {{score: 8, mod: -1, save: -1}}\n---\n{GOBLIN_BODY}" ), ); @@ -153,11 +161,10 @@ impl Layer { ); layer.write(ANIMAL_SOURCE, ANIMAL_SOURCE_TEXT); - let animal_body = super::stat_block::source_outside_region(ANIMAL_SOURCE_TEXT); layer.write( "rules/animals/wolf.md", &format!( - "---\nname: Wolf\ntype: animal\nsource: {ANIMAL_SOURCE}\nsize: Medium\ncreature_type: Beast\nalignment: Unaligned\nac: '12'\nhp: 11 (2d8 + 2)\nspeed: 40 ft.\ncr: 1/4\nskills: Perception +3, Stealth +4\nabilities:\n str: {{score: 12, mod: 1, save: 1}}\n dex: {{score: 15, mod: 2, save: 2}}\n con: {{score: 12, mod: 1, save: 1}}\n int: {{score: 3, mod: -4, save: -4}}\n wis: {{score: 12, mod: 1, save: 1}}\n cha: {{score: 6, mod: -2, save: -2}}\n---\n{animal_body}" + "---\nname: Wolf\ntype: animal\nsource: {ANIMAL_SOURCE}\nsize: Medium\ncreature_type: Beast\nalignment: Unaligned\nac: '12'\nhp: 11 (2d8 + 2)\nspeed: 40 ft.\ncr: 1/4\nskills: Perception +3, Stealth +4\nabilities:\n str: {{score: 12, mod: 1, save: 1}}\n dex: {{score: 15, mod: 2, save: 2}}\n con: {{score: 12, mod: 1, save: 1}}\n int: {{score: 3, mod: -4, save: -4}}\n wis: {{score: 12, mod: 1, save: 1}}\n cha: {{score: 6, mod: -2, save: -2}}\n---\n{ANIMAL_BODY}" ), ); diff --git a/src/srd/verify/mod.rs b/src/srd/verify/mod.rs index 1bcc1dc..8eaa4fa 100644 --- a/src/srd/verify/mod.rs +++ b/src/srd/verify/mod.rs @@ -288,12 +288,10 @@ mod tests { let report = verify(layer_root.path()); - assert!( - report - .failures - .iter() - .any(|f| f.message.contains("Source") || f.message.contains("Different")) - ); + assert!(report.failures.iter().any(|f| { + f.message + .contains("source has \"Source\" but the corpus body has \"Different\"") + })); } #[test] diff --git a/src/srd/verify/stat_block.rs b/src/srd/verify/stat_block.rs index 7aae683..39f39d9 100644 --- a/src/srd/verify/stat_block.rs +++ b/src/srd/verify/stat_block.rs @@ -411,11 +411,16 @@ mod tests { check_accounting(GOBLIN, &fields, &path, &mut failures); - assert!(!failures.is_empty()); - assert!( - failures - .iter() - .any(|f| f.message.contains("Skills") || f.message.contains("Senses")) + assert_eq!(failures.len(), 2); + assert_eq!( + failures[0].message, + "frontmatter 'Skills' value 'Darkvision 60 ft.; Passive Perception 9' does not \ + match the stat block's Skills segment 'Stealth +6'" + ); + assert_eq!( + failures[1].message, + "frontmatter 'Senses' value 'Stealth +6' does not match the stat block's Senses \ + segment 'Darkvision 60 ft.; Passive Perception 9'" ); } @@ -454,11 +459,16 @@ mod tests { check_accounting(GOBLIN, &fields, &path, &mut failures); - assert!(!failures.is_empty()); - assert!( - failures - .iter() - .any(|f| f.message.contains("abilities.str") || f.message.contains("abilities.dex")) + assert_eq!(failures.len(), 2); + assert_eq!( + failures[0].message, + "frontmatter abilities.str (score 15, mod +2, save +2) does not match the stat \ + block's str row (score 8, mod -1, save -1)" + ); + assert_eq!( + failures[1].message, + "frontmatter abilities.dex (score 8, mod -1, save -1) does not match the stat \ + block's dex row (score 15, mod +2, save +2)" ); } @@ -471,11 +481,14 @@ mod tests { check_accounting(GOBLIN, &fields, &path, &mut failures); - assert!(!failures.is_empty()); - assert!( - failures - .iter() - .any(|f| f.message.contains("'AC'") || f.message.contains("'HP'")) + assert_eq!(failures.len(), 2); + assert_eq!( + failures[0].message, + "frontmatter 'AC' value '10 (3d6)' does not match the stat block's AC segment '15'" + ); + assert_eq!( + failures[1].message, + "frontmatter 'HP' value '15' does not match the stat block's HP segment '10 (3d6)'" ); } diff --git a/tests/corpus.rs b/tests/corpus.rs index 13ca393..0933d75 100644 --- a/tests/corpus.rs +++ b/tests/corpus.rs @@ -101,12 +101,17 @@ fn count_md_files(kind_dir: &str) -> usize { .count() } +// These counts are exact, not floors: pinning the real committed count +// catches a corpus that loses half its entries just as well as one that +// loses one, which a "some minimum" floor would miss. Update the number +// only when the corpus is deliberately grown or trimmed. + #[test] -fn feats_directory_has_at_least_seventeen_entries() { - assert!(count_md_files("feats") >= 17); +fn feats_directory_has_exactly_seventeen_entries() { + assert_eq!(count_md_files("feats"), 17); } #[test] -fn glossary_directory_has_at_least_one_hundred_fifty_entries() { - assert!(count_md_files("glossary") >= 150); +fn glossary_directory_has_exactly_one_hundred_fifty_four_entries() { + assert_eq!(count_md_files("glossary"), 154); }