diff --git a/integration/hurl/tests_failed/assert_newline.err b/integration/hurl/tests_failed/assert_newline.err index e977dd086fd..811c38cd38c 100644 --- a/integration/hurl/tests_failed/assert_newline.err +++ b/integration/hurl/tests_failed/assert_newline.err @@ -5,7 +5,7 @@ error: Assert body value | ... 10 |

Hello

| ^ actual value is <

Hello

- -> + | + | > | diff --git a/integration/hurl/tests_failed/assert_value_error.err b/integration/hurl/tests_failed/assert_value_error.err index 39a7ef4910d..9725d3da57a 100644 --- a/integration/hurl/tests_failed/assert_value_error.err +++ b/integration/hurl/tests_failed/assert_value_error.err @@ -76,9 +76,9 @@ error: Assert failure | ... 11 | jsonpath "$.line_terminator" == "\r\n" | actual: string < - | > + |> | expected: string < - | > + |> | error: Assert failure diff --git a/integration/hurl/tests_failed/assert_value_error.out.pattern b/integration/hurl/tests_failed/assert_value_error.out.pattern index 4c6d0acd2ee..c0484d99b30 100644 --- a/integration/hurl/tests_failed/assert_value_error.out.pattern +++ b/integration/hurl/tests_failed/assert_value_error.out.pattern @@ -48,7 +48,7 @@ }, { "line": 11, - "message": "Assert failure\n --> tests_failed/assert_value_error.hurl:11:0\n |\n | GET http://localhost:8000/error-assert-value\n | ...\n11 | jsonpath \"$.line_terminator\" == \"\\r\\n\"\n | actual: string <\n | >\n | expected: string <\n | >\n |", + "message": "Assert failure\n --> tests_failed/assert_value_error.hurl:11:0\n |\n | GET http://localhost:8000/error-assert-value\n | ...\n11 | jsonpath \"$.line_terminator\" == \"\\r\\n\"\n | actual: string <\n |>\n | expected: string <\n |>\n |", "success": false }, { diff --git a/packages/hurl/src/runner/error.rs b/packages/hurl/src/runner/error.rs index f5f4b37d738..53896978bab 100644 --- a/packages/hurl/src/runner/error.rs +++ b/packages/hurl/src/runner/error.rs @@ -175,11 +175,11 @@ impl hurl_core::error::Error for Error { .. } => { let additional = if *type_mismatch { - "\n>>> types between actual and expected are not consistent" + "\n >>> types between actual and expected are not consistent" } else { "" }; - format!("actual: {actual}\nexpected: {expected}{additional}") + format!(" actual: {actual}\n expected: {expected}{additional}") } RunnerError::AssertHeaderValueError { actual } => { format!("actual value is <{actual}>") @@ -257,6 +257,14 @@ impl hurl_core::error::Error for Error { } } } + + fn show_source_line(&self) -> bool { + true + } + + fn show_caret(&self) -> bool { + !matches!(&self.inner, RunnerError::AssertFailure { .. }) + } } impl From for RunnerError { diff --git a/packages/hurl/src/util/logger.rs b/packages/hurl/src/util/logger.rs index 0571e9f9cf0..b720e6d96cc 100644 --- a/packages/hurl/src/util/logger.rs +++ b/packages/hurl/src/util/logger.rs @@ -474,11 +474,10 @@ pub(crate) fn error_string( }; let line = format!("{spaces}{arrow} {filename}:{error_line}:{error_column}"); text.push_str(&line); - text.push('\n'); - // 3. Appends line separator. - text.push_str(&prefix); + // 3. Appends additional empty line text.push('\n'); + text.push_str(&prefix); // 4. Appends the optional entry line. if let Some(entry_line) = entry_line { @@ -489,14 +488,15 @@ pub(crate) fn error_string( } else { line.to_string() }; + text.push('\n'); text.push_str(&prefix); text.push(' '); text.push_str(&line); - text.push('\n'); } if error_line - entry_line > 1 { + text.push('\n'); text.push_str(&prefix); - let dots = " ...\n"; + let dots = " ..."; let dots = if colored { dots.bright_black().to_string() } else { @@ -506,41 +506,73 @@ pub(crate) fn error_string( } } - // 5. Then, we build error line (whitespace is uniformized) - // ex. ` 2 | HTTP/1.0 200` - let line = line_with_loc(&lines, error_line, &separator, colored); - text.push_str(&line); - text.push('\n'); + // 5. Appends the error message (one or more lines) + // with the line number '|' prefix + let message = get_message(error, &lines, colored); + for (i, line) in split_lines(&message).iter().enumerate() { + if i == 0 { + let loc_max_width = max(lines.len().to_string().len(), 2); - // 6. Then, we append the error detailed message: - // ``` - // | actual: byte array - // | expected: byte array <00> - // ```` - - // Explicit asserts output is multi-line, with actual and expected value aligned, while - // other errors (implicit assert for instance) are one-line, with a column error marker "^^^..." - // on a second line. - // So, we have "marked" explicit asserts to suppress the display of the column error marker - // by setting their `source_info`'s column to 0 (see [`hurl::runner::predicate::eval_predicate::`]). - let message = if error_column == 0 { - let new_prefix = format!("{prefix} "); // actual and expected are prefixed by 2 spaces - let fix_me = error.fixme(); - add_line_prefix(&fix_me, &new_prefix, colored) - } else { - // We take tabs into account because we have normalize the display of the error line by replacing - // tabs with 4 spaces. - // TODO: add a unit test with tabs in source info. - let mut tab_shift = 0; - let line_raw = lines.get(error_line - 1).unwrap(); - for (i, c) in line_raw.chars().enumerate() { - if i >= error_column - 1 { - break; - }; - if c == '\t' { - tab_shift += 1; + let mut s = format!("{error_line:>loc_max_width$}"); + if colored { + s = s.blue().bold().to_string(); } + + text.push_str(format!("\n{s} |{line}").as_str()); + } else { + text.push_str(format!("\n{prefix}{line}").as_str()); } + } + + // 6. Appends additional empty line + if !message.ends_with('\n') { + text.push('\n'); + text.push_str(&prefix); + } + + text +} + +/// Return the constructed message for the error +/// +/// It may include: +/// - source line +/// - column position and number of characters (with one or more carets) +/// +/// Examples: +/// +/// GET abc +/// ^ expecting http://, https:// or {{ +/// +/// HTTP/1.0 200 +/// ^^^ actual value is <404> +/// +/// jsonpath "$.count" >= 5 +/// actual: int <2> +/// expected: greater than int <5> +/// +/// { +/// "name": "John", +///- "age": 27 +///+ "age": 28 +/// } +/// +fn get_message(error: &E, lines: &[&str], colored: bool) -> String { + let mut text = String::new(); + + if error.show_source_line() { + let line = lines.get(error.source_info().start.line - 1).unwrap(); + let line = line.replace('\t', " "); + text.push(' '); + text.push_str(&line); + text.push('\n'); + } + + let mut prefix = if error.show_caret() { + //let mut prefix = String::new(); + // the fixme message is offset with space and carets according to the error column + let error_line = error.source_info().start.line; + let error_column = error.source_info().start.column; // Error source info start and end can be on different lines, we insure a minimum width. let width = if error.source_info().end.column > error_column { @@ -549,61 +581,54 @@ pub(crate) fn error_string( 1 }; - let mut fix_me = "^".repeat(width); - fix_me.push(' '); - fix_me.push_str(&error.fixme()); - if colored { - fix_me = fix_me.red().bold().to_string(); + // We take tabs into account because we have normalize the display of the error line by replacing + // tabs with 4 spaces. + // TODO: add a unit test with tabs in source info. + let mut tab_shift = 0; + let line_raw = lines.get(error_line - 1).unwrap(); + for (i, c) in line_raw.chars().enumerate() { + if i >= (error_column - 1) { + break; + }; + if c == '\t' { + tab_shift += 1; + } } - format!( - "{spaces} {separator} {}{fix_me}", - " ".repeat(error_column - 1 + tab_shift * 3) - ) + + let mut value = " ".repeat(error_column + tab_shift * 3); + value.push_str("^".repeat(width).as_str()); + value.push(' '); + value + } else { + String::new() }; - text.push_str(&message); - text.push('\n'); - // 6. Appends final line separator. - text.push_str(&prefix); + let fixme = error.fixme(); + let lines = split_lines(&fixme); + for (i, line) in lines.iter().enumerate() { + if i > 0 { + if !prefix.is_empty() { + prefix = " ".repeat(prefix.len()); + } + text.push('\n'); + } + if !line.is_empty() { + text.push_str(color_if_needed(&prefix, colored).as_str()); + } + text.push_str(color_if_needed(line, colored).as_str()); + } text } -/// Returns the `line` count prefix. -/// Example: ` 45 ` -fn line_with_loc(lines: &[&str], loc: usize, separator: &str, colored: bool) -> String { - let mut text = String::new(); - let loc_max_width = max(lines.len().to_string().len(), 2); - let line = lines.get(loc - 1).unwrap(); - let line = line.replace('\t', " "); - let mut line_number = format!("{loc:>loc_max_width$}"); +// This function is temporary +// the fixme function in the Error trait should return the colored String directly +fn color_if_needed(s: &str, colored: bool) -> String { if colored { - line_number = line_number.blue().bold().to_string(); - } - text.push_str(&line_number); - text.push(' '); - text.push_str(separator); - if !line.is_empty() { - text.push(' '); - text.push_str(&line); + s.red().bold().to_string() + } else { + s.to_string() } - text -} - -/// Prefixes each line of the string `s` with a `prefix` and returns the new string. -/// If `colored` is true, each line is colored with ANSI escape codes. -fn add_line_prefix(s: &str, prefix: &str, colored: bool) -> String { - split_lines(s) - .iter() - .map(|line| { - if colored { - format!("{}{}", prefix, line.red().bold()) - } else { - format!("{prefix}{line}") - } - }) - .collect::>() - .join("\n") } /// Splits this `text` to a list of LF/CRLF separated lines. @@ -613,19 +638,12 @@ fn split_lines(text: &str) -> Vec<&str> { #[cfg(test)] pub mod tests { + use hurl_core::ast::{Pos, SourceInfo}; use super::*; use crate::runner; - #[test] - fn test_add_line_prefix_no_colored() { - assert_eq!( - add_line_prefix("line1\nline2\nline3", ">", false), - ">line1\n>line2\n>line3" - ); - } - #[test] fn test_error_timeout() { let content = "GET http://unknown"; @@ -635,6 +653,10 @@ pub mod tests { let error_source_info = SourceInfo::new(Pos::new(1, 5), Pos::new(1, 19)); let entry_source_info = SourceInfo::new(Pos::new(1, 1), Pos::new(1, 19)); let error = runner::Error::new(error_source_info, inner, true); + assert_eq!( + get_message(&error, &split_lines(content), false), + " GET http://unknown\n ^^^^^^^^^^^^^^ (6) Could not resolve host: unknown" + ); assert_eq!( error_string(filename, content, &error, Some(entry_source_info), false), r#"HTTP connection @@ -658,6 +680,17 @@ HTTP/1.0 200 let error_source_info = SourceInfo::new(Pos::new(2, 10), Pos::new(2, 13)); let entry_source_info = SourceInfo::new(Pos::new(1, 1), Pos::new(1, 18)); let error = runner::Error::new(error_source_info, inner, true); + + assert_eq!( + get_message(&error, &split_lines(content), false), + " HTTP/1.0 200\n ^^^ actual value is <404>" + ); + colored::control::set_override(true); + assert_eq!( + get_message(&error, &split_lines(content), true), + " HTTP/1.0 200\n\u{1b}[1;31m ^^^ \u{1b}[0m\u{1b}[1;31mactual value is <404>\u{1b}[0m" + ); + assert_eq!( error_string(filename, content, &error, Some(entry_source_info), false), r#"Assert status code @@ -685,6 +718,10 @@ xpath "strong(//head/title)" == "Hello" runner::RunnerError::QueryInvalidXpathEval, true, ); + assert_eq!( + get_message(&error, &split_lines(content), false), + " xpath \"strong(//head/title)\" == \"Hello\"\n ^^^^^^^^^^^^^^^^^^^^^^ the XPath expression is not valid" + ); assert_eq!( error_string(filename, content, &error, Some(entry_source_info), false), r#"Invalid XPath expression @@ -717,6 +754,14 @@ jsonpath "$.count" >= 5 }, assert: true, }; + + assert_eq!( + get_message(&error, &split_lines(content), false), + r#" jsonpath "$.count" >= 5 + actual: int <2> + expected: greater than int <5>"# + ); + assert_eq!( error_string(filename, content, &error, Some(entry_source_info), false), r#"Assert failure @@ -746,6 +791,11 @@ HTTP/1.0 200 let error_source_info = SourceInfo::new(Pos::new(3, 4), Pos::new(4, 1)); let entry_source_info = SourceInfo::new(Pos::new(1, 1), Pos::new(1, 20)); let error = runner::Error::new(error_source_info, inner, true); + + assert_eq!( + get_message(&error, &split_lines(content), false), + " ```

Hello

\n ^ actual value is <

Hello

\n\n >" + ); assert_eq!( error_string(filename, content, &error, Some(entry_source_info), false), r#"Assert body value @@ -755,8 +805,8 @@ HTTP/1.0 200 | ... 3 | ```

Hello

| ^ actual value is <

Hello

- -> + | + | > |"# ); } @@ -780,4 +830,70 @@ HTTP/1.0 200 |"# ); } + + #[test] + fn test_diff_error() { + let content = r#"GET http://localhost:8000/failed/multiline/json +HTTP 200 +``` +{ + "name": "John", + "age": 27 +} +``` +"#; + let filename = "test.hurl"; + struct E; + impl Error for E { + fn source_info(&self) -> SourceInfo { + SourceInfo::new(Pos::new(4, 1), Pos::new(4, 0)) + } + + fn description(&self) -> String { + "Assert body value".to_string() + } + + fn fixme(&self) -> String { + r#" { + "name": "John", +- "age": 27 ++ "age": 28 + } +"# + .to_string() + } + + fn show_source_line(&self) -> bool { + false + } + + fn show_caret(&self) -> bool { + false + } + } + let error = E; + + assert_eq!( + get_message(&error, &split_lines(content), false), + r#" { + "name": "John", +- "age": 27 ++ "age": 28 + } +"# + ); + + assert_eq!( + error_string(filename, content, &error, None, false), + r#"Assert body value + --> test.hurl:4:1 + | + 4 | { + | "name": "John", + |- "age": 27 + |+ "age": 28 + | } + |"# + ); + } } diff --git a/packages/hurl_core/src/error/mod.rs b/packages/hurl_core/src/error/mod.rs index 47d25dc1ecf..ee6bbbefe69 100644 --- a/packages/hurl_core/src/error/mod.rs +++ b/packages/hurl_core/src/error/mod.rs @@ -21,4 +21,6 @@ pub trait Error { fn source_info(&self) -> SourceInfo; fn description(&self) -> String; fn fixme(&self) -> String; + fn show_source_line(&self) -> bool; + fn show_caret(&self) -> bool; } diff --git a/packages/hurl_core/src/parser/error.rs b/packages/hurl_core/src/parser/error.rs index 1a4bb40f622..83076463d2c 100644 --- a/packages/hurl_core/src/parser/error.rs +++ b/packages/hurl_core/src/parser/error.rs @@ -235,6 +235,14 @@ impl crate::error::Error for Error { ParseError::Xml => "invalid XML".to_string(), } } + + fn show_source_line(&self) -> bool { + true + } + + fn show_caret(&self) -> bool { + true + } } fn did_you_mean(valid_values: &[&str], actual: &str, default: &str) -> String { diff --git a/packages/hurlfmt/src/linter/error.rs b/packages/hurlfmt/src/linter/error.rs index 458b9b71d27..3fed7704a06 100644 --- a/packages/hurlfmt/src/linter/error.rs +++ b/packages/hurlfmt/src/linter/error.rs @@ -44,4 +44,12 @@ impl Error for linter::Error { LinterError::OneSpace => "Use only one space".to_string(), } } + + fn show_source_line(&self) -> bool { + true + } + + fn show_caret(&self) -> bool { + true + } }