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 37b0b11..965dc62 100644 --- a/sqllogictest/src/parser.rs +++ b/sqllogictest/src/parser.rs @@ -1,6 +1,7 @@ //! Sqllogictest parser. use std::fmt; +use std::iter::Peekable; use std::path::Path; use std::sync::Arc; use std::time::Duration; @@ -12,6 +13,8 @@ use regex::Regex; use crate::ColumnType; use crate::ParseErrorKind::InvalidIncludeFile; +const RESULTS_DELIMITER: &str = "----"; + /// The location in source file. #[derive(Debug, PartialEq, Eq, Clone)] pub struct Location { @@ -84,8 +87,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 +104,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 +155,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 +185,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 +209,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, "{}", RESULTS_DELIMITER)?; + + 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: _, @@ -294,6 +283,120 @@ 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. + 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 parse_inline_tokens(tokens: &[&str]) -> Result { + Self::new_inline(tokens.join(" ")) + } + + /// 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)) + } + } + + /// 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 { + write!(f, " {regex}")?; + } + Ok(()) + } + + /// 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, "{}", RESULTS_DELIMITER)?; + writeln!(f, "{}", results.trim())?; + writeln!(f)?; // another empty line to indicate the end of multiline message + } + Ok(()) + } + + /// 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 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::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::new_inline(regex::escape(actual_err)).expect("escaped regex should be valid") + } + } +} + +impl std::fmt::Display for ExpectedError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + ExpectedError::Empty => write!(f, "(any)"), + ExpectedError::Inline(regex) => write!(f, "(regex) {}", regex), + ExpectedError::Multiline(results) => write!(f, "(multiline) {}", results.trim()), + } + } +} + +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, + } + } +} + #[derive(Debug, PartialEq, Eq, Clone)] #[non_exhaustive] pub enum Control { @@ -431,6 +534,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), @@ -446,6 +550,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:?}")] @@ -554,11 +662,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::parse_inline_tokens(tokens) + .map_err(|e| e.at(loc.clone()))?, + ); } ["count", count_str] => { expected_count = Some(count_str.parse::().map_err(|_| { @@ -567,7 +675,20 @@ fn parse_inner(loc: &Location, script: &str) -> Result return Err(ParseErrorKind::InvalidLine(line.into()).at(loc)), }; - let (sql, _) = parse_lines(&mut lines, &loc, None)?; + let (sql, has_results) = parse_lines(&mut lines, &loc, Some(RESULTS_DELIMITER))?; + if has_results { + if let Some(e) = &expected_error { + // If no inline error message is specified, it might be a multiline error. + if e.is_empty() { + expected_error = Some(parse_multiline_error(&mut lines)); + } 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), @@ -583,11 +704,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::parse_inline_tokens(tokens) + .map_err(|e| e.at(loc.clone()))?, + ); } [type_str, res @ ..] => { expected_types = type_str @@ -609,15 +730,24 @@ fn parse_inner(loc: &Location, script: &str) -> Result( Ok((out, found_delimiter)) } +/// Parse multiline error message under `----`. +fn parse_multiline_error<'a>( + lines: &mut Peekable>, +) -> ExpectedError { + 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'); + } + + ExpectedError::Multiline(results.trim().to_string()) +} + #[cfg(test)] mod tests { use std::io::Write; diff --git a/sqllogictest/src/runner.rs b/sqllogictest/src/runner.rs index adbfc3d..7dbd370 100644 --- a/sqllogictest/src/runner.rs +++ b/sqllogictest/src/runner.rs @@ -14,7 +14,6 @@ use futures::{stream, Future, FutureExt, StreamExt}; use itertools::Itertools; use md5::Digest; use owo_colors::OwoColorize; -use regex::Regex; use similar::{Change, ChangeTag, TextDiff}; use crate::parser::*; @@ -1312,9 +1311,9 @@ pub fn update_record_with_output( }) } // 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_actual_error(r.as_ref(), &e.to_string())), loc, conditions, connection, @@ -1353,10 +1352,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_actual_error( + r.as_ref(), + &e.to_string(), + )), loc, conditions, connection, @@ -1511,6 +1513,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 { @@ -1630,6 +1656,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 { @@ -1649,6 +1699,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 { @@ -1730,13 +1804,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