From 553eef02f80607017fe7521c33ef4c812b38fa6e Mon Sep 17 00:00:00 2001 From: Bugen Zhao Date: Wed, 8 Nov 2023 16:10:34 +0800 Subject: [PATCH 1/7] multiline line error Signed-off-by: Bugen Zhao --- sqllogictest/src/parser.rs | 236 ++++++++++++++++++++++++++++--------- sqllogictest/src/runner.rs | 12 +- 2 files changed, 186 insertions(+), 62 deletions(-) diff --git a/sqllogictest/src/parser.rs b/sqllogictest/src/parser.rs index 4de1e56..54d1210 100644 --- a/sqllogictest/src/parser.rs +++ b/sqllogictest/src/parser.rs @@ -84,8 +84,7 @@ pub enum Record { connection: Connection, /// The SQL command is expected to fail with an error messages that matches the given /// regex. If the regex is an empty string, any error message is accepted. - #[educe(PartialEq(method = "cmp_regex"))] - expected_error: Option, + expected_error: Option, /// The SQL command. sql: String, /// Expected rows affected. @@ -102,8 +101,7 @@ pub enum Record { label: Option, /// The SQL command is expected to fail with an error messages that matches the given /// regex. If the regex is an empty string, any error message is accepted. - #[educe(PartialEq(method = "cmp_regex"))] - expected_error: Option, + expected_error: Option, /// The SQL command. sql: String, /// The expected results. @@ -154,17 +152,6 @@ pub enum Record { Injected(Injected), } -/// Use string representation to determine if two regular -/// expressions came from the same text (rather than something -/// more deeply meaningful) -fn cmp_regex(l: &Option, r: &Option) -> bool { - match (l, r) { - (Some(l), Some(r)) => l.as_str().eq(r.as_str()), - (None, None) => true, - _ => false, - } -} - impl Record { /// Unparses the record to its string representation in the test file. /// @@ -195,19 +182,18 @@ impl std::fmt::Display for Record { write!(f, "statement ")?; match (expected_count, expected_error) { (None, None) => write!(f, "ok")?, - (None, Some(err)) => { - if err.as_str().is_empty() { - write!(f, "error")?; - } else { - write!(f, "error {err}")?; - } - } + (None, Some(err)) => err.fmt_inline(f)?, (Some(cnt), None) => write!(f, "count {cnt}")?, (Some(_), Some(_)) => unreachable!(), } writeln!(f)?; // statement always end with a blank line - writeln!(f, "{sql}") + writeln!(f, "{sql}")?; + + if let Some(err) = expected_error { + err.fmt_multiline(f)?; + } + Ok(()) } Record::Query { loc: _, @@ -220,32 +206,32 @@ impl std::fmt::Display for Record { sql, expected_results, } => { - write!(f, "query")?; + write!(f, "query ")?; if let Some(err) = expected_error { - writeln!(f, " error {err}")?; - return writeln!(f, "{sql}"); - } - - write!( - f, - " {}", - expected_types.iter().map(|c| c.to_char()).join("") - )?; - if let Some(sort_mode) = sort_mode { - write!(f, " {}", sort_mode.as_str())?; - } - if let Some(label) = label { - write!(f, " {label}")?; + err.fmt_inline(f)?; + } else { + write!(f, "{}", expected_types.iter().map(|c| c.to_char()).join(""))?; + if let Some(sort_mode) = sort_mode { + write!(f, " {}", sort_mode.as_str())?; + } + if let Some(label) = label { + write!(f, " {label}")?; + } } writeln!(f)?; writeln!(f, "{sql}")?; - write!(f, "----")?; - for result in expected_results { - write!(f, "\n{result}")?; + if let Some(err) = expected_error { + err.fmt_multiline(f) + } else { + write!(f, "----")?; + + for result in expected_results { + write!(f, "\n{result}")?; + } + // query always ends with a blank line + writeln!(f) } - // query always ends with a blank line - writeln!(f) } Record::System { loc: _, @@ -293,6 +279,86 @@ impl std::fmt::Display for Record { } } +#[derive(Debug, Clone)] +pub enum ExpectedError { + Inline(Regex), + Multiline(String), +} + +impl ExpectedError { + fn from_inline_tokens(tokens: &[&str]) -> Result { + let err_str = tokens.join(" "); + let regex = + Regex::new(&err_str).map_err(|_| ParseErrorKind::InvalidErrorMessage(err_str))?; + Ok(Self::Inline(regex)) + } + + fn is_empty_inline(&self) -> bool { + match self { + ExpectedError::Inline(regex) => regex.as_str().is_empty(), + ExpectedError::Multiline(_) => false, + } + } + + fn fmt_inline(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "error")?; + if let Self::Inline(regex) = self { + if !regex.as_str().is_empty() { + write!(f, " {regex}")?; + } + } + Ok(()) + } + + fn fmt_multiline(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + if let Self::Multiline(results) = self { + writeln!(f, "----")?; + writeln!(f, "{}", results.trim())?; + } + Ok(()) + } + + pub fn is_match(&self, err: &str) -> bool { + match self { + Self::Inline(regex) => regex.is_match(err), + Self::Multiline(results) => results.trim() == err.trim(), + } + } + + pub fn from_reference(reference: Option<&Self>, err: &str) -> Self { + let multiline = match reference { + Some(Self::Inline(_)) => false, + Some(Self::Multiline(_)) => true, + None => err.trim().lines().next_tuple::<(_, _)>().is_some(), + }; + + if multiline { + Self::Multiline(err.trim().to_string()) + } else { + Self::Inline(Regex::new(®ex::escape(err)).unwrap()) + } + } +} + +impl std::fmt::Display for ExpectedError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + ExpectedError::Inline(regex) => regex.fmt(f), + ExpectedError::Multiline(results) => write!(f, "{}", results.trim()), + } + } +} + +impl PartialEq for ExpectedError { + fn eq(&self, other: &Self) -> bool { + match (self, other) { + (Self::Inline(l0), Self::Inline(r0)) => l0.as_str() == r0.as_str(), + (Self::Multiline(l0), Self::Multiline(r0)) => l0 == r0, + _ => false, + } + } +} + #[derive(Debug, PartialEq, Eq, Clone)] pub enum Control { /// Control sort mode. @@ -416,6 +482,10 @@ pub enum ParseErrorKind { InvalidNumber(String), #[error("invalid error message: {0:?}")] InvalidErrorMessage(String), + #[error("duplicated error messages after error` and under `----`")] + DuplicatedErrorMessage, + #[error("statement should have no result, use `query` instead")] + StatementHasResults, #[error("invalid duration: {0:?}")] InvalidDuration(String), #[error("invalid control: {0:?}")] @@ -524,11 +594,11 @@ fn parse_inner(loc: &Location, script: &str) -> Result {} - ["error", err_str @ ..] => { - let err_str = err_str.join(" "); - expected_error = Some(Regex::new(&err_str).map_err(|_| { - ParseErrorKind::InvalidErrorMessage(err_str).at(loc.clone()) - })?); + ["error", tokens @ ..] => { + expected_error = Some( + ExpectedError::from_inline_tokens(tokens) + .map_err(|e| e.at(loc.clone()))?, + ); } ["count", count_str] => { expected_count = Some(count_str.parse::().map_err(|_| { @@ -541,13 +611,44 @@ fn parse_inner(loc: &Location, script: &str) -> Result line.into(), None => return Err(ParseErrorKind::UnexpectedEOF.at(loc.next_line())), }; + let mut has_multiline_error = false; for (_, line) in &mut lines { if line.is_empty() { break; } + if line == "----" { + has_multiline_error = true; + break; + } sql += "\n"; sql += line; } + if has_multiline_error { + if let Some(e) = &expected_error { + if e.is_empty_inline() { + let mut results = String::new(); + + while let Some((_, line)) = lines.next() { + // 2 consecutive empty lines + if line.is_empty() + && lines.peek().map(|(_, l)| l.is_empty()).unwrap_or(true) + { + lines.next(); + break; + } + results += line; + results.push('\n'); + } + expected_error = + Some(ExpectedError::Multiline(results.trim().to_string())); + } else { + return Err(ParseErrorKind::DuplicatedErrorMessage.at(loc.clone())); + } + } else { + return Err(ParseErrorKind::StatementHasResults.at(loc.clone())); + } + } + records.push(Record::Statement { loc, conditions: std::mem::take(&mut conditions), @@ -563,11 +664,11 @@ fn parse_inner(loc: &Location, script: &str) -> Result { - let err_str = err_str.join(" "); - expected_error = Some(Regex::new(&err_str).map_err(|_| { - ParseErrorKind::InvalidErrorMessage(err_str).at(loc.clone()) - })?); + ["error", tokens @ ..] => { + expected_error = Some( + ExpectedError::from_inline_tokens(tokens) + .map_err(|e| e.at(loc.clone()))?, + ); } [type_str, res @ ..] => { expected_types = type_str @@ -608,11 +709,32 @@ fn parse_inner(loc: &Location, script: &str) -> Result( }) } // Error mismatch, update expected error - (Some(e), _) => Some(Record::Statement { + (Some(e), r) => Some(Record::Statement { sql, - expected_error: Some(Regex::new(®ex::escape(&e.to_string())).unwrap()), + expected_error: Some(ExpectedError::from_reference(r.as_ref(), &e.to_string())), loc, conditions, connection, @@ -1336,10 +1335,13 @@ pub fn update_record_with_output( }); } // Error mismatch - (Some(e), _) => { + (Some(e), r) => { return Some(Record::Query { sql, - expected_error: Some(Regex::new(®ex::escape(&e.to_string())).unwrap()), + expected_error: Some(ExpectedError::from_reference( + r.as_ref(), + &e.to_string(), + )), loc, conditions, connection, From a62f98554e6cfef7fe5fbeef789dc3a35b082e36 Mon Sep 17 00:00:00 2001 From: Bugen Zhao Date: Wed, 8 Nov 2023 16:44:22 +0800 Subject: [PATCH 2/7] fix unparse Signed-off-by: Bugen Zhao --- sqllogictest/src/parser.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/sqllogictest/src/parser.rs b/sqllogictest/src/parser.rs index 54d1210..47bd8c2 100644 --- a/sqllogictest/src/parser.rs +++ b/sqllogictest/src/parser.rs @@ -314,6 +314,7 @@ impl ExpectedError { if let Self::Multiline(results) = self { writeln!(f, "----")?; writeln!(f, "{}", results.trim())?; + writeln!(f)?; // another empty line to indicate the end of multiline message } Ok(()) } @@ -326,14 +327,17 @@ impl ExpectedError { } pub fn from_reference(reference: Option<&Self>, err: &str) -> Self { + let trimmed_err = err.trim(); + let err_is_multiline = trimmed_err.lines().next_tuple::<(_, _)>().is_some(); + let multiline = match reference { - Some(Self::Inline(_)) => false, - Some(Self::Multiline(_)) => true, - None => err.trim().lines().next_tuple::<(_, _)>().is_some(), + Some(Self::Inline(_)) | None => err_is_multiline, // prefer inline as long as it fits + Some(Self::Multiline(_)) => true, /* always multiline if the ref is + * multiline */ }; if multiline { - Self::Multiline(err.trim().to_string()) + Self::Multiline(trimmed_err.to_string()) } else { Self::Inline(Regex::new(®ex::escape(err)).unwrap()) } From ae29a292d9a7f7b47e0c438d7fc594ea7ccfe8c0 Mon Sep 17 00:00:00 2001 From: Bugen Zhao Date: Wed, 8 Nov 2023 16:44:29 +0800 Subject: [PATCH 3/7] add tests Signed-off-by: Bugen Zhao --- sqllogictest/src/runner.rs | 80 ++++++++++++++++++++++++++++++++++++-- tests/harness.rs | 7 +++- tests/slt/basic.slt | 12 ++++++ 3 files changed, 94 insertions(+), 5 deletions(-) diff --git a/sqllogictest/src/runner.rs b/sqllogictest/src/runner.rs index f3563d3..b8fed68 100644 --- a/sqllogictest/src/runner.rs +++ b/sqllogictest/src/runner.rs @@ -1496,6 +1496,30 @@ mod tests { .run() } + #[test] + fn test_query_replacement_error_multiline() { + TestCase { + // input has no query results + input: "query III\n\ + select * from foo;\n\ + ----", + + // Model a run that produced a "MyAwesomeDB Error" + record_output: query_output_error("MyAwesomeDB Error\n\nCaused by:\n Inner Error"), + + expected: Some( + "query error +select * from foo; +---- +TestError: MyAwesomeDB Error + +Caused by: + Inner Error", + ), + } + .run() + } + #[test] fn test_statement_query_output() { TestCase { @@ -1615,6 +1639,30 @@ mod tests { .run() } + #[test] + fn test_statement_error_new_error_multiline() { + TestCase { + // statement expected error + input: "statement error bar\n\ + insert into foo values(2);", + + // Model a run that produced an error message + record_output: statement_output_error("foo\n\nCaused by:\n Inner Error"), + + // expect the output includes foo + expected: Some( + "statement error +insert into foo values(2); +---- +TestError: foo + +Caused by: + Inner Error", + ), + } + .run() + } + #[test] fn test_statement_error_ok_to_error() { TestCase { @@ -1634,6 +1682,30 @@ mod tests { .run() } + #[test] + fn test_statement_error_ok_to_error_multiline() { + TestCase { + // statement was ok + input: "statement ok\n\ + insert into foo values(2);", + + // Model a run that produced an error message + record_output: statement_output_error("foo\n\nCaused by:\n Inner Error"), + + // expect the output includes foo + expected: Some( + "statement error +insert into foo values(2); +---- +TestError: foo + +Caused by: + Inner Error", + ), + } + .run() + } + #[test] fn test_statement_error_special_chars() { TestCase { @@ -1715,13 +1787,13 @@ mod tests { } #[derive(Debug)] - struct TestCase { - input: &'static str, + struct TestCase<'a> { + input: &'a str, record_output: RecordOutput, - expected: Option<&'static str>, + expected: Option<&'a str>, } - impl TestCase { + impl TestCase<'_> { fn run(self) { let Self { input, diff --git a/tests/harness.rs b/tests/harness.rs index 3c15b4d..cd42e99 100644 --- a/tests/harness.rs +++ b/tests/harness.rs @@ -17,7 +17,7 @@ pub struct FakeDBError(String); impl std::fmt::Display for FakeDBError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{self:?}") + write!(f, "{}", self.0) } } @@ -75,6 +75,11 @@ impl sqllogictest::DB for FakeDB { "The operation (describe) is not supported. Did you mean [describe]?".to_string(), )); } + if sql.contains("multiline error") { + return Err(FakeDBError( + "Hey!\n\nYou got:\n Multiline FakeDBError!".to_string(), + )); + } Err(FakeDBError("Hey you got FakeDBError!".to_string())) } } diff --git a/tests/slt/basic.slt b/tests/slt/basic.slt index 2229d7e..6e290c8 100644 --- a/tests/slt/basic.slt +++ b/tests/slt/basic.slt @@ -29,6 +29,18 @@ give me an error statement error Hey you give me an error +statement error +give me a multiline error +---- +Hey! + +You got: + Multiline FakeDBError! + + +statement error Multiline +give me a multiline error + # statement is expected to fail with error: "Hey we", but got error: "Hey you got FakeDBError!" # statement error Hey we # give me an error From fc4c222496b9465680b7c9946268eb8a1eabc13f Mon Sep 17 00:00:00 2001 From: Bugen Zhao Date: Wed, 8 Nov 2023 16:50:41 +0800 Subject: [PATCH 4/7] add docs Signed-off-by: Bugen Zhao --- sqllogictest/src/parser.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/sqllogictest/src/parser.rs b/sqllogictest/src/parser.rs index 47bd8c2..b65d1d0 100644 --- a/sqllogictest/src/parser.rs +++ b/sqllogictest/src/parser.rs @@ -279,13 +279,22 @@ impl std::fmt::Display for Record { } } +/// Expected error message after `error` or under `----`. #[derive(Debug, Clone)] pub enum ExpectedError { + /// An inline regular expression after `error`. + /// + /// The actual error message that matches the regex is considered as a match. Inline(Regex), + /// A multiline error message under `----`, ends with 2 consecutive empty lines. + /// + /// The actual error message that's exactly the same as the expected one is considered as a + /// match. Multiline(String), } impl ExpectedError { + /// Parses an inline regex variant from tokens. fn from_inline_tokens(tokens: &[&str]) -> Result { let err_str = tokens.join(" "); let regex = @@ -293,6 +302,7 @@ impl ExpectedError { Ok(Self::Inline(regex)) } + /// Returns whether it's an empty inline regex. fn is_empty_inline(&self) -> bool { match self { ExpectedError::Inline(regex) => regex.as_str().is_empty(), @@ -300,6 +310,7 @@ impl ExpectedError { } } + /// Unparses the expected message after `error`, if it's inline. fn fmt_inline(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "error")?; if let Self::Inline(regex) = self { @@ -310,6 +321,7 @@ impl ExpectedError { Ok(()) } + /// Unparses the expected message under `----`, if it's multiline. fn fmt_multiline(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { if let Self::Multiline(results) = self { writeln!(f, "----")?; @@ -319,6 +331,7 @@ impl ExpectedError { Ok(()) } + /// Returns whether the given error message matches the expected one. pub fn is_match(&self, err: &str) -> bool { match self { Self::Inline(regex) => regex.is_match(err), @@ -326,6 +339,7 @@ impl ExpectedError { } } + /// Creates an expected error message from a reference and an expected error message. pub fn from_reference(reference: Option<&Self>, err: &str) -> Self { let trimmed_err = err.trim(); let err_is_multiline = trimmed_err.lines().next_tuple::<(_, _)>().is_some(); From 8039d2579ccb8f9094791f6cb1f564471fd5397a Mon Sep 17 00:00:00 2001 From: Bugen Zhao Date: Wed, 8 Nov 2023 17:25:37 +0800 Subject: [PATCH 5/7] bump version and add changelog Signed-off-by: Bugen Zhao --- CHANGELOG.md | 22 ++++++++++++++++++++++ Cargo.lock | 6 +++--- Cargo.toml | 2 +- sqllogictest-bin/Cargo.toml | 4 ++-- sqllogictest-engines/Cargo.toml | 2 +- sqllogictest/src/parser.rs | 1 + 6 files changed, 30 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1cb0367..372fc82 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,28 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +## [0.18.0] - 2023-11-08 + +* Support matching multiline error message under `----` for both `statement error` and `query error`. + ``` + query error + SELECT 1/0; + ---- + db error: ERROR: Failed to execute query + + Caused by these errors: + 1: Failed to evaluate expression: 1/0 + 2: Division by zero + ``` + + The output error message must be the exact match of the expected one to pass the test, except for the leading and trailing whitespaces. Users may use `--override` to let the runner update the test files with the actual output. + + Empty lines are allowed in the expected error message. As a result, the message must end with two consecutive empty lines. + + Breaking changes in the parser: + - Add new variants to `ParseErrorKind`. Mark it as `#[non_exhaustive]`. + - Change the type of `expected_error` from `Regex` to `ExpectedError`, which is either a inline `Regex` or multiline `String`. + ## [0.17.2] - 2023-11-01 * fix(runner): fix parallel testing db name duplication. Now we use full file path instead of filename as the temporary db name in `run_parallel_async`. diff --git a/Cargo.lock b/Cargo.lock index 11bd38a..126521b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1403,7 +1403,7 @@ dependencies = [ [[package]] name = "sqllogictest" -version = "0.17.2" +version = "0.18.0" dependencies = [ "async-trait", "educe", @@ -1426,7 +1426,7 @@ dependencies = [ [[package]] name = "sqllogictest-bin" -version = "0.17.2" +version = "0.18.0" dependencies = [ "anyhow", "async-trait", @@ -1447,7 +1447,7 @@ dependencies = [ [[package]] name = "sqllogictest-engines" -version = "0.17.2" +version = "0.18.0" dependencies = [ "async-trait", "bytes", diff --git a/Cargo.toml b/Cargo.toml index b025df8..aebe372 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,7 +3,7 @@ resolver = "2" members = ["sqllogictest", "sqllogictest-bin", "sqllogictest-engines", "tests"] [workspace.package] -version = "0.17.2" +version = "0.18.0" edition = "2021" homepage = "https://github.com/risinglightdb/sqllogictest-rs" keywords = ["sql", "database", "parser", "cli"] diff --git a/sqllogictest-bin/Cargo.toml b/sqllogictest-bin/Cargo.toml index c3f7c33..af04e20 100644 --- a/sqllogictest-bin/Cargo.toml +++ b/sqllogictest-bin/Cargo.toml @@ -24,8 +24,8 @@ glob = "0.3" itertools = "0.11" quick-junit = { version = "0.3" } rand = "0.8" -sqllogictest = { path = "../sqllogictest", version = "0.17" } -sqllogictest-engines = { path = "../sqllogictest-engines", version = "0.17" } +sqllogictest = { path = "../sqllogictest", version = "0.18" } +sqllogictest-engines = { path = "../sqllogictest-engines", version = "0.18" } tokio = { version = "1", features = [ "rt", "rt-multi-thread", diff --git a/sqllogictest-engines/Cargo.toml b/sqllogictest-engines/Cargo.toml index 0504b2e..8530c97 100644 --- a/sqllogictest-engines/Cargo.toml +++ b/sqllogictest-engines/Cargo.toml @@ -19,7 +19,7 @@ postgres-types = { version = "0.2.5", features = ["derive", "with-chrono-0_4"] } rust_decimal = { version = "1.30.0", features = ["tokio-pg"] } serde = { version = "1", features = ["derive"] } serde_json = "1" -sqllogictest = { path = "../sqllogictest", version = "0.17" } +sqllogictest = { path = "../sqllogictest", version = "0.18" } thiserror = "1" tokio = { version = "1", features = [ "rt", diff --git a/sqllogictest/src/parser.rs b/sqllogictest/src/parser.rs index 227b7f5..3971a0a 100644 --- a/sqllogictest/src/parser.rs +++ b/sqllogictest/src/parser.rs @@ -515,6 +515,7 @@ impl ParseError { /// The error kind for parsing sqllogictest. #[derive(thiserror::Error, Debug, Eq, PartialEq, Clone)] +#[non_exhaustive] pub enum ParseErrorKind { #[error("unexpected token: {0:?}")] UnexpectedToken(String), From 19e77e021a1b034e70fb4febf4742ff5a4027fd3 Mon Sep 17 00:00:00 2001 From: Bugen Zhao Date: Thu, 9 Nov 2023 13:23:35 +0800 Subject: [PATCH 6/7] add empty variant to make it more clear Signed-off-by: Bugen Zhao --- sqllogictest/src/parser.rs | 70 ++++++++++++++++++++++++-------------- sqllogictest/src/runner.rs | 4 +-- 2 files changed, 46 insertions(+), 28 deletions(-) diff --git a/sqllogictest/src/parser.rs b/sqllogictest/src/parser.rs index 3971a0a..c7c4b67 100644 --- a/sqllogictest/src/parser.rs +++ b/sqllogictest/src/parser.rs @@ -283,6 +283,10 @@ impl std::fmt::Display for Record { /// Expected error message after `error` or under `----`. #[derive(Debug, Clone)] pub enum ExpectedError { + /// No expected error message. + /// + /// Any error message is considered as a match. + Empty, /// An inline regular expression after `error`. /// /// The actual error message that matches the regex is considered as a match. @@ -296,33 +300,38 @@ pub enum ExpectedError { impl ExpectedError { /// Parses an inline regex variant from tokens. - fn from_inline_tokens(tokens: &[&str]) -> Result { - let err_str = tokens.join(" "); - let regex = - Regex::new(&err_str).map_err(|_| ParseErrorKind::InvalidErrorMessage(err_str))?; - Ok(Self::Inline(regex)) + fn parse_inline_tokens(tokens: &[&str]) -> Result { + Self::new_inline(tokens.join(" ")) } - /// Returns whether it's an empty inline regex. - fn is_empty_inline(&self) -> bool { - match self { - ExpectedError::Inline(regex) => regex.as_str().is_empty(), - ExpectedError::Multiline(_) => false, + /// Creates an inline expected error message from a regex string. + /// + /// If the regex is empty, it's considered as [`ExpectedError::Empty`]. + fn new_inline(regex: String) -> Result { + if regex.is_empty() { + Ok(Self::Empty) + } else { + let regex = + Regex::new(®ex).map_err(|_| ParseErrorKind::InvalidErrorMessage(regex))?; + Ok(Self::Inline(regex)) } } - /// Unparses the expected message after `error`, if it's inline. + /// Returns whether it's an empty match. + fn is_empty(&self) -> bool { + matches!(self, Self::Empty) + } + + /// Unparses the expected message after `statement`. fn fmt_inline(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "error")?; if let Self::Inline(regex) = self { - if !regex.as_str().is_empty() { - write!(f, " {regex}")?; - } + write!(f, " {regex}")?; } Ok(()) } - /// Unparses the expected message under `----`, if it's multiline. + /// Unparses the expected message with `----`, if it's multiline. fn fmt_multiline(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { if let Self::Multiline(results) = self { writeln!(f, "----")?; @@ -335,26 +344,31 @@ impl ExpectedError { /// Returns whether the given error message matches the expected one. pub fn is_match(&self, err: &str) -> bool { match self { + Self::Empty => true, Self::Inline(regex) => regex.is_match(err), Self::Multiline(results) => results.trim() == err.trim(), } } - /// Creates an expected error message from a reference and an expected error message. - pub fn from_reference(reference: Option<&Self>, err: &str) -> Self { - let trimmed_err = err.trim(); + /// Creates an expected error message from the actual error message. Used by the runner + /// to update the test cases with `--override`. + /// + /// A reference might be provided to help decide whether to use inline or multiline. + pub fn from_actual_error(reference: Option<&Self>, actual_err: &str) -> Self { + let trimmed_err = actual_err.trim(); let err_is_multiline = trimmed_err.lines().next_tuple::<(_, _)>().is_some(); let multiline = match reference { - Some(Self::Inline(_)) | None => err_is_multiline, // prefer inline as long as it fits - Some(Self::Multiline(_)) => true, /* always multiline if the ref is - * multiline */ + Some(Self::Multiline(_)) => true, // always multiline if the ref is multiline + _ => err_is_multiline, // prefer inline as long as it fits }; if multiline { + // Even if the actual error is empty, we still use `Multiline` to indicate that + // an exact empty error is expected, instead of any error by `Empty`. Self::Multiline(trimmed_err.to_string()) } else { - Self::Inline(Regex::new(®ex::escape(err)).unwrap()) + Self::new_inline(regex::escape(actual_err)).expect("escaped regex should be valid") } } } @@ -362,6 +376,7 @@ impl ExpectedError { impl std::fmt::Display for ExpectedError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { + ExpectedError::Empty => Ok(()), ExpectedError::Inline(regex) => regex.fmt(f), ExpectedError::Multiline(results) => write!(f, "{}", results.trim()), } @@ -371,6 +386,7 @@ impl std::fmt::Display for ExpectedError { impl PartialEq for ExpectedError { fn eq(&self, other: &Self) -> bool { match (self, other) { + (Self::Empty, Self::Empty) => true, (Self::Inline(l0), Self::Inline(r0)) => l0.as_str() == r0.as_str(), (Self::Multiline(l0), Self::Multiline(r0)) => l0 == r0, _ => false, @@ -645,7 +661,7 @@ fn parse_inner(loc: &Location, script: &str) -> Result {} ["error", tokens @ ..] => { expected_error = Some( - ExpectedError::from_inline_tokens(tokens) + ExpectedError::parse_inline_tokens(tokens) .map_err(|e| e.at(loc.clone()))?, ); } @@ -674,7 +690,8 @@ fn parse_inner(loc: &Location, script: &str) -> Result(loc: &Location, script: &str) -> Result { expected_error = Some( - ExpectedError::from_inline_tokens(tokens) + ExpectedError::parse_inline_tokens(tokens) .map_err(|e| e.at(loc.clone()))?, ); } @@ -759,7 +776,8 @@ fn parse_inner(loc: &Location, script: &str) -> Result( // Error mismatch, update expected error (Some(e), r) => Some(Record::Statement { sql, - expected_error: Some(ExpectedError::from_reference(r.as_ref(), &e.to_string())), + expected_error: Some(ExpectedError::from_actual_error(r.as_ref(), &e.to_string())), loc, conditions, connection, @@ -1355,7 +1355,7 @@ pub fn update_record_with_output( (Some(e), r) => { return Some(Record::Query { sql, - expected_error: Some(ExpectedError::from_reference( + expected_error: Some(ExpectedError::from_actual_error( r.as_ref(), &e.to_string(), )), From bd0127df56314f4ee95ca863a68084d5636230fd Mon Sep 17 00:00:00 2001 From: Bugen Zhao Date: Thu, 9 Nov 2023 13:25:38 +0800 Subject: [PATCH 7/7] refine display impl Signed-off-by: Bugen Zhao --- sqllogictest/src/parser.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sqllogictest/src/parser.rs b/sqllogictest/src/parser.rs index c7c4b67..c63ed34 100644 --- a/sqllogictest/src/parser.rs +++ b/sqllogictest/src/parser.rs @@ -376,9 +376,9 @@ impl ExpectedError { impl std::fmt::Display for ExpectedError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - ExpectedError::Empty => Ok(()), - ExpectedError::Inline(regex) => regex.fmt(f), - ExpectedError::Multiline(results) => write!(f, "{}", results.trim()), + ExpectedError::Empty => write!(f, "(any)"), + ExpectedError::Inline(regex) => write!(f, "(regex) {}", regex), + ExpectedError::Multiline(results) => write!(f, "(multiline) {}", results.trim()), } } }