From 2676e7f36ba25e3d89983d547e998ad42c00b61f Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 11 Nov 2021 09:42:21 -0600 Subject: [PATCH 1/8] refactor: Consolidate binary check --- src/runner.rs | 61 ++++++++++++++++++++++++++++++--------------------- 1 file changed, 36 insertions(+), 25 deletions(-) diff --git a/src/runner.rs b/src/runner.rs index cc2eea2d..6c236083 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -276,33 +276,33 @@ impl Case { } }; if let Some(mut stdout) = output.stdout { - if !step.binary { - stdout = stdout.utf8(); - } - if stdout.is_ok() { - stdout = match self.validate_stream(stdout, step.expected_stdout.as_ref(), mode) { - Ok(stdout) => stdout, - Err(stdout) => { - ok = false; - stdout - } - }; - } + stdout = match self.validate_stream( + stdout, + step.expected_stdout.as_ref(), + step.binary, + mode, + ) { + Ok(stdout) => stdout, + Err(stdout) => { + ok = false; + stdout + } + }; output.stdout = Some(stdout); } if let Some(mut stderr) = output.stderr { - if !step.binary { - stderr = stderr.utf8(); - } - if stderr.is_ok() { - stderr = match self.validate_stream(stderr, step.expected_stderr.as_ref(), mode) { - Ok(stderr) => stderr, - Err(stderr) => { - ok = false; - stderr - } - }; - } + stderr = match self.validate_stream( + stderr, + step.expected_stderr.as_ref(), + step.binary, + mode, + ) { + Ok(stderr) => stderr, + Err(stderr) => { + ok = false; + stderr + } + }; output.stderr = Some(stderr); } @@ -350,6 +350,7 @@ impl Case { &self, mut stream: Stream, expected_content: Option<&crate::File>, + binary: bool, mode: &Mode, ) -> Result { if let Mode::Dump(root) = mode { @@ -364,7 +365,17 @@ impl Case { stream.status = StreamStatus::Failure(e); stream })?; - } else if let Some(expected_content) = expected_content { + return Ok(stream); + } + + if !binary { + stream = stream.utf8(); + if !stream.is_ok() { + return Err(stream); + } + } + + if let Some(expected_content) = expected_content { if let crate::File::Text(e) = &expected_content { stream.content = stream.content.map_text(|t| crate::elide::normalize(t, e)); } From ad3a8d94f081523056f61e02106eacf00db47d2e Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 11 Nov 2021 09:56:09 -0600 Subject: [PATCH 2/8] refactor: Split out all stream logic --- src/runner.rs | 84 ++++++++++++++++++++++++++++++++------------------- 1 file changed, 53 insertions(+), 31 deletions(-) diff --git a/src/runner.rs b/src/runner.rs index 6c236083..d4436f15 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -268,43 +268,20 @@ impl Case { // For dump mode's sake, allow running all let mut ok = output.is_ok(); - let mut output = match self.validate_spawn(output, step.expected_status()) { + let output = match self.validate_spawn(output, step.expected_status()) { + Ok(output) => output, + Err(output) => { + ok = false; + output + } + }; + let output = match self.validate_streams(output, step, mode) { Ok(output) => output, Err(output) => { ok = false; output } }; - if let Some(mut stdout) = output.stdout { - stdout = match self.validate_stream( - stdout, - step.expected_stdout.as_ref(), - step.binary, - mode, - ) { - Ok(stdout) => stdout, - Err(stdout) => { - ok = false; - stdout - } - }; - output.stdout = Some(stdout); - } - if let Some(mut stderr) = output.stderr { - stderr = match self.validate_stream( - stderr, - step.expected_stderr.as_ref(), - step.binary, - mode, - ) { - Ok(stderr) => stderr, - Err(stderr) => { - ok = false; - stderr - } - }; - output.stderr = Some(stderr); - } if ok { Ok(output) @@ -346,6 +323,51 @@ impl Case { Ok(output) } + fn validate_streams( + &self, + mut output: Output, + step: &crate::schema::Step, + mode: &Mode, + ) -> Result { + let mut ok = true; + if let Some(mut stdout) = output.stdout { + stdout = match self.validate_stream( + stdout, + step.expected_stdout.as_ref(), + step.binary, + mode, + ) { + Ok(stdout) => stdout, + Err(stdout) => { + ok = false; + stdout + } + }; + output.stdout = Some(stdout); + } + if let Some(mut stderr) = output.stderr { + stderr = match self.validate_stream( + stderr, + step.expected_stderr.as_ref(), + step.binary, + mode, + ) { + Ok(stderr) => stderr, + Err(stderr) => { + ok = false; + stderr + } + }; + output.stderr = Some(stderr); + } + + if ok { + Ok(output) + } else { + Err(output) + } + } + fn validate_stream( &self, mut stream: Stream, From f3d3d3fe68beb98d5c28485055faa2b350f957f2 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 11 Nov 2021 10:01:49 -0600 Subject: [PATCH 3/8] refactor: Split out dump logic --- src/runner.rs | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/src/runner.rs b/src/runner.rs index d4436f15..3a512a46 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -376,18 +376,7 @@ impl Case { mode: &Mode, ) -> Result { if let Mode::Dump(root) = mode { - let stdout_path = root.join( - self.path - .with_extension(stream.stream.as_str()) - .file_name() - .unwrap(), - ); - stream.content.write_to(&stdout_path).map_err(|e| { - let mut stream = stream.clone(); - stream.status = StreamStatus::Failure(e); - stream - })?; - return Ok(stream); + return self.dump_stream(root, stream); } if !binary { @@ -431,6 +420,23 @@ impl Case { Ok(stream) } + fn dump_stream(&self, root: &std::path::Path, stream: Stream) -> Result { + let stream_path = root.join( + self.path + .with_extension(stream.stream.as_str()) + .file_name() + .unwrap(), + ); + stream.content.write_to(&stream_path).map_err(|e| { + let mut stream = stream.clone(); + if stream.is_ok() { + stream.status = StreamStatus::Failure(e); + } + stream + })?; + return Ok(stream); + } + fn validate_fs( &self, actual_root: &std::path::Path, From e1fff4b84a71dbc77d455818a66986fc63606fce Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 11 Nov 2021 10:11:50 -0600 Subject: [PATCH 4/8] refactor: Pull out overwrite logic This will centralize file format knowledge --- src/runner.rs | 161 +++++++++++++++++++++++++------------------------- src/schema.rs | 28 +++++++++ 2 files changed, 110 insertions(+), 79 deletions(-) diff --git a/src/runner.rs b/src/runner.rs index 3a512a46..60ea2d42 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -330,35 +330,52 @@ impl Case { mode: &Mode, ) -> Result { let mut ok = true; - if let Some(mut stdout) = output.stdout { - stdout = match self.validate_stream( - stdout, - step.expected_stdout.as_ref(), - step.binary, - mode, - ) { - Ok(stdout) => stdout, - Err(stdout) => { + + output.stdout = + match self.validate_stream(output.stdout, step.expected_stdout.as_ref(), step.binary) { + Ok(stream) => stream, + Err(stream) => { ok = false; - stdout + stream } }; - output.stdout = Some(stdout); - } - if let Some(mut stderr) = output.stderr { - stderr = match self.validate_stream( - stderr, - step.expected_stderr.as_ref(), - step.binary, - mode, - ) { - Ok(stderr) => stderr, - Err(stderr) => { + output.stderr = + match self.validate_stream(output.stderr, step.expected_stderr.as_ref(), step.binary) { + Ok(stream) => stream, + Err(stream) => { ok = false; - stderr + stream } }; - output.stderr = Some(stderr); + + match mode { + Mode::Dump(root) => { + output.stdout = match self.dump_stream(root, output.stdout) { + Ok(stream) => stream, + Err(stream) => { + ok = false; + stream + } + }; + output.stderr = match self.dump_stream(root, output.stderr) { + Ok(stream) => stream, + Err(stream) => { + ok = false; + stream + } + }; + } + Mode::Overwrite => { + if let Err(_) = crate::schema::TryCmd::overwrite( + &self.path, + step.id.as_deref(), + output.stdout.as_ref().map(|s| &s.content), + output.stderr.as_ref().map(|s| &s.content), + ) { + ok = false; + } + } + Mode::Fail => {} } if ok { @@ -370,71 +387,57 @@ impl Case { fn validate_stream( &self, - mut stream: Stream, + stream: Option, expected_content: Option<&crate::File>, binary: bool, - mode: &Mode, - ) -> Result { - if let Mode::Dump(root) = mode { - return self.dump_stream(root, stream); - } - - if !binary { - stream = stream.utf8(); - if !stream.is_ok() { - return Err(stream); + ) -> Result, Option> { + if let Some(mut stream) = stream { + if !binary { + stream = stream.utf8(); + if !stream.is_ok() { + return Err(Some(stream)); + } } - } - if let Some(expected_content) = expected_content { - if let crate::File::Text(e) = &expected_content { - stream.content = stream.content.map_text(|t| crate::elide::normalize(t, e)); - } - if stream.content != *expected_content { - match mode { - Mode::Fail => { - stream.status = StreamStatus::Expected(expected_content.clone()); - return Err(stream); - } - Mode::Overwrite => { - let stdout_path = self.path.with_extension(stream.stream.as_str()); - if stdout_path.exists() { - stream.content.write_to(&stdout_path).map_err(|e| { - let mut stream = stream.clone(); - stream.status = StreamStatus::Failure(e); - stream - })?; - stream.status = StreamStatus::Expected(expected_content.clone()); - return Ok(stream); - } else { - // `.trycmd` files do not support overwrite, see issue #23 - stream.status = StreamStatus::Expected(expected_content.clone()); - return Err(stream); - } - } - Mode::Dump(_) => unreachable!("handled earlier"), + if let Some(expected_content) = expected_content { + if let crate::File::Text(e) = &expected_content { + stream.content = stream.content.map_text(|t| crate::elide::normalize(t, e)); + } + if stream.content != *expected_content { + stream.status = StreamStatus::Expected(expected_content.clone()); + return Err(Some(stream)); } } - } - Ok(stream) + Ok(Some(stream)) + } else { + Ok(None) + } } - fn dump_stream(&self, root: &std::path::Path, stream: Stream) -> Result { - let stream_path = root.join( - self.path - .with_extension(stream.stream.as_str()) - .file_name() - .unwrap(), - ); - stream.content.write_to(&stream_path).map_err(|e| { - let mut stream = stream.clone(); - if stream.is_ok() { - stream.status = StreamStatus::Failure(e); - } - stream - })?; - return Ok(stream); + fn dump_stream( + &self, + root: &std::path::Path, + stream: Option, + ) -> Result, Option> { + if let Some(stream) = stream { + let stream_path = root.join( + self.path + .with_extension(stream.stream.as_str()) + .file_name() + .unwrap(), + ); + stream.content.write_to(&stream_path).map_err(|e| { + let mut stream = stream.clone(); + if stream.is_ok() { + stream.status = StreamStatus::Failure(e); + } + stream + })?; + Ok(Some(stream)) + } else { + Ok(None) + } } fn validate_fs( diff --git a/src/schema.rs b/src/schema.rs index 08fafda7..cef9c0f5 100644 --- a/src/schema.rs +++ b/src/schema.rs @@ -81,6 +81,34 @@ impl TryCmd { Ok(sequence) } + pub(crate) fn overwrite( + path: &std::path::Path, + id: Option<&str>, + stdout: Option<&crate::File>, + stderr: Option<&crate::File>, + ) -> Result<(), String> { + if let Some(ext) = path.extension() { + if ext == std::ffi::OsStr::new("toml") { + assert_eq!(id, None); + if let Some(stdout) = stdout { + let stdout_path = path.with_extension("stdout"); + stdout.write_to(&stdout_path)?; + } + if let Some(stderr) = stderr { + let stderr_path = path.with_extension("stderr"); + stderr.write_to(&stderr_path)?; + } + } else if ext == std::ffi::OsStr::new("trycmd") || ext == std::ffi::OsStr::new("md") { + } else { + return Err(format!("Unsupported extension: {}", ext.to_string_lossy())); + } + } else { + return Err("No extension".into()); + } + + Ok(()) + } + fn parse_trycmd(s: &str) -> Result { let mut steps = Vec::new(); From 593b6875bf4942b666cd0fbef572d6ba9cb58980 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 11 Nov 2021 12:06:34 -0600 Subject: [PATCH 5/8] fix: Overwrite support for `*.trycmd` files The downside is this makes it so overwrite runs are considered failures, even obscuring the real failure or making the user think it didn't work. Fixes #23 --- src/filesystem.rs | 101 ++++++++++++++++++++++++++++++++++++++++++++++ src/runner.rs | 72 ++++++++++++++++----------------- src/schema.rs | 53 +++++++++++++++++++++++- 3 files changed, 188 insertions(+), 38 deletions(-) diff --git a/src/filesystem.rs b/src/filesystem.rs index e1ee1113..3e0c6fd7 100644 --- a/src/filesystem.rs +++ b/src/filesystem.rs @@ -24,6 +24,35 @@ impl File { .map_err(|e| format!("Failed to write {}: {}", path.display(), e)) } + pub(crate) fn replace_lines( + &mut self, + line_nums: std::ops::Range, + text: &str, + ) -> Result<(), String> { + let mut output_lines = String::new(); + + let s = self + .as_str() + .ok_or("Binary file can't have lines replaced")?; + for (line_num, line) in crate::lines::LinesWithTerminator::new(s) + .enumerate() + .map(|(i, l)| (i + 1, l)) + { + if line_num == line_nums.start { + output_lines.push_str(text); + if !text.is_empty() && !text.ends_with("\n") { + output_lines.push('\n'); + } + } + if !line_nums.contains(&line_num) { + output_lines.push_str(line); + } + } + + *self = Self::Text(output_lines); + Ok(()) + } + pub(crate) fn map_text(self, op: impl FnOnce(&str) -> String) -> Self { match self { Self::Binary(data) => Self::Binary(data), @@ -70,6 +99,13 @@ impl File { } } + pub(crate) fn as_str(&self) -> Option<&str> { + match self { + Self::Binary(_) => None, + Self::Text(data) => Some(data.as_str()), + } + } + pub(crate) fn as_bytes(&self) -> &[u8] { match self { Self::Binary(data) => data, @@ -249,3 +285,68 @@ fn symlink_to_file(link: &std::path::Path, target: &std::path::Path) -> Result<( fn symlink_to_file(link: &std::path::Path, target: &std::path::Path) -> Result<(), std::io::Error> { std::os::unix::fs::symlink(target, link) } + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn replace_lines_same_line_count() { + let input = "One\nTwo\nThree"; + let line_nums = 2..3; + let replacement = "World\n"; + let expected = File::Text("One\nWorld\nThree".into()); + + let mut actual = File::Text(input.into()); + actual.replace_lines(line_nums, replacement).unwrap(); + assert_eq!(expected, actual); + } + + #[test] + fn replace_lines_grow() { + let input = "One\nTwo\nThree"; + let line_nums = 2..3; + let replacement = "World\nTrees\n"; + let expected = File::Text("One\nWorld\nTrees\nThree".into()); + + let mut actual = File::Text(input.into()); + actual.replace_lines(line_nums, replacement).unwrap(); + assert_eq!(expected, actual); + } + + #[test] + fn replace_lines_shrink() { + let input = "One\nTwo\nThree"; + let line_nums = 2..3; + let replacement = ""; + let expected = File::Text("One\nThree".into()); + + let mut actual = File::Text(input.into()); + actual.replace_lines(line_nums, replacement).unwrap(); + assert_eq!(expected, actual); + } + + #[test] + fn replace_lines_no_trailing() { + let input = "One\nTwo\nThree"; + let line_nums = 2..3; + let replacement = "World"; + let expected = File::Text("One\nWorld\nThree".into()); + + let mut actual = File::Text(input.into()); + actual.replace_lines(line_nums, replacement).unwrap(); + assert_eq!(expected, actual); + } + + #[test] + fn replace_lines_empty_range() { + let input = "One\nTwo\nThree"; + let line_nums = 2..2; + let replacement = "World\n"; + let expected = File::Text("One\nWorld\nTwo\nThree".into()); + + let mut actual = File::Text(input.into()); + actual.replace_lines(line_nums, replacement).unwrap(); + assert_eq!(expected, actual); + } +} diff --git a/src/runner.rs b/src/runner.rs index 60ea2d42..5ff8df8b 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -185,12 +185,44 @@ impl Case { step.expected_status = Some(crate::schema::CommandStatus::Skipped); } - let step_status = self.run_step(step, cwd.as_deref(), mode, bins); - if step_status.is_err() { + let step_status = self.run_step(step, cwd.as_deref(), bins); + if step_status.is_err() && *mode == Mode::Fail { prior_step_failed = true; } outputs.push(step_status); } + match mode { + Mode::Dump(root) => { + for output in &mut outputs { + let output = match output { + Ok(output) => output, + Err(output) => output, + }; + output.stdout = match self.dump_stream(root, output.stdout.take()) { + Ok(stream) => stream, + Err(stream) => stream, + }; + output.stderr = match self.dump_stream(root, output.stderr.take()) { + Ok(stream) => stream, + Err(stream) => stream, + }; + } + } + Mode::Overwrite => { + // `rev()` to ensure we don't mess up our line number info + for output in outputs.iter().rev() { + if let Err(output) = output { + let _ = sequence.overwrite( + &self.path, + output.id.as_deref(), + output.stdout.as_ref().map(|s| &s.content), + output.stderr.as_ref().map(|s| &s.content), + ); + } + } + } + Mode::Fail => {} + } if sequence.fs.sandbox() { let mut ok = true; @@ -232,7 +264,6 @@ impl Case { &self, step: &mut crate::schema::Step, cwd: Option<&std::path::Path>, - mode: &Mode, bins: &crate::BinRegistry, ) -> Result { let output = if let Some(id) = step.id.clone() { @@ -266,7 +297,7 @@ impl Case { let cmd_output = step.to_output(cwd).map_err(|e| output.clone().error(e))?; let output = output.output(cmd_output); - // For dump mode's sake, allow running all + // For Mode::Dump's sake, allow running all let mut ok = output.is_ok(); let output = match self.validate_spawn(output, step.expected_status()) { Ok(output) => output, @@ -275,7 +306,7 @@ impl Case { output } }; - let output = match self.validate_streams(output, step, mode) { + let output = match self.validate_streams(output, step) { Ok(output) => output, Err(output) => { ok = false; @@ -327,7 +358,6 @@ impl Case { &self, mut output: Output, step: &crate::schema::Step, - mode: &Mode, ) -> Result { let mut ok = true; @@ -348,36 +378,6 @@ impl Case { } }; - match mode { - Mode::Dump(root) => { - output.stdout = match self.dump_stream(root, output.stdout) { - Ok(stream) => stream, - Err(stream) => { - ok = false; - stream - } - }; - output.stderr = match self.dump_stream(root, output.stderr) { - Ok(stream) => stream, - Err(stream) => { - ok = false; - stream - } - }; - } - Mode::Overwrite => { - if let Err(_) = crate::schema::TryCmd::overwrite( - &self.path, - step.id.as_deref(), - output.stdout.as_ref().map(|s| &s.content), - output.stderr.as_ref().map(|s| &s.content), - ) { - ok = false; - } - } - Mode::Fail => {} - } - if ok { Ok(output) } else { diff --git a/src/schema.rs b/src/schema.rs index cef9c0f5..ae3b49b0 100644 --- a/src/schema.rs +++ b/src/schema.rs @@ -82,6 +82,7 @@ impl TryCmd { } pub(crate) fn overwrite( + &self, path: &std::path::Path, id: Option<&str>, stdout: Option<&crate::File>, @@ -90,15 +91,33 @@ impl TryCmd { if let Some(ext) = path.extension() { if ext == std::ffi::OsStr::new("toml") { assert_eq!(id, None); + if let Some(stdout) = stdout { let stdout_path = path.with_extension("stdout"); stdout.write_to(&stdout_path)?; } + if let Some(stderr) = stderr { let stderr_path = path.with_extension("stderr"); stderr.write_to(&stderr_path)?; } } else if ext == std::ffi::OsStr::new("trycmd") || ext == std::ffi::OsStr::new("md") { + assert_eq!(stderr, Some(&crate::File::Text("".into()))); + if let (Some(id), Some(stdout)) = (id, stdout) { + let step = self + .steps + .iter() + .find(|s| s.id.as_deref() == Some(id)) + .expect("id is valid"); + let line_nums = step + .expected_stdout_source + .clone() + .expect("always present for .trycmd"); + let stdout = stdout.as_str().expect("already converted to Text"); + let mut raw = crate::File::read_from(path, false)?; + raw.replace_lines(line_nums, stdout)?; + raw.write_to(path)?; + } } else { return Err(format!("Unsupported extension: {}", ext.to_string_lossy())); } @@ -149,11 +168,13 @@ impl TryCmd { let mut expected_status = Some(CommandStatus::Success); let mut stdout = String::new(); let cmd_start; + let mut stdout_start; if let Some((line_num, line)) = lines.pop_front() { if let Some(raw) = line.strip_prefix("$ ") { cmdline.extend(shlex::Shlex::new(raw.trim())); cmd_start = line_num; + stdout_start = line_num + 1; } else { return Err(format!("Expected `$` on line {}, got `{}`", line_num, line)); } @@ -163,6 +184,7 @@ impl TryCmd { while let Some((line_num, line)) = lines.pop_front() { if let Some(raw) = line.strip_prefix("> ") { cmdline.extend(shlex::Shlex::new(raw.trim())); + stdout_start = line_num + 1; } else { lines.push_front((line_num, line)); break; @@ -171,20 +193,25 @@ impl TryCmd { if let Some((line_num, line)) = lines.pop_front() { if let Some(raw) = line.strip_prefix("? ") { expected_status = Some(raw.trim().parse::()?); + stdout_start = line_num + 1; } else { lines.push_front((line_num, line)); } } + let mut post_stdout_start = stdout_start; let mut block_done = false; while let Some((line_num, line)) = lines.pop_front() { if line.starts_with("$ ") { lines.push_front((line_num, line)); + post_stdout_start = line_num; break; } else if line.starts_with("```") { block_done = true; + post_stdout_start = line_num; break; } else { stdout.push_str(line); + post_stdout_start = line_num + 1; } } @@ -206,10 +233,15 @@ impl TryCmd { bin: Some(Bin::Name(bin)), args: cmdline, env, - expected_status, + stdin: None, stderr_to_stdout: true, + expected_status, + expected_stdout_source: Some(stdout_start..post_stdout_start), expected_stdout: Some(crate::File::Text(stdout)), - ..Default::default() + expected_stderr_source: None, + expected_stderr: None, + binary: false, + timeout: None, }; steps.push(step); if block_done { @@ -254,7 +286,9 @@ impl From for TryCmd { stdin: None, stderr_to_stdout, expected_status: status, + expected_stdout_source: None, expected_stdout: None, + expected_stderr_source: None, expected_stderr: None, binary, timeout, @@ -273,7 +307,9 @@ pub(crate) struct Step { pub(crate) stdin: Option, pub(crate) stderr_to_stdout: bool, pub(crate) expected_status: Option, + pub(crate) expected_stdout_source: Option>, pub(crate) expected_stdout: Option, + pub(crate) expected_stderr_source: Option>, pub(crate) expected_stderr: Option, pub(crate) binary: bool, pub(crate) timeout: Option, @@ -634,6 +670,7 @@ mod test { bin: Some(Bin::Name("cmd".into())), expected_status: Some(CommandStatus::Success), stderr_to_stdout: true, + expected_stdout_source: Some(4..4), expected_stdout: Some(crate::File::Text("".into())), expected_stderr: None, ..Default::default() @@ -660,6 +697,7 @@ $ cmd args: vec!["arg1".into(), "arg with space".into()], expected_status: Some(CommandStatus::Success), stderr_to_stdout: true, + expected_stdout_source: Some(4..4), expected_stdout: Some(crate::File::Text("".into())), expected_stderr: None, ..Default::default() @@ -686,6 +724,7 @@ $ cmd arg1 'arg with space' args: vec!["arg1".into(), "arg with space".into()], expected_status: Some(CommandStatus::Success), stderr_to_stdout: true, + expected_stdout_source: Some(5..5), expected_stdout: Some(crate::File::Text("".into())), expected_stderr: None, ..Default::default() @@ -720,6 +759,7 @@ $ cmd arg1 }, expected_status: Some(CommandStatus::Success), stderr_to_stdout: true, + expected_stdout_source: Some(4..4), expected_stdout: Some(crate::File::Text("".into())), expected_stderr: None, ..Default::default() @@ -745,6 +785,7 @@ $ KEY1=VALUE1 KEY2='VALUE2 with space' cmd bin: Some(Bin::Name("cmd".into())), expected_status: Some(CommandStatus::Skipped), stderr_to_stdout: true, + expected_stdout_source: Some(5..5), expected_stdout: Some(crate::File::Text("".into())), expected_stderr: None, ..Default::default() @@ -771,6 +812,7 @@ $ cmd bin: Some(Bin::Name("cmd".into())), expected_status: Some(CommandStatus::Code(-1)), stderr_to_stdout: true, + expected_stdout_source: Some(5..5), expected_stdout: Some(crate::File::Text("".into())), expected_stderr: None, ..Default::default() @@ -797,6 +839,7 @@ $ cmd bin: Some(Bin::Name("cmd".into())), expected_status: Some(CommandStatus::Success), stderr_to_stdout: true, + expected_stdout_source: Some(4..5), expected_stdout: Some(crate::File::Text("Hello World\n".into())), expected_stderr: None, ..Default::default() @@ -823,6 +866,7 @@ Hello World bin: Some(Bin::Name("cmd1".into())), expected_status: Some(CommandStatus::Code(1)), stderr_to_stdout: true, + expected_stdout_source: Some(5..5), expected_stdout: Some(crate::File::Text("".into())), expected_stderr: None, ..Default::default() @@ -832,6 +876,7 @@ Hello World bin: Some(Bin::Name("cmd2".into())), expected_status: Some(CommandStatus::Success), stderr_to_stdout: true, + expected_stdout_source: Some(6..6), expected_stdout: Some(crate::File::Text("".into())), expected_stderr: None, ..Default::default() @@ -861,6 +906,7 @@ $ cmd2 bin: Some(Bin::Name("bare-cmd".into())), expected_status: Some(CommandStatus::Code(1)), stderr_to_stdout: true, + expected_stdout_source: Some(5..5), expected_stdout: Some(crate::File::Text("".into())), expected_stderr: None, ..Default::default() @@ -870,6 +916,7 @@ $ cmd2 bin: Some(Bin::Name("trycmd-cmd".into())), expected_status: Some(CommandStatus::Code(1)), stderr_to_stdout: true, + expected_stdout_source: Some(10..10), expected_stdout: Some(crate::File::Text("".into())), expected_stderr: None, ..Default::default() @@ -879,6 +926,7 @@ $ cmd2 bin: Some(Bin::Name("sh-cmd".into())), expected_status: Some(CommandStatus::Code(1)), stderr_to_stdout: true, + expected_stdout_source: Some(15..15), expected_stdout: Some(crate::File::Text("".into())), expected_stderr: None, ..Default::default() @@ -888,6 +936,7 @@ $ cmd2 bin: Some(Bin::Name("bash-cmd".into())), expected_status: Some(CommandStatus::Code(1)), stderr_to_stdout: true, + expected_stdout_source: Some(20..20), expected_stdout: Some(crate::File::Text("".into())), expected_stderr: None, ..Default::default() From f564c39e36232b85b0b6c53ed39a889c5f72c4e0 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 11 Nov 2021 13:42:39 -0600 Subject: [PATCH 6/8] fix: Do not overwrite dumps on multi-step files --- src/runner.rs | 43 +++++++++++++++++++++++++++++-------------- 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/src/runner.rs b/src/runner.rs index 5ff8df8b..58757c51 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -198,14 +198,16 @@ impl Case { Ok(output) => output, Err(output) => output, }; - output.stdout = match self.dump_stream(root, output.stdout.take()) { - Ok(stream) => stream, - Err(stream) => stream, - }; - output.stderr = match self.dump_stream(root, output.stderr.take()) { - Ok(stream) => stream, - Err(stream) => stream, - }; + output.stdout = + match self.dump_stream(root, output.id.as_deref(), output.stdout.take()) { + Ok(stream) => stream, + Err(stream) => stream, + }; + output.stderr = + match self.dump_stream(root, output.id.as_deref(), output.stderr.take()) { + Ok(stream) => stream, + Err(stream) => stream, + }; } } Mode::Overwrite => { @@ -418,15 +420,28 @@ impl Case { fn dump_stream( &self, root: &std::path::Path, + id: Option<&str>, stream: Option, ) -> Result, Option> { if let Some(stream) = stream { - let stream_path = root.join( - self.path - .with_extension(stream.stream.as_str()) - .file_name() - .unwrap(), - ); + let file_name = match id { + Some(id) => { + format!( + "{}-{}.{}", + self.path.file_stem().unwrap().to_string_lossy(), + id, + stream.stream.as_str(), + ) + } + None => { + format!( + "{}.{}", + self.path.file_stem().unwrap().to_string_lossy(), + stream.stream.as_str(), + ) + } + }; + let stream_path = root.join(file_name); stream.content.write_to(&stream_path).map_err(|e| { let mut stream = stream.clone(); if stream.is_ok() { From 0cf131ed38490baae27c3017ff0e5abe28d0249a Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 11 Nov 2021 14:28:06 -0600 Subject: [PATCH 7/8] refactor: Clean up validation logic The main reason for the `Result` was when the `mode` overwrote the error state in `T`. Now that `mode`s work is done elsewhere, we can simplify things --- src/runner.rs | 93 ++++++++++++++------------------------------------- 1 file changed, 26 insertions(+), 67 deletions(-) diff --git a/src/runner.rs b/src/runner.rs index 58757c51..0f67b781 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -300,34 +300,17 @@ impl Case { let output = output.output(cmd_output); // For Mode::Dump's sake, allow running all - let mut ok = output.is_ok(); - let output = match self.validate_spawn(output, step.expected_status()) { - Ok(output) => output, - Err(output) => { - ok = false; - output - } - }; - let output = match self.validate_streams(output, step) { - Ok(output) => output, - Err(output) => { - ok = false; - output - } - }; + let output = self.validate_spawn(output, step.expected_status()); + let output = self.validate_streams(output, step); - if ok { + if output.is_ok() { Ok(output) } else { Err(output) } } - fn validate_spawn( - &self, - mut output: Output, - expected: crate::schema::CommandStatus, - ) -> Result { + fn validate_spawn(&self, mut output: Output, expected: crate::schema::CommandStatus) -> Output { let status = output.spawn.exit.expect("bale out before now"); match expected { crate::schema::CommandStatus::Success => { @@ -353,38 +336,16 @@ impl Case { } } - Ok(output) + output } - fn validate_streams( - &self, - mut output: Output, - step: &crate::schema::Step, - ) -> Result { - let mut ok = true; - + fn validate_streams(&self, mut output: Output, step: &crate::schema::Step) -> Output { output.stdout = - match self.validate_stream(output.stdout, step.expected_stdout.as_ref(), step.binary) { - Ok(stream) => stream, - Err(stream) => { - ok = false; - stream - } - }; + self.validate_stream(output.stdout, step.expected_stdout.as_ref(), step.binary); output.stderr = - match self.validate_stream(output.stderr, step.expected_stderr.as_ref(), step.binary) { - Ok(stream) => stream, - Err(stream) => { - ok = false; - stream - } - }; + self.validate_stream(output.stderr, step.expected_stderr.as_ref(), step.binary); - if ok { - Ok(output) - } else { - Err(output) - } + output } fn validate_stream( @@ -392,29 +353,27 @@ impl Case { stream: Option, expected_content: Option<&crate::File>, binary: bool, - ) -> Result, Option> { - if let Some(mut stream) = stream { - if !binary { - stream = stream.utf8(); - if !stream.is_ok() { - return Err(Some(stream)); - } - } + ) -> Option { + let mut stream = stream?; - if let Some(expected_content) = expected_content { - if let crate::File::Text(e) = &expected_content { - stream.content = stream.content.map_text(|t| crate::elide::normalize(t, e)); - } - if stream.content != *expected_content { - stream.status = StreamStatus::Expected(expected_content.clone()); - return Err(Some(stream)); - } + if !binary { + stream = stream.utf8(); + if !stream.is_ok() { + return Some(stream); } + } - Ok(Some(stream)) - } else { - Ok(None) + if let Some(expected_content) = expected_content { + if let crate::File::Text(e) = &expected_content { + stream.content = stream.content.map_text(|t| crate::elide::normalize(t, e)); + } + if stream.content != *expected_content { + stream.status = StreamStatus::Expected(expected_content.clone()); + return Some(stream); + } } + + Some(stream) } fn dump_stream( From a8eaa72a4a4346d6cd7312c87c78a2f6bd52926a Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 11 Nov 2021 14:33:37 -0600 Subject: [PATCH 8/8] style: Make clippy happy --- src/filesystem.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/filesystem.rs b/src/filesystem.rs index 3e0c6fd7..f9abc2d8 100644 --- a/src/filesystem.rs +++ b/src/filesystem.rs @@ -40,7 +40,7 @@ impl File { { if line_num == line_nums.start { output_lines.push_str(text); - if !text.is_empty() && !text.ends_with("\n") { + if !text.is_empty() && !text.ends_with('\n') { output_lines.push('\n'); } }