From 8d6bfde5a03a2383d9990e53c9589ed1d91831fe Mon Sep 17 00:00:00 2001 From: Jason Volk Date: Wed, 1 Oct 2025 02:41:24 +0000 Subject: [PATCH] Use insta and criterion for main integration test and benches respectively. docker/ci: Separate integration and unit tests and benches jobs. Add directives to remove db before/after integration tests are performed. Split start/run/stop phases; add more granular smoketests. Split main integration tests into units for isolation. Signed-off-by: Jason Volk --- .github/workflows/test.yml | 13 +- Cargo.lock | 4 + Cargo.toml | 2 +- docker/Dockerfile.cargo | 2 +- docker/Dockerfile.nix | 12 +- docker/bake.hcl | 133 +++++++++++------- src/core/Cargo.toml | 2 +- src/core/args.rs | 29 +++- src/database/Cargo.toml | 8 +- src/database/benches/ser.rs | 2 +- src/database/engine/context.rs | 57 +++++++- src/database/engine/open.rs | 2 + src/macros/Cargo.toml | 1 + src/main/Cargo.toml | 20 ++- src/main/benches/main.rs | 35 +++++ src/main/lib.rs | 58 ++++++-- src/main/tests/smoke.rs | 16 ++- src/main/tests/smoke_async.rs | 24 ++++ src/main/tests/smoke_shutdown.rs | 29 ++++ .../snapshots/smoke__smoke@smoke_test.snap | 8 ++ .../smoke_async__smoke_async@smoke_async.snap | 8 ++ ...utdown__smoke_shutdown@smoke_shutdown.snap | 8 ++ 22 files changed, 373 insertions(+), 100 deletions(-) create mode 100644 src/main/benches/main.rs create mode 100644 src/main/tests/smoke_async.rs create mode 100644 src/main/tests/smoke_shutdown.rs create mode 100644 src/main/tests/snapshots/smoke__smoke@smoke_test.snap create mode 100644 src/main/tests/snapshots/smoke_async__smoke_async@smoke_async.snap create mode 100644 src/main/tests/snapshots/smoke_shutdown__smoke_shutdown@smoke_shutdown.snap diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index dc0336bb..07044197 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -87,7 +87,7 @@ jobs: name: Unit uses: ./.github/workflows/bake.yml with: - bake_targets: '["unit"]' + bake_targets: '["unit", "integration"]' cargo_profiles: '["test"]' feat_sets: '["all"]' rust_toolchains: ${{inputs.rust_toolchains}} @@ -110,7 +110,7 @@ jobs: name: Bench uses: ./.github/workflows/bake.yml with: - bake_targets: '["unit"]' + bake_targets: '["unit", "integration"]' cargo_profiles: '["bench"]' feat_sets: '["all"]' rust_toolchains: '["nightly"]' @@ -222,7 +222,7 @@ jobs: excludes: ${{inputs.excludes}} includes: ${{inputs.includes}} - rust-sdk-integration: + matrix-rust-sdk-integration: if: > !failure() && !cancelled() && !contains(inputs.head_msg, '[ci no build]') @@ -232,7 +232,8 @@ jobs: && contains(fromJSON(inputs.rust_toolchains), fromJSON('["nightly"]')[0]) && contains(fromJSON(inputs.sys_targets), fromJSON('["x86_64-v1-linux-gnu"]')[0]) - name: Rust SDK Integration + name: Matrix SDK Integration + needs: [smoke] uses: ./.github/workflows/bake.yml with: bake_targets: '["matrix-rust-sdk-integration"]' @@ -251,7 +252,7 @@ jobs: { "matrix-rust-sdk-integration": { "src": "/var/log/tuwunel.log", - "dst": "rust-sdk-integration.tuwunel.log", + "dst": "matrix-rust-sdk-integration.tuwunel.log", } } @@ -293,7 +294,7 @@ jobs: && contains(fromJSON(inputs.sys_targets), fromJSON('["x86_64-v1-linux-gnu"]')[0]) name: Matrix Compliance - needs: [complement] + needs: [complement, smoke] runs-on: ["${{matrix.machine}}", "${{inputs.complement_runner}}"] concurrency: group: complement-cant-walk-and-chew-bubblegum diff --git a/Cargo.lock b/Cargo.lock index d2ce90d5..552edc82 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4962,13 +4962,17 @@ version = "1.4.2" dependencies = [ "clap", "const-str", + "criterion", "ctor", + "insta", "log", + "maplit", "opentelemetry", "opentelemetry_sdk", "sentry", "sentry-tower", "sentry-tracing", + "similar", "tokio", "tokio-metrics", "tracing", diff --git a/Cargo.toml b/Cargo.toml index b08a2f3b..67526ed1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -703,7 +703,7 @@ inherits = "release-native.build-override" #] [profile.bench] -debug = "limited" +debug = false strip = false #rustflags = [ # "-Cremark=all", diff --git a/docker/Dockerfile.cargo b/docker/Dockerfile.cargo index 7678bd1a..eb255fa6 100644 --- a/docker/Dockerfile.cargo +++ b/docker/Dockerfile.cargo @@ -87,7 +87,7 @@ RUN \ "--manifest-path=Cargo.toml" \ ${recipe_args} \ ${color_args} \ - ${cargo_args} + $cargo_args # If this image is further reused with other cargo commands, all # modifications made by cargo chef cook outside of target-dir have to be diff --git a/docker/Dockerfile.nix b/docker/Dockerfile.nix index f3e1feae..e4b421d0 100644 --- a/docker/Dockerfile.nix +++ b/docker/Dockerfile.nix @@ -26,14 +26,15 @@ ARG sys_target WORKDIR /usr/src/tuwunel COPY --link --from=source /usr/src/tuwunel . -ENV TUWUNEL_DATABASE_PATH="/tmp/buildtest.db" RUN \ --mount=type=cache,dst=/nix,sharing=shared \ --mount=type=cache,dst=/root/.cache/nix,sharing=shared \ --mount=type=cache,dst=/root/.local/state/nix,sharing=shared \ <, - /// Set functional testing modes if available. Ex '--test=smoke' - #[arg(long, hide(true))] + /// Set functional testing modes if available. Ex '--test=smoke'. Empty + /// values are permitted for compatibility with testing and benchmarking + /// frameworks which may simply pass `--test` to the same execution. + #[arg( + long, + hide(true), + num_args = 0..=1, + require_equals(false), + default_missing_value = "", + )] pub test: Vec, + /// Compatibility option for benchmark frameworks which pass `--bench` to + /// the same execution and must be silently accepted without error. + #[arg( + long, + hide(true), + num_args = 0..=1, + require_equals(false), + default_missing_value = "", + )] + pub bench: Vec, + /// Override the tokio worker_thread count. #[arg( long, @@ -151,12 +170,12 @@ pub struct Args { impl Args { #[must_use] - pub fn default_test(name: &str) -> Self { + pub fn default_test(name: &[&str]) -> Self { let mut args = Self::default(); - args.test.push(name.into()); + args.test + .extend(name.iter().copied().map(ToOwned::to_owned)); args.option .push("server_name=\"localhost\"".into()); - args } } diff --git a/src/database/Cargo.toml b/src/database/Cargo.toml index 28a17214..89c63ca9 100644 --- a/src/database/Cargo.toml +++ b/src/database/Cargo.toml @@ -17,10 +17,6 @@ crate-type = [ # "dylib", ] -[[bench]] -name = "ser" -harness = false - [features] bzip2_compression = [ "rust-rocksdb/bzip2", @@ -76,3 +72,7 @@ criterion.workspace = true [lints] workspace = true + +[[bench]] +name = "ser" +harness = false diff --git a/src/database/benches/ser.rs b/src/database/benches/ser.rs index a25a29c0..7ac20fe2 100644 --- a/src/database/benches/ser.rs +++ b/src/database/benches/ser.rs @@ -1,6 +1,6 @@ use criterion::{Criterion, criterion_group, criterion_main}; -criterion_group!(benches, ser_str,); +criterion_group!(benches, ser_str); criterion_main!(benches); diff --git a/src/database/engine/context.rs b/src/database/engine/context.rs index c089da2a..cc1ee517 100644 --- a/src/database/engine/context.rs +++ b/src/database/engine/context.rs @@ -1,10 +1,15 @@ use std::{ collections::BTreeMap, + fs::remove_dir_all, + path::Path, sync::{Arc, Mutex}, }; use rocksdb::{Cache, Env, LruCacheOptions}; -use tuwunel_core::{Result, Server, debug, utils::math::usize_from_f64}; +use tuwunel_core::{ + Result, Server, debug, + utils::{math::usize_from_f64, result::LogErr}, +}; use crate::{or_else, pool::Pool}; @@ -78,5 +83,55 @@ impl Drop for Context { debug!("Joining background threads..."); env.join_all_threads(); + + after_close(self, &self.server.config.database_path) + .expect("Failed to execute after_close handler"); } } + +/// For unit and integration tests the 'fresh' directive deletes found db. +pub(super) fn before_open(ctx: &Arc, path: &Path) -> Result { + if ctx.server.config.test.contains("fresh") { + match delete_database_for_testing(ctx, path) { + | Err(e) if !e.is_not_found() => return Err(e), + | _ => (), + } + } + + Ok(()) +} + +/// For unit and integration tests the 'cleanup' directive deletes after close +/// to cleanup. +fn after_close(ctx: &Context, path: &Path) -> Result { + if ctx.server.config.test.contains("cleanup") { + delete_database_for_testing(ctx, path) + .log_err() + .ok(); + } + + Ok(()) +} + +/// For unit and integration tests; removes the database directory when called. +/// To prevent misuse, cfg!(test) must be true for a unit test or the +/// integration test server is named localhost. +#[tracing::instrument(level = "debug", skip_all)] +fn delete_database_for_testing(ctx: &Context, path: &Path) -> Result { + let config = &ctx.server.config; + let localhost = config + .server_name + .as_str() + .starts_with("localhost"); + + if !cfg!(test) && !localhost { + return Ok(()); + } + + debug_assert!( + config.test.contains("cleanup") | config.test.contains("fresh"), + "missing any test directive legitimating this call.", + ); + + remove_dir_all(path).map_err(Into::into) +} diff --git a/src/database/engine/open.rs b/src/database/engine/open.rs index c87fd64a..5d63dcc8 100644 --- a/src/database/engine/open.rs +++ b/src/database/engine/open.rs @@ -10,6 +10,7 @@ use tuwunel_core::{Result, debug, implement, info, warn}; use super::{ Db, Engine, cf_opts::cf_options, + context, db_opts::db_options, descriptor::{self, Descriptor}, repair::repair, @@ -23,6 +24,7 @@ pub(crate) async fn open(ctx: Arc, desc: &[Descriptor]) -> Result, runtime: Runtime) -> Result { } pub fn run(server: &Arc, runtime: &Runtime) -> Result { - runtime.spawn(signals::enable(server.clone())); - runtime.block_on(run_async(server)) + runtime.block_on(async_exec(server)) } /// Operate the server normally in release-mode static builds. This will start, /// run and stop the server within the asynchronous runtime. -#[cfg(any(not(tuwunel_mods), not(feature = "tuwunel_mods")))] #[tracing::instrument( name = "main", parent = None, skip_all )] -pub async fn run_async(server: &Arc) -> Result { +pub async fn async_exec(server: &Arc) -> Result { + let signals = server + .server + .runtime() + .spawn(signals::enable(server.clone())); + + async_start(server).await?; + async_run(server).await?; + async_stop(server).await?; + signals.await?; + + debug_info!("Exit runtime"); + Ok(()) +} + +#[cfg(any(not(tuwunel_mods), not(feature = "tuwunel_mods")))] +pub async fn async_start(server: &Arc) -> Result> { extern crate tuwunel_router as router; - match router::start(&server.server).await { - | Ok(services) => server.services.lock().await.insert(services), + Ok(match router::start(&server.server).await { + | Ok(services) => server + .services + .lock() + .await + .insert(services) + .clone(), + | Err(error) => { error!("Critical error starting server: {error}"); return Err(error); }, - }; + }) +} + +/// Operate the server normally in release-mode static builds. This will start, +/// run and stop the server within the asynchronous runtime. +#[cfg(any(not(tuwunel_mods), not(feature = "tuwunel_mods")))] +pub async fn async_run(server: &Arc) -> Result { + extern crate tuwunel_router as router; if let Err(error) = router::run( server @@ -63,6 +89,13 @@ pub async fn run_async(server: &Arc) -> Result { return Err(error); } + Ok(()) +} + +#[cfg(any(not(tuwunel_mods), not(feature = "tuwunel_mods")))] +pub async fn async_stop(server: &Arc) -> Result { + extern crate tuwunel_router as router; + if let Err(error) = router::stop( server .services @@ -77,7 +110,6 @@ pub async fn run_async(server: &Arc) -> Result { return Err(error); } - debug_info!("Exit runtime"); Ok(()) } @@ -85,7 +117,7 @@ pub async fn run_async(server: &Arc) -> Result { /// and hot-reload portions of the server as-needed before returning for an /// actual shutdown. This is not available in release-mode or static builds. #[cfg(all(tuwunel_mods, feature = "tuwunel_mods"))] -pub async fn run_async(server: &Arc) -> Result { +pub async fn async_exec(server: &Arc) -> Result { let mut starts = true; let mut reloads = true; while reloads { diff --git a/src/main/tests/smoke.rs b/src/main/tests/smoke.rs index b0bb47f0..cc1d2a1f 100644 --- a/src/main/tests/smoke.rs +++ b/src/main/tests/smoke.rs @@ -1,5 +1,6 @@ #![cfg(test)] +use insta::{assert_debug_snapshot, with_settings}; use tuwunel::Server; use tuwunel_core::{Args, Result, runtime}; @@ -12,9 +13,16 @@ fn panic_dummy() { panic!("dummy") } #[test] fn smoke() -> Result { - let args = Args::default_test("smoke"); - let runtime = runtime::new(Some(&args))?; - let server = Server::new(Some(&args), Some(runtime.handle()))?; + with_settings!({ + description => "Smoke Test", + snapshot_suffix => "smoke_test", + }, { + let args = Args::default_test(&["smoke", "fresh", "cleanup"]); + let runtime = runtime::new(Some(&args))?; + let server = Server::new(Some(&args), Some(runtime.handle()))?; + let result = tuwunel::exec(&server, runtime); - tuwunel::exec(&server, runtime) + assert_debug_snapshot!(result); + result + }) } diff --git a/src/main/tests/smoke_async.rs b/src/main/tests/smoke_async.rs new file mode 100644 index 00000000..e50955d3 --- /dev/null +++ b/src/main/tests/smoke_async.rs @@ -0,0 +1,24 @@ +#![cfg(test)] + +use insta::{assert_debug_snapshot, with_settings}; +use tuwunel::Server; +use tuwunel_core::{Args, Result, runtime}; + +#[test] +fn smoke_async() -> Result { + with_settings!({ + description => "Smoke Async", + snapshot_suffix => "smoke_async", + }, { + let args = Args::default_test(&["smoke", "fresh", "cleanup"]); + let runtime = runtime::new(Some(&args))?; + let server = Server::new(Some(&args), Some(runtime.handle()))?; + let result = runtime.block_on(async { + tuwunel::async_exec(&server).await + }); + + runtime::shutdown(&server.server, runtime)?; + assert_debug_snapshot!(result); + result + }) +} diff --git a/src/main/tests/smoke_shutdown.rs b/src/main/tests/smoke_shutdown.rs new file mode 100644 index 00000000..9008a84d --- /dev/null +++ b/src/main/tests/smoke_shutdown.rs @@ -0,0 +1,29 @@ +#![cfg(test)] + +use insta::{assert_debug_snapshot, with_settings}; +use tracing::Level; +use tuwunel::Server; +use tuwunel_core::{Args, Result, runtime, utils::result::ErrLog}; + +#[test] +fn smoke_shutdown() -> Result { + with_settings!({ + description => "Smoke Shutdown", + snapshot_suffix => "smoke_shutdown", + }, { + let args = Args::default_test(&["fresh", "cleanup"]); + let runtime = runtime::new(Some(&args))?; + let server = Server::new(Some(&args), Some(runtime.handle()))?; + let result = runtime.block_on(async { + tuwunel::async_start(&server).await?; + let run = tuwunel::async_run(&server); + server.server.shutdown().log_err(Level::WARN).ok(); + run.await?; + tuwunel::async_stop(&server).await + }); + + runtime::shutdown(&server.server, runtime)?; + assert_debug_snapshot!(result); + result + }) +} diff --git a/src/main/tests/snapshots/smoke__smoke@smoke_test.snap b/src/main/tests/snapshots/smoke__smoke@smoke_test.snap new file mode 100644 index 00000000..596ad022 --- /dev/null +++ b/src/main/tests/snapshots/smoke__smoke@smoke_test.snap @@ -0,0 +1,8 @@ +--- +source: src/main/tests/smoke.rs +description: Smoke Test +expression: result +--- +Ok( + (), +) diff --git a/src/main/tests/snapshots/smoke_async__smoke_async@smoke_async.snap b/src/main/tests/snapshots/smoke_async__smoke_async@smoke_async.snap new file mode 100644 index 00000000..6090f83c --- /dev/null +++ b/src/main/tests/snapshots/smoke_async__smoke_async@smoke_async.snap @@ -0,0 +1,8 @@ +--- +source: src/main/tests/smoke_async.rs +description: Smoke Async +expression: result +--- +Ok( + (), +) diff --git a/src/main/tests/snapshots/smoke_shutdown__smoke_shutdown@smoke_shutdown.snap b/src/main/tests/snapshots/smoke_shutdown__smoke_shutdown@smoke_shutdown.snap new file mode 100644 index 00000000..76d5335d --- /dev/null +++ b/src/main/tests/snapshots/smoke_shutdown__smoke_shutdown@smoke_shutdown.snap @@ -0,0 +1,8 @@ +--- +source: src/main/tests/smoke_shutdown.rs +description: Smoke Shutdown +expression: result +--- +Ok( + (), +)