From d554415e9c3c62bbcaaef01c065ea3cbbbbd52f4 Mon Sep 17 00:00:00 2001 From: Bugen Zhao Date: Fri, 15 Sep 2023 14:15:37 +0800 Subject: [PATCH] feat: run system commands (#193) --- CHANGELOG.md | 15 +++++ Cargo.lock | 30 ++++++++- Cargo.toml | 2 +- sqllogictest-bin/Cargo.toml | 4 +- sqllogictest-bin/src/engines.rs | 36 +++++++--- sqllogictest-engines/Cargo.toml | 2 +- sqllogictest-engines/src/external.rs | 6 +- sqllogictest-engines/src/postgres/extended.rs | 5 ++ sqllogictest-engines/src/postgres/simple.rs | 5 ++ sqllogictest/Cargo.toml | 3 + sqllogictest/src/parser.rs | 53 +++++++++++---- sqllogictest/src/runner.rs | 66 +++++++++++++++++++ tests/Cargo.toml | 5 ++ tests/system_command/system_command.rs | 62 +++++++++++++++++ tests/system_command/system_command.slt | 15 +++++ tests/system_command/system_command_fail.slt | 2 + 16 files changed, 282 insertions(+), 29 deletions(-) create mode 100644 tests/system_command/system_command.rs create mode 100644 tests/system_command/system_command.slt create mode 100644 tests/system_command/system_command_fail.slt diff --git a/CHANGELOG.md b/CHANGELOG.md index 1651c17..e615195 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +## [0.16.0] - 2023-09-15 + +* Support running external system commands with the syntax below. This is useful for manipulating some external resources during the test. + ``` + system ok + echo "Hello, world!" + ``` + The runner will check the exit code of the command, and the output will be ignored. Currently, only `ok` is supported. + + Changes: + - (parser) **Breaking change**: Add `Record::System`, and corresponding `TestErrorKind` and `RecordOutput`. Mark `TestErrorKind` and `RecordOutput` as `#[non_exhaustive]`. + - (runner) Add `run_command` to `AsyncDB` trait. The default implementation will run the command with `std::process::Command::status`. Implementors can override this method to utilize an asynchronous runtime such as `tokio`. + +* fix(runner): fix database name duplication for parallel tests by using the **full path** of the test file (instead of the file name) as the database name. + ## [0.15.3] - 2023-08-02 * fix(bin): fix error context display. To avoid stack backtrace being printed, unset `RUST_BACKTRACE` environment variable, or use pre-built binaries built with stable toolchain instead. diff --git a/Cargo.lock b/Cargo.lock index ac26143..0dedb65 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -392,6 +392,12 @@ dependencies = [ "typenum", ] +[[package]] +name = "diff" +version = "0.1.13" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "56254986775e3233ffa9c4d7d3faaf6d36a2c09d30b20687e9f88bc8bafc16c8" + [[package]] name = "digest" version = "0.10.7" @@ -1053,6 +1059,16 @@ version = "0.2.17" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5b40af805b3121feab8a3c29f04d8ad262fa8e0561883e7653e024ae4479e6de" +[[package]] +name = "pretty_assertions" +version = "1.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "af7cee1a6c8a5b9208b3cb1061f10c0cb689087b3d8ce85fb9d2dd7a29b6ba66" +dependencies = [ + "diff", + "yansi", +] + [[package]] name = "proc-macro-crate" version = "0.1.5" @@ -1411,7 +1427,7 @@ dependencies = [ [[package]] name = "sqllogictest" -version = "0.15.3" +version = "0.16.0" dependencies = [ "async-trait", "educe", @@ -1423,6 +1439,7 @@ dependencies = [ "libtest-mimic", "md-5", "owo-colors", + "pretty_assertions", "regex", "similar", "tempfile", @@ -1432,7 +1449,7 @@ dependencies = [ [[package]] name = "sqllogictest-bin" -version = "0.15.3" +version = "0.16.0" dependencies = [ "anyhow", "async-trait", @@ -1453,7 +1470,7 @@ dependencies = [ [[package]] name = "sqllogictest-engines" -version = "0.15.3" +version = "0.16.0" dependencies = [ "async-trait", "bytes", @@ -1549,6 +1566,7 @@ dependencies = [ name = "tests" version = "0.1.0" dependencies = [ + "regex", "sqllogictest", ] @@ -2013,3 +2031,9 @@ checksum = "05f360fc0b24296329c78fda852a1e9ae82de9cf7b27dae4b7f62f118f77b9ed" dependencies = [ "tap", ] + +[[package]] +name = "yansi" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "09041cd90cf85f7f8b2df60c646f853b7f535ce68f85244eb6731cf89fa498ec" diff --git a/Cargo.toml b/Cargo.toml index 486de8c..02e22d3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -2,7 +2,7 @@ members = ["sqllogictest", "sqllogictest-bin", "sqllogictest-engines", "tests"] [workspace.package] -version = "0.15.3" +version = "0.16.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 401ac6f..fd30801 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.15" } -sqllogictest-engines = { path = "../sqllogictest-engines", version = "0.15" } +sqllogictest = { path = "../sqllogictest", version = "0.16" } +sqllogictest-engines = { path = "../sqllogictest-engines", version = "0.16" } tokio = { version = "1", features = [ "rt", "rt-multi-thread", diff --git a/sqllogictest-bin/src/engines.rs b/sqllogictest-bin/src/engines.rs index 05c5bc3..d178f55 100644 --- a/sqllogictest-bin/src/engines.rs +++ b/sqllogictest-bin/src/engines.rs @@ -1,4 +1,6 @@ use std::fmt::Display; +use std::process::ExitStatus; +use std::time::Duration; use async_trait::async_trait; use clap::ValueEnum; @@ -97,14 +99,14 @@ impl std::error::Error for EnginesError { } } -impl Engines { - async fn run(&mut self, sql: &str) -> Result, anyhow::Error> { - Ok(match self { - Engines::Postgres(e) => e.run(sql).await?, - Engines::PostgresExtended(e) => e.run(sql).await?, - Engines::External(e) => e.run(sql).await?, - }) - } +macro_rules! dispatch_engines { + ($impl:expr, $inner:ident, $body:tt) => {{ + match $impl { + Engines::Postgres($inner) => $body, + Engines::PostgresExtended($inner) => $body, + Engines::External($inner) => $body, + } + }}; } #[async_trait] @@ -113,6 +115,22 @@ impl AsyncDB for Engines { type ColumnType = DefaultColumnType; async fn run(&mut self, sql: &str) -> Result, Self::Error> { - self.run(sql).await.map_err(EnginesError) + dispatch_engines!(self, e, { + e.run(sql) + .await + .map_err(|e| EnginesError(anyhow::Error::from(e))) + }) + } + + fn engine_name(&self) -> &str { + dispatch_engines!(self, e, { e.engine_name() }) + } + + async fn sleep(dur: Duration) { + tokio::time::sleep(dur).await + } + + async fn run_command(command: std::process::Command) -> std::io::Result { + Command::from(command).status().await } } diff --git a/sqllogictest-engines/Cargo.toml b/sqllogictest-engines/Cargo.toml index a1143e9..43acb3e 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.15" } +sqllogictest = { path = "../sqllogictest", version = "0.16" } thiserror = "1" tokio = { version = "1", features = [ "rt", diff --git a/sqllogictest-engines/src/external.rs b/sqllogictest-engines/src/external.rs index a03fa49..64a5238 100644 --- a/sqllogictest-engines/src/external.rs +++ b/sqllogictest-engines/src/external.rs @@ -1,6 +1,6 @@ use std::io; use std::marker::PhantomData; -use std::process::Stdio; +use std::process::{ExitStatus, Stdio}; use std::time::Duration; use async_trait::async_trait; @@ -120,6 +120,10 @@ impl AsyncDB for ExternalDriver { async fn sleep(dur: Duration) { tokio::time::sleep(dur).await } + + async fn run_command(command: std::process::Command) -> std::io::Result { + Command::from(command).status().await + } } struct JsonDecoder(PhantomData); diff --git a/sqllogictest-engines/src/postgres/extended.rs b/sqllogictest-engines/src/postgres/extended.rs index 1dce8f0..cef6beb 100644 --- a/sqllogictest-engines/src/postgres/extended.rs +++ b/sqllogictest-engines/src/postgres/extended.rs @@ -1,4 +1,5 @@ use std::fmt::Write; +use std::process::{Command, ExitStatus}; use std::time::Duration; use async_trait::async_trait; @@ -317,4 +318,8 @@ impl sqllogictest::AsyncDB for Postgres { async fn sleep(dur: Duration) { tokio::time::sleep(dur).await } + + async fn run_command(command: Command) -> std::io::Result { + tokio::process::Command::from(command).status().await + } } diff --git a/sqllogictest-engines/src/postgres/simple.rs b/sqllogictest-engines/src/postgres/simple.rs index 767acfc..690a7ad 100644 --- a/sqllogictest-engines/src/postgres/simple.rs +++ b/sqllogictest-engines/src/postgres/simple.rs @@ -1,3 +1,4 @@ +use std::process::{Command, ExitStatus}; use std::time::Duration; use async_trait::async_trait; @@ -64,4 +65,8 @@ impl sqllogictest::AsyncDB for Postgres { async fn sleep(dur: Duration) { tokio::time::sleep(dur).await } + + async fn run_command(command: Command) -> std::io::Result { + tokio::process::Command::from(command).status().await + } } diff --git a/sqllogictest/Cargo.toml b/sqllogictest/Cargo.toml index b0740d9..f2fdf73 100644 --- a/sqllogictest/Cargo.toml +++ b/sqllogictest/Cargo.toml @@ -24,3 +24,6 @@ regex = "1.9.1" owo-colors = "3.5.0" md-5 = "0.10" fs-err = "2.9.0" + +[dev-dependencies] +pretty_assertions = "1" diff --git a/sqllogictest/src/parser.rs b/sqllogictest/src/parser.rs index 14a7a24..4de1e56 100644 --- a/sqllogictest/src/parser.rs +++ b/sqllogictest/src/parser.rs @@ -109,6 +109,14 @@ pub enum Record { /// The expected results. expected_results: Vec, }, + /// A system command is an external command that is to be executed by the shell. Currently it + /// must succeed and the output is ignored. + System { + loc: Location, + conditions: Vec, + /// The external command. + command: String, + }, /// A sleep period. Sleep { loc: Location, @@ -239,6 +247,13 @@ impl std::fmt::Display for Record { // query always ends with a blank line writeln!(f) } + Record::System { + loc: _, + conditions: _, + command, + } => { + writeln!(f, "system ok\n{command}") + } Record::Sleep { loc: _, duration } => { write!(f, "sleep {}", humantime::format_duration(*duration)) } @@ -612,6 +627,25 @@ fn parse_inner(loc: &Location, script: &str) -> Result { + // TODO: we don't support asserting error message for system command + let mut command = match lines.next() { + Some((_, line)) => line.into(), + None => return Err(ParseErrorKind::UnexpectedEOF.at(loc.next_line())), + }; + for (_, line) in &mut lines { + if line.is_empty() { + break; + } + command += "\n"; + command += line; + } + records.push(Record::System { + loc, + conditions: std::mem::take(&mut conditions), + command, + }); + } ["control", res @ ..] => match res { ["sortmode", sort_mode] => match SortMode::try_from_str(sort_mode) { Ok(sort_mode) => records.push(Record::Control(Control::SortMode(sort_mode))), @@ -739,6 +773,11 @@ mod tests { parse_roundtrip::("../tests/custom_type/custom_type.slt") } + #[test] + fn test_system_command() { + parse_roundtrip::("../tests/system_command/system_command.slt") + } + #[test] fn test_fail_unknown_type() { let script = "\ @@ -805,18 +844,7 @@ select * from foo; let records = normalize_filename(records); let reparsed_records = normalize_filename(reparsed_records); - assert_eq!( - records, reparsed_records, - "Mismatch in reparsed records\n\ - *********\n\ - original:\n\ - *********\n\ - {records:#?}\n\n\ - *********\n\ - reparsed:\n\ - *********\n\ - {reparsed_records:#?}\n\n", - ); + pretty_assertions::assert_eq!(records, reparsed_records, "Mismatch in reparsed records"); } /// Replaces the actual filename in all Records with @@ -829,6 +857,7 @@ select * from foo; match &mut record { Record::Include { loc, .. } => normalize_loc(loc), Record::Statement { loc, .. } => normalize_loc(loc), + Record::System { loc, .. } => normalize_loc(loc), Record::Query { loc, .. } => normalize_loc(loc), Record::Sleep { loc, .. } => normalize_loc(loc), Record::Subtest { loc, .. } => normalize_loc(loc), diff --git a/sqllogictest/src/runner.rs b/sqllogictest/src/runner.rs index ecb48fc..6916eaa 100644 --- a/sqllogictest/src/runner.rs +++ b/sqllogictest/src/runner.rs @@ -3,6 +3,7 @@ use std::collections::HashSet; use std::fmt::{Debug, Display}; use std::path::Path; +use std::process::{Command, ExitStatus}; use std::sync::Arc; use std::time::Duration; use std::vec; @@ -21,6 +22,7 @@ use crate::parser::*; use crate::{ColumnType, Connections, MakeConnection}; #[derive(Debug, Clone)] +#[non_exhaustive] pub enum RecordOutput { Nothing, Query { @@ -32,6 +34,9 @@ pub enum RecordOutput { count: u64, error: Option>, }, + System { + error: Option>, + }, } #[non_exhaustive] @@ -72,6 +77,15 @@ pub trait AsyncDB { async fn sleep(dur: Duration) { std::thread::sleep(dur); } + + /// [`Runner`] calls this function to run a system command. + /// + /// The default implementation is `std::process::Command::status`, which is universial to any + /// async runtime but would block the current thread. If you are running in tokio runtime, you + /// should override this by `tokio::process::Command::status`. + async fn run_command(mut command: Command) -> std::io::Result { + command.status() + } } /// The database to be tested. @@ -225,6 +239,7 @@ impl std::fmt::Display for RecordKind { /// /// For colored error message, use `self.display()`. #[derive(thiserror::Error, Debug, Clone)] +#[non_exhaustive] pub enum TestErrorKind { #[error("parse error: {0}")] ParseError(ParseErrorKind), @@ -236,6 +251,11 @@ pub enum TestErrorKind { err: Arc, kind: RecordKind, }, + #[error("system command failed: {err}\n[CMD] {command}")] + SystemFail { + command: String, + err: Arc, + }, // Remember to also update [`TestErrorKindDisplay`] if this message is changed. #[error("{kind} is expected to fail with error:\n\t{expected_err}\nbut got error:\n\t{err}\n[SQL] {sql}")] ErrorMismatch { @@ -570,6 +590,40 @@ impl> Runner { }, } } + Record::System { + conditions, + command, + loc: _, + } => { + let command = self.replace_keywords(command); + + if should_skip(&self.labels, "", &conditions) { + return RecordOutput::Nothing; + } + + let cmd = if cfg!(target_os = "windows") { + let mut cmd = std::process::Command::new("cmd"); + cmd.arg("/C").arg(command); + cmd + } else { + let mut cmd = std::process::Command::new("sh"); + cmd.arg("-c").arg(command); + cmd + }; + let result = D::run_command(cmd).await; + + #[derive(thiserror::Error, Debug)] + #[error("process exited unsuccessfully: {0}")] // message from unstable `ExitStatusError` + struct SystemError(ExitStatus); + + let error: Option> = match result { + Ok(status) if status.success() => None, + Ok(status) => Some(Arc::new(SystemError(status))), + Err(e) => Some(Arc::new(e)), + }; + + RecordOutput::System { error } + } Record::Query { conditions, connection, @@ -839,6 +893,18 @@ impl> Runner { .at(loc)); } } + ( + Record::System { + loc, + conditions: _, + command, + }, + RecordOutput::System { error }, + ) => { + if let Some(err) = error { + return Err(TestErrorKind::SystemFail { command, err }.at(loc)); + } + } _ => unreachable!(), } diff --git a/tests/Cargo.toml b/tests/Cargo.toml index 449f551..3fad772 100644 --- a/tests/Cargo.toml +++ b/tests/Cargo.toml @@ -5,6 +5,7 @@ edition = "2021" publish = false [dependencies] +regex = "1.9.1" sqllogictest = { path = "../sqllogictest" } [[test]] @@ -23,3 +24,7 @@ path = "./validator/validator.rs" [[test]] name = "test_dir_escape" path = "./test_dir_escape/test_dir_escape.rs" + +[[test]] +name = "system_command" +path = "./system_command/system_command.rs" diff --git a/tests/system_command/system_command.rs b/tests/system_command/system_command.rs new file mode 100644 index 0000000..5566afa --- /dev/null +++ b/tests/system_command/system_command.rs @@ -0,0 +1,62 @@ +use sqllogictest::{DBOutput, DefaultColumnType}; + +pub struct FakeDB; + +#[derive(Debug)] +pub struct FakeDBError; + +impl std::fmt::Display for FakeDBError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{self:?}") + } +} + +impl std::error::Error for FakeDBError {} + +impl sqllogictest::DB for FakeDB { + type Error = FakeDBError; + type ColumnType = DefaultColumnType; + + fn run(&mut self, sql: &str) -> Result, FakeDBError> { + let path = regex::Regex::new(r#"^select read\("(.+)"\)$"#) + .unwrap() + .captures(sql) + .and_then(|c| c.get(1)) + .ok_or(FakeDBError)? + .as_str(); + + let content = std::fs::read_to_string(path) + .map_err(|_| FakeDBError)? + .trim() + .to_string(); + + Ok(DBOutput::Rows { + types: vec![DefaultColumnType::Text], + rows: vec![vec![content]], + }) + } +} + +#[test] +fn test() { + let mut tester = sqllogictest::Runner::new(|| async { Ok(FakeDB) }); + // enable `__TEST_DIR__` override + tester.enable_testdir(); + + tester + .run_file("./system_command/system_command.slt") + .unwrap(); +} + +#[test] +fn test_fail() { + let mut tester = sqllogictest::Runner::new(|| async { Ok(FakeDB) }); + // enable `__TEST_DIR__` override + tester.enable_testdir(); + + let err = tester + .run_file("./system_command/system_command_fail.slt") + .unwrap_err(); + + assert!(err.to_string().contains("system command failed"), "{err}"); +} diff --git a/tests/system_command/system_command.slt b/tests/system_command/system_command.slt new file mode 100644 index 0000000..d0f78d9 --- /dev/null +++ b/tests/system_command/system_command.slt @@ -0,0 +1,15 @@ +system ok +echo 114514 > __TEST_DIR__/test.txt + +query T +select read("__TEST_DIR__/test.txt") +---- +114514 + +system ok +echo 1919810 > __TEST_DIR__/test.txt + +query T +select read("__TEST_DIR__/test.txt") +---- +1919810 diff --git a/tests/system_command/system_command_fail.slt b/tests/system_command/system_command_fail.slt new file mode 100644 index 0000000..2831572 --- /dev/null +++ b/tests/system_command/system_command_fail.slt @@ -0,0 +1,2 @@ +system ok +__TEST_DIR__/something_than_not_exist