From 594539c5f79dfc8d98c85960b1e207de7de2bd35 Mon Sep 17 00:00:00 2001 From: discord9 Date: Tue, 3 Feb 2026 21:48:36 +0800 Subject: [PATCH] per review Signed-off-by: discord9 --- Cargo.lock | 3 ++ tests/runner/src/cmd/bare.rs | 36 ++++++++++++--------- tests/runner/src/cmd/compat.rs | 58 ++++++++++++++-------------------- tests/runner/src/cmd/kube.rs | 22 ++++++------- tests/runner/src/main.rs | 7 +++- tests/runner/src/version.rs | 38 +++++++++++++++++++++- 6 files changed, 100 insertions(+), 64 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f11ee4c9a3..17673cb0a5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -12605,6 +12605,7 @@ dependencies = [ name = "sqlness-runner" version = "1.0.0-rc.1" dependencies = [ + "anyhow", "async-trait", "clap", "client", @@ -12619,9 +12620,11 @@ dependencies = [ "mysql", "num_cpus", "reqwest", + "semver", "serde", "serde_json", "sha2", + "snafu 0.8.6", "sqlness", "tar", "tempfile", diff --git a/tests/runner/src/cmd/bare.rs b/tests/runner/src/cmd/bare.rs index 03fb88d742..137c4bc0e6 100644 --- a/tests/runner/src/cmd/bare.rs +++ b/tests/runner/src/cmd/bare.rs @@ -15,6 +15,7 @@ use std::path::PathBuf; use std::sync::Arc; +use anyhow::{Context, Result, ensure}; use clap::{Parser, ValueEnum}; use sqlness::interceptor::Registry; use sqlness::{ConfigBuilder, Runner}; @@ -110,11 +111,11 @@ pub struct BareCommand { } impl BareCommand { - pub async fn run(mut self) { + pub async fn run(mut self) -> Result<()> { let temp_dir = tempfile::Builder::new() .prefix("sqlness") .tempdir() - .unwrap(); + .context("Failed to create sqlness temp dir")?; let sqlness_home = temp_dir.keep(); let mut interceptor_registry: Registry = Default::default(); @@ -127,10 +128,8 @@ impl BareCommand { Arc::new(ignore_result::IgnoreResultInterceptorFactory), ); - if let Some(d) = &self.config.case_dir - && !d.is_dir() - { - panic!("{} is not a directory", d.display()); + if let Some(d) = &self.config.case_dir { + ensure!(d.is_dir(), "{} is not a directory", d.display()); } if self.jobs == 0 { self.jobs = num_cpus::get() / 2; @@ -160,7 +159,7 @@ impl BareCommand { .interceptor_registry(interceptor_registry) .parallelism(self.jobs) .build() - .unwrap(); + .context("Failed to build sqlness config")?; let wal = match self.wal { Wal::RaftEngine => WalConfig::RaftEngine, @@ -194,13 +193,7 @@ impl BareCommand { self.extra_args, ), ); - match runner.run().await { - Ok(_) => println!("\x1b[32mAll sqlness tests passed!\x1b[0m"), - Err(e) => { - println!("\x1b[31mTest failed: {}\x1b[0m", e); - std::process::exit(1); - } - } + let run_result = runner.run().await; // clean up and exit if !self.preserve_state { @@ -210,7 +203,20 @@ impl BareCommand { } // TODO(weny): remove postgre and mysql containers println!("Removing state in {:?}", sqlness_home); - tokio::fs::remove_dir_all(sqlness_home).await.unwrap(); + if let Err(err) = tokio::fs::remove_dir_all(&sqlness_home).await { + println!( + "Warning: failed to remove sqlness state dir {}: {}", + sqlness_home.display(), + err + ); + } } + + if let Err(err) = run_result { + return Err(err).context("Sqlness tests failed"); + } + + println!("\x1b[32mAll sqlness tests passed!\x1b[0m"); + Ok(()) } } diff --git a/tests/runner/src/cmd/compat.rs b/tests/runner/src/cmd/compat.rs index f25de3702f..4c2364f86c 100644 --- a/tests/runner/src/cmd/compat.rs +++ b/tests/runner/src/cmd/compat.rs @@ -14,6 +14,7 @@ use std::path::PathBuf; +use anyhow::{Context, Result}; use clap::Parser; use crate::compatibility_runner::CompatibilityRunner; @@ -43,30 +44,20 @@ pub struct CompatCommand { } impl CompatCommand { - pub async fn run(self) { - let from_version = match Version::parse(&self.from) { - Ok(v) => v, - Err(e) => { - println!("Error parsing 'from' version: {}", e); - std::process::exit(1); - } - }; + pub async fn run(self) -> Result<()> { + let from_version = Version::parse(&self.from) + .with_context(|| format!("Error parsing 'from' version: {}", self.from))?; - let to_version = match Version::parse(&self.to) { - Ok(v) => v, - Err(e) => { - println!("Error parsing 'to' version: {}", e); - std::process::exit(1); - } - }; + let to_version = Version::parse(&self.to) + .with_context(|| format!("Error parsing 'to' version: {}", self.to))?; let temp_dir = tempfile::Builder::new() .prefix("compat-test") .tempdir() - .unwrap(); + .context("Failed to create compatibility temp dir")?; let data_dir = temp_dir.keep(); - let runner = match CompatibilityRunner::new( + let runner = CompatibilityRunner::new( from_version, to_version, self.case_dir, @@ -75,28 +66,27 @@ impl CompatCommand { self.fail_fast, ) .await - { - Ok(r) => r, - Err(e) => { - println!("Failed to create compatibility runner: {}", e); - std::process::exit(1); - } - }; + .context("Failed to create compatibility runner")?; - match runner.run().await { - Ok(_) => { - println!("\x1b[32mCompatibility tests passed!\x1b[0m"); - } - Err(e) => { - println!("\x1b[31mCompatibility tests failed: {}\x1b[0m", e); - std::process::exit(1); - } - } + let run_result = runner.run().await; if !self.preserve_state { println!("Stopping etcd"); crate::util::stop_rm_etcd(); - tokio::fs::remove_dir_all(data_dir).await.unwrap(); + if let Err(err) = tokio::fs::remove_dir_all(&data_dir).await { + println!( + "Warning: failed to remove compatibility data dir {}: {}", + data_dir.display(), + err + ); + } } + + if let Err(err) = run_result { + return Err(err).context("Compatibility tests failed"); + } + + println!("\x1b[32mCompatibility tests passed!\x1b[0m"); + Ok(()) } } diff --git a/tests/runner/src/cmd/kube.rs b/tests/runner/src/cmd/kube.rs index 650ed690b2..4a5f6e2dad 100644 --- a/tests/runner/src/cmd/kube.rs +++ b/tests/runner/src/cmd/kube.rs @@ -14,6 +14,7 @@ use std::sync::Arc; +use anyhow::{Context, Result, ensure}; use clap::Parser; use sqlness::interceptor::Registry; use sqlness::{ConfigBuilder, Runner}; @@ -50,17 +51,15 @@ pub struct KubeCommand { } impl KubeCommand { - pub async fn run(self) { + pub async fn run(self) -> Result<()> { let mut interceptor_registry: Registry = Default::default(); interceptor_registry.register( protocol_interceptor::PREFIX, Arc::new(protocol_interceptor::ProtocolInterceptorFactory), ); - if let Some(d) = &self.config.case_dir - && !d.is_dir() - { - panic!("{} is not a directory", d.display()); + if let Some(d) = &self.config.case_dir { + ensure!(d.is_dir(), "{} is not a directory", d.display()); } let config = ConfigBuilder::default() @@ -71,7 +70,7 @@ impl KubeCommand { .env_config_file(self.config.env_config_file) .interceptor_registry(interceptor_registry) .build() - .unwrap(); + .context("Failed to build sqlness config")?; let runner = Runner::new( config, @@ -84,12 +83,9 @@ impl KubeCommand { resources_manager: Arc::new(NaiveResourcesManager::new(self.namespace)), }, ); - match runner.run().await { - Ok(_) => println!("\x1b[32mAll sqlness tests passed!\x1b[0m"), - Err(e) => { - println!("\x1b[31mTest failed: {}\x1b[0m", e); - std::process::exit(1); - } - } + runner.run().await.context("Sqlness tests failed")?; + + println!("\x1b[32mAll sqlness tests passed!\x1b[0m"); + Ok(()) } } diff --git a/tests/runner/src/main.rs b/tests/runner/src/main.rs index 4dcf4d3dd1..de94097d68 100644 --- a/tests/runner/src/main.rs +++ b/tests/runner/src/main.rs @@ -33,9 +33,14 @@ mod version; async fn main() { let cmd = Command::parse(); - match cmd.subcmd { + let result = match cmd.subcmd { SubCommand::Bare(cmd) => cmd.run().await, SubCommand::Kube(cmd) => cmd.run().await, SubCommand::Compat(cmd) => cmd.run().await, + }; + + if let Err(err) = result { + println!("\x1b[31mError: {err:?}\x1b[0m"); + std::process::exit(1); } } diff --git a/tests/runner/src/version.rs b/tests/runner/src/version.rs index f2eb9e081a..12c6dfc2ad 100644 --- a/tests/runner/src/version.rs +++ b/tests/runner/src/version.rs @@ -111,11 +111,46 @@ impl Version { let rank2 = Self::pre_release_rank(&v2.pre); rank1 .cmp(&rank2) - .then_with(|| v1.pre.as_str().cmp(v2.pre.as_str())) + .then_with(|| Self::compare_pre_release_identifiers(&v1.pre, &v2.pre)) } } } + fn compare_pre_release_identifiers(pre1: &Prerelease, pre2: &Prerelease) -> Ordering { + let mut parts1 = pre1.as_str().split('.'); + let mut parts2 = pre2.as_str().split('.'); + + loop { + match (parts1.next(), parts2.next()) { + (None, None) => return Ordering::Equal, + (None, Some(_)) => return Ordering::Less, + (Some(_), None) => return Ordering::Greater, + (Some(a), Some(b)) => { + let ordering = Self::compare_pre_release_part(a, b); + if ordering != Ordering::Equal { + return ordering; + } + } + } + } + } + + fn compare_pre_release_part(a: &str, b: &str) -> Ordering { + let a_numeric = a.chars().all(|c| c.is_ascii_digit()); + let b_numeric = b.chars().all(|c| c.is_ascii_digit()); + + match (a_numeric, b_numeric) { + (true, true) => { + let a_num = a.parse::().unwrap_or(u64::MAX); + let b_num = b.parse::().unwrap_or(u64::MAX); + a_num.cmp(&b_num) + } + (true, false) => Ordering::Less, + (false, true) => Ordering::Greater, + (false, false) => a.cmp(b), + } + } + fn pre_release_rank(pre: &Prerelease) -> u8 { let s = pre.as_str(); if s.starts_with("alpha") { @@ -194,6 +229,7 @@ mod tests { Version::parse("1.0.0-alpha.1").unwrap() < Version::parse("1.0.0-alpha.2").unwrap() ); assert!(Version::parse("1.0.0-alpha.2").unwrap() < Version::parse("1.0.0-beta.1").unwrap()); + assert!(Version::parse("1.0.0-beta.2").unwrap() < Version::parse("1.0.0-beta.10").unwrap()); assert!(Version::parse("1.0.0-beta.1").unwrap() < Version::parse("1.0.0-rc.1").unwrap()); assert!(Version::parse("1.0.0-rc.1").unwrap() < Version::parse("1.0.0").unwrap()); }