From 8b267451dea4f151682ddbbaa9de55723e3db352 Mon Sep 17 00:00:00 2001 From: Jason Hall Date: Thu, 1 Jan 2026 13:53:10 -0500 Subject: [PATCH] improve error reporting Signed-off-by: Jason Hall --- Cargo.lock | 55 ++++- Cargo.toml | 8 +- README.md | 59 ++++- src/error.rs | 322 +++++++++++++++++++++++++++- tests/stdout_stderr_verification.rs | 88 ++++---- 5 files changed, 478 insertions(+), 54 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6d7e7d9..35a81eb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -26,6 +26,17 @@ dependencies = [ "derive_arbitrary", ] +[[package]] +name = "atty" +version = "0.2.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d9b39be18770d11421cdb1b9947a45dd3f37e93092cbf377614828a319d5fee8" +dependencies = [ + "hermit-abi", + "libc", + "winapi", +] + [[package]] name = "bitflags" version = "2.9.4" @@ -77,6 +88,15 @@ dependencies = [ "wasi", ] +[[package]] +name = "hermit-abi" +version = "0.1.19" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "62b467343b94ba476dcb2500d242dadbb39557df889310ac77c5d99100aaac33" +dependencies = [ + "libc", +] + [[package]] name = "libc" version = "0.2.176" @@ -200,14 +220,25 @@ dependencies = [ "windows-sys", ] +[[package]] +name = "termcolor" +version = "1.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "06794f8f6c5c898b3275aebefa6b8a1cb24cd2c6c79397ab15774837a0bc5755" +dependencies = [ + "winapi-util", +] + [[package]] name = "testscript-rs" -version = "0.2.10" +version = "0.2.11" dependencies = [ "anyhow", "arbitrary", + "atty", "regex", "tempfile", + "termcolor", "thiserror", "walkdir", ] @@ -266,6 +297,22 @@ dependencies = [ "wit-bindgen", ] +[[package]] +name = "winapi" +version = "0.3.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5c839a674fcd7a98952e593242ea400abe93992746761e38641405d28b00f419" +dependencies = [ + "winapi-i686-pc-windows-gnu", + "winapi-x86_64-pc-windows-gnu", +] + +[[package]] +name = "winapi-i686-pc-windows-gnu" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ac3b87c63620426dd9b991e5ce0329eff545bccbbb34f3be09ff6fb6ab51b7b6" + [[package]] name = "winapi-util" version = "0.1.11" @@ -275,6 +322,12 @@ dependencies = [ "windows-sys", ] +[[package]] +name = "winapi-x86_64-pc-windows-gnu" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" + [[package]] name = "windows-link" version = "0.2.0" diff --git a/Cargo.toml b/Cargo.toml index 67c8aec..1524262 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "testscript-rs" -version = "0.2.10" +version = "0.2.11" edition = "2021" authors = ["Jason Hall"] license = "Apache-2.0" @@ -15,6 +15,12 @@ thiserror = "2.0" tempfile = "3.0" walkdir = "2.0" regex = "1.0" +termcolor = { version = "1.4", optional = true } +atty = { version = "0.2", optional = true } + +[features] +default = [] +colors = ["termcolor", "atty"] [dev-dependencies] arbitrary = { version = "1.0", features = ["derive"] } diff --git a/README.md b/README.md index 36e0506..44013d7 100644 --- a/README.md +++ b/README.md @@ -118,18 +118,18 @@ custom-cmd arg1 arg2 arg3 Test scripts use the [`txtar`](https://pkg.go.dev/github.com/rogpeppe/go-internal/txtar) format. For complete format documentation, see the [original Go testscript documentation](https://pkg.go.dev/github.com/rogpeppe/go-internal/testscript). -### Built-in Commands +## Built-in Commands - **exec** - Execute external commands - **cmp** - Compare two files -- **stdout/stderr** - Check command output (supports regex) +- **stdout/stderr** - Check command output (supports regex and `-count=N` option) - **exists** - Check file existence - **mkdir** - Create directories - **cp** - Copy files (supports stdout/stderr as source) - **mv** - Move/rename files - **rm** - Remove files/directories - **chmod** - Change file permissions -- **env** - Set environment variables +- **env** - Set environment variables (supports `${VAR@R}` regex quoting) - **cmpenv** - Compare files with environment variable substitution - **stdin** - Set stdin for next command - **cd** - Change working directory @@ -143,9 +143,19 @@ Test scripts use the [`txtar`](https://pkg.go.dev/github.com/rogpeppe/go-interna Commands can be prefixed with conditions (`[unix]`) or negated (`!`). +### Go testscript Compatibility + +testscript-rs implements full compatibility with Go's testscript package, including: + +- **`${VAR@R}` syntax** - Escape regex metacharacters in environment variables +- **`-count=N` option** - Count exact number of matches for stdout/stderr +- **Regex pattern detection** - Automatic detection based on regex metacharacters +- **Environment variable substitution** - Full `$VAR` and `${VAR}` support +- **Whitespace handling** - Matches Go's exact trimming behavior + ## Error Messages -testscript-rs provides detailed error messages with script context to make debugging easy: +testscript-rs provides detailed, readable error messages with script context to make debugging easy: ``` Error in testdata/hello.txt at line 6: @@ -155,8 +165,49 @@ Error in testdata/hello.txt at line 6: > 6 | exec nonexistent-command arg1 arg2 7 | stdout "should not get here" 8 | + +Command 'nonexistent-command' failed: command not found ``` +### Output Comparison Errors + +When stdout/stderr assertions fail, you get clear, formatted output comparisons: + +``` +Error in testdata/test.txt at line 3: + 1 | exec echo "hello world" + 2 | +> 3 | stdout "goodbye world" + +Expected: 'goodbye world' + Actual: 'hello world' +``` + +For multi-line output, line numbers are shown: + +``` +Expected: + 1 | line1 + 2 | expected + 3 | line3 + +Actual: + 1 | line1 + 2 | actual + 3 | line3 +``` + +### Optional Color Support + +Enable colored error output for terminal-friendly debugging: + +```toml +[dev-dependencies] +testscript-rs = { version = "", features = ["colors"] } +``` + +With colors enabled, the failing line and error details are highlighted in red for easy identification. + > Note: Some features of `testscript` in Go are not supported in this Rust port: > > - `[gc]` for whether Go was built with gc diff --git a/src/error.rs b/src/error.rs index 2fb84da..f60429b 100644 --- a/src/error.rs +++ b/src/error.rs @@ -2,6 +2,9 @@ use thiserror::Error; +#[cfg(feature = "colors")] +use termcolor::Color; + /// Result type alias for testscript operations pub type Result = std::result::Result; @@ -32,7 +35,7 @@ pub enum Error { FileCompare { message: String }, /// Output comparison error - #[error("Output comparison failed: expected {expected}, got {actual}")] + #[error("{}", format_output_comparison(.expected, .actual))] OutputCompare { expected: String, actual: String }, /// Unknown command error @@ -48,7 +51,7 @@ pub enum Error { Generic(String), /// Script execution error with context - #[error("Error in {script_file} at line {line_num}:\n{context}")] + #[error("Error in {script_file} at line {line_num}:\n{context}\n\n{source}")] ScriptError { script_file: String, line_num: usize, @@ -108,6 +111,18 @@ fn generate_error_context(script_content: &str, error_line: usize) -> String { let line_content = lines.get(line_num - 1).unwrap_or(&""); if line_num == error_line { + #[cfg(feature = "colors")] + { + if atty::is(atty::Stream::Stderr) { + context.push_str(&format!( + "\x1b[31m> {} | {}\x1b[0m\n", + line_num, line_content + )); + } else { + context.push_str(&format!("> {} | {}\n", line_num, line_content)); + } + } + #[cfg(not(feature = "colors"))] context.push_str(&format!("> {} | {}\n", line_num, line_content)); } else { context.push_str(&format!(" {} | {}\n", line_num, line_content)); @@ -116,3 +131,306 @@ fn generate_error_context(script_content: &str, error_line: usize) -> String { context.trim_end().to_string() } + +/// Format output comparison error for better readability +fn format_output_comparison(expected: &str, actual: &str) -> String { + #[cfg(feature = "colors")] + { + if atty::is(atty::Stream::Stderr) { + return format_output_comparison_colored(expected, actual); + } + } + + format_output_comparison_plain(expected, actual) +} + +#[cfg(feature = "colors")] +fn format_output_comparison_colored(expected: &str, actual: &str) -> String { + // Handle empty strings specially + if expected.is_empty() && actual.is_empty() { + return "Both expected and actual output are empty".to_string(); + } + if expected.is_empty() { + return format!( + "Expected empty output, but got:\n{}", + colorize_output_value(actual, Color::Red) + ); + } + if actual.is_empty() { + return format!( + "Expected output:\n{}\nBut got empty output", + colorize_output_value(expected, Color::Green) + ); + } + + // For short outputs, show inline + if expected.len() <= 50 + && actual.len() <= 50 + && !expected.contains('\n') + && !actual.contains('\n') + { + return format!( + "Expected: {}\n Actual: {}", + colorize_text(format!("'{}'", expected), Color::Green), + colorize_text(format!("'{}'", actual), Color::Red) + ); + } + + // For longer outputs, show formatted comparison + format!( + "Output mismatch:\n\n{}:\n{}\n\n{}:\n{}", + colorize_text("Expected".to_string(), Color::Green), + colorize_output_value(expected, Color::Green), + colorize_text("Actual".to_string(), Color::Red), + colorize_output_value(actual, Color::Red) + ) +} + +#[cfg(feature = "colors")] +fn colorize_text(text: String, color: Color) -> String { + // Simple ANSI color codes for basic coloring + let color_code = match color { + Color::Red => "\x1b[31m", + Color::Green => "\x1b[32m", + _ => "", + }; + let reset = "\x1b[0m"; + + if color_code.is_empty() { + text + } else { + format!("{}{}{}", color_code, text, reset) + } +} + +#[cfg(feature = "colors")] +fn colorize_output_value(value: &str, color: Color) -> String { + let formatted = format_output_value_plain(value); + colorize_text(formatted, color) +} + +fn format_output_comparison_plain(expected: &str, actual: &str) -> String { + // Handle empty strings specially + if expected.is_empty() && actual.is_empty() { + return "Both expected and actual output are empty".to_string(); + } + if expected.is_empty() { + return format!( + "Expected empty output, but got:\n{}", + format_output_value(actual) + ); + } + if actual.is_empty() { + return format!( + "Expected output:\n{}\nBut got empty output", + format_output_value(expected) + ); + } + + // For short outputs, show inline + if expected.len() <= 50 + && actual.len() <= 50 + && !expected.contains('\n') + && !actual.contains('\n') + { + return format!("Expected: '{}'\n Actual: '{}'", expected, actual); + } + + // For longer outputs, show formatted comparison + format!( + "Output mismatch:\n\nExpected:\n{}\n\nActual:\n{}", + format_output_value(expected), + format_output_value(actual) + ) +} + +/// Format a single output value for display +fn format_output_value(value: &str) -> String { + format_output_value_plain(value) +} + +fn format_output_value_plain(value: &str) -> String { + if value.is_empty() { + return "".to_string(); + } + + // Add line numbers for multi-line output + if value.contains('\n') { + let lines: Vec<&str> = value.lines().collect(); + if lines.len() > 1 { + return lines + .iter() + .enumerate() + .map(|(i, line)| format!(" {} | {}", i + 1, line)) + .collect::>() + .join("\n"); + } + } + + // For single line, add quotes if it contains special characters + if value.chars().any(|c| c.is_whitespace() || c.is_control()) { + format!("'{}'", value) + } else { + value.to_string() + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_format_output_comparison_simple() { + let result = format_output_comparison("expected", "actual"); + assert!(result.contains("Expected: 'expected'\n Actual: 'actual'")); + } + + #[test] + fn test_format_output_comparison_empty() { + let result = format_output_comparison("", ""); + assert_eq!(result, "Both expected and actual output are empty"); + + let result = format_output_comparison("", "something"); + assert!(result.contains("Expected empty output, but got:")); + assert!(result.contains("something")); + + let result = format_output_comparison("expected", ""); + assert!(result.contains("Expected output:")); + assert!(result.contains("But got empty output")); + } + + #[test] + fn test_format_output_comparison_multiline() { + let expected = "line1\nline2\nline3"; + let actual = "line1\nwrong\nline3"; + let result = format_output_comparison(expected, actual); + + assert!(result.contains("Output mismatch:")); + assert!(result.contains("Expected:")); + assert!(result.contains("Actual:")); + assert!(result.contains("1 | line1")); + assert!(result.contains("2 | line2")); + assert!(result.contains("2 | wrong")); + assert!(result.contains("3 | line3")); + } + + #[test] + fn test_format_output_value_empty() { + assert_eq!(format_output_value(""), ""); + } + + #[test] + fn test_format_output_value_simple() { + assert_eq!(format_output_value("simple"), "simple"); + assert_eq!(format_output_value("with spaces"), "'with spaces'"); + assert_eq!(format_output_value("with\ttabs"), "'with\ttabs'"); + } + + #[test] + fn test_format_output_value_multiline() { + let result = format_output_value("line1\nline2\nline3"); + assert!(result.contains("1 | line1")); + assert!(result.contains("2 | line2")); + assert!(result.contains("3 | line3")); + } + + #[test] + fn test_format_output_value_single_line_with_newline() { + let result = format_output_value("single line\n"); + assert_eq!(result, "'single line\n'"); + } + + #[test] + fn test_error_creation() { + let parse_err = Error::parse_error(42, "test message"); + match parse_err { + Error::Parse { line, message } => { + assert_eq!(line, 42); + assert_eq!(message, "test message"); + } + _ => panic!("Wrong error type"), + } + + let cmd_err = Error::command_error("test_cmd", "test failure"); + match cmd_err { + Error::Command { command, message } => { + assert_eq!(command, "test_cmd"); + assert_eq!(message, "test failure"); + } + _ => panic!("Wrong error type"), + } + } + + #[test] + fn test_script_error_creation() { + let source = Error::command_error("test", "failed"); + let script_err = Error::script_error( + "test.txt", + 5, + "line1\nline2\nline3\nline4\nline5\nline6", + source, + ); + + match script_err { + Error::ScriptError { + script_file, + line_num, + context, + .. + } => { + assert_eq!(script_file, "test.txt"); + assert_eq!(line_num, 5); + assert!(context.contains("> 5 | line5")); + assert!(context.contains(" 3 | line3")); + assert!(context.contains(" 4 | line4")); + assert!(context.contains(" 6 | line6")); + } + _ => panic!("Wrong error type"), + } + } + + #[test] + fn test_generate_error_context() { + let script = "line1\nline2\nline3\nline4\nline5\nline6\nline7"; + let context = generate_error_context(script, 4); + + assert!(context.contains(" 1 | line1")); + assert!(context.contains(" 2 | line2")); + assert!(context.contains(" 3 | line3")); + assert!(context.contains("> 4 | line4")); + assert!(context.contains(" 5 | line5")); + assert!(context.contains(" 6 | line6")); + assert!(!context.contains("line7")); + } + + #[test] + fn test_generate_error_context_edge_cases() { + // Test first line error + let script = "line1\nline2\nline3"; + let context = generate_error_context(script, 1); + assert!(context.contains("> 1 | line1")); + assert!(context.contains(" 2 | line2")); + assert!(context.contains(" 3 | line3")); + + // Test last line error + let context = generate_error_context(script, 3); + assert!(context.contains(" 1 | line1")); + assert!(context.contains(" 2 | line2")); + assert!(context.contains("> 3 | line3")); + + // Test empty script + let context = generate_error_context("", 1); + assert!(context.is_empty()); + } + + #[test] + fn test_output_compare_error_display() { + let error = Error::OutputCompare { + expected: "test".to_string(), + actual: "different".to_string(), + }; + let display = format!("{}", error); + assert!(display.contains("Expected: 'test'")); + assert!(display.contains("Actual: 'different'")); + } +} diff --git a/tests/stdout_stderr_verification.rs b/tests/stdout_stderr_verification.rs index 5aa25e2..f986451 100644 --- a/tests/stdout_stderr_verification.rs +++ b/tests/stdout_stderr_verification.rs @@ -34,11 +34,9 @@ stdout "no newline" fs::write(&script_path, script_content).unwrap(); let result = run_test(&script_path); - assert!( - result.is_ok(), - "Stdout exact match test failed: {:?}", - result - ); + if let Err(error) = result { + panic!("Stdout exact match test failed:\n{}", error); + } } #[test] @@ -56,11 +54,9 @@ stderr "error no newline" fs::write(&script_path, script_content).unwrap(); let result = run_test(&script_path); - assert!( - result.is_ok(), - "Stderr exact match test failed: {:?}", - result - ); + if let Err(error) = result { + panic!("Stderr exact match test failed:\n{}", error); + } } #[test] @@ -88,7 +84,9 @@ stdout "line1.*line2" fs::write(&script_path, script_content).unwrap(); let result = run_test(&script_path); - assert!(result.is_ok(), "Stdout regex test failed: {:?}", result); + if let Err(error) = result { + panic!("Stdout regex test failed:\n{}", error); + } } #[test] @@ -106,7 +104,9 @@ stderr "start.*error.*end" fs::write(&script_path, script_content).unwrap(); let result = run_test(&script_path); - assert!(result.is_ok(), "Stderr regex test failed: {:?}", result); + if let Err(error) = result { + panic!("Stderr regex test failed:\n{}", error); + } } #[test] @@ -130,11 +130,9 @@ stdout "$MSG $NUM test" fs::write(&script_path, script_content).unwrap(); let result = run_test(&script_path); - assert!( - result.is_ok(), - "Env var substitution test failed: {:?}", - result - ); + if let Err(error) = result { + panic!("Env var substitution test failed:\n{}", error); + } } #[test] @@ -187,7 +185,9 @@ stdout "中文测试" fs::write(&script_path, script_content).unwrap(); let result = run_test(&script_path); - assert!(result.is_ok(), "Unicode handling test failed: {:?}", result); + if let Err(error) = result { + panic!("Unicode handling test failed:\n{}", error); + } } #[test] @@ -209,7 +209,9 @@ exec sh -c 'true' fs::write(&script_path, script_content).unwrap(); let result = run_test(&script_path); - assert!(result.is_ok(), "Empty output test failed: {:?}", result); + if let Err(error) = result { + panic!("Empty output test failed:\n{}", error); + } } #[test] @@ -233,7 +235,9 @@ stdout "just newline" fs::write(&script_path, script_content).unwrap(); let result = run_test(&script_path); - assert!(result.is_ok(), "Newline handling test failed: {:?}", result); + if let Err(error) = result { + panic!("Newline handling test failed:\n{}", error); + } } #[test] @@ -260,7 +264,9 @@ pattern[0-9]+ fs::write(&script_path, script_content).unwrap(); let result = run_test(&script_path); - assert!(result.is_ok(), "File vs literal test failed: {:?}", result); + if let Err(error) = result { + panic!("File vs literal test failed:\n{}", error); + } } #[test] @@ -281,11 +287,9 @@ stdout "tabs" fs::write(&script_path, script_content).unwrap(); let result = run_test(&script_path); - assert!( - result.is_ok(), - "Whitespace trimming test failed: {:?}", - result - ); + if let Err(error) = result { + panic!("Whitespace trimming test failed:\n{}", error); + } } #[test] @@ -369,11 +373,9 @@ stdout "third" fs::write(&script_path, script_content).unwrap(); let result = run_test(&script_path); - assert!( - result.is_ok(), - "Multiple commands test failed: {:?}", - result - ); + if let Err(error) = result { + panic!("Multiple commands test failed:\n{}", error); + } } #[test] @@ -425,11 +427,9 @@ exec true fs::write(&script_path, script_content).unwrap(); let result = run_test(&script_path); - assert!( - result.is_ok(), - "Negated stdout/stderr test failed: {:?}", - result - ); + if let Err(error) = result { + panic!("Negated stdout/stderr test failed:\n{}", error); + } } #[test] @@ -474,11 +474,9 @@ stdout "$PREFIX-test[0-9]+" fs::write(&script_path, script_content).unwrap(); let result = run_test(&script_path); - assert!( - result.is_ok(), - "Env substitution in regex test failed: {:?}", - result - ); + if let Err(error) = result { + panic!("Env substitution in regex test failed:\n{}", error); + } } #[test] @@ -514,11 +512,9 @@ stdout - fs::write(&script_path, script_content).unwrap(); let result = run_test(&script_path); - assert!( - result.is_ok(), - "Dash for empty output test failed: {:?}", - result - ); + if let Err(error) = result { + panic!("Dash for empty output test failed:\n{}", error); + } } #[test]