From da5ef9472643a350449e3d0ca9cffd77f5e2840b Mon Sep 17 00:00:00 2001
From: Bugen Zhao <i@bugenzhao.com>
Date: Thu, 20 Jun 2024 16:09:30 +0800
Subject: [PATCH] fix: inherit configuration in `run_parallel` (#215)

---
 .github/workflows/ci.yml         |  2 +-
 CHANGELOG.md                     |  4 ++++
 Cargo.lock                       |  6 +++---
 Cargo.toml                       |  2 +-
 sqllogictest/src/runner.rs       | 21 ++++++++++++++++++---
 sqllogictest/src/substitution.rs |  6 +++---
 6 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index fcc49ac..06cca21 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -63,7 +63,7 @@ jobs:
         uses: actions-rs/cargo@v1
         with:
           command: install
-          args: cargo-semver-checks --version ^0.27 --locked
+          args: cargo-semver-checks --version ^0.32 --locked
       - name: Check semver
         run: |
          cargo semver-checks check-release -p sqllogictest
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 35253b2..7298d1a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## Unreleased
 
+## [0.20.5] - 2024-06-20
+
+* fix(runner): when running in parallel, the runner will correctly inherit configuration like `sort_mode` and `labels` from the main runner.
+
 ## [0.20.4] - 2024-06-06
 
 * bump dependencies
diff --git a/Cargo.lock b/Cargo.lock
index 08157c9..df2b6eb 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1414,7 +1414,7 @@ dependencies = [
 
 [[package]]
 name = "sqllogictest"
-version = "0.20.4"
+version = "0.20.5"
 dependencies = [
  "async-trait",
  "educe",
@@ -1437,7 +1437,7 @@ dependencies = [
 
 [[package]]
 name = "sqllogictest-bin"
-version = "0.20.4"
+version = "0.20.5"
 dependencies = [
  "anyhow",
  "async-trait",
@@ -1459,7 +1459,7 @@ dependencies = [
 
 [[package]]
 name = "sqllogictest-engines"
-version = "0.20.4"
+version = "0.20.5"
 dependencies = [
  "async-trait",
  "bytes",
diff --git a/Cargo.toml b/Cargo.toml
index 4a6718b..5d28fb5 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -3,7 +3,7 @@ resolver = "2"
 members = ["sqllogictest", "sqllogictest-bin", "sqllogictest-engines", "tests"]
 
 [workspace.package]
-version = "0.20.4"
+version = "0.20.5"
 edition = "2021"
 homepage = "https://github.com/risinglightdb/sqllogictest-rs"
 keywords = ["sql", "database", "parser", "cli"]
diff --git a/sqllogictest/src/runner.rs b/sqllogictest/src/runner.rs
index 4cc5221..2fa9f49 100644
--- a/sqllogictest/src/runner.rs
+++ b/sqllogictest/src/runner.rs
@@ -649,7 +649,8 @@ impl<D: AsyncDB, M: MakeConnection<Conn = D>> Runner<D, M> {
                 };
 
                 if is_background {
-                    // Spawn a new process, but don't wait for stdout, otherwise it will block until the process exits.
+                    // Spawn a new process, but don't wait for stdout, otherwise it will block until
+                    // the process exits.
                     let error: Option<AnyError> = match cmd.spawn() {
                         Ok(_) => None,
                         Err(e) => Some(Arc::new(e)),
@@ -1090,6 +1091,9 @@ impl<D: AsyncDB, M: MakeConnection<Conn = D>> Runner<D, M> {
 
     /// accept the tasks, spawn jobs task to run slt test. the tasks are (AsyncDB, slt filename)
     /// pairs.
+    // TODO: This is not a good interface, as the `make_conn` passed to `new` is unused but we
+    // accept a new `conn_builder` here. May change `MakeConnection` to support specifying the
+    // database name in the future.
     pub async fn run_parallel_async<Fut>(
         &mut self,
         glob: &str,
@@ -1115,9 +1119,20 @@ impl<D: AsyncDB, M: MakeConnection<Conn = D>> Runner<D, M> {
                 .await
                 .expect("create db failed");
             let target = hosts[idx % hosts.len()].clone();
+
+            let mut tester = Runner {
+                conn: Connections::new(move || {
+                    conn_builder(target.clone(), db_name.clone()).map(Ok)
+                }),
+                validator: self.validator,
+                column_type_validator: self.column_type_validator,
+                substitution: self.substitution.clone(),
+                sort_mode: self.sort_mode,
+                hash_threshold: self.hash_threshold,
+                labels: self.labels.clone(),
+            };
+
             tasks.push(async move {
-                let mut tester =
-                    Runner::new(move || conn_builder(target.clone(), db_name.clone()).map(Ok));
                 let filename = file.to_string_lossy().to_string();
                 tester.run_file_async(filename).await
             })
diff --git a/sqllogictest/src/substitution.rs b/sqllogictest/src/substitution.rs
index 7ca27da..f350163 100644
--- a/sqllogictest/src/substitution.rs
+++ b/sqllogictest/src/substitution.rs
@@ -1,14 +1,14 @@
-use std::sync::OnceLock;
+use std::sync::{Arc, OnceLock};
 
 use subst::Env;
 use tempfile::{tempdir, TempDir};
 
 /// Substitute environment variables and special variables like `__TEST_DIR__` in SQL.
-#[derive(Default)]
+#[derive(Default, Clone)]
 pub(crate) struct Substitution {
     /// The temporary directory for `__TEST_DIR__`.
     /// Lazily initialized and cleaned up when dropped.
-    test_dir: OnceLock<TempDir>,
+    test_dir: Arc<OnceLock<TempDir>>,
 }
 
 impl<'a> subst::VariableMap<'a> for Substitution {