From 8710fcdd1b72abcbbc9450ddc6581d6942fd54c3 Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 29 Jun 2026 13:48:19 +0200 Subject: [PATCH 1/8] Enforce enhanced lint requirements Add the template rustfmt configuration and move the project toolchain to nightly so the imported unstable formatting rules are available. Apply the resulting Rust formatting changes across the workspace. Extend `make lint` with `interrogate --fail-under 100 .` so Python docstring coverage is mandatory, and install `interrogate` as a uv tool in CI before the lint step runs. --- .github/workflows/ci.yml | 7 +- rust-toolchain.toml | 2 +- rustfmt.toml | 14 +++ src/bin/pg_worker.rs | 7 +- src/bin/pg_worker/tests.rs | 15 +-- src/bootstrap/env.rs | 41 ++++--- src/bootstrap/env_tests.rs | 8 +- src/bootstrap/env_types.rs | 2 +- src/bootstrap/mod.rs | 32 ++--- src/bootstrap/mod_tests.rs | 12 +- src/bootstrap/mode.rs | 7 +- src/bootstrap/prepare/mod.rs | 26 +++-- src/bootstrap/prepare/tests.rs | 21 ++-- src/cache/config.rs | 17 ++- src/cache/lock.rs | 13 ++- src/cache/mod.rs | 9 +- src/cache/operations/copy.rs | 5 +- src/cache/operations/lookup.rs | 7 +- src/cache/operations/populate.rs | 3 +- src/cache/operations/tests.rs | 15 +-- src/cleanup_helpers.rs | 6 +- src/cluster/cache_integration.rs | 19 ++- src/cluster/cleanup.rs | 24 ++-- src/cluster/connection.rs | 72 ++++-------- src/cluster/delegation.rs | 12 +- src/cluster/drop_logging_tests.rs | 2 +- src/cluster/guard.rs | 32 +++-- src/cluster/handle.rs | 46 ++++---- src/cluster/installation.rs | 17 ++- src/cluster/lifecycle.rs | 49 ++++---- src/cluster/mod.rs | 62 +++++----- src/cluster/mod_tests.rs | 21 ++-- src/cluster/runtime.rs | 2 +- src/cluster/runtime_mode.rs | 4 +- src/cluster/shutdown.rs | 28 +++-- src/cluster/shutdown_hook.rs | 15 ++- src/cluster/startup.rs | 23 ++-- src/cluster/startup_tests.rs | 23 ++-- src/cluster/temporary_database.rs | 20 +--- src/cluster/worker_invoker/mod.rs | 24 ++-- src/cluster/worker_invoker/tests.rs | 23 ++-- src/env/loom_tests.rs | 21 ++-- src/env/mod.rs | 14 +-- src/env/state.rs | 29 ++--- src/env/tests/corruption.rs | 19 +-- src/env/tests/mod.rs | 44 ++++--- src/env/tests/thread_helpers.rs | 17 +-- src/error.rs | 19 +-- src/fs.rs | 13 ++- src/lib.rs | 123 +++++++++++--------- src/privileges.rs | 16 +-- src/test_support/errors.rs | 8 +- src/test_support/filesystem.rs | 12 +- src/test_support/fixtures.rs | 29 +++-- src/test_support/fixtures_docs.rs | 29 ++--- src/test_support/hash.rs | 11 +- src/test_support/hook.rs | 34 +++--- src/test_support/logging.rs | 22 ++-- src/test_support/mod.rs | 31 +++-- src/test_support/partial_data_dir.rs | 7 +- src/test_support/scoped_env.rs | 7 +- src/test_support/shared_singleton.rs | 15 +-- src/test_support/worker_env.rs | 20 ++-- src/test_support/worker_env_tests.rs | 7 +- src/worker.rs | 38 +++--- src/worker_process/mod.rs | 43 +++---- src/worker_process/output.rs | 8 +- src/worker_process/privileges.rs | 18 +-- tests/bootstrap_for_tests.rs | 32 ++--- tests/bootstrap_privileges.rs | 5 +- tests/bootstrap_worker_binary.rs | 17 ++- tests/cluster_handle_send.rs | 26 +++-- tests/cluster_split_constructors.rs | 1 + tests/data_dir_recovery.rs | 11 +- tests/database_lifecycle.rs | 21 ++-- tests/e2e_postgresql_embedded_diesel.rs | 38 +++--- tests/observability.rs | 26 ++--- tests/recovery_integration.rs | 7 +- tests/settings.rs | 19 +-- tests/settings_logging.rs | 14 ++- tests/settings_snapshot_roundtrip.rs | 3 +- tests/shared_cluster_handle_failure.rs | 8 +- tests/shared_cluster_handle_success.rs | 6 +- tests/shutdown_hook_lifecycle.rs | 4 +- tests/shutdown_timeout.rs | 3 +- tests/support/bootstrap_sandbox.rs | 37 +++--- tests/support/cap_fs_bootstrap.rs | 1 - tests/support/cap_fs_privileged.rs | 7 +- tests/support/database_lifecycle_helpers.rs | 18 ++- tests/support/diesel_e2e_helpers.rs | 14 ++- tests/support/env.rs | 8 +- tests/support/env_isolation.rs | 6 +- tests/support/pg_worker_hang.rs | 6 +- tests/support/pg_worker_helpers.rs | 4 +- tests/support/sandbox.rs | 43 ++++--- tests/support/serial.rs | 24 ++-- tests/test_cluster.rs | 32 +++-- tests/test_cluster_async.rs | 3 +- tests/test_cluster_behaviour.rs | 11 +- tests/test_cluster_connection.rs | 28 ++--- tests/test_cluster_drop.rs | 9 +- tests/test_cluster_fixture/process_utils.rs | 8 +- tests/test_cluster_fixture/unit_tests.rs | 5 +- tests/test_cluster_fixture/world.rs | 14 +-- tests/test_workflow_integration.py | 4 +- tests/worker_process.rs | 30 +++-- 106 files changed, 1043 insertions(+), 951 deletions(-) create mode 100644 rustfmt.toml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index df994432..d10afc45 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -32,11 +32,10 @@ jobs: **/*.md !**/target/** !**/dist/** + - name: Setup uv + uses: astral-sh/setup-uv@12d13f90bc3a5a1971bebad4beb09a4dfa962e91 - name: Install interrogate - run: | - python -m pip install --user uv==0.11.19 - echo "$HOME/.local/bin" >> "$GITHUB_PATH" - "$HOME/.local/bin/uv" tool install interrogate==1.7.0 + run: uv tool install interrogate==1.7.0 - name: Lint run: make lint - name: Install cargo-nextest diff --git a/rust-toolchain.toml b/rust-toolchain.toml index 73cb934d..8e275b74 100644 --- a/rust-toolchain.toml +++ b/rust-toolchain.toml @@ -1,3 +1,3 @@ [toolchain] -channel = "stable" +channel = "nightly" components = ["rustfmt", "clippy"] diff --git a/rustfmt.toml b/rustfmt.toml new file mode 100644 index 00000000..f7ad026b --- /dev/null +++ b/rustfmt.toml @@ -0,0 +1,14 @@ +unstable_features = true +comment_width = 100 +format_code_in_doc_comments = true +imports_granularity = "Crate" +imports_layout = "HorizontalVertical" +wrap_comments = true +group_imports = "StdExternalCrate" +use_try_shorthand = true +hex_literal_case = "Lower" +format_strings = true +format_macro_matchers = true +fn_single_line = true +condense_wildcard_suffixes = true +use_field_init_shorthand = true diff --git a/src/bin/pg_worker.rs b/src/bin/pg_worker.rs index 456da5e9..ed9ec097 100644 --- a/src/bin/pg_worker.rs +++ b/src/bin/pg_worker.rs @@ -77,16 +77,15 @@ impl Operation { "cleanup" => Ok(Self::Cleanup), "cleanup-full" => Ok(Self::CleanupFull), other => Err(WorkerError::InvalidArgs(format!( - "unknown operation '{other}'; expected setup, start, stop, cleanup, or cleanup-full" + "unknown operation '{other}'; expected setup, start, stop, cleanup, or \ + cleanup-full" ))), } } } #[cfg(unix)] -fn main() -> Result<(), BoxError> { - run_worker(env::args_os()).map_err(Into::into) -} +fn main() -> Result<(), BoxError> { run_worker(env::args_os()).map_err(Into::into) } #[cfg(unix)] fn run_worker(args: impl Iterator) -> Result<(), WorkerError> { diff --git a/src/bin/pg_worker/tests.rs b/src/bin/pg_worker/tests.rs index f1ea5f6f..100870d2 100644 --- a/src/bin/pg_worker/tests.rs +++ b/src/bin/pg_worker/tests.rs @@ -1,21 +1,21 @@ //! Unit tests for `pg_worker` data directory recovery and argument parsing. -use super::*; -use pg_embedded_setup_unpriv::test_support::create_partial_data_dir; -use rstest::{fixture, rstest}; use std::{ ffi::{OsStr, OsString}, fs, os::unix::ffi::OsStrExt, }; + +use pg_embedded_setup_unpriv::test_support::create_partial_data_dir; +use rstest::{fixture, rstest}; use tempfile::{TempDir, tempdir}; +use super::*; + type R = Result>; type TempDataDirResult = R<(TempDir, Utf8PathBuf)>; -fn ensure(is_valid: bool, msg: &str) -> R { - if is_valid { Ok(()) } else { Err(msg.into()) } -} +fn ensure(is_valid: bool, msg: &str) -> R { if is_valid { Ok(()) } else { Err(msg.into()) } } #[fixture] fn temp_data_dir() -> TempDataDirResult { @@ -81,7 +81,8 @@ fn reset_removes_partial(temp_data_dir: TempDataDirResult) -> R { #[rstest] fn reset_ok_for_missing(temp_data_dir: TempDataDirResult) -> R { - reset_data_dir(&temp_data_dir?.1) + let (_temp_dir, data_dir) = temp_data_dir?; + reset_data_dir(&data_dir) } #[test] diff --git a/src/bootstrap/env.rs b/src/bootstrap/env.rs index c3b4f54d..c51f6e1c 100644 --- a/src/bootstrap/env.rs +++ b/src/bootstrap/env.rs @@ -1,20 +1,25 @@ //! Parses environment variables used by the bootstrapper and surfaces the //! resulting configuration for the filesystem preparers. -pub use crate::bootstrap::env_types::TestBootstrapEnvironment; -use crate::bootstrap::env_types::TimezoneEnv; -pub(super) use crate::bootstrap::env_types::XdgDirs; -use crate::bootstrap::mode::ExecutionPrivileges; -use crate::error::{BootstrapError, BootstrapErrorKind, BootstrapResult}; -use crate::fs::ambient_dir_and_path; +use std::{ + env::{self, VarError}, + ffi::OsString, + io::ErrorKind, + path::PathBuf, + time::Duration, +}; + use camino::{Utf8Path, Utf8PathBuf}; #[cfg(unix)] use cap_std::fs::PermissionsExt; use color_eyre::eyre::Report; -use std::env::{self, VarError}; -use std::ffi::OsString; -use std::io::ErrorKind; -use std::path::PathBuf; -use std::time::Duration; + +pub use crate::bootstrap::env_types::TestBootstrapEnvironment; +pub(super) use crate::bootstrap::env_types::XdgDirs; +use crate::{ + bootstrap::{env_types::TimezoneEnv, mode::ExecutionPrivileges}, + error::{BootstrapError, BootstrapErrorKind, BootstrapResult}, + fs::ambient_dir_and_path, +}; #[cfg(unix)] const WORKER_BINARY_NAME: &str = "pg_worker"; #[cfg(windows)] @@ -39,7 +44,8 @@ fn discover_worker_from_path_value( let dir = Utf8PathBuf::from_path_buf(entry).map_err(|invalid_entry| { let invalid_value = invalid_entry.as_os_str().to_string_lossy(); let report = color_eyre::eyre::eyre!( - "PATH contains a non-UTF-8 entry: {invalid_value:?}; remove or replace the malformed entry." + "PATH contains a non-UTF-8 entry: {invalid_value:?}; remove or replace the \ + malformed entry." ); BootstrapError::new(BootstrapErrorKind::WorkerBinaryPathNonUtf8, report) })?; @@ -67,9 +73,7 @@ fn is_executable(path: &Utf8Path) -> bool { } #[cfg(not(unix))] -fn is_executable(_path: &Utf8Path) -> bool { - true -} +fn is_executable(_path: &Utf8Path) -> bool { true } /// Common Unix paths where time zone databases may be installed. /// @@ -133,7 +137,8 @@ pub(super) fn shutdown_timeout_from_env() -> BootstrapResult { if seconds > MAX_SHUTDOWN_TIMEOUT_SECS { return Err(BootstrapError::from(color_eyre::eyre::eyre!( - "{SHUTDOWN_TIMEOUT_ENV} must be {MAX_SHUTDOWN_TIMEOUT_SECS} seconds or less (received {trimmed})" + "{SHUTDOWN_TIMEOUT_ENV} must be {MAX_SHUTDOWN_TIMEOUT_SECS} seconds or less \ + (received {trimmed})" ))); } @@ -154,8 +159,8 @@ pub(super) fn worker_binary_from_env( let path = Utf8PathBuf::from_path_buf(PathBuf::from(&raw)).map_err(|_| { let invalid_value = raw.to_string_lossy().to_string(); BootstrapError::from(color_eyre::eyre::eyre!( - "PG_EMBEDDED_WORKER contains a non-UTF-8 value: {invalid_value:?}. \ - Provide a UTF-8 encoded absolute path to the worker binary." + "PG_EMBEDDED_WORKER contains a non-UTF-8 value: {invalid_value:?}. Provide a \ + UTF-8 encoded absolute path to the worker binary." )) })?; diff --git a/src/bootstrap/env_tests.rs b/src/bootstrap/env_tests.rs index 04433a6c..d1d0c3fa 100644 --- a/src/bootstrap/env_tests.rs +++ b/src/bootstrap/env_tests.rs @@ -1,9 +1,11 @@ //! Tests for bootstrap environment discovery helpers. +use std::{ + ffi::OsString, + os::unix::{ffi::OsStringExt, fs::PermissionsExt}, +}; + use super::{BootstrapErrorKind, WORKER_BINARY_NAME, discover_worker_from_path_value}; -use std::ffi::OsString; -use std::os::unix::ffi::OsStringExt; -use std::os::unix::fs::PermissionsExt; #[test] fn discover_worker_returns_none_when_path_is_absent() { diff --git a/src/bootstrap/env_types.rs b/src/bootstrap/env_types.rs index 66dd91b0..09f0e188 100644 --- a/src/bootstrap/env_types.rs +++ b/src/bootstrap/env_types.rs @@ -61,8 +61,8 @@ impl TestBootstrapEnvironment { /// /// # Examples /// ``` - /// use pg_embedded_setup_unpriv::TestBootstrapEnvironment; /// use camino::Utf8PathBuf; + /// use pg_embedded_setup_unpriv::TestBootstrapEnvironment; /// /// let env = TestBootstrapEnvironment { /// home: Utf8PathBuf::from("/tmp/home"), diff --git a/src/bootstrap/mod.rs b/src/bootstrap/mod.rs index 38635d7e..22c01ade 100644 --- a/src/bootstrap/mod.rs +++ b/src/bootstrap/mod.rs @@ -7,27 +7,25 @@ mod env_types; mod mode; mod prepare; -use std::time::Duration; - -use color_eyre::eyre::{Context, eyre}; -use postgresql_embedded::Settings; -use serde::{Deserialize, Serialize}; #[cfg(test)] use std::sync::{Arc, Mutex, OnceLock}; +use std::time::Duration; -use crate::{ - PgEnvCfg, - error::{BootstrapResult, Result as CrateResult}, -}; - +use color_eyre::eyre::{Context, eyre}; pub use env::{TestBootstrapEnvironment, find_timezone_dir}; pub use mode::{ExecutionMode, ExecutionPrivileges, detect_execution_privileges}; +use postgresql_embedded::Settings; +use serde::{Deserialize, Serialize}; use self::{ env::{shutdown_timeout_from_env, worker_binary_from_env}, mode::determine_execution_mode, prepare::prepare_bootstrap, }; +use crate::{ + PgEnvCfg, + error::{BootstrapResult, Result as CrateResult}, +}; const DEFAULT_SETUP_TIMEOUT: Duration = Duration::from_secs(180); const DEFAULT_START_TIMEOUT: Duration = Duration::from_secs(60); @@ -129,10 +127,13 @@ pub fn run() -> CrateResult<()> { /// /// # fn main() -> pg_embedded_setup_unpriv::BootstrapResult<()> { /// let bootstrap = bootstrap_for_tests()?; -/// with_vars(bootstrap.environment.to_env(), || -> pg_embedded_setup_unpriv::BootstrapResult<()> { -/// // Launch application logic that relies on `bootstrap.settings` here. -/// Ok(()) -/// })?; +/// with_vars( +/// bootstrap.environment.to_env(), +/// || -> pg_embedded_setup_unpriv::BootstrapResult<()> { +/// // Launch application logic that relies on `bootstrap.settings` here. +/// Ok(()) +/// }, +/// )?; /// # Ok(()) /// # } /// ``` @@ -189,7 +190,8 @@ fn validate_backend_selection() -> BootstrapResult<()> { return Ok(()); } Err(eyre!( - "SKIP-TEST-CLUSTER: unsupported PG_TEST_BACKEND '{trimmed}'; supported backends: postgresql_embedded" + "SKIP-TEST-CLUSTER: unsupported PG_TEST_BACKEND '{trimmed}'; supported backends: \ + postgresql_embedded" ) .into()) } diff --git a/src/bootstrap/mod_tests.rs b/src/bootstrap/mod_tests.rs index bf730fd2..4ae21341 100644 --- a/src/bootstrap/mod_tests.rs +++ b/src/bootstrap/mod_tests.rs @@ -1,14 +1,18 @@ //! Tests for bootstrap orchestration and backend selection. -use super::*; -use crate::test_support::scoped_env; +use std::{ + ffi::OsString, + sync::{Arc, Mutex}, +}; + use camino::Utf8PathBuf; use rstest::{fixture, rstest}; use serial_test::serial; -use std::ffi::OsString; -use std::sync::{Arc, Mutex}; use tempfile::tempdir; +use super::*; +use crate::test_support::scoped_env; + /// Converts string key-value pairs to `OsString` pairs for `scoped_env`. fn env_vars(pairs: [(&str, Option<&str>); N]) -> Vec<(OsString, Option)> { pairs diff --git a/src/bootstrap/mode.rs b/src/bootstrap/mode.rs index 07dd7659..c1628072 100644 --- a/src/bootstrap/mode.rs +++ b/src/bootstrap/mode.rs @@ -1,12 +1,11 @@ //! Detects execution privileges and selects the appropriate orchestration mode. use camino::Utf8PathBuf; - -use crate::error::{BootstrapError, BootstrapResult}; - #[cfg(unix)] use nix::unistd::geteuid; +use crate::error::{BootstrapError, BootstrapResult}; + /// Represents the privileges the process is running with when bootstrapping `PostgreSQL`. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum ExecutionPrivileges { @@ -31,7 +30,7 @@ pub enum ExecutionMode { /// /// # Examples /// ``` -/// use pg_embedded_setup_unpriv::{detect_execution_privileges, ExecutionPrivileges}; +/// use pg_embedded_setup_unpriv::{ExecutionPrivileges, detect_execution_privileges}; /// /// let privileges = detect_execution_privileges(); /// let mode = match privileges { diff --git a/src/bootstrap/prepare/mod.rs b/src/bootstrap/prepare/mod.rs index 1bf3d198..08438895 100644 --- a/src/bootstrap/prepare/mod.rs +++ b/src/bootstrap/prepare/mod.rs @@ -1,10 +1,24 @@ //! Prepares filesystem state for the bootstrap flows. +#[cfg(unix)] +use std::net::TcpListener; + use camino::{Utf8Path, Utf8PathBuf}; #[cfg(unix)] use color_eyre::eyre::{Context, eyre}; +#[cfg(unix)] +use nix::unistd::{Uid, User, fchown, geteuid}; use postgresql_embedded::Settings; +use tracing::debug; +use super::env::{TestBootstrapEnvironment, XdgDirs, prepare_timezone_env}; +#[cfg(unix)] +use crate::privileges::{ + default_paths_for, + ensure_dir_for_user, + ensure_tree_owned_by_user, + make_data_dir_private, +}; use crate::{ PgEnvCfg, error::{BootstrapError, BootstrapResult}, @@ -12,18 +26,6 @@ use crate::{ observability::LOG_TARGET, }; -use super::env::{TestBootstrapEnvironment, XdgDirs, prepare_timezone_env}; - -#[cfg(unix)] -use crate::privileges::{ - default_paths_for, ensure_dir_for_user, ensure_tree_owned_by_user, make_data_dir_private, -}; -#[cfg(unix)] -use nix::unistd::{Uid, User, fchown, geteuid}; -#[cfg(unix)] -use std::net::TcpListener; -use tracing::debug; - const PGPASS_MODE: u32 = 0o600; pub(super) fn prepare_bootstrap( diff --git a/src/bootstrap/prepare/tests.rs b/src/bootstrap/prepare/tests.rs index d97c9570..b91259cb 100644 --- a/src/bootstrap/prepare/tests.rs +++ b/src/bootstrap/prepare/tests.rs @@ -3,12 +3,13 @@ use super::*; mod sanitized_settings { - use super::log_sanitized_settings; - use crate::test_support::capture_debug_logs; + use std::{collections::HashMap, time::Duration}; + use color_eyre::eyre::{Result, ensure}; use postgresql_embedded::VersionReq; - use std::collections::HashMap; - use std::time::Duration; + + use super::log_sanitized_settings; + use crate::test_support::capture_debug_logs; fn sample_settings() -> Result { let mut configuration = HashMap::new(); @@ -77,11 +78,13 @@ mod sanitized_settings { } mod behaviour_tests { - use super::*; - use crate::test_support::scoped_env; use std::ffi::OsString; + use tempfile::tempdir; + use super::*; + use crate::test_support::scoped_env; + #[test] fn bootstrap_unprivileged_sets_up_directories() { let runtime = tempdir().expect("runtime dir"); @@ -126,11 +129,13 @@ mod behaviour_tests { #[cfg(unix)] mod unix_tests { - use super::*; - use nix::unistd::{Uid, User, geteuid}; use std::os::unix::fs::{MetadataExt, PermissionsExt}; + + use nix::unistd::{Uid, User, geteuid}; use tempfile::tempdir; + use super::*; + #[test] fn ensure_settings_paths_applies_defaults() { let cfg = PgEnvCfg::default(); diff --git a/src/cache/config.rs b/src/cache/config.rs index 2b106a70..b4ad62d5 100644 --- a/src/cache/config.rs +++ b/src/cache/config.rs @@ -3,9 +3,10 @@ //! Resolves the cache directory from environment variables with XDG-compliant //! fallback paths. -use camino::Utf8PathBuf; use std::path::PathBuf; +use camino::Utf8PathBuf; + /// Subdirectory path within the XDG cache home. const CACHE_SUBDIR: &str = "pg-embedded/binaries"; @@ -27,15 +28,11 @@ impl BinaryCacheConfig { /// Creates a cache configuration with a custom directory. #[must_use] - pub const fn with_dir(cache_dir: Utf8PathBuf) -> Self { - Self { cache_dir } - } + pub const fn with_dir(cache_dir: Utf8PathBuf) -> Self { Self { cache_dir } } } impl Default for BinaryCacheConfig { - fn default() -> Self { - Self::new() - } + fn default() -> Self { Self::new() } } /// Resolves the binary cache directory from environment and XDG conventions. @@ -110,10 +107,12 @@ fn resolve_from_home() -> Option { #[cfg(test)] mod tests { + use std::ffi::OsString; + + use rstest::rstest; + use super::*; use crate::test_support::scoped_env; - use rstest::rstest; - use std::ffi::OsString; /// Consolidated test for `resolve_cache_dir` with various environment configurations. #[rstest] diff --git a/src/cache/lock.rs b/src/cache/lock.rs index dc022962..2673fa7f 100644 --- a/src/cache/lock.rs +++ b/src/cache/lock.rs @@ -4,12 +4,14 @@ //! parallel test runners. On Unix systems, uses `flock(2)` for advisory locking. //! On non-Unix platforms, locking is a no-op. -use camino::Utf8Path; -use std::fs::{File, OpenOptions}; -use std::io; - #[cfg(unix)] use std::os::unix::io::AsRawFd; +use std::{ + fs::{File, OpenOptions}, + io, +}; + +use camino::Utf8Path; /// Subdirectory within the cache for lock files. const LOCKS_SUBDIR: &str = ".locks"; @@ -163,10 +165,11 @@ fn validate_version(version: &str) -> io::Result<()> { #[cfg(test)] mod tests { - use super::*; use rstest::{fixture, rstest}; use tempfile::TempDir; + use super::*; + /// Fixture providing a temporary cache directory as a UTF-8 path. #[fixture] fn cache_fixture() -> (TempDir, camino::Utf8PathBuf) { diff --git a/src/cache/mod.rs b/src/cache/mod.rs index 22130c55..c47e377e 100644 --- a/src/cache/mod.rs +++ b/src/cache/mod.rs @@ -27,6 +27,11 @@ mod operations; pub use config::{BinaryCacheConfig, resolve_cache_dir}; pub use lock::CacheLock; pub use operations::{ - CacheLookupResult, check_cache, copy_from_cache, find_matching_cached_version, populate_cache, - try_populate_cache, try_use_cache, + CacheLookupResult, + check_cache, + copy_from_cache, + find_matching_cached_version, + populate_cache, + try_populate_cache, + try_use_cache, }; diff --git a/src/cache/operations/copy.rs b/src/cache/operations/copy.rs index d5ed7403..c74a9d8f 100644 --- a/src/cache/operations/copy.rs +++ b/src/cache/operations/copy.rs @@ -2,11 +2,10 @@ //! //! Provides recursive directory copying with permission preservation. +use std::{fs, io, path::Path}; + use camino::Utf8Path; use color_eyre::eyre::Context; -use std::fs; -use std::io; -use std::path::Path; use tracing::debug; use crate::error::BootstrapResult; diff --git a/src/cache/operations/lookup.rs b/src/cache/operations/lookup.rs index c3018909..3ae4c00b 100644 --- a/src/cache/operations/lookup.rs +++ b/src/cache/operations/lookup.rs @@ -2,9 +2,10 @@ //! //! Provides functions for checking cache status and finding matching versions. +use std::fs; + use camino::{Utf8Path, Utf8PathBuf}; use postgresql_embedded::{Version, VersionReq}; -use std::fs; use tracing::{debug, warn}; use super::copy::copy_from_cache; @@ -54,7 +55,7 @@ fn is_cache_entry_complete(version_dir: &Utf8Path) -> bool { /// /// ```no_run /// use camino::Utf8Path; -/// use pg_embedded_setup_unpriv::cache::{check_cache, CacheLookupResult}; +/// use pg_embedded_setup_unpriv::cache::{CacheLookupResult, check_cache}; /// /// let cache_dir = Utf8Path::new("/home/user/.cache/pg-embedded/binaries"); /// match check_cache(cache_dir, "17.4.0") { @@ -118,8 +119,8 @@ fn log_cache_miss(version_dir: &Utf8Path, version: &str) { /// /// ```no_run /// use camino::Utf8Path; -/// use postgresql_embedded::VersionReq; /// use pg_embedded_setup_unpriv::cache::find_matching_cached_version; +/// use postgresql_embedded::VersionReq; /// /// let cache_dir = Utf8Path::new("/home/user/.cache/pg-embedded/binaries"); /// let version_req = VersionReq::parse("^17").expect("valid version req"); diff --git a/src/cache/operations/populate.rs b/src/cache/operations/populate.rs index 585e9833..b6e4e5e8 100644 --- a/src/cache/operations/populate.rs +++ b/src/cache/operations/populate.rs @@ -2,9 +2,10 @@ //! //! Provides functions to copy freshly downloaded binaries into the cache. +use std::fs; + use camino::Utf8Path; use color_eyre::eyre::Context; -use std::fs; use tracing::{debug, warn}; use super::copy::copy_dir_recursive; diff --git a/src/cache/operations/tests.rs b/src/cache/operations/tests.rs index 71ffd805..4eb867c4 100644 --- a/src/cache/operations/tests.rs +++ b/src/cache/operations/tests.rs @@ -1,13 +1,14 @@ //! Tests for cache operations. -use super::lookup::COMPLETION_MARKER; -use super::*; +use std::fs; + use camino::Utf8Path; use postgresql_embedded::VersionReq; use rstest::{fixture, rstest}; -use std::fs; use tempfile::{TempDir, tempdir}; +use super::{lookup::COMPLETION_MARKER, *}; + /// Creates a `bin` subdirectory in the given path with mock `postgres` and /// `pg_ctl` binary files for cache tests. fn create_mock_binaries(dir: &Utf8Path) { @@ -152,14 +153,10 @@ fn assert_try_use_cache(populate_cache: bool, expected_result: bool, check_files } #[test] -fn try_use_cache_returns_false_on_miss() { - assert_try_use_cache(false, false, false); -} +fn try_use_cache_returns_false_on_miss() { assert_try_use_cache(false, false, false); } #[test] -fn try_use_cache_returns_true_on_hit() { - assert_try_use_cache(true, true, true); -} +fn try_use_cache_returns_true_on_hit() { assert_try_use_cache(true, true, true); } #[rstest] fn find_matching_cached_version_returns_none_for_empty_cache( diff --git a/src/cleanup_helpers.rs b/src/cleanup_helpers.rs index 57f9a383..4b04f2dd 100644 --- a/src/cleanup_helpers.rs +++ b/src/cleanup_helpers.rs @@ -1,7 +1,9 @@ //! Shared directory removal helpers with safety guards. -use std::io::ErrorKind; -use std::path::{Component, Path}; +use std::{ + io::ErrorKind, + path::{Component, Path}, +}; /// Records the outcome of a guarded directory removal attempt. /// diff --git a/src/cluster/cache_integration.rs b/src/cluster/cache_integration.rs index 2da22a0a..5bda3c83 100644 --- a/src/cluster/cache_integration.rs +++ b/src/cluster/cache_integration.rs @@ -3,17 +3,24 @@ //! Provides methods to check and populate the shared `PostgreSQL` binary cache, //! avoiding repeated downloads across test runs. -use crate::TestBootstrapSettings; -use crate::cache::{ - BinaryCacheConfig, CacheLock, CacheLookupResult, check_cache, copy_from_cache, - find_matching_cached_version, populate_cache, -}; -use crate::observability::LOG_TARGET; use camino::Utf8PathBuf; use postgresql_embedded::{Settings, VersionReq}; use tracing::{debug, info, warn}; use super::installation; +use crate::{ + TestBootstrapSettings, + cache::{ + BinaryCacheConfig, + CacheLock, + CacheLookupResult, + check_cache, + copy_from_cache, + find_matching_cached_version, + populate_cache, + }, + observability::LOG_TARGET, +}; /// Sets the exact version requirement in settings to skip GitHub API resolution. fn set_exact_version(settings: &mut Settings, version: &str) { diff --git a/src/cluster/cleanup.rs b/src/cluster/cleanup.rs index 2c1347a8..9f7111da 100644 --- a/src/cluster/cleanup.rs +++ b/src/cluster/cleanup.rs @@ -1,14 +1,16 @@ //! Cleanup helpers for `TestCluster` shutdown. -use crate::cleanup_helpers::{RemovalOutcome, has_parent_dir, try_remove_dir_all}; -use crate::observability::LOG_TARGET; -use crate::{CleanupMode, TestBootstrapSettings}; +use std::{error::Error, path::Path}; + use postgresql_embedded::Settings; -use std::error::Error; -use std::path::Path; -use super::worker_invoker::WorkerInvoker as ClusterWorkerInvoker; -use super::worker_operation; +use super::{worker_invoker::WorkerInvoker as ClusterWorkerInvoker, worker_operation}; +use crate::{ + CleanupMode, + TestBootstrapSettings, + cleanup_helpers::{RemovalOutcome, has_parent_dir, try_remove_dir_all}, + observability::LOG_TARGET, +}; #[derive(Debug, Clone, Copy)] enum DirectoryLabel { @@ -207,13 +209,15 @@ fn warn_cleanup_removal_failure( #[cfg(test)] mod tests { - use super::cleanup_in_process; - use crate::CleanupMode; + use std::fs; + use postgresql_embedded::Settings; use rstest::rstest; - use std::fs; use tempfile::tempdir; + use super::cleanup_in_process; + use crate::CleanupMode; + #[rstest] #[case::data_only(CleanupMode::DataOnly, false, true)] #[case::full(CleanupMode::Full, false, false)] diff --git a/src/cluster/connection.rs b/src/cluster/connection.rs index 2c77dd90..8cf8e025 100644 --- a/src/cluster/connection.rs +++ b/src/cluster/connection.rs @@ -7,16 +7,13 @@ use color_eyre::eyre::eyre; use postgres::{Client, NoTls}; use postgresql_embedded::Settings; -use crate::TestBootstrapSettings; -use crate::error::BootstrapResult; +use crate::{TestBootstrapSettings, error::BootstrapResult}; /// Escapes a SQL identifier by doubling embedded double quotes. /// /// `PostgreSQL` identifiers are quoted with double quotes. Any embedded /// double quote must be escaped by doubling it. -pub(crate) fn escape_identifier(name: &str) -> String { - name.replace('"', "\"\"") -} +pub(crate) fn escape_identifier(name: &str) -> String { name.replace('"', "\"\"") } /// Creates a new `PostgreSQL` client connection from the given URL. /// @@ -57,40 +54,28 @@ impl ConnectionMetadata { /// Returns the configured database host. #[must_use] - pub fn host(&self) -> &str { - self.settings.host.as_str() - } + pub fn host(&self) -> &str { self.settings.host.as_str() } /// Returns the configured port. #[must_use] - pub const fn port(&self) -> u16 { - self.settings.port - } + pub const fn port(&self) -> u16 { self.settings.port } /// Returns the configured superuser name. #[must_use] - pub fn superuser(&self) -> &str { - self.settings.username.as_str() - } + pub fn superuser(&self) -> &str { self.settings.username.as_str() } /// Returns the generated superuser password. #[must_use] - pub fn password(&self) -> &str { - self.settings.password.as_str() - } + pub fn password(&self) -> &str { self.settings.password.as_str() } /// Returns the prepared `.pgpass` file path. #[must_use] - pub fn pgpass_file(&self) -> &Utf8Path { - self.pgpass_file.as_ref() - } + pub fn pgpass_file(&self) -> &Utf8Path { self.pgpass_file.as_ref() } /// Constructs a libpq-compatible URL for `database` using the underlying /// `postgresql_embedded` helper. #[must_use] - pub fn database_url(&self, database: &str) -> String { - self.settings.url(database) - } + pub fn database_url(&self, database: &str) -> String { self.settings.url(database) } } /// Accessor for connection helpers derived from a @@ -123,45 +108,31 @@ impl TestClusterConnection { /// Returns host metadata without exposing internal storage. #[must_use] - pub fn host(&self) -> &str { - self.metadata.host() - } + pub fn host(&self) -> &str { self.metadata.host() } /// Returns the configured port. #[must_use] - pub const fn port(&self) -> u16 { - self.metadata.port() - } + pub const fn port(&self) -> u16 { self.metadata.port() } /// Returns the configured superuser account name. #[must_use] - pub fn superuser(&self) -> &str { - self.metadata.superuser() - } + pub fn superuser(&self) -> &str { self.metadata.superuser() } /// Returns the generated password for the superuser. #[must_use] - pub fn password(&self) -> &str { - self.metadata.password() - } + pub fn password(&self) -> &str { self.metadata.password() } /// Returns the `.pgpass` file prepared during bootstrap. #[must_use] - pub fn pgpass_file(&self) -> &Utf8Path { - self.metadata.pgpass_file() - } + pub fn pgpass_file(&self) -> &Utf8Path { self.metadata.pgpass_file() } /// Provides an owned snapshot of the connection metadata. #[must_use] - pub fn metadata(&self) -> ConnectionMetadata { - self.metadata.clone() - } + pub fn metadata(&self) -> ConnectionMetadata { self.metadata.clone() } /// Builds a libpq-compatible database URL for `database`. #[must_use] - pub fn database_url(&self, database: &str) -> String { - self.metadata.database_url(database) - } + pub fn database_url(&self, database: &str) -> String { self.metadata.database_url(database) } /// Establishes a Diesel connection for the target `database`. /// @@ -184,12 +155,17 @@ impl TestClusterConnection { #[cfg(test)] mod tests { - use super::*; - use crate::bootstrap::{ExecutionMode, ExecutionPrivileges, TestBootstrapEnvironment}; - use crate::{CleanupMode, TestBootstrapSettings}; - use postgresql_embedded::Settings; use std::time::Duration; + use postgresql_embedded::Settings; + + use super::*; + use crate::{ + CleanupMode, + TestBootstrapSettings, + bootstrap::{ExecutionMode, ExecutionPrivileges, TestBootstrapEnvironment}, + }; + fn sample_settings() -> TestBootstrapSettings { let settings = Settings { host: "127.0.0.1".into(), diff --git a/src/cluster/delegation.rs b/src/cluster/delegation.rs index c4176b0a..f57e015f 100644 --- a/src/cluster/delegation.rs +++ b/src/cluster/delegation.rs @@ -4,11 +4,13 @@ //! `TestCluster`, eliminating the need for callers to explicitly call `.connection()` //! before invoking methods like `create_database` or `drop_database`. -use super::lifecycle::DatabaseName; -use super::temporary_database::TemporaryDatabase; -use super::{ClusterHandle, TestCluster}; -use crate::CleanupMode; -use crate::error::BootstrapResult; +use super::{ + ClusterHandle, + TestCluster, + lifecycle::DatabaseName, + temporary_database::TemporaryDatabase, +}; +use crate::{CleanupMode, error::BootstrapResult}; /// Generates delegation methods on `TestCluster` that forward to `TestClusterConnection`. /// diff --git a/src/cluster/drop_logging_tests.rs b/src/cluster/drop_logging_tests.rs index 9d5da124..7a74f1ed 100644 --- a/src/cluster/drop_logging_tests.rs +++ b/src/cluster/drop_logging_tests.rs @@ -1,9 +1,9 @@ //! Tests for drop-time warning logs. -use crate::test_support::capture_warn_logs; use rstest::rstest; use super::shutdown; +use crate::test_support::capture_warn_logs; #[rstest] #[case::timeout( diff --git a/src/cluster/guard.rs b/src/cluster/guard.rs index 9951951e..5a88e09a 100644 --- a/src/cluster/guard.rs +++ b/src/cluster/guard.rs @@ -8,10 +8,9 @@ //! # Architecture //! //! The guard holds: -//! - **Environment guards**: `ScopedEnv` instances that restore environment -//! variables when dropped -//! - **Shutdown resources**: Runtime, `PostgreSQL` instance, and configuration -//! needed to cleanly stop the cluster +//! - **Environment guards**: `ScopedEnv` instances that restore environment variables when dropped +//! - **Shutdown resources**: Runtime, `PostgreSQL` instance, and configuration needed to cleanly +//! stop the cluster //! - **Tracing span**: Keeps the cluster's observability span alive //! //! # Drop Behaviour @@ -31,21 +30,19 @@ //! `ClusterGuard`'s shutdown and environment-restoration behaviour is tested //! in `tests/cluster_split_constructors.rs`: //! -//! - `new_split_creates_working_handle_and_guard`: Verifies that dropping the -//! guard stops the `PostgreSQL` cluster (`postmaster.pid` is removed) and -//! restores environment variables to their pre-cluster state. +//! - `new_split_creates_working_handle_and_guard`: Verifies that dropping the guard stops the +//! `PostgreSQL` cluster (`postmaster.pid` is removed) and restores environment variables to their +//! pre-cluster state. //! -//! - `start_async_split_creates_working_handle_and_guard`: Tests the async -//! variant with the same shutdown and restoration assertions. +//! - `start_async_split_creates_working_handle_and_guard`: Tests the async variant with the same +//! shutdown and restoration assertions. -use super::runtime_mode::ClusterRuntime; -use super::shutdown; -use crate::env::ScopedEnv; -use crate::observability::LOG_TARGET; -use crate::{CleanupMode, TestBootstrapSettings}; use postgresql_embedded::PostgreSQL; use tracing::{info, warn}; +use super::{runtime_mode::ClusterRuntime, shutdown}; +use crate::{CleanupMode, TestBootstrapSettings, env::ScopedEnv, observability::LOG_TARGET}; + /// Lifecycle guard for a running `PostgreSQL` cluster. /// /// This guard manages cluster shutdown and environment restoration. It is @@ -77,15 +74,16 @@ use tracing::{info, warn}; /// /// ```no_run /// use std::sync::OnceLock; +/// /// use pg_embedded_setup_unpriv::{ClusterHandle, TestCluster}; /// /// static SHARED: OnceLock = OnceLock::new(); /// /// fn shared_handle() -> &'static ClusterHandle { /// SHARED.get_or_init(|| { -/// let (handle, guard) = TestCluster::new_split() -/// .expect("cluster bootstrap failed"); -/// handle.register_shutdown_on_exit() +/// let (handle, guard) = TestCluster::new_split().expect("cluster bootstrap failed"); +/// handle +/// .register_shutdown_on_exit() /// .expect("shutdown hook registration failed"); /// std::mem::forget(guard); /// handle diff --git a/src/cluster/handle.rs b/src/cluster/handle.rs index 6edaa4c9..47a680bf 100644 --- a/src/cluster/handle.rs +++ b/src/cluster/handle.rs @@ -22,15 +22,16 @@ //! //! ```no_run //! use std::sync::OnceLock; +//! //! use pg_embedded_setup_unpriv::{ClusterHandle, TestCluster}; //! //! static SHARED: OnceLock = OnceLock::new(); //! //! fn shared_handle() -> &'static ClusterHandle { //! SHARED.get_or_init(|| { -//! let (handle, guard) = TestCluster::new_split() -//! .expect("cluster bootstrap failed"); -//! handle.register_shutdown_on_exit() +//! let (handle, guard) = TestCluster::new_split().expect("cluster bootstrap failed"); +//! handle +//! .register_shutdown_on_exit() //! .expect("shutdown hook registration failed"); //! std::mem::forget(guard); //! handle @@ -38,13 +39,15 @@ //! } //! ``` -use super::connection::TestClusterConnection; -use super::lifecycle::DatabaseName; -use super::temporary_database::TemporaryDatabase; -use crate::error::BootstrapResult; -use crate::{TestBootstrapEnvironment, TestBootstrapSettings}; use postgresql_embedded::Settings; +use super::{ + connection::TestClusterConnection, + lifecycle::DatabaseName, + temporary_database::TemporaryDatabase, +}; +use crate::{TestBootstrapEnvironment, TestBootstrapSettings, error::BootstrapResult}; + /// Send-safe handle providing read-only access to a running `PostgreSQL` cluster. /// /// Handles are lightweight and cloneable. They contain only the bootstrap @@ -83,16 +86,12 @@ const _: () = { }; impl From for ClusterHandle { - fn from(bootstrap: TestBootstrapSettings) -> Self { - Self { bootstrap } - } + fn from(bootstrap: TestBootstrapSettings) -> Self { Self { bootstrap } } } impl ClusterHandle { /// Creates a new handle from bootstrap settings. - pub(super) const fn new(bootstrap: TestBootstrapSettings) -> Self { - Self { bootstrap } - } + pub(super) const fn new(bootstrap: TestBootstrapSettings) -> Self { Self { bootstrap } } /// Returns the prepared `PostgreSQL` settings for the running cluster. /// @@ -106,9 +105,7 @@ impl ClusterHandle { /// # Ok::<(), pg_embedded_setup_unpriv::BootstrapError>(()) /// ``` #[must_use] - pub const fn settings(&self) -> &Settings { - &self.bootstrap.settings - } + pub const fn settings(&self) -> &Settings { &self.bootstrap.settings } /// Returns the environment required for clients to interact with the cluster. /// @@ -122,9 +119,7 @@ impl ClusterHandle { /// # Ok::<(), pg_embedded_setup_unpriv::BootstrapError>(()) /// ``` #[must_use] - pub const fn environment(&self) -> &TestBootstrapEnvironment { - &self.bootstrap.environment - } + pub const fn environment(&self) -> &TestBootstrapEnvironment { &self.bootstrap.environment } /// Returns the bootstrap metadata captured when the cluster was started. /// @@ -138,9 +133,7 @@ impl ClusterHandle { /// # Ok::<(), pg_embedded_setup_unpriv::BootstrapError>(()) /// ``` #[must_use] - pub const fn bootstrap(&self) -> &TestBootstrapSettings { - &self.bootstrap - } + pub const fn bootstrap(&self) -> &TestBootstrapSettings { &self.bootstrap } /// Returns helper methods for constructing connection artefacts. /// @@ -294,15 +287,16 @@ impl ClusterHandle { /// /// ```no_run /// use std::sync::OnceLock; + /// /// use pg_embedded_setup_unpriv::{ClusterHandle, TestCluster}; /// /// static SHARED: OnceLock = OnceLock::new(); /// /// fn shared_handle() -> &'static ClusterHandle { /// SHARED.get_or_init(|| { - /// let (handle, guard) = TestCluster::new_split() - /// .expect("cluster bootstrap failed"); - /// handle.register_shutdown_on_exit() + /// let (handle, guard) = TestCluster::new_split().expect("cluster bootstrap failed"); + /// handle + /// .register_shutdown_on_exit() /// .expect("shutdown hook registration failed"); /// std::mem::forget(guard); /// handle diff --git a/src/cluster/installation.rs b/src/cluster/installation.rs index 5aef1b67..020ebee8 100644 --- a/src/cluster/installation.rs +++ b/src/cluster/installation.rs @@ -3,13 +3,17 @@ //! Handles refreshing the installation directory and port after worker setup, //! as well as reading `postmaster.pid` for port discovery. -use crate::error::BootstrapResult; -use crate::observability::LOG_TARGET; -use crate::{ExecutionPrivileges, TestBootstrapSettings}; +use std::{path::Path, time::Duration}; + use color_eyre::eyre::eyre; use postgresql_embedded::{Settings, Version}; -use std::path::Path; -use std::time::Duration; + +use crate::{ + ExecutionPrivileges, + TestBootstrapSettings, + error::BootstrapResult, + observability::LOG_TARGET, +}; /// Number of attempts to read the postmaster port. pub(super) const POSTMASTER_PORT_ATTEMPTS: usize = 10; @@ -215,9 +219,10 @@ pub(super) fn resolve_installed_dir(settings: &Settings) -> Option) -> Self { - Self(name.into()) - } + pub fn new(name: impl Into) -> Self { Self(name.into()) } /// Returns the database name as a string slice. #[must_use] - pub fn as_str(&self) -> &str { - &self.0 - } + pub fn as_str(&self) -> &str { &self.0 } } impl AsRef for DatabaseName { - fn as_ref(&self) -> &str { - &self.0 - } + fn as_ref(&self) -> &str { &self.0 } } impl From<&str> for DatabaseName { - fn from(s: &str) -> Self { - Self(s.to_owned()) - } + fn from(s: &str) -> Self { Self(s.to_owned()) } } impl From for DatabaseName { - fn from(s: String) -> Self { - Self(s) - } + fn from(s: String) -> Self { Self(s) } } /// Global per-template locks to prevent concurrent template creation. @@ -152,7 +144,9 @@ impl TestClusterConnection { /// // ... run migrations on my_template ... /// /// // Clone the template for a test - /// cluster.connection().create_database_from_template("test_db", "my_template")?; + /// cluster + /// .connection() + /// .create_database_from_template("test_db", "my_template")?; /// # Ok(()) /// # } /// ``` @@ -258,14 +252,18 @@ impl TestClusterConnection { /// let cluster = TestCluster::new()?; /// /// // Ensure template exists, running migrations if needed - /// cluster.connection().ensure_template_exists("my_template", |db_name| { - /// // Run migrations on the newly created template database - /// // e.g., diesel::migration::run(&mut conn)?; - /// Ok(()) - /// })?; + /// cluster + /// .connection() + /// .ensure_template_exists("my_template", |db_name| { + /// // Run migrations on the newly created template database + /// // e.g., diesel::migration::run(&mut conn)?; + /// Ok(()) + /// })?; /// /// // Clone the template for each test - /// cluster.connection().create_database_from_template("test_db_1", "my_template")?; + /// cluster + /// .connection() + /// .create_database_from_template("test_db_1", "my_template")?; /// # Ok(()) /// # } /// ``` @@ -357,7 +355,8 @@ impl TestClusterConnection { /// cluster.ensure_template_exists("migrated_template", |_| Ok(()))?; /// /// // Each test gets its own database cloned from the template - /// let temp_db = cluster.connection() + /// let temp_db = cluster + /// .connection() /// .temporary_database_from_template("test_db", "migrated_template")?; /// /// // Database is dropped automatically when temp_db goes out of scope diff --git a/src/cluster/mod.rs b/src/cluster/mod.rs index f3e0447e..ffc3b955 100644 --- a/src/cluster/mod.rs +++ b/src/cluster/mod.rs @@ -16,6 +16,7 @@ //! let url = cluster.settings().url("my_database"); //! // Perform test database work here. //! drop(cluster); // `PostgreSQL` stops automatically. +//! //! # Ok(()) //! # } //! ``` @@ -70,28 +71,35 @@ mod temporary_database; mod worker_invoker; mod worker_operation; -pub use self::connection::{ConnectionMetadata, TestClusterConnection}; -pub use self::guard::ClusterGuard; -pub use self::handle::ClusterHandle; -pub use self::lifecycle::DatabaseName; -pub use self::temporary_database::TemporaryDatabase; -#[cfg(any(doc, test, feature = "cluster-unit-tests", feature = "dev-worker"))] -pub use self::worker_invoker::WorkerInvoker; -#[doc(hidden)] -pub use self::worker_operation::WorkerOperation; +use std::ops::Deref; + +use tracing::info_span; -use self::runtime::build_runtime; -use self::runtime_mode::ClusterRuntime; pub(crate) use self::startup::setup_postgres_only; #[cfg(feature = "async-api")] use self::startup::start_postgres_async; -use self::startup::{cache_config_from_bootstrap, start_postgres}; -use crate::bootstrap_for_tests; -use crate::env::ScopedEnv; -use crate::error::BootstrapResult; -use crate::observability::LOG_TARGET; -use std::ops::Deref; -use tracing::info_span; +#[cfg(any(doc, test, feature = "cluster-unit-tests", feature = "dev-worker"))] +pub use self::worker_invoker::WorkerInvoker; +#[doc(hidden)] +pub use self::worker_operation::WorkerOperation; +pub use self::{ + connection::{ConnectionMetadata, TestClusterConnection}, + guard::ClusterGuard, + handle::ClusterHandle, + lifecycle::DatabaseName, + temporary_database::TemporaryDatabase, +}; +use self::{ + runtime::build_runtime, + runtime_mode::ClusterRuntime, + startup::{cache_config_from_bootstrap, start_postgres}, +}; +use crate::{ + bootstrap_for_tests, + env::ScopedEnv, + error::BootstrapResult, + observability::LOG_TARGET, +}; /// Embedded `PostgreSQL` instance whose lifecycle follows Rust's drop semantics. /// @@ -108,15 +116,16 @@ use tracing::info_span; /// /// ```no_run /// use std::sync::OnceLock; +/// /// use pg_embedded_setup_unpriv::{ClusterHandle, TestCluster}; /// /// static SHARED: OnceLock = OnceLock::new(); /// /// fn shared_cluster() -> &'static ClusterHandle { /// SHARED.get_or_init(|| { -/// let (handle, guard) = TestCluster::new_split() -/// .expect("cluster bootstrap failed"); -/// handle.register_shutdown_on_exit() +/// let (handle, guard) = TestCluster::new_split().expect("cluster bootstrap failed"); +/// handle +/// .register_shutdown_on_exit() /// .expect("shutdown hook registration failed"); /// std::mem::forget(guard); /// handle @@ -178,15 +187,16 @@ impl TestCluster { /// /// ```no_run /// use std::sync::OnceLock; + /// /// use pg_embedded_setup_unpriv::{ClusterHandle, TestCluster}; /// /// static SHARED: OnceLock = OnceLock::new(); /// /// fn shared_cluster() -> &'static ClusterHandle { /// SHARED.get_or_init(|| { - /// let (handle, guard) = TestCluster::new_split() - /// .expect("cluster bootstrap failed"); - /// handle.register_shutdown_on_exit() + /// let (handle, guard) = TestCluster::new_split().expect("cluster bootstrap failed"); + /// handle + /// .register_shutdown_on_exit() /// .expect("shutdown hook registration failed"); /// std::mem::forget(guard); /// handle @@ -404,9 +414,7 @@ impl TestCluster { impl Deref for TestCluster { type Target = ClusterHandle; - fn deref(&self) -> &Self::Target { - &self.handle - } + fn deref(&self) -> &Self::Target { &self.handle } } // Note: TestCluster does NOT implement Drop because the ClusterGuard handles shutdown. diff --git a/src/cluster/mod_tests.rs b/src/cluster/mod_tests.rs index 34faccdd..f4db8c46 100644 --- a/src/cluster/mod_tests.rs +++ b/src/cluster/mod_tests.rs @@ -2,16 +2,21 @@ use std::ffi::OsString; -use super::TestCluster; -use super::guard::ClusterGuard; -use super::handle::ClusterHandle; -use super::runtime_mode::ClusterRuntime; -use crate::ExecutionPrivileges; -use crate::env::ScopedEnv; -use crate::observability::LOG_TARGET; -use crate::test_support::{dummy_settings, scoped_env}; use tracing::info_span; +use super::{ + TestCluster, + guard::ClusterGuard, + handle::ClusterHandle, + runtime_mode::ClusterRuntime, +}; +use crate::{ + ExecutionPrivileges, + env::ScopedEnv, + observability::LOG_TARGET, + test_support::{dummy_settings, scoped_env}, +}; + #[test] fn with_worker_guard_restores_environment() { const KEY: &str = "PG_EMBEDDED_WORKER_GUARD_TEST"; diff --git a/src/cluster/runtime.rs b/src/cluster/runtime.rs index 99cb5fab..2077fe7b 100644 --- a/src/cluster/runtime.rs +++ b/src/cluster/runtime.rs @@ -1,10 +1,10 @@ //! Helpers for constructing Tokio runtimes used by `TestCluster`. -use crate::error::{BootstrapError, BootstrapResult}; use color_eyre::eyre::Context; use tokio::runtime::{Builder, Runtime}; use super::panic_utils::nested_runtime_thread_panic; +use crate::error::{BootstrapError, BootstrapResult}; /// Constructs a current-thread Tokio runtime for `TestCluster` lifecycle work. /// diff --git a/src/cluster/runtime_mode.rs b/src/cluster/runtime_mode.rs index 8dc0f4d0..65312eb6 100644 --- a/src/cluster/runtime_mode.rs +++ b/src/cluster/runtime_mode.rs @@ -21,7 +21,5 @@ pub(super) enum ClusterRuntime { impl ClusterRuntime { /// Returns `true` if this is async mode. - pub(super) const fn is_async(&self) -> bool { - matches!(self, Self::Async) - } + pub(super) const fn is_async(&self) -> bool { matches!(self, Self::Async) } } diff --git a/src/cluster/shutdown.rs b/src/cluster/shutdown.rs index b96258dd..1fe0f322 100644 --- a/src/cluster/shutdown.rs +++ b/src/cluster/shutdown.rs @@ -3,17 +3,23 @@ //! Provides synchronous and asynchronous shutdown methods, as well as //! drop-time cleanup for both worker-managed and in-process clusters. -use crate::error::BootstrapResult; -use crate::observability::LOG_TARGET; -use crate::{CleanupMode, TestBootstrapSettings}; -use postgresql_embedded::{PostgreSQL, Settings}; use std::{fmt::Display, time::Duration}; + +use postgresql_embedded::{PostgreSQL, Settings}; use tokio::time; -use super::cleanup; -use super::runtime::build_runtime; -use super::worker_invoker::WorkerInvoker as ClusterWorkerInvoker; -use super::worker_operation; +use super::{ + cleanup, + runtime::build_runtime, + worker_invoker::WorkerInvoker as ClusterWorkerInvoker, + worker_operation, +}; +use crate::{ + CleanupMode, + TestBootstrapSettings, + error::BootstrapResult, + observability::LOG_TARGET, +}; /// Context for cluster drop operations, grouping related shutdown state. pub(super) struct DropContext<'a> { @@ -354,15 +360,17 @@ pub(super) fn warn_stop_failure(context: &str, err: &impl Display) { /// Logs a warning when stopping the cluster times out. pub(super) fn warn_stop_timeout(timeout_secs: u64, context: &str) { tracing::warn!( - "SKIP-TEST-CLUSTER: stop() timed out after {timeout_secs}s ({context}); proceeding with drop" + "SKIP-TEST-CLUSTER: stop() timed out after {timeout_secs}s ({context}); proceeding with \ + drop" ); } #[cfg(all(test, feature = "cluster-unit-tests"))] mod tests { + use rstest::rstest; + use super::*; use crate::test_support::capture_warn_logs; - use rstest::rstest; #[rstest] #[case::timeout( diff --git a/src/cluster/shutdown_hook.rs b/src/cluster/shutdown_hook.rs index 8141cbff..f1cadb45 100644 --- a/src/cluster/shutdown_hook.rs +++ b/src/cluster/shutdown_hook.rs @@ -10,14 +10,12 @@ //! postmaster PID from disk, sends SIGTERM (signal 15, terminate), polls for //! exit, and escalates to SIGKILL if the timeout elapses. -use std::path::Path; -use std::sync::Mutex; -use std::time::Duration; +use std::{path::Path, sync::Mutex, time::Duration}; -use crate::CleanupMode; -use crate::error::BootstrapResult; use postgresql_embedded::Settings; +use crate::{CleanupMode, error::BootstrapResult}; + /// State captured at registration time and read by the atexit callback. struct ShutdownState { settings: Settings, @@ -238,16 +236,17 @@ fn best_effort_cleanup(state: &ShutdownState) { #[cfg(all(test, feature = "cluster-unit-tests"))] mod tests { - use super::*; - use color_eyre::eyre::{Result, ensure}; use rstest::{fixture, rstest}; use tempfile::TempDir; + use super::*; + /// Creates a fresh temporary directory for PID file tests. #[fixture] fn pid_dir() -> Result { - Ok(tempfile::tempdir()?) + let pid_dir = tempfile::tempdir()?; + Ok(pid_dir) } #[rstest] diff --git a/src/cluster/startup.rs b/src/cluster/startup.rs index 16efc18e..0beeb586 100644 --- a/src/cluster/startup.rs +++ b/src/cluster/startup.rs @@ -5,21 +5,26 @@ //! The [`setup_postgres_only`] entry point drives download + `initdb` without //! starting the server, used by the CLI binary. -use crate::cache::BinaryCacheConfig; -use crate::env::ScopedEnv; -use crate::error::BootstrapResult; -use crate::observability::LOG_TARGET; -use crate::{ExecutionPrivileges, TestBootstrapSettings}; use postgresql_embedded::PostgreSQL; use tokio::runtime::Runtime; use tracing::info; -use super::cache_integration; -use super::installation; #[cfg(feature = "async-api")] use super::worker_invoker::AsyncInvoker; -use super::worker_invoker::WorkerInvoker as ClusterWorkerInvoker; -use super::worker_operation; +use super::{ + cache_integration, + installation, + worker_invoker::WorkerInvoker as ClusterWorkerInvoker, + worker_operation, +}; +use crate::{ + ExecutionPrivileges, + TestBootstrapSettings, + cache::BinaryCacheConfig, + env::ScopedEnv, + error::BootstrapResult, + observability::LOG_TARGET, +}; #[derive(Clone, Copy)] enum LifecycleStep { diff --git a/src/cluster/startup_tests.rs b/src/cluster/startup_tests.rs index 4a7b397f..dc829686 100644 --- a/src/cluster/startup_tests.rs +++ b/src/cluster/startup_tests.rs @@ -1,10 +1,12 @@ //! Tests for setup-only startup orchestration. -use std::ffi::OsString; -use std::fs; -use std::panic::{AssertUnwindSafe, catch_unwind}; -use std::sync::{Arc, Mutex}; -use std::time::Duration; +use std::{ + ffi::OsString, + fs, + panic::{AssertUnwindSafe, catch_unwind}, + sync::{Arc, Mutex}, + time::Duration, +}; use camino::{Utf8Path, Utf8PathBuf}; use color_eyre::eyre::{Result, ensure, eyre}; @@ -15,7 +17,10 @@ use tempfile::tempdir; use super::*; use crate::test_support::{ - dummy_settings, install_run_root_operation_hook, scoped_env, test_runtime, + dummy_settings, + install_run_root_operation_hook, + scoped_env, + test_runtime, }; const TEST_POSTGRES_VERSION: &str = "17.4.0"; @@ -214,7 +219,8 @@ fn setup_postgres_only_resolves_cache_before_scoped_env_and_runs_setup_only( let observed_xdg_cache_home = std::env::var("XDG_CACHE_HOME").ok(); ensure!( observed_xdg_cache_home.as_deref() == Some(operations_hook.host_cache_home.as_str()), - "expected host XDG_CACHE_HOME to be set for cache resolution (observed: {observed_xdg_cache_home:?})", + "expected host XDG_CACHE_HOME to be set for cache resolution (observed: \ + {observed_xdg_cache_home:?})", ); create_complete_cache_entry(&operations_hook.host_cache_home, TEST_POSTGRES_VERSION)?; @@ -225,7 +231,8 @@ fn setup_postgres_only_resolves_cache_before_scoped_env_and_runs_setup_only( .join("binaries"); ensure!( resolved_cache_dir == expected_cache_dir, - "cache config should resolve from host env before ScopedEnv (expected: {expected_cache_dir}, observed: {resolved_cache_dir})", + "cache config should resolve from host env before ScopedEnv (expected: \ + {expected_cache_dir}, observed: {resolved_cache_dir})", ); let expected_install_dir = utf8_path( root_bootstrap.settings.installation_dir.clone(), diff --git a/src/cluster/temporary_database.rs b/src/cluster/temporary_database.rs index 412ab14e..a08164df 100644 --- a/src/cluster/temporary_database.rs +++ b/src/cluster/temporary_database.rs @@ -7,8 +7,7 @@ use color_eyre::eyre::WrapErr; use tracing::info_span; use super::connection::{connect_admin, escape_identifier}; -use crate::error::BootstrapResult; -use crate::observability::LOG_TARGET; +use crate::{error::BootstrapResult, observability::LOG_TARGET}; /// RAII guard that drops a database when it goes out of scope. /// @@ -58,15 +57,11 @@ impl TemporaryDatabase { /// Returns the database name. #[must_use] - pub fn name(&self) -> &str { - &self.name - } + pub fn name(&self) -> &str { &self.name } /// Returns the connection URL for this database. #[must_use] - pub fn url(&self) -> &str { - &self.database_url - } + pub fn url(&self) -> &str { &self.database_url } /// Drops the database, failing if connections exist. /// @@ -94,9 +89,7 @@ impl TemporaryDatabase { /// # Ok(()) /// # } /// ``` - pub fn drop_database(self) -> BootstrapResult<()> { - self.try_drop() - } + pub fn drop_database(self) -> BootstrapResult<()> { self.try_drop() } /// Drops the database, terminating any active connections first. /// @@ -132,9 +125,8 @@ impl TemporaryDatabase { // Terminate active connections using parameterized query client .execute( - "SELECT pg_terminate_backend(pid) \ - FROM pg_stat_activity \ - WHERE datname = $1 AND pid <> pg_backend_pid()", + "SELECT pg_terminate_backend(pid) FROM pg_stat_activity WHERE datname = $1 AND \ + pid <> pg_backend_pid()", &[&self.name], ) .wrap_err(format!( diff --git a/src/cluster/worker_invoker/mod.rs b/src/cluster/worker_invoker/mod.rs index e6955a6a..2f6f9916 100644 --- a/src/cluster/worker_invoker/mod.rs +++ b/src/cluster/worker_invoker/mod.rs @@ -1,18 +1,21 @@ -//! Dispatches `PostgreSQL` lifecycle operations either in-process or via the privileged worker binary. +//! Dispatches `PostgreSQL` lifecycle operations either in-process or via the privileged worker +//! binary. use std::future::Future; use color_eyre::eyre::{Context, eyre}; use tokio::runtime::Runtime; - -use crate::error::{BootstrapError, BootstrapResult}; -use crate::observability::LOG_TARGET; -use crate::worker_process::{self, WorkerRequest, WorkerRequestArgs}; -use crate::{ExecutionMode, ExecutionPrivileges, TestBootstrapSettings}; - -use super::WorkerOperation; -use super::panic_utils::nested_runtime_thread_panic; use tracing::{error, info, info_span}; +use super::{WorkerOperation, panic_utils::nested_runtime_thread_panic}; +use crate::{ + ExecutionMode, + ExecutionPrivileges, + TestBootstrapSettings, + error::{BootstrapError, BootstrapResult}, + observability::LOG_TARGET, + worker_process::{self, WorkerRequest, WorkerRequestArgs}, +}; + // ============================================================================ // Shared helper functions // ============================================================================ @@ -111,7 +114,8 @@ fn spawn_worker_inner( { let worker = bootstrap.worker_binary.as_ref().ok_or_else(|| { BootstrapError::from(eyre!(concat!( - "pg_worker binary not found. Install it with 'cargo install --path . --bin pg_worker' ", + "pg_worker binary not found. Install it with 'cargo install --path . --bin \ + pg_worker' ", "and ensure it is in PATH, or set PG_EMBEDDED_WORKER to its absolute path" ))) })?; diff --git a/src/cluster/worker_invoker/tests.rs b/src/cluster/worker_invoker/tests.rs index 38f121ac..7350b8c4 100644 --- a/src/cluster/worker_invoker/tests.rs +++ b/src/cluster/worker_invoker/tests.rs @@ -1,19 +1,26 @@ //! Unit tests for the [`WorkerInvoker`] component, verifying both in-process //! execution for unprivileged operations and hook delegation for root operations. -use super::*; -use crate::ExecutionPrivileges; -use crate::test_support::{ - RunRootOperationHookInstallError, drain_hook_install_logs, dummy_settings, - install_run_root_operation_hook, test_runtime, -}; -use color_eyre::eyre::{Result, ensure, eyre}; -use serial_test::serial; use std::sync::{ Arc, atomic::{AtomicUsize, Ordering}, }; +use color_eyre::eyre::{Result, ensure, eyre}; +use serial_test::serial; + +use super::*; +use crate::{ + ExecutionPrivileges, + test_support::{ + RunRootOperationHookInstallError, + drain_hook_install_logs, + dummy_settings, + install_run_root_operation_hook, + test_runtime, + }, +}; + #[test] fn unprivileged_operations_execute_in_process() -> Result<()> { let runtime = test_runtime()?; diff --git a/src/env/loom_tests.rs b/src/env/loom_tests.rs index b760a61e..657b5015 100644 --- a/src/env/loom_tests.rs +++ b/src/env/loom_tests.rs @@ -1,12 +1,19 @@ //! Loom-backed concurrency checks for `ScopedEnv`. -use super::ScopedEnv; -use super::state::{EnvLockOps, ThreadStateInner}; -use loom::sync::Arc; -use loom::sync::atomic::{AtomicUsize, Ordering}; -use loom::thread; -use std::cell::RefCell; -use std::ffi::OsString; +use std::{cell::RefCell, ffi::OsString}; + +use loom::{ + sync::{ + Arc, + atomic::{AtomicUsize, Ordering}, + }, + thread, +}; + +use super::{ + ScopedEnv, + state::{EnvLockOps, ThreadStateInner}, +}; loom::lazy_static! { static ref LOOM_ENV_LOCK: loom::sync::Mutex<()> = loom::sync::Mutex::new(()); diff --git a/src/env/mod.rs b/src/env/mod.rs index d01e4cf1..c175fc79 100644 --- a/src/env/mod.rs +++ b/src/env/mod.rs @@ -22,14 +22,12 @@ //! depth so callers can compose helpers without deadlocking. Different threads //! are still serialized. -use crate::observability::LOG_TARGET; -use std::cell::RefCell; -use std::ffi::OsString; -use std::marker::PhantomData; -use std::rc::Rc; -use std::thread_local; +use std::{cell::RefCell, ffi::OsString, marker::PhantomData, rc::Rc, thread_local}; + use tracing::{info, info_span}; +use crate::observability::LOG_TARGET; + #[cfg(all(test, feature = "loom-tests"))] mod loom_tests; mod state; @@ -61,9 +59,7 @@ fn enter_scope_std(vars: Vec<(OsString, Option)>) -> usize { with_state(|state| state.enter_scope(vars)) } -fn exit_scope_std(index: usize) { - with_state(|state| state.exit_scope(index)); -} +fn exit_scope_std(index: usize) { with_state(|state| state.exit_scope(index)); } /// Restores the process environment when dropped, reverting to prior values. #[derive(Debug)] diff --git a/src/env/state.rs b/src/env/state.rs index 944bccd6..68f0fec2 100644 --- a/src/env/state.rs +++ b/src/env/state.rs @@ -1,9 +1,12 @@ //! Thread-local state and mutex management for scoped environment guards. +use std::{ + env, + ffi::OsString, + sync::{Mutex, MutexGuard}, +}; + use crate::observability::LOG_TARGET; -use std::env; -use std::ffi::OsString; -use std::sync::{Mutex, MutexGuard}; pub(crate) static ENV_LOCK: Mutex<()> = Mutex::new(()); @@ -69,9 +72,7 @@ impl ThreadState { self.inner.enter_scope(vars) } - pub fn exit_scope(&mut self, index: usize) { - self.inner.exit_scope(index); - } + pub fn exit_scope(&mut self, index: usize) { self.inner.exit_scope(index); } } #[cfg(all(test, feature = "loom-tests"))] @@ -179,9 +180,7 @@ impl ThreadStateCore { } #[cfg(not(any(unix, windows)))] - fn contains_equals(key: &OsString) -> bool { - key.to_string_lossy().contains('=') - } + fn contains_equals(key: &OsString) -> bool { key.to_string_lossy().contains('=') } fn apply_single_var(key: &OsString, new_value: Option) -> Option { debug_assert!( @@ -302,17 +301,11 @@ impl ThreadStateCore { #[cfg(test)] impl ThreadState { - pub const fn depth(&self) -> usize { - self.inner.depth - } + pub const fn depth(&self) -> usize { self.inner.depth } - pub fn is_stack_empty(&self) -> bool { - self.inner.stack.is_empty() - } + pub fn is_stack_empty(&self) -> bool { self.inner.stack.is_empty() } - pub const fn has_lock(&self) -> bool { - self.inner.lock.is_some() - } + pub const fn has_lock(&self) -> bool { self.inner.lock.is_some() } } fn restore_saved(saved: Vec<(OsString, Option)>) { diff --git a/src/env/tests/corruption.rs b/src/env/tests/corruption.rs index 8607eaec..dcc2b413 100644 --- a/src/env/tests/corruption.rs +++ b/src/env/tests/corruption.rs @@ -5,10 +5,9 @@ //! drops). These helpers are used by the parameterised `rstest` cases in //! `mod.rs`. -use super::ScopedEnv; -use super::THREAD_STATE; -use std::ffi::OsString; -use std::panic; +use std::{ffi::OsString, panic}; + +use super::{ScopedEnv, THREAD_STATE}; /// Runs a corruption scenario with a unique env key and delegated assertions. pub(super) fn run_scoped_env_corruption_test(test_name: &str, setup_and_corrupt: F) @@ -83,14 +82,8 @@ pub(super) fn apply_invalid_scope_exit() -> bool { } /// Returns false so callers skip restoration assertions. -pub(super) const fn no_corruption() -> bool { - false -} +pub(super) const fn no_corruption() -> bool { false } -pub(super) fn drop_guards_in_order(guards: GuardSet) { - guards.drop_in_order(); -} +pub(super) fn drop_guards_in_order(guards: GuardSet) { guards.drop_in_order(); } -pub(super) fn drop_guards_out_of_order(guards: GuardSet) { - guards.drop_out_of_order(); -} +pub(super) fn drop_guards_out_of_order(guards: GuardSet) { guards.drop_out_of_order(); } diff --git a/src/env/tests/mod.rs b/src/env/tests/mod.rs index 49a9799c..98d2a20c 100644 --- a/src/env/tests/mod.rs +++ b/src/env/tests/mod.rs @@ -1,28 +1,44 @@ //! Tests for environment scoping and logging. -use super::ScopedEnv; -use super::THREAD_STATE; -use super::state::{ENV_LOCK, ThreadState}; -#[cfg(feature = "cluster-unit-tests")] -use crate::test_support::capture_info_logs; +use std::{ + env, + ffi::{OsStr, OsString}, + panic, + sync::{Arc, Barrier, TryLockError, mpsc}, + thread, + time::{Duration, Instant}, +}; + use rstest::rstest; use serial_test::serial; -use std::env; -use std::ffi::{OsStr, OsString}; -use std::panic; -use std::sync::{Arc, Barrier, TryLockError, mpsc}; -use std::thread; -use std::time::{Duration, Instant}; + +use super::{ + ScopedEnv, + THREAD_STATE, + state::{ENV_LOCK, ThreadState}, +}; +#[cfg(feature = "cluster-unit-tests")] +use crate::test_support::capture_info_logs; mod corruption; mod thread_helpers; use corruption::{ - CorruptionCase, apply_invalid_scope_exit, drop_guards_in_order, drop_guards_out_of_order, - no_corruption, run_scoped_env_corruption_test, setup_nested_guards, setup_single_guard, + CorruptionCase, + apply_invalid_scope_exit, + drop_guards_in_order, + drop_guards_out_of_order, + no_corruption, + run_scoped_env_corruption_test, + setup_nested_guards, + setup_single_guard, }; use thread_helpers::{ - ReleaseOnDrop, RestoreEnv, ThreadAChannels, ThreadBChannels, spawn_inner_guard_thread, + ReleaseOnDrop, + RestoreEnv, + ThreadAChannels, + ThreadBChannels, + spawn_inner_guard_thread, spawn_outer_guard_thread, }; diff --git a/src/env/tests/thread_helpers.rs b/src/env/tests/thread_helpers.rs index b1112aa2..4556068b 100644 --- a/src/env/tests/thread_helpers.rs +++ b/src/env/tests/thread_helpers.rs @@ -3,11 +3,14 @@ //! Provides the drop guards and spawn routines used by //! `serialises_env_across_threads` to exercise cross-thread ordering. +use std::{ + env, + ffi::{OsStr, OsString}, + sync::{Arc, Barrier, mpsc}, + thread, +}; + use super::{ENV_LOCK, ScopedEnv, remove_env_var_unlocked, set_env_var_unlocked}; -use std::env; -use std::ffi::{OsStr, OsString}; -use std::sync::{Arc, Barrier, mpsc}; -use std::thread; /// Sends a unit on drop via `mpsc::Sender` and ignores send errors. pub(super) struct ReleaseOnDrop { @@ -115,10 +118,8 @@ pub(super) fn spawn_inner_guard_thread( /// - `channels`: `ThreadAChannels` containing the coordination primitives: /// - `barrier`: `Arc` used to co-ordinate with other threads. /// - `ready_tx`: `mpsc::Sender<()>` used to signal readiness after applying. -/// - `release_rx`: `mpsc::Receiver<()>` used to wait for release before -/// dropping the guard. -/// - `done_tx`: `mpsc::Sender<()>` used to signal completion after the guard -/// is dropped. +/// - `release_rx`: `mpsc::Receiver<()>` used to wait for release before dropping the guard. +/// - `done_tx`: `mpsc::Sender<()>` used to signal completion after the guard is dropped. /// /// # Behaviour /// diff --git a/src/error.rs b/src/error.rs index 7dbae136..145d84bf 100644 --- a/src/error.rs +++ b/src/error.rs @@ -54,26 +54,18 @@ impl BootstrapError { /// Constructs a new bootstrap error with the provided kind and diagnostic /// report. #[must_use] - pub const fn new(kind: BootstrapErrorKind, report: Report) -> Self { - Self { kind, report } - } + pub const fn new(kind: BootstrapErrorKind, report: Report) -> Self { Self { kind, report } } /// Returns the semantic category for this bootstrap failure. #[must_use] - pub const fn kind(&self) -> BootstrapErrorKind { - self.kind - } + pub const fn kind(&self) -> BootstrapErrorKind { self.kind } /// Extracts the underlying diagnostic report. - pub fn into_report(self) -> Report { - self.report - } + pub fn into_report(self) -> Report { self.report } } impl From for BootstrapError { - fn from(report: Report) -> Self { - Self::new(BootstrapErrorKind::Other, report) - } + fn from(report: Report) -> Self { Self::new(BootstrapErrorKind::Other, report) } } impl From for BootstrapError { @@ -114,10 +106,11 @@ pub struct ConfigError(#[from] Report); mod tests { //! Unit tests for error display formats. - use super::*; use color_eyre::eyre::eyre; use rstest::rstest; + use super::*; + #[rstest] #[case::bootstrap( "PG_EMBEDDED_WORKER must be set", diff --git a/src/fs.rs b/src/fs.rs index e94692be..9e0eb13e 100644 --- a/src/fs.rs +++ b/src/fs.rs @@ -1,15 +1,17 @@ //! Shared filesystem helpers that operate within the capability sandbox. -use crate::observability::LOG_TARGET; +use std::io::ErrorKind; + use camino::{Utf8Path, Utf8PathBuf}; use cap_std::{ ambient_authority, fs::{Dir, Metadata, Permissions, PermissionsExt}, }; use color_eyre::eyre::{Context, Result}; -use std::io::ErrorKind; use tracing::{error, info, info_span}; +use crate::observability::LOG_TARGET; + /// Resolves a path to an ambient directory handle paired with the relative path component. /// /// Absolute paths are opened relative to their parent directory; relative paths reuse the current @@ -228,13 +230,14 @@ fn log_dir_metadata_error(path: &Utf8Path, err: std::io::Error) -> std::io::Erro mod tests { //! Unit tests for filesystem helpers. - use super::{ensure_dir_exists, ensure_existing_path_is_dir, find_existing_ancestor}; + use std::{fs::File, io::ErrorKind}; + use camino::{Utf8Path, Utf8PathBuf}; use rstest::rstest; - use std::fs::File; - use std::io::ErrorKind; use tempfile::tempdir; + use super::{ensure_dir_exists, ensure_existing_path_is_dir, find_existing_ancestor}; + /// Test-case container: `create_file` selects file vs directory, and /// `error_kind` records the expected `ErrorKind` outcome. struct ExistingPathCase { diff --git a/src/lib.rs b/src/lib.rs index e9c651fa..ff472f43 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -34,10 +34,7 @@ pub(crate) mod worker_process; pub mod worker_process_test_api { //! Integration test shims for worker process orchestration. - pub use crate::cluster::WorkerOperation; use crate::worker_process; - pub use crate::worker_process::WorkerRequestArgs; - #[cfg(all( unix, any( @@ -50,6 +47,7 @@ pub mod worker_process_test_api { any(test, doc, feature = "privileged-tests"), ))] use crate::worker_process::PrivilegeDropGuard as InnerPrivilegeDropGuard; + pub use crate::{cluster::WorkerOperation, worker_process::WorkerRequestArgs}; /// Test-visible wrapper around the internal worker request. /// @@ -89,9 +87,7 @@ pub mod worker_process_test_api { } /// Returns a reference to the wrapped worker request. - pub(crate) const fn inner(&self) -> &worker_process::WorkerRequest<'a> { - &self.0 - } + pub(crate) const fn inner(&self) -> &worker_process::WorkerRequest<'a> { &self.0 } } /// Executes a worker request whilst returning crate-level errors. @@ -145,47 +141,20 @@ pub mod worker_process_test_api { } } -/// Resolves a path to an ambient directory handle paired with the relative path component. -/// -/// This function provides capability-based filesystem access by opening paths relative to -/// ambient authority. Absolute paths are opened relative to their parent directory; relative -/// paths reuse the current working directory. -/// -/// # Returns -/// -/// Returns a tuple containing: -/// - A [`cap_std::fs::Dir`] handle for the parent directory -/// - A [`camino::Utf8PathBuf`] with the relative component -/// -/// For absolute paths like `/foo/bar`, returns `(Dir("/foo"), "bar")`. -/// For relative paths like `baz/qux`, returns `(Dir("."), "baz/qux")`. -/// For root paths like `/`, returns `(Dir("/"), "")` with an empty relative component. -/// -/// # Errors -/// -/// Returns an error if the path cannot be opened as a directory or if path operations fail. -/// -/// # Examples -/// -/// ```no_run -/// use pg_embedded_setup_unpriv::ambient_dir_and_path; -/// use camino::Utf8Path; -/// -/// # fn main() -> color_eyre::Result<()> { -/// let (dir, relative) = ambient_dir_and_path(Utf8Path::new("./data"))?; -/// // Use dir handle for capability-based operations on relative path -/// # Ok(()) -/// # } -/// ``` -pub use crate::fs::ambient_dir_and_path; +use std::ffi::OsString; -#[doc(hidden)] -pub use crate::env::ScopedEnv; pub use bootstrap::{ - CleanupMode, ExecutionMode, ExecutionPrivileges, TestBootstrapEnvironment, - TestBootstrapSettings, bootstrap_for_tests, detect_execution_privileges, find_timezone_dir, + CleanupMode, + ExecutionMode, + ExecutionPrivileges, + TestBootstrapEnvironment, + TestBootstrapSettings, + bootstrap_for_tests, + detect_execution_privileges, + find_timezone_dir, run, }; +use camino::Utf8PathBuf; #[cfg(any(doc, test, feature = "cluster-unit-tests", feature = "dev-worker"))] #[doc(hidden)] pub use cluster::WorkerInvoker; @@ -193,15 +162,28 @@ pub use cluster::WorkerInvoker; #[doc(hidden)] pub use cluster::WorkerOperation; pub use cluster::{ - ClusterGuard, ClusterHandle, ConnectionMetadata, DatabaseName, TemporaryDatabase, TestCluster, + ClusterGuard, + ClusterHandle, + ConnectionMetadata, + DatabaseName, + TemporaryDatabase, + TestCluster, TestClusterConnection, }; +use color_eyre::eyre::{Context, eyre}; #[doc(hidden)] pub use error::BootstrapResult; -pub use error::PgEmbeddedError as Error; pub use error::{ - BootstrapError, BootstrapErrorKind, PgEmbeddedError, PrivilegeError, PrivilegeResult, Result, + BootstrapError, + BootstrapErrorKind, + PgEmbeddedError as Error, + PgEmbeddedError, + PrivilegeError, + PrivilegeResult, + Result, }; +use ortho_config::OrthoConfig; +use postgresql_embedded::{Settings, VersionReq}; #[cfg(feature = "privileged-tests")] #[cfg(all( unix, @@ -229,15 +211,44 @@ pub use privileges::with_temp_euid; ), ))] pub use privileges::{default_paths_for, make_data_dir_private, make_dir_accessible, nobody_uid}; - -use color_eyre::eyre::{Context, eyre}; -use ortho_config::OrthoConfig; -use postgresql_embedded::{Settings, VersionReq}; use serde::{Deserialize, Serialize}; +#[doc(hidden)] +pub use crate::env::ScopedEnv; use crate::error::{ConfigError, ConfigResult}; -use camino::Utf8PathBuf; -use std::ffi::OsString; +/// Resolves a path to an ambient directory handle paired with the relative path component. +/// +/// This function provides capability-based filesystem access by opening paths relative to +/// ambient authority. Absolute paths are opened relative to their parent directory; relative +/// paths reuse the current working directory. +/// +/// # Returns +/// +/// Returns a tuple containing: +/// - A [`cap_std::fs::Dir`] handle for the parent directory +/// - A [`camino::Utf8PathBuf`] with the relative component +/// +/// For absolute paths like `/foo/bar`, returns `(Dir("/foo"), "bar")`. +/// For relative paths like `baz/qux`, returns `(Dir("."), "baz/qux")`. +/// For root paths like `/`, returns `(Dir("/"), "")` with an empty relative component. +/// +/// # Errors +/// +/// Returns an error if the path cannot be opened as a directory or if path operations fail. +/// +/// # Examples +/// +/// ```no_run +/// use camino::Utf8Path; +/// use pg_embedded_setup_unpriv::ambient_dir_and_path; +/// +/// # fn main() -> color_eyre::Result<()> { +/// let (dir, relative) = ambient_dir_and_path(Utf8Path::new("./data"))?; +/// // Use dir handle for capability-based operations on relative path +/// # Ok(()) +/// # } +/// ``` +pub use crate::fs::ambient_dir_and_path; /// Captures `PostgreSQL` settings supplied via environment variables. #[derive(Debug, Clone, Serialize, Deserialize, OrthoConfig, Default)] @@ -311,9 +322,7 @@ impl PgEnvCfg { /// /// # Errors /// Returns an error when the semantic version requirement cannot be parsed. - pub fn to_settings(&self) -> Result { - self.to_settings_with_context(false) - } + pub fn to_settings(&self) -> Result { self.to_settings_with_context(false) } /// Converts the configuration into `Settings`, applying test-only worker limits. /// @@ -330,9 +339,7 @@ impl PgEnvCfg { /// /// # Errors /// Returns an error when the semantic version requirement cannot be parsed. - pub fn to_settings_for_tests(&self) -> Result { - self.to_settings_with_context(true) - } + pub fn to_settings_for_tests(&self) -> Result { self.to_settings_with_context(true) } /// Converts the configuration into `Settings`, optionally applying test limits. /// diff --git a/src/privileges.rs b/src/privileges.rs index b526c534..bc5f5dce 100644 --- a/src/privileges.rs +++ b/src/privileges.rs @@ -9,9 +9,8 @@ target_os = "dragonfly", ) ))] -use crate::error::{PrivilegeError, PrivilegeResult}; -use crate::fs::{ensure_dir_exists, set_permissions}; -use crate::observability::LOG_TARGET; +use std::io::ErrorKind; + use camino::{Utf8Path, Utf8PathBuf}; use cap_std::{ ambient_authority, @@ -19,9 +18,14 @@ use cap_std::{ }; use color_eyre::eyre::{Context, eyre}; use nix::unistd::{Uid, User, chown}; -use std::io::ErrorKind; use tracing::{info, info_span}; +use crate::{ + error::{PrivilegeError, PrivilegeResult}, + fs::{ensure_dir_exists, set_permissions}, + observability::LOG_TARGET, +}; + pub(crate) fn ensure_dir_for_user>( directory: P, user: &User, @@ -197,9 +201,7 @@ fn chown_entry(path: &Utf8Path, user: &User) -> PrivilegeResult<()> { Ok(()) } -fn is_directory(entry: &DirEntry) -> bool { - entry.file_type().is_ok_and(|ft| ft.is_dir()) -} +fn is_directory(entry: &DirEntry) -> bool { entry.file_type().is_ok_and(|ft| ft.is_dir()) } /// Retrieves the UID of the `nobody` account, defaulting to 65534 when absent. /// diff --git a/src/test_support/errors.rs b/src/test_support/errors.rs index 3e9ca528..ef8d0eb8 100644 --- a/src/test_support/errors.rs +++ b/src/test_support/errors.rs @@ -24,9 +24,7 @@ use crate::error::{BootstrapError, PrivilegeError}; /// ``` #[cfg(any(doc, test, feature = "cluster-unit-tests", feature = "dev-worker"))] #[must_use] -pub fn bootstrap_error(err: Report) -> Error { - Error::Bootstrap(BootstrapError::from(err)) -} +pub fn bootstrap_error(err: Report) -> Error { Error::Bootstrap(BootstrapError::from(err)) } /// Converts a privilege-related report into the library's public [`Error`] type. /// This helper exists for test scaffolding and should not be used in published @@ -43,6 +41,4 @@ pub fn bootstrap_error(err: Report) -> Error { /// ``` #[cfg(any(doc, test, feature = "cluster-unit-tests", feature = "dev-worker"))] #[must_use] -pub fn privilege_error(err: Report) -> Error { - Error::Privilege(PrivilegeError::from(err)) -} +pub fn privilege_error(err: Report) -> Error { Error::Privilege(PrivilegeError::from(err)) } diff --git a/src/test_support/filesystem.rs b/src/test_support/filesystem.rs index 1d256844..29f45771 100644 --- a/src/test_support/filesystem.rs +++ b/src/test_support/filesystem.rs @@ -51,9 +51,7 @@ pub fn ambient_dir_and_path(path: &Utf8Path) -> Result<(Dir, Utf8PathBuf)> { /// # } /// ``` #[cfg(any(doc, test, feature = "cluster-unit-tests", feature = "dev-worker"))] -pub fn ensure_dir_exists(path: &Utf8Path) -> Result<()> { - fs::ensure_dir_exists(path) -} +pub fn ensure_dir_exists(path: &Utf8Path) -> Result<()> { fs::ensure_dir_exists(path) } /// Applies POSIX permissions to the provided path when it already exists. /// @@ -68,9 +66,7 @@ pub fn ensure_dir_exists(path: &Utf8Path) -> Result<()> { /// # } /// ``` #[cfg(any(doc, test, feature = "cluster-unit-tests", feature = "dev-worker"))] -pub fn set_permissions(path: &Utf8Path, mode: u32) -> Result<()> { - fs::set_permissions(path, mode) -} +pub fn set_permissions(path: &Utf8Path, mode: u32) -> Result<()> { fs::set_permissions(path, mode) } /// Retrieves metadata for the provided path using capability APIs. #[cfg(any(doc, test, feature = "cluster-unit-tests", feature = "dev-worker"))] @@ -207,9 +203,7 @@ impl CapabilityTempDir { /// Returns the UTF-8 path to the temporary directory. #[must_use] - pub fn path(&self) -> &Utf8Path { - &self.path - } + pub fn path(&self) -> &Utf8Path { &self.path } fn remove_dir(dir: Dir, path: &Utf8Path) { if let Err(err) = dir.remove_open_dir_all() { diff --git a/src/test_support/fixtures.rs b/src/test_support/fixtures.rs index 75520361..929b6613 100644 --- a/src/test_support/fixtures.rs +++ b/src/test_support/fixtures.rs @@ -1,20 +1,26 @@ //! Shared fixtures for tests that need bootstrap scaffolding. -use super::scoped_env::scoped_env; +use std::{ffi::OsString, time::Duration}; + use camino::Utf8PathBuf; use color_eyre::eyre::{Result, eyre}; +use postgresql_embedded::Settings; #[cfg(not(doc))] use rstest::fixture; -use std::ffi::OsString; -use std::time::Duration; use tokio::runtime::{Builder, Runtime}; -use super::worker_env; +use super::{scoped_env::scoped_env, worker_env}; use crate::{ - CleanupMode, ClusterHandle, ExecutionMode, ExecutionPrivileges, TestBootstrapEnvironment, - TestBootstrapSettings, TestCluster, detect_execution_privileges, env::ScopedEnv, + CleanupMode, + ClusterHandle, + ExecutionMode, + ExecutionPrivileges, + TestBootstrapEnvironment, + TestBootstrapSettings, + TestCluster, + detect_execution_privileges, + env::ScopedEnv, }; -use postgresql_embedded::Settings; /// Builds a single-threaded Tokio runtime for synchronous tests. /// @@ -61,8 +67,7 @@ pub fn dummy_environment() -> TestBootstrapEnvironment { /// /// # Examples /// ```rust -/// use pg_embedded_setup_unpriv::test_support::dummy_settings; -/// use pg_embedded_setup_unpriv::ExecutionPrivileges; +/// use pg_embedded_setup_unpriv::{ExecutionPrivileges, test_support::dummy_settings}; /// /// let settings = dummy_settings(ExecutionPrivileges::Unprivileged); /// assert_eq!(settings.privileges, ExecutionPrivileges::Unprivileged); @@ -207,16 +212,18 @@ pub fn shared_test_cluster_handle() -> &'static ClusterHandle { match shared_cluster_handle() { Ok(handle) => handle, Err(err) => panic!( - "SKIP-TEST-CLUSTER: shared_test_cluster_handle fixture failed to start PostgreSQL: {err:?}" + "SKIP-TEST-CLUSTER: shared_test_cluster_handle fixture failed to start PostgreSQL: \ + {err:?}" ), } } #[cfg(test)] mod tests { - use super::*; use rstest::rstest; + use super::*; + /// Unprivileged users should not require the worker binary, regardless of /// whether it exists or whether `PG_EMBEDDED_WORKER` is set. #[rstest] diff --git a/src/test_support/fixtures_docs.rs b/src/test_support/fixtures_docs.rs index 09c37b81..b610cc29 100644 --- a/src/test_support/fixtures_docs.rs +++ b/src/test_support/fixtures_docs.rs @@ -5,9 +5,7 @@ //! applied to items with doc comments. This keeps Whitaker's //! `function_attrs_follow_docs` lint happy while preserving documentation. -use crate::ClusterHandle; -use crate::TestCluster; -use crate::test_support::fixtures as runtime_fixtures; +use crate::{ClusterHandle, TestCluster, test_support::fixtures as runtime_fixtures}; /// `rstest` fixture that yields a running [`TestCluster`]. /// @@ -17,8 +15,7 @@ use crate::test_support::fixtures as runtime_fixtures; /// /// # Examples /// ```no_run -/// use pg_embedded_setup_unpriv::TestCluster; -/// use pg_embedded_setup_unpriv::test_support::test_cluster; +/// use pg_embedded_setup_unpriv::{TestCluster, test_support::test_cluster}; /// use rstest::rstest; /// /// #[rstest] @@ -28,9 +25,7 @@ use crate::test_support::fixtures as runtime_fixtures; /// } /// ``` #[must_use] -pub fn test_cluster() -> TestCluster { - runtime_fixtures::test_cluster() -} +pub fn test_cluster() -> TestCluster { runtime_fixtures::test_cluster() } /// `rstest` fixture that yields a reference to the shared [`TestCluster`]. /// @@ -47,8 +42,7 @@ pub fn test_cluster() -> TestCluster { /// # Examples /// /// ```no_run -/// use pg_embedded_setup_unpriv::TestCluster; -/// use pg_embedded_setup_unpriv::test_support::shared_test_cluster; +/// use pg_embedded_setup_unpriv::{TestCluster, test_support::shared_test_cluster}; /// use rstest::rstest; /// /// #[rstest] @@ -58,9 +52,7 @@ pub fn test_cluster() -> TestCluster { /// } /// ``` #[must_use] -pub fn shared_test_cluster() -> &'static TestCluster { - runtime_fixtures::shared_test_cluster() -} +pub fn shared_test_cluster() -> &'static TestCluster { runtime_fixtures::shared_test_cluster() } /// `rstest` fixture that yields a reference to the shared [`ClusterHandle`]. /// @@ -78,15 +70,16 @@ pub fn shared_test_cluster() -> &'static TestCluster { /// # Examples /// /// ```no_run -/// use pg_embedded_setup_unpriv::ClusterHandle; -/// use pg_embedded_setup_unpriv::test_support::shared_test_cluster_handle; +/// use pg_embedded_setup_unpriv::{ClusterHandle, test_support::shared_test_cluster_handle}; /// use rstest::rstest; /// /// #[rstest] /// fn uses_shared_handle(shared_test_cluster_handle: &'static ClusterHandle) { -/// assert!(shared_test_cluster_handle -/// .database_exists("postgres") -/// .expect("expected 'postgres' database to exist in shared_test_cluster_handle")); +/// assert!( +/// shared_test_cluster_handle +/// .database_exists("postgres") +/// .expect("expected 'postgres' database to exist in shared_test_cluster_handle") +/// ); /// } /// ``` #[must_use] diff --git a/src/test_support/hash.rs b/src/test_support/hash.rs index 6616cdc2..ae057207 100644 --- a/src/test_support/hash.rs +++ b/src/test_support/hash.rs @@ -1,9 +1,12 @@ //! Directory hashing utilities for template naming. +use std::{ + io::Read, + path::{Path, PathBuf}, +}; + use cap_std::{ambient_authority, fs::Dir}; use sha2::{Digest, Sha256}; -use std::io::Read; -use std::path::{Path, PathBuf}; use crate::error::BootstrapResult; @@ -136,10 +139,12 @@ fn join_path(base: &Path, relative: &Path) -> PathBuf { #[cfg(test)] mod tests { - use super::*; use std::fs; + use tempfile::TempDir; + use super::*; + #[test] fn hash_directory_produces_consistent_results() { let temp = TempDir::new().expect("tempdir"); diff --git a/src/test_support/hook.rs b/src/test_support/hook.rs index 5962b13b..fde3fb4e 100644 --- a/src/test_support/hook.rs +++ b/src/test_support/hook.rs @@ -1,16 +1,21 @@ //! Hook infrastructure that intercepts privileged worker operations during //! tests to assert behaviour and control cluster bootstrapping. -use std::future::Future; -use std::mem; -use std::sync::{Arc, Mutex, OnceLock}; -use std::thread; - -use crate::TestBootstrapSettings; -use crate::cluster::{WorkerInvoker, WorkerOperation}; -use crate::error::BootstrapResult; +use std::{ + future::Future, + mem, + sync::{Arc, Mutex, OnceLock}, + thread, +}; + use tracing::debug_span; +use crate::{ + TestBootstrapSettings, + cluster::{WorkerInvoker, WorkerOperation}, + error::BootstrapResult, +}; + #[doc(hidden)] /// Signature for intercepting privileged worker operations triggered by `TestCluster`. /// @@ -18,9 +23,7 @@ use tracing::debug_span; /// ``` /// use pg_embedded_setup_unpriv::test_support::RunRootOperationHook; /// -/// fn installs_hook(hook: RunRootOperationHook) { -/// let _ = hook; -/// } +/// fn installs_hook(hook: RunRootOperationHook) { let _ = hook; } /// ``` pub type RunRootOperationHook = Arc< dyn Fn( @@ -62,8 +65,7 @@ pub fn drain_hook_install_logs() -> Vec { /// run_root_operation_hook, /// }; /// -/// let guard = install_run_root_operation_hook(|_, _, _| Ok(())) -/// .expect("hook should install"); +/// let guard = install_run_root_operation_hook(|_, _, _| Ok(())).expect("hook should install"); /// assert!( /// run_root_operation_hook() /// .lock() @@ -82,8 +84,7 @@ pub fn run_root_operation_hook() -> &'static Mutex> /// ``` /// use pg_embedded_setup_unpriv::test_support::install_run_root_operation_hook; /// -/// let guard = install_run_root_operation_hook(|_, _, _| Ok(())) -/// .expect("hook should install"); +/// let guard = install_run_root_operation_hook(|_, _, _| Ok(())).expect("hook should install"); /// drop(guard); // hook removed automatically /// ``` pub struct HookGuard; @@ -165,8 +166,7 @@ where /// ``` /// use pg_embedded_setup_unpriv::test_support::install_run_root_operation_hook; /// -/// let guard = install_run_root_operation_hook(|_, _, _| Ok(())) -/// .expect("hook should install"); +/// let guard = install_run_root_operation_hook(|_, _, _| Ok(())).expect("hook should install"); /// drop(guard); /// ``` pub fn install_run_root_operation_hook( diff --git a/src/test_support/logging.rs b/src/test_support/logging.rs index 1ba42ee0..5ab25a3c 100644 --- a/src/test_support/logging.rs +++ b/src/test_support/logging.rs @@ -3,14 +3,15 @@ //! The helper records `WARN`-level logs without timestamps so assertions can //! match human-readable messages directly. -use std::io::{Result as IoResult, Write}; -use std::sync::{Arc, Mutex}; +use std::{ + io::{Result as IoResult, Write}, + sync::{Arc, Mutex}, +}; + +use tracing::{Level, subscriber::with_default}; +use tracing_subscriber::{fmt, fmt::format::FmtSpan}; use crate::observability::LOG_TARGET; -use tracing::Level; -use tracing::subscriber::with_default; -use tracing_subscriber::fmt; -use tracing_subscriber::fmt::format::FmtSpan; struct BufferWriter { buffer: Arc>>, @@ -26,9 +27,7 @@ impl Write for BufferWriter { Ok(buf.len()) } - fn flush(&mut self) -> IoResult<()> { - Ok(()) - } + fn flush(&mut self) -> IoResult<()> { Ok(()) } } /// Runs `action`, capturing warning logs and returning them alongside the @@ -159,9 +158,10 @@ fn decode_logs(bytes: Vec) -> Vec { #[cfg(test)] mod tests { - use super::{capture_debug_logs, capture_info_logs_with_spans, capture_warn_logs, decode_logs}; use tracing::info_span; + use super::{capture_debug_logs, capture_info_logs_with_spans, capture_warn_logs, decode_logs}; + #[test] fn captures_span_enter_and_close_events() { let (logs, ()) = capture_info_logs_with_spans(|| { @@ -196,7 +196,7 @@ mod tests { #[test] fn decode_logs_uses_lossy_utf8_for_invalid_bytes() { - let bytes = vec![b'a', b'b', b'\n', 0xF0, 0x28, 0x8C, 0x28]; + let bytes = vec![b'a', b'b', b'\n', 0xf0, 0x28, 0x8c, 0x28]; let (warn_logs, logs) = capture_warn_logs(|| decode_logs(bytes)); assert_eq!( diff --git a/src/test_support/mod.rs b/src/test_support/mod.rs index a205bbbb..63bef143 100644 --- a/src/test_support/mod.rs +++ b/src/test_support/mod.rs @@ -25,18 +25,17 @@ mod worker_env; #[cfg(doc)] mod fixtures_docs; -#[cfg(all( - unix, - any(doc, test, feature = "cluster-unit-tests", feature = "dev-worker") -))] -pub use crate::cluster::{process_is_running, read_postmaster_pid}; #[cfg(any(doc, test, feature = "cluster-unit-tests", feature = "dev-worker"))] pub use errors::{bootstrap_error, privilege_error}; pub use filesystem::ambient_dir_and_path; #[cfg(any(doc, test, feature = "cluster-unit-tests", feature = "dev-worker"))] pub use filesystem::{CapabilityTempDir, ensure_dir_exists, metadata, set_permissions}; pub use fixtures::{ - dummy_environment, dummy_settings, ensure_worker_env, shared_cluster, shared_cluster_handle, + dummy_environment, + dummy_settings, + ensure_worker_env, + shared_cluster, + shared_cluster_handle, test_runtime, }; #[cfg(not(doc))] @@ -46,12 +45,20 @@ pub use fixtures_docs::{shared_test_cluster, shared_test_cluster_handle, test_cl pub use hash::hash_directory; #[cfg(any(doc, test, feature = "cluster-unit-tests", feature = "dev-worker"))] pub use hook::{ - HookGuard, RunRootOperationHook, RunRootOperationHookInstallError, drain_hook_install_logs, - install_run_root_operation_hook, invoke_with_privileges, run_root_operation_hook, + HookGuard, + RunRootOperationHook, + RunRootOperationHookInstallError, + drain_hook_install_logs, + install_run_root_operation_hook, + invoke_with_privileges, + run_root_operation_hook, }; #[cfg(any(test, feature = "cluster-unit-tests", feature = "dev-worker"))] pub use logging::{ - capture_debug_logs, capture_info_logs, capture_info_logs_with_spans, capture_warn_logs, + capture_debug_logs, + capture_info_logs, + capture_info_logs_with_spans, + capture_warn_logs, }; #[cfg(any(doc, test, feature = "cluster-unit-tests", feature = "dev-worker"))] pub use panic::panic_payload_to_string; @@ -59,3 +66,9 @@ pub use panic::panic_payload_to_string; pub use partial_data_dir::create_partial_data_dir; pub use scoped_env::scoped_env; pub use worker_env::worker_binary_for_tests; + +#[cfg(all( + unix, + any(doc, test, feature = "cluster-unit-tests", feature = "dev-worker") +))] +pub use crate::cluster::{process_is_running, read_postmaster_pid}; diff --git a/src/test_support/partial_data_dir.rs b/src/test_support/partial_data_dir.rs index 47b8ae4b..4205cffe 100644 --- a/src/test_support/partial_data_dir.rs +++ b/src/test_support/partial_data_dir.rs @@ -1,7 +1,6 @@ //! Shared helpers for creating partial `PostgreSQL` data directories in tests. -use std::fs; -use std::path::Path; +use std::{fs, path::Path}; /// Creates a partial data directory structure that simulates an interrupted `initdb`. /// @@ -21,6 +20,7 @@ use std::path::Path; /// /// ``` /// use std::path::Path; +/// /// use pg_embedded_setup_unpriv::test_support::create_partial_data_dir; /// /// let temp = tempfile::tempdir().unwrap(); @@ -40,9 +40,10 @@ pub fn create_partial_data_dir(data_dir: &Path) -> std::io::Result<()> { #[cfg(test)] mod tests { - use super::*; use tempfile::tempdir; + use super::*; + #[test] fn creates_expected_structure() { let temp = tempdir().expect("failed to create temp dir"); diff --git a/src/test_support/scoped_env.rs b/src/test_support/scoped_env.rs index 247bbfd8..4e8abc1f 100644 --- a/src/test_support/scoped_env.rs +++ b/src/test_support/scoped_env.rs @@ -13,9 +13,10 @@ use crate::env::ScopedEnv; /// /// use pg_embedded_setup_unpriv::test_support; /// -/// let guard = test_support::scoped_env(vec![ -/// (OsString::from("PGUSER"), Some(OsString::from("postgres"))), -/// ]); +/// let guard = test_support::scoped_env(vec![( +/// OsString::from("PGUSER"), +/// Some(OsString::from("postgres")), +/// )]); /// drop(guard); /// ``` #[doc(hidden)] diff --git a/src/test_support/shared_singleton.rs b/src/test_support/shared_singleton.rs index 220af102..e3bc9dd2 100644 --- a/src/test_support/shared_singleton.rs +++ b/src/test_support/shared_singleton.rs @@ -6,10 +6,12 @@ use std::sync::{Arc, Mutex, OnceLock}; -use crate::error::{BootstrapError, BootstrapResult}; -use crate::{ClusterHandle, TestCluster}; - use super::fixtures::ensure_worker_env; +use crate::{ + ClusterHandle, + TestCluster, + error::{BootstrapError, BootstrapResult}, +}; // ============================================================================ // Shared cluster handle singleton @@ -188,15 +190,14 @@ enum SharedClusterState { /// `PhantomData>`). The pointer is safe to share across threads because: /// 1. The cluster is only initialised once and never moved. /// 2. All access goes through immutable references. -/// 3. The cluster's public API is thread-safe (database operations use -/// independent connections). +/// 3. The cluster's public API is thread-safe (database operations use independent connections). struct SharedClusterPtr(*const TestCluster); // SAFETY: SharedClusterPtr upholds the following invariants: // 1. The pointer targets a `Box::leak`ed allocation that outlives all references. // 2. No mutable access occurs through this pointer; all usage is via `&TestCluster`. -// 3. `TestCluster` methods internally handle synchronisation (each database -// operation creates an independent connection). +// 3. `TestCluster` methods internally handle synchronisation (each database operation creates an +// independent connection). unsafe impl Send for SharedClusterPtr {} unsafe impl Sync for SharedClusterPtr {} diff --git a/src/test_support/worker_env.rs b/src/test_support/worker_env.rs index 709fbc82..feb71468 100644 --- a/src/test_support/worker_env.rs +++ b/src/test_support/worker_env.rs @@ -1,19 +1,18 @@ //! Resolves and stages worker binaries for privileged test runs. -use std::ffi::OsString; -use std::sync::OnceLock; - -#[cfg(unix)] -use sha2::{Digest, Sha256}; #[cfg(unix)] use std::os::unix::ffi::OsStrExt; #[cfg(unix)] use std::os::unix::fs::PermissionsExt; #[cfg(unix)] use std::path::{Path, PathBuf}; +use std::{ffi::OsString, sync::OnceLock}; #[cfg(unix)] use std::{fs, io}; +#[cfg(unix)] +use sha2::{Digest, Sha256}; + /// Returns the worker binary path staged for privileged test execution. /// /// The path is resolved once per process and, on Unix, staged into a @@ -28,9 +27,7 @@ use std::{fs, io}; /// # let _ = worker; /// ``` #[must_use] -pub fn worker_binary_for_tests() -> Option { - worker_binary() -} +pub fn worker_binary_for_tests() -> Option { worker_binary() } pub(super) fn worker_binary() -> Option { static WORKER_PATH: OnceLock> = OnceLock::new(); @@ -99,9 +96,10 @@ fn try_stage_worker_binary(original: &OsString) -> io::Result { /// 3. After creation, the directory is not a symlink and is owned by current user #[cfg(unix)] fn create_staging_directory_secure(staged_dir: &Path) -> io::Result<()> { - use nix::unistd::geteuid; use std::os::unix::fs::MetadataExt; + use nix::unistd::geteuid; + let current_uid = geteuid().as_raw(); // Check if path already exists @@ -232,9 +230,7 @@ fn check_deps_parent_for_profile( /// This function is only called from `check_deps_parent_for_profile` after /// "debug" and "release" have already been handled, so it always returns "unknown". #[cfg(unix)] -const fn profile_name_to_static(_name: &str) -> &'static str { - "unknown" -} +const fn profile_name_to_static(_name: &str) -> &'static str { "unknown" } /// Computes a short hash of the source path for staging directory uniqueness. #[cfg(unix)] diff --git a/src/test_support/worker_env_tests.rs b/src/test_support/worker_env_tests.rs index 10c363ac..ee8c5ed7 100644 --- a/src/test_support/worker_env_tests.rs +++ b/src/test_support/worker_env_tests.rs @@ -1,11 +1,12 @@ //! Tests for worker binary staging logic. -use super::*; -use rstest::{fixture, rstest}; - #[cfg(unix)] use std::path::PathBuf; +use rstest::{fixture, rstest}; + +use super::*; + /// Guard that cleans up a staged directory when dropped. #[cfg(unix)] struct StagedDirCleanup(Option); diff --git a/src/worker.rs b/src/worker.rs index b932cd3f..ab53d12e 100644 --- a/src/worker.rs +++ b/src/worker.rs @@ -5,10 +5,10 @@ //! //! # Examples //! ```no_run +//! use std::{error::Error, time::Duration}; +//! //! use pg_embedded_setup_unpriv::worker::{SettingsSnapshot, WorkerPayload}; //! use postgresql_embedded::Settings; -//! use std::error::Error; -//! use std::time::Duration; //! //! fn main() -> Result<(), Box> { //! let mut settings = Settings::default(); @@ -22,7 +22,9 @@ //! settings.password = "secret".into(); //! settings.temporary = false; //! settings.timeout = Some(Duration::from_secs(30)); -//! settings.configuration.insert("log_min_messages".into(), "debug".into()); +//! settings +//! .configuration +//! .insert("log_min_messages".into(), "debug".into()); //! settings.trust_installation_dir = true; //! //! let snapshot = SettingsSnapshot::try_from(&settings)?; @@ -40,15 +42,16 @@ //! Ok(()) //! } //! ``` -use crate::error::BootstrapError; +use std::{collections::HashMap, time::Duration}; + use camino::Utf8PathBuf; use color_eyre::eyre::eyre; use postgresql_embedded::{Settings, VersionReq}; use secrecy::{ExposeSecret, SecretString}; use serde::{Deserialize, Serialize}; use serde_with::{DisplayFromStr, DurationSeconds, serde_as}; -use std::collections::HashMap; -use std::time::Duration; + +use crate::error::BootstrapError; /// Serialised representation of [`Settings`] for subprocess helpers. #[serde_as] @@ -74,9 +77,7 @@ pub struct SettingsSnapshot { impl SettingsSnapshot { /// Converts the snapshot back into [`Settings`]. - pub fn into_settings(self) -> Result { - Ok(self.into()) - } + pub fn into_settings(self) -> Result { Ok(self.into()) } } impl TryFrom<&Settings> for SettingsSnapshot { @@ -143,7 +144,8 @@ impl From for Settings { // sensible value until this snapshot explicitly models them. #[expect( clippy::needless_update, - reason = "Keep upstream defaults for future Settings fields this snapshot does not yet model" + reason = "Keep upstream defaults for future Settings fields this snapshot does not \ + yet model" )] Self { releases_url: snapshot.releases_url, @@ -171,27 +173,19 @@ pub struct PlainSecret(SecretString); impl PlainSecret { #[must_use] - pub fn expose(&self) -> &str { - self.0.expose_secret() - } + pub fn expose(&self) -> &str { self.0.expose_secret() } } impl From for PlainSecret { - fn from(secret: String) -> Self { - Self(SecretString::from(secret)) - } + fn from(secret: String) -> Self { Self(SecretString::from(secret)) } } impl From<&str> for PlainSecret { - fn from(secret: &str) -> Self { - Self(SecretString::from(secret.to_owned())) - } + fn from(secret: &str) -> Self { Self(SecretString::from(secret.to_owned())) } } impl From for SecretString { - fn from(secret: PlainSecret) -> Self { - secret.0 - } + fn from(secret: PlainSecret) -> Self { secret.0 } } impl Serialize for PlainSecret { diff --git a/src/worker_process/mod.rs b/src/worker_process/mod.rs index 820a9980..42c49b7b 100644 --- a/src/worker_process/mod.rs +++ b/src/worker_process/mod.rs @@ -6,25 +6,16 @@ mod output; mod privileges; -pub(crate) use self::output::render_failure_for_tests; -use self::output::{append_error_context, combine_errors, render_failure}; -use crate::cluster::WorkerOperation; -use crate::error::{BootstrapError, BootstrapResult}; -use crate::observability::LOG_TARGET; -use crate::worker::WorkerPayload; +use std::{ + io::{ErrorKind, Write as _}, + path::Path, + process::{Child, Command, Output, Stdio}, + time::Duration, +}; + use camino::Utf8Path; use color_eyre::eyre::{Context, Report, eyre}; use postgresql_embedded::Settings; -use serde_json::to_writer; -use std::io::ErrorKind; -use std::io::Write as _; -use std::path::Path; -use std::process::{Child, Command, Output, Stdio}; -use std::time::Duration; -use tempfile::{NamedTempFile, TempPath}; -use tracing::{info, info_span}; -use wait_timeout::ChildExt; - #[cfg(all( unix, any( @@ -37,6 +28,19 @@ use wait_timeout::ChildExt; any(test, doc, feature = "privileged-tests"), ))] pub(crate) use privileges::{PrivilegeDropGuard, disable_privilege_drop_for_tests}; +use serde_json::to_writer; +use tempfile::{NamedTempFile, TempPath}; +use tracing::{info, info_span}; +use wait_timeout::ChildExt; + +pub(crate) use self::output::render_failure_for_tests; +use self::output::{append_error_context, combine_errors, render_failure}; +use crate::{ + cluster::WorkerOperation, + error::{BootstrapError, BootstrapResult}, + observability::LOG_TARGET, + worker::WorkerPayload, +}; /// Captures inputs for launching a worker subprocess. /// @@ -164,8 +168,7 @@ impl<'a> WorkerRequest<'a> { /// - the worker payload cannot be created, serialised, or flushed to disk; /// - the worker command cannot be spawned or its output cannot be collected; /// - the worker exceeds the configured timeout and must be terminated; or -/// - the worker exits unsuccessfully, in which case the captured output is -/// surfaced for context. +/// - the worker exits unsuccessfully, in which case the captured output is surfaced for context. /// /// # Examples /// @@ -207,9 +210,7 @@ struct WorkerProcess<'a> { } impl<'a> WorkerProcess<'a> { - const fn new(request: &'a WorkerRequest<'a>) -> Self { - Self { request } - } + const fn new(request: &'a WorkerRequest<'a>) -> Self { Self { request } } #[expect( clippy::cognitive_complexity, diff --git a/src/worker_process/output.rs b/src/worker_process/output.rs index 475658de..dc553b6f 100644 --- a/src/worker_process/output.rs +++ b/src/worker_process/output.rs @@ -1,10 +1,10 @@ //! Output truncation and error rendering helpers for worker processes. -use crate::error::BootstrapError; +use std::{borrow::Cow, fmt::Write as FmtWrite, process::Output}; + use color_eyre::eyre::eyre; -use std::borrow::Cow; -use std::fmt::Write as FmtWrite; -use std::process::Output; + +use crate::error::BootstrapError; pub(super) const OUTPUT_CHAR_LIMIT: usize = 2_048; pub(super) const TRUNCATION_SUFFIX: &str = "… [truncated]"; diff --git a/src/worker_process/privileges.rs b/src/worker_process/privileges.rs index 3f2b4308..8b9f59e7 100644 --- a/src/worker_process/privileges.rs +++ b/src/worker_process/privileges.rs @@ -3,13 +3,13 @@ //! The helper enforces that payload files are owned by the target unprivileged //! account before execing the worker binary with the downgraded identity. -use crate::error::BootstrapResult; -use crate::observability::LOG_TARGET; +use std::{path::Path, process::Command}; + use color_eyre::eyre::{Context, eyre}; -use std::path::Path; -use std::process::Command; use tracing::{info, info_span}; +use crate::{error::BootstrapResult, observability::LOG_TARGET}; + macro_rules! cfg_privilege_drop { ($($item:item)*) => { $( @@ -240,9 +240,7 @@ cfg_privilege_drop! { target_os = "dragonfly", ), )))] -const fn skip_privilege_drop_for_tests() -> bool { - false -} +const fn skip_privilege_drop_for_tests() -> bool { false } #[cfg(all( test, @@ -257,11 +255,13 @@ const fn skip_privilege_drop_for_tests() -> bool { feature = "cluster-unit-tests" ))] mod tests { - use super::*; - use crate::test_support::capture_info_logs; use std::process::Command; + use tempfile::NamedTempFile; + use super::*; + use crate::test_support::capture_info_logs; + #[test] fn skip_guard_logs_observability() { let payload = NamedTempFile::new().expect("payload file"); diff --git a/tests/bootstrap_for_tests.rs b/tests/bootstrap_for_tests.rs index d155ac91..f9cfe478 100644 --- a/tests/bootstrap_for_tests.rs +++ b/tests/bootstrap_for_tests.rs @@ -1,17 +1,22 @@ //! Behavioural coverage for the `bootstrap_for_tests` helper. #![cfg(unix)] -use std::cell::RefCell; -use std::ffi::OsStr; -use std::fs; -use std::os::unix::fs::{MetadataExt, PermissionsExt}; +use std::{ + cell::RefCell, + ffi::OsStr, + fs, + os::unix::fs::{MetadataExt, PermissionsExt}, +}; use camino::Utf8PathBuf; use color_eyre::eyre::{Context, Report, Result, ensure, eyre}; use nix::unistd::User; -use pg_embedded_setup_unpriv::test_support::worker_binary_for_tests; use pg_embedded_setup_unpriv::{ - ExecutionPrivileges, TestBootstrapSettings, bootstrap_for_tests, detect_execution_privileges, + ExecutionPrivileges, + TestBootstrapSettings, + bootstrap_for_tests, + detect_execution_privileges, + test_support::worker_binary_for_tests, }; use rstest::fixture; use rstest_bdd_macros::{given, scenario, then, when}; @@ -64,9 +69,7 @@ impl BootstrapWorld { self.skip_reason = Some(message); } - const fn is_skipped(&self) -> bool { - self.skip_reason.is_some() - } + const fn is_skipped(&self) -> bool { self.skip_reason.is_some() } fn record_settings(&mut self, settings: TestBootstrapSettings) { self.settings = Some(settings); @@ -81,13 +84,9 @@ impl BootstrapWorld { self.env_expected = None; } - fn record_restored_env(&mut self, snapshot: EnvSnapshot) { - self.env_restored = Some(snapshot); - } + fn record_restored_env(&mut self, snapshot: EnvSnapshot) { self.env_restored = Some(snapshot); } - fn record_expected_env(&mut self, snapshot: EnvSnapshot) { - self.env_expected = Some(snapshot); - } + fn record_expected_env(&mut self, snapshot: EnvSnapshot) { self.env_expected = Some(snapshot); } fn handle_outcome(&mut self, outcome: Result) -> Result<()> { match outcome { @@ -162,7 +161,8 @@ fn borrow_world(world: &BootstrapWorldFixture) -> Result<&RefCell BootstrapWorldFixture { - Ok(RefCell::new(BootstrapWorld::new()?)) + let world = BootstrapWorld::new()?; + Ok(RefCell::new(world)) } #[given("a bootstrap sandbox for tests")] diff --git a/tests/bootstrap_privileges.rs b/tests/bootstrap_privileges.rs index 37c7e820..8c30716d 100644 --- a/tests/bootstrap_privileges.rs +++ b/tests/bootstrap_privileges.rs @@ -20,7 +20,10 @@ mod serial; mod skip; use bootstrap_sandbox::{ - BootstrapSandboxFixture, borrow_sandbox, run_bootstrap_with_temp_drop, sandbox, + BootstrapSandboxFixture, + borrow_sandbox, + run_bootstrap_with_temp_drop, + sandbox, }; use scenario::expect_fixture; use serial::{ScenarioSerialGuard, serial_guard}; diff --git a/tests/bootstrap_worker_binary.rs b/tests/bootstrap_worker_binary.rs index 145543dc..42f15d71 100644 --- a/tests/bootstrap_worker_binary.rs +++ b/tests/bootstrap_worker_binary.rs @@ -1,17 +1,16 @@ //! Integration tests for the `pg_worker` binary. //! //! This module covers: -//! - Bootstrap failure paths when the worker binary is misconfigured, ensuring -//! the bootstrapper validates helper paths eagerly so privileged orchestration -//! does not defer errors to runtime. -//! - Binary invocation tests validating argument parsing, error messages, and -//! output formatting. +//! - Bootstrap failure paths when the worker binary is misconfigured, ensuring the bootstrapper +//! validates helper paths eagerly so privileged orchestration does not defer errors to runtime. +//! - Binary invocation tests validating argument parsing, error messages, and output formatting. #![cfg(unix)] -use std::ffi::{OsStr, OsString}; -use std::fs; -use std::os::unix::ffi::OsStringExt; -use std::os::unix::fs::PermissionsExt; +use std::{ + ffi::{OsStr, OsString}, + fs, + os::unix::{ffi::OsStringExt, fs::PermissionsExt}, +}; use color_eyre::eyre::{Result, ensure, eyre}; use nix::unistd::geteuid; diff --git a/tests/cluster_handle_send.rs b/tests/cluster_handle_send.rs index 94a2c826..55a6de72 100644 --- a/tests/cluster_handle_send.rs +++ b/tests/cluster_handle_send.rs @@ -4,11 +4,14 @@ //! contexts through the `ClusterHandle` type. #![cfg(unix)] -use std::sync::OnceLock; -use std::thread; +use std::{sync::OnceLock, thread}; -use pg_embedded_setup_unpriv::test_support::dummy_settings; -use pg_embedded_setup_unpriv::{ClusterGuard, ClusterHandle, ExecutionPrivileges}; +use pg_embedded_setup_unpriv::{ + ClusterGuard, + ClusterHandle, + ExecutionPrivileges, + test_support::dummy_settings, +}; use rstest::{fixture, rstest}; // ============================================================================ @@ -175,15 +178,14 @@ fn cluster_guard_is_not_send_documented() { // **Explicit caching tests exist in separate test binaries** (Cargo compiles // each `tests/*.rs` as its own binary, providing natural `OnceLock` isolation): // -// - `tests/shared_cluster_handle_success.rs`: Verifies that successful -// initialisation is cached. Calls `shared_cluster_handle()` three times -// and asserts pointer equality (`std::ptr::eq`) on returned handles. +// - `tests/shared_cluster_handle_success.rs`: Verifies that successful initialisation is cached. +// Calls `shared_cluster_handle()` three times and asserts pointer equality (`std::ptr::eq`) on +// returned handles. // -// - `tests/shared_cluster_handle_failure.rs`: Verifies that failed -// initialisation is cached. Injects failure by setting `TZDIR` to a -// non-existent path, then calls `shared_cluster_handle()` three times -// and asserts that returned errors have identical `BootstrapErrorKind` -// and contain "previously failed" in the message. +// - `tests/shared_cluster_handle_failure.rs`: Verifies that failed initialisation is cached. +// Injects failure by setting `TZDIR` to a non-existent path, then calls `shared_cluster_handle()` +// three times and asserts that returned errors have identical `BootstrapErrorKind` and contain +// "previously failed" in the message. // // This file focuses on compile-time trait verification and thread-safety // patterns that don't require `OnceLock` state manipulation. diff --git a/tests/cluster_split_constructors.rs b/tests/cluster_split_constructors.rs index 32ed181e..9d1b93ce 100644 --- a/tests/cluster_split_constructors.rs +++ b/tests/cluster_split_constructors.rs @@ -255,6 +255,7 @@ async fn wait_for_postmaster_shutdown_async( data_dir: &Utf8PathBuf, ) -> std::result::Result<(), color_eyre::Report> { use std::time::Instant; + use tokio::time::sleep; let pid = data_dir.join("postmaster.pid"); diff --git a/tests/data_dir_recovery.rs b/tests/data_dir_recovery.rs index 04f82d74..eed2781f 100644 --- a/tests/data_dir_recovery.rs +++ b/tests/data_dir_recovery.rs @@ -12,16 +12,15 @@ //! subprocess needs permission to modify the data directory). #![cfg(all(unix, feature = "cluster-unit-tests", feature = "privileged-tests"))] -use std::fs; -use std::time::Duration; +use std::{fs, time::Duration}; use camino::Utf8PathBuf; use color_eyre::eyre::{Context, Result, ensure, eyre}; use nix::unistd::{Gid, Uid, User, chown, geteuid}; -use pg_embedded_setup_unpriv::bootstrap_for_tests; -use pg_embedded_setup_unpriv::test_support::worker_binary_for_tests; -use pg_embedded_setup_unpriv::worker_process_test_api::{ - WorkerOperation, WorkerRequest, WorkerRequestArgs, +use pg_embedded_setup_unpriv::{ + bootstrap_for_tests, + test_support::worker_binary_for_tests, + worker_process_test_api::{WorkerOperation, WorkerRequest, WorkerRequestArgs}, }; use rstest::rstest; diff --git a/tests/database_lifecycle.rs b/tests/database_lifecycle.rs index 5ce8ba34..930886d3 100644 --- a/tests/database_lifecycle.rs +++ b/tests/database_lifecycle.rs @@ -1,8 +1,7 @@ //! Behavioural coverage for database lifecycle methods on `TestClusterConnection`. #![cfg(unix)] -use std::cell::RefCell; -use std::sync::atomic::Ordering; +use std::{cell::RefCell, sync::atomic::Ordering}; use color_eyre::eyre::{Context, Result, ensure}; use postgres::NoTls; @@ -27,8 +26,15 @@ mod serial; mod skip; use database_lifecycle_helpers::{ - DatabaseWorld, DatabaseWorldFixture, SETUP_CALL_COUNT, borrow_world, check_db_exists, - check_db_exists_via_delegation, execute_db_op, setup_sandboxed_cluster, verify_error, + DatabaseWorld, + DatabaseWorldFixture, + SETUP_CALL_COUNT, + borrow_world, + check_db_exists, + check_db_exists_via_delegation, + execute_db_op, + setup_sandboxed_cluster, + verify_error, }; use scenario::expect_fixture; use serial::{ScenarioSerialGuard, serial_guard}; @@ -40,7 +46,8 @@ const CLONED_DB_NAME: &str = "cloned_from_template_db"; #[fixture] fn world() -> DatabaseWorldFixture { - Ok(RefCell::new(DatabaseWorld::new()?)) + let world = DatabaseWorld::new()?; + Ok(RefCell::new(world)) } #[given("a sandboxed TestCluster is running")] @@ -244,8 +251,8 @@ fn when_template_created_and_populated(world: &DatabaseWorldFixture) -> Result<( postgres::Client::connect(&url, NoTls).context("connect to template database")?; client .batch_execute( - "CREATE TABLE test_table (id SERIAL PRIMARY KEY, value TEXT); \ - INSERT INTO test_table (value) VALUES ('template_data');", + "CREATE TABLE test_table (id SERIAL PRIMARY KEY, value TEXT); INSERT INTO test_table \ + (value) VALUES ('template_data');", ) .context("create test table and insert data")?; Ok(()) diff --git a/tests/e2e_postgresql_embedded_diesel.rs b/tests/e2e_postgresql_embedded_diesel.rs index 02a97277..76ee1bf7 100644 --- a/tests/e2e_postgresql_embedded_diesel.rs +++ b/tests/e2e_postgresql_embedded_diesel.rs @@ -3,17 +3,17 @@ //! downgrading to the `nobody` user for database operations. #![cfg(all(unix, feature = "privileged-tests"))] -use std::io::Write; -use std::time::Duration; +use std::{io::Write, time::Duration}; use camino::Utf8PathBuf; use cap_std::fs::{OpenOptions, PermissionsExt}; use color_eyre::eyre::{Context, Result, ensure, eyre}; -use diesel::prelude::*; -use diesel::sql_types::{Int4, Text}; +use diesel::{ + prelude::*, + sql_types::{Int4, Text}, +}; use nix::unistd::{User, fchown, geteuid}; -use pg_embedded_setup_unpriv::PgEnvCfg; -use pg_embedded_setup_unpriv::worker_process_test_api::WorkerOperation; +use pg_embedded_setup_unpriv::{PgEnvCfg, worker_process_test_api::WorkerOperation}; use postgresql_embedded::PostgreSQL; use tokio::runtime::{Builder, Runtime}; @@ -26,7 +26,11 @@ mod env; use cap_fs::{ensure_dir, open_dir, remove_tree}; use diesel_e2e_helpers::{ - PostgresHandle, WorkerHandle, ensure_database_exists, run_worker_operation, worker_from_env, + PostgresHandle, + WorkerHandle, + ensure_database_exists, + run_worker_operation, + worker_from_env, }; use env::{ScopedEnvVars, build_env, with_scoped_env}; @@ -85,25 +89,15 @@ impl TestConfig { } } - const fn base_dir(&self) -> &Utf8PathBuf { - &self.base_dir - } + const fn base_dir(&self) -> &Utf8PathBuf { &self.base_dir } - const fn install_dir(&self) -> &Utf8PathBuf { - &self.install_dir - } + const fn install_dir(&self) -> &Utf8PathBuf { &self.install_dir } - fn cache_dir(&self) -> Utf8PathBuf { - self.install_dir.join("cache") - } + fn cache_dir(&self) -> Utf8PathBuf { self.install_dir.join("cache") } - fn runtime_dir(&self) -> Utf8PathBuf { - self.install_dir.join("run") - } + fn runtime_dir(&self) -> Utf8PathBuf { self.install_dir.join("run") } - fn password_file(&self) -> Utf8PathBuf { - self.install_dir.join(".pgpass") - } + fn password_file(&self) -> Utf8PathBuf { self.install_dir.join(".pgpass") } fn bootstrap_env(&self) -> ScopedEnvVars { let port = self.port.to_string(); diff --git a/tests/observability.rs b/tests/observability.rs index 8437c206..0bcdf860 100644 --- a/tests/observability.rs +++ b/tests/observability.rs @@ -1,19 +1,19 @@ //! Behavioural coverage for observability instrumentation. #![cfg(unix)] -use std::cell::RefCell; -use std::ffi::OsString; -use std::fs; -use std::os::unix::fs::PermissionsExt; +use std::{cell::RefCell, ffi::OsString, fs, os::unix::fs::PermissionsExt}; +use camino::{Utf8Path, Utf8PathBuf}; use color_eyre::eyre::{Context, Report, Result, ensure, eyre}; -use pg_embedded_setup_unpriv::test_support::capture_info_logs_with_spans; -use pg_embedded_setup_unpriv::{BootstrapResult, TestCluster, WorkerOperation}; +use pg_embedded_setup_unpriv::{ + BootstrapResult, + TestCluster, + WorkerOperation, + test_support::capture_info_logs_with_spans, +}; use rstest::fixture; use rstest_bdd_macros::{given, scenario, then, when}; -use camino::{Utf8Path, Utf8PathBuf}; - #[path = "support/cap_fs_bootstrap.rs"] mod cap_fs; #[path = "support/cluster_skip.rs"] @@ -75,7 +75,8 @@ impl ObservabilityWorld { } else if coverage_mode() && message.contains("postgresql_embedded::setup() failed") { self.skip_reason = Some( - "skipping observability success scenario under coverage: embedded postgres setup failed" + "skipping observability success scenario under coverage: embedded \ + postgres setup failed" .to_owned(), ); } else if let Some(reason) = cluster_skip_message(&message, Some(&debug)) { @@ -97,7 +98,8 @@ fn borrow_world(world: &WorldFixture) -> Result<&RefCell> { #[fixture] fn world() -> WorldFixture { - Ok(RefCell::new(ObservabilityWorld::new()?)) + let world = ObservabilityWorld::new()?; + Ok(RefCell::new(world)) } #[given("an observability sandbox")] @@ -296,9 +298,7 @@ fn permission_denied_in_chain(report: &Report) -> bool { }) } -fn coverage_mode() -> bool { - std::env::var("CARGO_LLVM_COV").is_ok() -} +fn coverage_mode() -> bool { std::env::var("CARGO_LLVM_COV").is_ok() } fn ensure_runtime_dir_matches( env_vars: &[(OsString, Option)], diff --git a/tests/recovery_integration.rs b/tests/recovery_integration.rs index cdc9badb..3d26ca3f 100644 --- a/tests/recovery_integration.rs +++ b/tests/recovery_integration.rs @@ -4,8 +4,7 @@ //! from partial data directories (missing `global/pg_filenode.map`). #![cfg(unix)] -use std::fs; -use std::path::Path; +use std::{fs, path::Path}; use color_eyre::eyre::{Result, ensure, eyre}; use pg_embedded_setup_unpriv::test_support::create_partial_data_dir; @@ -81,8 +80,8 @@ fn stderr_indicates_missing_postgres(stderr: &str) -> bool { /// This test creates a partial data directory with `PG_VERSION` but without /// the marker file, runs `pg_worker` with setup operation, and verifies: /// 1. The partial data directory is removed by recovery -/// 2. If the binary fails, it's due to missing `PostgreSQL` installation -/// (not due to recovery failure or other unexpected reasons) +/// 2. If the binary fails, it's due to missing `PostgreSQL` installation (not due to recovery +/// failure or other unexpected reasons) /// /// Note: The setup may succeed if `PostgreSQL` binaries are cached, or fail /// if no real installation is available. Either outcome is acceptable as diff --git a/tests/settings.rs b/tests/settings.rs index 76bb7aa9..a786d436 100644 --- a/tests/settings.rs +++ b/tests/settings.rs @@ -1,9 +1,9 @@ //! Validates translating environment settings into `PostgreSQL` configuration. -use camino::Utf8PathBuf; -use color_eyre::eyre::{ensure, eyre}; use std::path::Path; +use camino::Utf8PathBuf; +use color_eyre::eyre::{ensure, eyre}; use nix::unistd::geteuid; #[cfg(feature = "privileged-tests")] use pg_embedded_setup_unpriv::Error as PgEmbeddedError; @@ -35,7 +35,7 @@ fn invoke_deprecated_with_temp_euid() -> pg_embedded_setup_unpriv::Result<()> { /// /// # Examples /// ```no_run -/// to_settings_roundtrip()?; +/// to_settings_roundtrip()?; /// ``` #[rstest] fn to_settings_roundtrip() -> color_eyre::Result<()> { @@ -99,7 +99,9 @@ fn to_settings_default_config() -> color_eyre::Result<()> { #[fixture] fn default_pg_env() -> PgEnvCfg { - PgEnvCfg::default() + PgEnvCfg { + ..PgEnvCfg::default() + } } #[rstest] @@ -175,7 +177,8 @@ fn with_temp_euid_changes_uid() -> color_eyre::Result<()> { /// Stub variant ensuring the suite reports skipped when privilege drops are unavailable. fn with_temp_euid_changes_uid() -> color_eyre::Result<()> { tracing::warn!( - "skipping root-dependent test: enable the privileged-tests feature to exercise privilege drops", + "skipping root-dependent test: enable the privileged-tests feature to exercise privilege \ + drops", ); Ok(()) } @@ -186,13 +189,13 @@ mod cap_fs; #[cfg(all(unix, feature = "cluster-unit-tests"))] mod dir_accessible_tests { - use super::*; - use cap_std::fs::{MetadataExt, PermissionsExt}; - use cap_fs::{CapabilityTempDir, metadata}; + use cap_std::fs::{MetadataExt, PermissionsExt}; use color_eyre::eyre::{Context, ensure}; use nix::unistd::User; + use super::*; + #[rstest] fn make_dir_accessible_allows_nobody() -> color_eyre::Result<()> { if !geteuid().is_root() { diff --git a/tests/settings_logging.rs b/tests/settings_logging.rs index 75551e5f..bbed5386 100644 --- a/tests/settings_logging.rs +++ b/tests/settings_logging.rs @@ -1,14 +1,15 @@ //! Behavioural coverage for settings observability and redaction. #![cfg(all(unix, any(feature = "cluster-unit-tests", feature = "dev-worker")))] -use std::cell::RefCell; -use std::ffi::OsString; -use std::fs; +use std::{cell::RefCell, ffi::OsString, fs}; use camino::Utf8PathBuf; use color_eyre::eyre::{Context, Report, Result, ensure, eyre}; -use pg_embedded_setup_unpriv::test_support::capture_debug_logs; -use pg_embedded_setup_unpriv::{BootstrapResult, bootstrap_for_tests}; +use pg_embedded_setup_unpriv::{ + BootstrapResult, + bootstrap_for_tests, + test_support::capture_debug_logs, +}; use rstest::fixture; use rstest_bdd_macros::{given, scenario, then, when}; @@ -96,7 +97,8 @@ fn borrow_world(world: &WorldFixture) -> Result<&RefCell> #[fixture] fn world() -> WorldFixture { - Ok(RefCell::new(SettingsLoggingWorld::new()?)) + let world = SettingsLoggingWorld::new()?; + Ok(RefCell::new(world)) } #[given("a settings logging sandbox")] diff --git a/tests/settings_snapshot_roundtrip.rs b/tests/settings_snapshot_roundtrip.rs index 9b6df6b6..c02754c5 100644 --- a/tests/settings_snapshot_roundtrip.rs +++ b/tests/settings_snapshot_roundtrip.rs @@ -1,6 +1,5 @@ //! Validates that worker settings snapshots faithfully round-trip `PostgreSQL` settings. -use std::collections::HashMap; -use std::time::Duration; +use std::{collections::HashMap, time::Duration}; use color_eyre::eyre::{Result, WrapErr, ensure}; use pg_embedded_setup_unpriv::worker::{SettingsSnapshot, WorkerPayload}; diff --git a/tests/shared_cluster_handle_failure.rs b/tests/shared_cluster_handle_failure.rs index 462bde78..e5b1524e 100644 --- a/tests/shared_cluster_handle_failure.rs +++ b/tests/shared_cluster_handle_failure.rs @@ -9,8 +9,12 @@ //! verification. #![cfg(unix)] -use pg_embedded_setup_unpriv::test_support::shared_cluster_handle; -use pg_embedded_setup_unpriv::{BootstrapError, BootstrapErrorKind, ClusterHandle}; +use pg_embedded_setup_unpriv::{ + BootstrapError, + BootstrapErrorKind, + ClusterHandle, + test_support::shared_cluster_handle, +}; use tracing::warn; #[expect(dead_code, reason = "required by env_isolation module")] diff --git a/tests/shared_cluster_handle_success.rs b/tests/shared_cluster_handle_success.rs index d47d858b..72a07d83 100644 --- a/tests/shared_cluster_handle_success.rs +++ b/tests/shared_cluster_handle_success.rs @@ -11,8 +11,10 @@ mod cluster_skip; mod skip; use cluster_skip::cluster_skip_message; -use pg_embedded_setup_unpriv::BootstrapError; -use pg_embedded_setup_unpriv::test_support::{scoped_env, shared_cluster_handle}; +use pg_embedded_setup_unpriv::{ + BootstrapError, + test_support::{scoped_env, shared_cluster_handle}, +}; use tempfile::tempdir; use tracing::warn; diff --git a/tests/shutdown_hook_lifecycle.rs b/tests/shutdown_hook_lifecycle.rs index b8fa570c..8f8e05d6 100644 --- a/tests/shutdown_hook_lifecycle.rs +++ b/tests/shutdown_hook_lifecycle.rs @@ -13,9 +13,7 @@ mod cluster_skip; #[path = "support/skip.rs"] mod skip; -use std::path::Path; -use std::time::Duration; -use std::{env, fs, thread}; +use std::{env, fs, path::Path, thread, time::Duration}; use cluster_skip::cluster_skip_message; use color_eyre::eyre::{Context, Result, eyre}; diff --git a/tests/shutdown_timeout.rs b/tests/shutdown_timeout.rs index 6d691f85..fe9dfee4 100644 --- a/tests/shutdown_timeout.rs +++ b/tests/shutdown_timeout.rs @@ -1,8 +1,7 @@ //! Validates configuration of the shutdown timeout environment variable. #![cfg(unix)] -use std::ffi::OsString; -use std::time::Duration; +use std::{ffi::OsString, time::Duration}; use color_eyre::eyre::{Result, ensure, eyre}; use pg_embedded_setup_unpriv::bootstrap_for_tests; diff --git a/tests/support/bootstrap_sandbox.rs b/tests/support/bootstrap_sandbox.rs index dc9e4bfa..d778d3fa 100644 --- a/tests/support/bootstrap_sandbox.rs +++ b/tests/support/bootstrap_sandbox.rs @@ -1,22 +1,24 @@ //! Sandbox environment for bootstrap privilege tests. -use std::cell::RefCell; -use std::ffi::OsString; -use std::io::ErrorKind; +use std::{cell::RefCell, ffi::OsString, io::ErrorKind}; use camino::{Utf8Path, Utf8PathBuf}; use cap_std::fs::MetadataExt; use color_eyre::eyre::{Context, Result, ensure, eyre}; use nix::unistd::Uid; -use pg_embedded_setup_unpriv::ExecutionPrivileges; #[cfg(feature = "privileged-tests")] use pg_embedded_setup_unpriv::nobody_uid; -use pg_embedded_setup_unpriv::test_support::{CapabilityTempDir, metadata}; +use pg_embedded_setup_unpriv::{ + ExecutionPrivileges, + test_support::{CapabilityTempDir, metadata}, +}; use rstest::fixture; -use super::cap_fs_bootstrap::{remove_tree, set_permissions}; -use super::env::{build_env, with_scoped_env}; -use super::skip::skip_message; +use super::{ + cap_fs_bootstrap::{remove_tree, set_permissions}, + env::{build_env, with_scoped_env}, + skip::skip_message, +}; /// Test sandbox for bootstrap privilege scenarios. #[derive(Debug)] @@ -107,9 +109,7 @@ impl BootstrapSandbox { Ok(()) } - fn remove_if_present(path: &Utf8Path) -> Result<()> { - remove_tree(path) - } + fn remove_if_present(path: &Utf8Path) -> Result<()> { remove_tree(path) } /// Records the detected execution privileges. pub const fn record_privileges(&mut self, privileges: ExecutionPrivileges) { @@ -117,9 +117,7 @@ impl BootstrapSandbox { } /// Sets the expected owner UID for directory ownership checks. - pub const fn set_expected_owner(&mut self, uid: Uid) { - self.expected_owner = Some(uid); - } + pub const fn set_expected_owner(&mut self, uid: Uid) { self.expected_owner = Some(uid); } /// Records an error message from a failed bootstrap attempt. pub fn record_error(&mut self, error: impl Into) { @@ -134,14 +132,10 @@ impl BootstrapSandbox { } /// Returns whether this scenario has been marked as skipped. - pub const fn is_skipped(&self) -> bool { - self.skip_reason.is_some() - } + pub const fn is_skipped(&self) -> bool { self.skip_reason.is_some() } /// Returns the last recorded error message. - pub fn last_error(&self) -> Option<&str> { - self.last_error.as_deref() - } + pub fn last_error(&self) -> Option<&str> { self.last_error.as_deref() } /// Asserts that the detected privileges match the expected value. pub fn assert_detected(&self, expected: ExecutionPrivileges) -> Result<()> { @@ -256,5 +250,6 @@ pub fn borrow_sandbox(sandbox: &BootstrapSandboxFixture) -> Result<&RefCell BootstrapSandboxFixture { - Ok(RefCell::new(BootstrapSandbox::new()?)) + let sandbox = BootstrapSandbox::new()?; + Ok(RefCell::new(sandbox)) } diff --git a/tests/support/cap_fs_bootstrap.rs b/tests/support/cap_fs_bootstrap.rs index 56e38181..14032212 100644 --- a/tests/support/cap_fs_bootstrap.rs +++ b/tests/support/cap_fs_bootstrap.rs @@ -2,7 +2,6 @@ use camino::Utf8Path; use color_eyre::eyre::{Context, Result}; - use pg_embedded_setup_unpriv::test_support::set_permissions as shared_set_permissions; #[expect( unused_imports, diff --git a/tests/support/cap_fs_privileged.rs b/tests/support/cap_fs_privileged.rs index cb3f8247..57608bb0 100644 --- a/tests/support/cap_fs_privileged.rs +++ b/tests/support/cap_fs_privileged.rs @@ -3,10 +3,10 @@ use camino::Utf8Path; use cap_std::{ambient_authority, fs::Dir}; use color_eyre::eyre::{Context, Result}; - pub use pg_embedded_setup_unpriv::test_support::ambient_dir_and_path; use pg_embedded_setup_unpriv::test_support::{ - ensure_dir_exists as shared_ensure_dir_exists, set_permissions as shared_set_permissions, + ensure_dir_exists as shared_ensure_dir_exists, + set_permissions as shared_set_permissions, }; /// Splits an absolute or relative path into a capability directory and the relative path. @@ -54,9 +54,10 @@ fn is_not_found(err: &color_eyre::Report) -> bool { mod tests { //! Unit tests for capability-based filesystem helpers. - use super::*; use camino::Utf8PathBuf; + use super::*; + fn temp_utf8_dir() -> Utf8PathBuf { let temp = std::env::temp_dir(); Utf8PathBuf::from_path_buf(temp).expect("temp dir should be valid UTF-8") diff --git a/tests/support/database_lifecycle_helpers.rs b/tests/support/database_lifecycle_helpers.rs index 04bffad9..1cd77616 100644 --- a/tests/support/database_lifecycle_helpers.rs +++ b/tests/support/database_lifecycle_helpers.rs @@ -1,16 +1,16 @@ //! Helper functions for database lifecycle behavioural tests. -use std::cell::RefCell; -use std::ffi::{OsStr, OsString}; -use std::sync::atomic::{AtomicUsize, Ordering}; -use std::time::Duration; +use std::{ + cell::RefCell, + ffi::{OsStr, OsString}, + sync::atomic::{AtomicUsize, Ordering}, + time::Duration, +}; use color_eyre::eyre::{Context, Result, ensure, eyre}; use pg_embedded_setup_unpriv::{TemporaryDatabase, TestCluster, find_timezone_dir}; -use super::cluster_skip::cluster_skip_message; -use super::env::ScopedEnvVars; -use super::sandbox::TestSandbox; +use super::{cluster_skip::cluster_skip_message, env::ScopedEnvVars, sandbox::TestSandbox}; const BOOTSTRAP_RETRY_ATTEMPTS: usize = 3; const BOOTSTRAP_RETRY_DELAY: Duration = Duration::from_millis(250); @@ -84,9 +84,7 @@ impl DatabaseWorld { /// Returns whether the scenario is skipped. #[must_use] - pub const fn is_skipped(&self) -> bool { - self.skip_reason.is_some() - } + pub const fn is_skipped(&self) -> bool { self.skip_reason.is_some() } /// Returns an error if the scenario is skipped. /// diff --git a/tests/support/diesel_e2e_helpers.rs b/tests/support/diesel_e2e_helpers.rs index c2d34d8e..bf7a3f66 100644 --- a/tests/support/diesel_e2e_helpers.rs +++ b/tests/support/diesel_e2e_helpers.rs @@ -1,14 +1,18 @@ //! Helpers for the Diesel-based `PostgreSQL` embedded e2e test. -use std::path::PathBuf; -use std::time::Duration; +use std::{path::PathBuf, time::Duration}; use camino::Utf8PathBuf; use color_eyre::eyre::{Context, Result, eyre}; -use diesel::prelude::*; -use diesel::sql_types::{Bool, Text}; +use diesel::{ + prelude::*, + sql_types::{Bool, Text}, +}; use pg_embedded_setup_unpriv::worker_process_test_api::{ - WorkerOperation, WorkerRequest, WorkerRequestArgs, run as run_worker, + WorkerOperation, + WorkerRequest, + WorkerRequestArgs, + run as run_worker, }; use postgresql_embedded::PostgreSQL; diff --git a/tests/support/env.rs b/tests/support/env.rs index 17bbbe6b..b1898318 100644 --- a/tests/support/env.rs +++ b/tests/support/env.rs @@ -2,8 +2,7 @@ use std::ffi::{OsStr, OsString}; -use pg_embedded_setup_unpriv::ScopedEnv; -use pg_embedded_setup_unpriv::test_support; +use pg_embedded_setup_unpriv::{ScopedEnv, test_support}; /// Collection type for guarded environment variables. pub type ScopedEnvVars = Vec<(OsString, Option)>; @@ -43,9 +42,7 @@ where /// nested scopes on the same thread share the mutex whilst recording the outer /// state. Scopes on different threads still serialise to avoid interleaving /// process-level environment mutations. -pub fn apply_env(vars: ScopedEnvVars) -> ScopedEnvGuard { - test_support::scoped_env(vars) -} +pub fn apply_env(vars: ScopedEnvVars) -> ScopedEnvGuard { test_support::scoped_env(vars) } /// Runs `body` with the provided environment variables temporarily set. /// @@ -54,7 +51,6 @@ pub fn apply_env(vars: ScopedEnvVars) -> ScopedEnvGuard { /// access so concurrent tests cannot interleave environment mutations. Calls on the /// same thread are re-entrant, enabling helpers to compose without risking /// deadlocks. -/// pub fn with_scoped_env( vars: impl IntoIterator)>, body: impl FnOnce() -> R, diff --git a/tests/support/env_isolation.rs b/tests/support/env_isolation.rs index cf445dbd..ae4a594f 100644 --- a/tests/support/env_isolation.rs +++ b/tests/support/env_isolation.rs @@ -1,7 +1,9 @@ //! Environment helpers for isolating test scenarios. -use std::collections::HashSet; -use std::ffi::{OsStr, OsString}; +use std::{ + collections::HashSet, + ffi::{OsStr, OsString}, +}; use camino::Utf8Path; diff --git a/tests/support/pg_worker_hang.rs b/tests/support/pg_worker_hang.rs index c9d373be..55fdce39 100644 --- a/tests/support/pg_worker_hang.rs +++ b/tests/support/pg_worker_hang.rs @@ -1,11 +1,7 @@ //! Helper binary that deliberately stalls to exercise worker timeouts. #![cfg(unix)] -use std::env; -use std::fs; -use std::path::PathBuf; -use std::thread; -use std::time::Duration; +use std::{env, fs, path::PathBuf, thread, time::Duration}; use color_eyre::eyre::{Context, Report, Result}; use pg_embedded_setup_unpriv::worker::WorkerPayload; diff --git a/tests/support/pg_worker_helpers.rs b/tests/support/pg_worker_helpers.rs index 2cfb27fa..b6f99e34 100644 --- a/tests/support/pg_worker_helpers.rs +++ b/tests/support/pg_worker_helpers.rs @@ -8,9 +8,7 @@ use color_eyre::eyre::Result; /// /// Returns `None` when `CARGO_BIN_EXE_pg_worker` is not set, which can occur /// when running tests without building the binary target. -pub const fn pg_worker_binary() -> Option<&'static str> { - option_env!("CARGO_BIN_EXE_pg_worker") -} +pub const fn pg_worker_binary() -> Option<&'static str> { option_env!("CARGO_BIN_EXE_pg_worker") } /// Runs the `pg_worker` binary with the given arguments. /// diff --git a/tests/support/sandbox.rs b/tests/support/sandbox.rs index 60853ffa..e711f550 100644 --- a/tests/support/sandbox.rs +++ b/tests/support/sandbox.rs @@ -4,12 +4,16 @@ use std::ffi::OsString; use camino::{Utf8Path, Utf8PathBuf}; use color_eyre::eyre::{Context, Result, eyre}; +use pg_embedded_setup_unpriv::{ + ExecutionPrivileges, + detect_execution_privileges, + test_support::CapabilityTempDir, +}; -use pg_embedded_setup_unpriv::test_support::CapabilityTempDir; -use pg_embedded_setup_unpriv::{ExecutionPrivileges, detect_execution_privileges}; - -use super::cap_fs::{remove_tree, set_permissions}; -use super::env::{ScopedEnvVars, build_env, with_scoped_env}; +use super::{ + cap_fs::{remove_tree, set_permissions}, + env::{ScopedEnvVars, build_env, with_scoped_env}, +}; /// Provides a capability-backed directory tree for behavioural `PostgreSQL` /// tests. Each sandbox supplies dedicated installation and data directories so @@ -91,9 +95,7 @@ impl TestSandbox { /// # } /// # docs().expect("install_dir example should succeed"); /// ``` - pub fn install_dir(&self) -> &Utf8Path { - &self.install_dir - } + pub fn install_dir(&self) -> &Utf8Path { &self.install_dir } /// Returns the `PostgreSQL` data directory assigned to the sandbox. /// @@ -111,9 +113,7 @@ impl TestSandbox { /// # } /// # docs().expect("data_dir example should succeed"); /// ``` - pub fn data_dir(&self) -> &Utf8Path { - &self.data_dir - } + pub fn data_dir(&self) -> &Utf8Path { &self.data_dir } /// Provides the base environment variables required for `PostgreSQL` to run /// within the sandbox. @@ -127,7 +127,9 @@ impl TestSandbox { /// # fn docs() -> Result<()> { /// let sandbox = TestSandbox::new("example-base-env")?; /// let vars = sandbox.base_env(); - /// let has_runtime = vars.iter().any(|(key, _)| key == OsStr::new("PG_RUNTIME_DIR")); + /// let has_runtime = vars + /// .iter() + /// .any(|(key, _)| key == OsStr::new("PG_RUNTIME_DIR")); /// assert!(has_runtime, "runtime directory should be present"); /// sandbox.reset()?; /// # Ok(()) @@ -155,9 +157,16 @@ impl TestSandbox { /// # fn docs() -> Result<()> { /// let sandbox = TestSandbox::new("example-without-tz")?; /// let vars = sandbox.env_without_timezone(); - /// let tz_missing = vars.iter().any(|(key, value)| key == OsStr::new("TZ") && value.is_none()); - /// let tzdir_missing = vars.iter().any(|(key, value)| key == OsStr::new("TZDIR") && value.is_none()); - /// assert!(tz_missing && tzdir_missing, "time zone variables should be cleared"); + /// let tz_missing = vars + /// .iter() + /// .any(|(key, value)| key == OsStr::new("TZ") && value.is_none()); + /// let tzdir_missing = vars + /// .iter() + /// .any(|(key, value)| key == OsStr::new("TZDIR") && value.is_none()); + /// assert!( + /// tz_missing && tzdir_missing, + /// "time zone variables should be cleared" + /// ); /// sandbox.reset()?; /// # Ok(()) /// # } @@ -278,10 +287,10 @@ fn base_dir_mode() -> u32 { mod tests { //! Tests for sandbox environment helpers. - use super::*; - use color_eyre::eyre::Result; + use super::*; + #[test] fn env_with_timezone_override_sets_tzdir() -> Result<()> { let sandbox = TestSandbox::new("sandbox-tz-override")?; diff --git a/tests/support/serial.rs b/tests/support/serial.rs index ff6ec2a3..a4794cea 100644 --- a/tests/support/serial.rs +++ b/tests/support/serial.rs @@ -6,15 +6,15 @@ //! mutex, then environment mutex). Following this order prevents deadlocks when //! multiple suites mutate process-wide state. -use rstest::fixture; -use std::sync::{Mutex, MutexGuard}; - #[cfg(unix)] use std::fs::OpenOptions; #[cfg(unix)] use std::os::unix::io::AsRawFd; #[cfg(unix)] use std::path::PathBuf; +use std::sync::{Mutex, MutexGuard}; + +use rstest::fixture; static SCENARIO_MUTEX: std::sync::LazyLock> = std::sync::LazyLock::new(|| Mutex::new(())); @@ -50,8 +50,8 @@ pub struct ScenarioLocalGuard { /// # Behaviour /// /// - Acquires the global `SCENARIO_MUTEX` and wraps the guard. -/// - If the mutex is poisoned (a previous test panicked whilst holding the lock), -/// the poison is cleared and execution continues. +/// - If the mutex is poisoned (a previous test panicked whilst holding the lock), the poison is +/// cleared and execution continues. /// - The guard is automatically released when dropped at the end of the test. /// /// # Examples @@ -85,8 +85,8 @@ pub fn serial_guard() -> ScenarioSerialGuard { /// # Behaviour /// /// - Acquires the global `SCENARIO_MUTEX` and wraps the guard. -/// - If the mutex is poisoned (a previous test panicked whilst holding the lock), -/// the poison is cleared and execution continues. +/// - If the mutex is poisoned (a previous test panicked whilst holding the lock), the poison is +/// cleared and execution continues. /// - The guard is automatically released when dropped at the end of the test. /// /// # Examples @@ -153,9 +153,7 @@ fn acquire_process_lock() -> ProcessLock { } #[cfg(not(unix))] -fn acquire_process_lock() -> ProcessLock { - () -} +fn acquire_process_lock() -> ProcessLock { () } #[cfg(test)] mod tests { @@ -185,8 +183,7 @@ mod tests { fn acquire_process_lock_places_lock_file_in_cargo_target_dir( serial_guard: ScenarioSerialGuard, ) { - use std::ffi::OsString; - use std::{env, fs}; + use std::{env, ffi::OsString, fs}; use pg_embedded_setup_unpriv::test_support::scoped_env; @@ -212,7 +209,8 @@ mod tests { .collect(); assert!( !entries.is_empty(), - "expected acquire_process_lock to create a lock file in {tmp_dir:?}, but directory was empty" + "expected acquire_process_lock to create a lock file in {tmp_dir:?}, but directory \ + was empty" ); // Best-effort cleanup; errors are non-fatal in test teardown. diff --git a/tests/test_cluster.rs b/tests/test_cluster.rs index ad4682cb..edee8f7f 100644 --- a/tests/test_cluster.rs +++ b/tests/test_cluster.rs @@ -1,7 +1,10 @@ //! Unit tests covering `TestCluster` privilege dispatch behaviour. #![cfg(unix)] -use serial_test::serial; +#[cfg(feature = "privileged-tests")] +use std::fs; +#[cfg(feature = "privileged-tests")] +use std::os::unix::fs::PermissionsExt; use std::sync::{ Arc, atomic::{AtomicBool, AtomicUsize, Ordering}, @@ -9,20 +12,25 @@ use std::sync::{ #[cfg(feature = "privileged-tests")] use std::time::Duration; -#[cfg(not(feature = "cluster-unit-tests"))] -use crate as cluster_crate; #[cfg(feature = "privileged-tests")] use camino::Utf8Path; #[cfg(feature = "privileged-tests")] use camino::Utf8PathBuf; -use cluster_crate::BootstrapError; #[cfg(feature = "privileged-tests")] use cluster_crate::test_support::capture_warn_logs; -use cluster_crate::test_support::{ - RunRootOperationHookInstallError, drain_hook_install_logs, dummy_settings, - install_run_root_operation_hook, test_runtime, +use cluster_crate::{ + BootstrapError, + ExecutionPrivileges, + WorkerInvoker, + WorkerOperation, + test_support::{ + RunRootOperationHookInstallError, + drain_hook_install_logs, + dummy_settings, + install_run_root_operation_hook, + test_runtime, + }, }; -use cluster_crate::{ExecutionPrivileges, WorkerInvoker, WorkerOperation}; #[cfg(feature = "privileged-tests")] use color_eyre::eyre::Context; use color_eyre::eyre::{Result, ensure, eyre}; @@ -30,13 +38,13 @@ use color_eyre::eyre::{Result, ensure, eyre}; use nix::unistd::geteuid; #[cfg(feature = "cluster-unit-tests")] use pg_embedded_setup_unpriv as cluster_crate; -#[cfg(feature = "privileged-tests")] -use std::fs; -#[cfg(feature = "privileged-tests")] -use std::os::unix::fs::PermissionsExt; +use serial_test::serial; #[cfg(feature = "privileged-tests")] use tempfile::tempdir; +#[cfg(not(feature = "cluster-unit-tests"))] +use crate as cluster_crate; + #[test] fn unprivileged_operations_run_in_process() -> Result<()> { let runtime = test_runtime()?; diff --git a/tests/test_cluster_async.rs b/tests/test_cluster_async.rs index ce7cf115..c3a22bff 100644 --- a/tests/test_cluster_async.rs +++ b/tests/test_cluster_async.rs @@ -10,8 +10,7 @@ #![cfg(all(unix, feature = "async-api"))] use color_eyre::eyre::{Result, ensure}; -use pg_embedded_setup_unpriv::test_support::scoped_env; -use pg_embedded_setup_unpriv::{ScopedEnv, TestCluster}; +use pg_embedded_setup_unpriv::{ScopedEnv, TestCluster, test_support::scoped_env}; use rstest::rstest; #[path = "support/cap_fs_bootstrap.rs"] diff --git a/tests/test_cluster_behaviour.rs b/tests/test_cluster_behaviour.rs index 79cc21bb..27ed71d1 100644 --- a/tests/test_cluster_behaviour.rs +++ b/tests/test_cluster_behaviour.rs @@ -59,9 +59,7 @@ impl ClusterWorld { self.skip_reason = Some(message); } - const fn is_skipped(&self) -> bool { - self.skip_reason.is_some() - } + const fn is_skipped(&self) -> bool { self.skip_reason.is_some() } fn ensure_not_skipped(&self) -> Result<()> { if self.is_skipped() { @@ -137,9 +135,7 @@ impl ClusterWorld { } impl Drop for ClusterWorld { - fn drop(&mut self) { - drop(self.cluster.take()); - } + fn drop(&mut self) { drop(self.cluster.take()); } } type ClusterWorldFixture = Result>; @@ -152,7 +148,8 @@ fn borrow_world(world: &ClusterWorldFixture) -> Result<&RefCell> { #[fixture] fn world() -> ClusterWorldFixture { - Ok(RefCell::new(ClusterWorld::new()?)) + let world = ClusterWorld::new()?; + Ok(RefCell::new(world)) } #[given("a cluster sandbox for tests")] diff --git a/tests/test_cluster_connection.rs b/tests/test_cluster_connection.rs index f44a49aa..af321dd4 100644 --- a/tests/test_cluster_connection.rs +++ b/tests/test_cluster_connection.rs @@ -1,15 +1,16 @@ //! Behavioural coverage for the connection helpers exposed by `TestCluster`. #![cfg(unix)] -use std::cell::RefCell; +use std::{ + cell::RefCell, + ffi::{OsStr, OsString}, +}; use color_eyre::eyre::{Context, Result, ensure, eyre}; -use diesel::prelude::*; -use diesel::sql_types::Integer; +use diesel::{prelude::*, sql_types::Integer}; use pg_embedded_setup_unpriv::{ConnectionMetadata, TestCluster}; use rstest::fixture; use rstest_bdd_macros::{given, scenario, then, when}; -use std::ffi::{OsStr, OsString}; #[path = "support/cap_fs_bootstrap.rs"] mod cap_fs; @@ -67,9 +68,7 @@ impl ConnectionWorld { self.skip_reason = Some(message); } - const fn is_skipped(&self) -> bool { - self.skip_reason.is_some() - } + const fn is_skipped(&self) -> bool { self.skip_reason.is_some() } fn ensure_not_skipped(&self) -> Result<()> { if self.is_skipped() { @@ -101,13 +100,9 @@ impl ConnectionWorld { } } - fn record_metadata(&mut self, metadata: ConnectionMetadata) { - self.metadata = Some(metadata); - } + fn record_metadata(&mut self, metadata: ConnectionMetadata) { self.metadata = Some(metadata); } - const fn record_selected_value(&mut self, value: i32) { - self.selected_value = Some(value); - } + const fn record_selected_value(&mut self, value: i32) { self.selected_value = Some(value); } fn record_query_error(&mut self, err: impl Into) { self.query_error = Some(err.into()); @@ -149,9 +144,7 @@ impl ConnectionWorld { } impl Drop for ConnectionWorld { - fn drop(&mut self) { - drop(self.cluster.take()); - } + fn drop(&mut self) { drop(self.cluster.take()); } } type ConnectionWorldFixture = Result>; @@ -164,7 +157,8 @@ fn borrow_world(world: &ConnectionWorldFixture) -> Result<&RefCell ConnectionWorldFixture { - Ok(RefCell::new(ConnectionWorld::new()?)) + let world = ConnectionWorld::new()?; + Ok(RefCell::new(world)) } #[given("a sandboxed TestCluster is running")] diff --git a/tests/test_cluster_drop.rs b/tests/test_cluster_drop.rs index 6e4523b2..9854a246 100644 --- a/tests/test_cluster_drop.rs +++ b/tests/test_cluster_drop.rs @@ -1,12 +1,15 @@ //! Unit coverage for the `TestCluster` RAII guard. #![cfg(unix)] +use std::{thread, time::Duration}; + use camino::Utf8PathBuf; -use color_eyre::Report; -use color_eyre::eyre::{Context, Result, ensure, eyre}; +use color_eyre::{ + Report, + eyre::{Context, Result, ensure, eyre}, +}; use pg_embedded_setup_unpriv::{CleanupMode, TestCluster}; use rstest::rstest; -use std::{thread, time::Duration}; #[path = "support/cap_fs_bootstrap.rs"] mod cap_fs; diff --git a/tests/test_cluster_fixture/process_utils.rs b/tests/test_cluster_fixture/process_utils.rs index 4a47d269..a56fddc7 100644 --- a/tests/test_cluster_fixture/process_utils.rs +++ b/tests/test_cluster_fixture/process_utils.rs @@ -1,8 +1,10 @@ //! Helpers for fixture process management in cluster tests. -use std::path::{Path, PathBuf}; -use std::thread; -use std::time::Duration; +use std::{ + path::{Path, PathBuf}, + thread, + time::Duration, +}; use camino::Utf8PathBuf; use color_eyre::eyre::{Context, Result, eyre}; diff --git a/tests/test_cluster_fixture/unit_tests.rs b/tests/test_cluster_fixture/unit_tests.rs index 29f7b0fc..59244f39 100644 --- a/tests/test_cluster_fixture/unit_tests.rs +++ b/tests/test_cluster_fixture/unit_tests.rs @@ -8,7 +8,10 @@ use super::{ TestSandbox, env_isolation::{EnvIsolationGuard, set_env_var}, process_utils::{ - read_postmaster_pid, sandbox_root_path, wait_for_pid_file_removal, wait_for_process_exit, + read_postmaster_pid, + sandbox_root_path, + wait_for_pid_file_removal, + wait_for_process_exit, }, serial::{ScenarioSerialGuard, serial_guard}, }; diff --git a/tests/test_cluster_fixture/world.rs b/tests/test_cluster_fixture/world.rs index 7dbbb17a..4306b60d 100644 --- a/tests/test_cluster_fixture/world.rs +++ b/tests/test_cluster_fixture/world.rs @@ -8,7 +8,8 @@ use pg_embedded_setup_unpriv::test_support::panic_payload_to_string; use rstest::fixture; use super::{ - TestCluster, cap_fs, + TestCluster, + cap_fs, cluster_skip::cluster_skip_message, env::ScopedEnvVars, env_isolation::{override_env_os, override_env_path}, @@ -53,9 +54,7 @@ impl FixtureWorld { self.skip_reason = Some(message); } - pub(super) const fn is_skipped(&self) -> bool { - self.skip_reason.is_some() - } + pub(super) const fn is_skipped(&self) -> bool { self.skip_reason.is_some() } pub(super) fn ensure_not_skipped(&self) -> Result<()> { if self.is_skipped() { @@ -97,9 +96,7 @@ impl FixtureWorld { } impl Drop for FixtureWorld { - fn drop(&mut self) { - drop(self.cluster.take()); - } + fn drop(&mut self) { drop(self.cluster.take()); } } pub(super) type FixtureWorldFixture = Result>; @@ -112,7 +109,8 @@ pub(super) fn borrow_world(world: &FixtureWorldFixture) -> Result<&RefCell FixtureWorldFixture { - Ok(RefCell::new(FixtureWorld::new()?)) + let world = FixtureWorld::new()?; + Ok(RefCell::new(world)) } pub(super) fn env_for_profile( diff --git a/tests/test_workflow_integration.py b/tests/test_workflow_integration.py index 4b09f35e..cf8fc0f0 100644 --- a/tests/test_workflow_integration.py +++ b/tests/test_workflow_integration.py @@ -17,7 +17,7 @@ def run_act( *, artifact_dir: Path, ) -> tuple[int, Path, str]: - """Run an `act` job and return its exit code, artefact directory, and logs.""" + """Run an `act` pull request job and return its exit code, artefacts, and logs.""" if shutil.which("act") is None: pytest.skip("act CLI not installed") artifact_dir.mkdir(parents=True, exist_ok=True) @@ -53,7 +53,7 @@ def run_act( def test_workflow_produces_expected_artefact_and_logs(tmp_path: Path) -> None: - """Verify the self-test workflow writes its artefact and greeting logs.""" + """Verify the self-test workflow writes its result artefact and greeting log.""" artifact_dir = tmp_path / "act-artifacts" code, artdir, logs = run_act(artifact_dir=artifact_dir) assert code == 0, f"act failed:\n{logs}" diff --git a/tests/worker_process.rs b/tests/worker_process.rs index 5cf168af..7866cdf8 100644 --- a/tests/worker_process.rs +++ b/tests/worker_process.rs @@ -12,19 +12,28 @@ feature = "privileged-tests", ))] +use std::{ + fs, + os::unix::{fs::PermissionsExt, process::ExitStatusExt}, + sync::{Mutex, OnceLock}, + time::Duration, +}; + use camino::{Utf8Path, Utf8PathBuf}; use color_eyre::eyre::{Context, eyre}; -use pg_embedded_setup_unpriv::worker_process_test_api::{ - WorkerOperation, WorkerRequest, WorkerRequestArgs, disable_privilege_drop_for_tests, - render_failure_for_tests, run, +use pg_embedded_setup_unpriv::{ + BootstrapError, + BootstrapResult, + worker_process_test_api::{ + WorkerOperation, + WorkerRequest, + WorkerRequestArgs, + disable_privilege_drop_for_tests, + render_failure_for_tests, + run, + }, }; -use pg_embedded_setup_unpriv::{BootstrapError, BootstrapResult}; use postgresql_embedded::Settings; -use std::fs; -use std::os::unix::fs::PermissionsExt; -use std::os::unix::process::ExitStatusExt; -use std::sync::{Mutex, OnceLock}; -use std::time::Duration; use tempfile::tempdir; const TRUNCATION_SUFFIX: &str = "… [truncated]"; @@ -122,7 +131,8 @@ fn run_truncates_stdout_and_stderr_on_failure() -> BootstrapResult<()> { let env_vars = Vec::new(); let long_output = "A".repeat(5_000); let script_body = format!( - "#!/bin/sh\ncat <<'EOF'\n{long_output}\nEOF\ncat <<'EOF' >&2\n{long_output}\nEOF\nexit 1\n" + "#!/bin/sh\ncat <<'EOF'\n{long_output}\nEOF\ncat <<'EOF' \ + >&2\n{long_output}\nEOF\nexit 1\n" ); let worker_path = write_script(sandbox.path(), "fail.sh", &script_body)?; let request = request( From d401285283f9a7a8025305ff7f3cf28d6fd64487 Mon Sep 17 00:00:00 2001 From: leynos Date: Tue, 30 Jun 2026 03:11:42 +0200 Subject: [PATCH 2/8] Stabilize lint review fixes Pin the CI `interrogate` tool installation and the nightly Rust toolchain so lint and formatting behaviour remains reproducible. Restrict shared singleton test helper exports to documentation, tests, and explicit testing features while leaving the dummy fixture constructors available for the intended unit-test support API. --- rust-toolchain.toml | 2 +- src/test_support/mod.rs | 11 +++-------- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/rust-toolchain.toml b/rust-toolchain.toml index 8e275b74..843be084 100644 --- a/rust-toolchain.toml +++ b/rust-toolchain.toml @@ -1,3 +1,3 @@ [toolchain] -channel = "nightly" +channel = "nightly-2026-04-25" components = ["rustfmt", "clippy"] diff --git a/src/test_support/mod.rs b/src/test_support/mod.rs index 63bef143..4f9c379d 100644 --- a/src/test_support/mod.rs +++ b/src/test_support/mod.rs @@ -30,14 +30,9 @@ pub use errors::{bootstrap_error, privilege_error}; pub use filesystem::ambient_dir_and_path; #[cfg(any(doc, test, feature = "cluster-unit-tests", feature = "dev-worker"))] pub use filesystem::{CapabilityTempDir, ensure_dir_exists, metadata, set_permissions}; -pub use fixtures::{ - dummy_environment, - dummy_settings, - ensure_worker_env, - shared_cluster, - shared_cluster_handle, - test_runtime, -}; +pub use fixtures::{dummy_environment, dummy_settings, test_runtime}; +#[cfg(any(doc, test, feature = "cluster-unit-tests", feature = "dev-worker"))] +pub use fixtures::{ensure_worker_env, shared_cluster, shared_cluster_handle}; #[cfg(not(doc))] pub use fixtures::{shared_test_cluster, shared_test_cluster_handle, test_cluster}; #[cfg(doc)] From a4ea672f140cc630dc68e9433d34a4ee39d00a1c Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 3 Jul 2026 00:00:05 +0200 Subject: [PATCH 3/8] Document enhanced lint requirements Add contributor guidance for the pinned nightly rustfmt toolchain and Interrogate-backed lint gate so developers can reproduce CI locally. Record the quality-gate import in the roadmap and include the Markdown normalisation produced by the repository formatter. --- AGENTS.md | 4 +- ...antipatterns-and-refactoring-strategies.md | 4 +- docs/developers-guide.md | 18 +++++ docs/execplans/configure-pg-worker-count.md | 7 +- .../data-directory-recovery-logic.md | 13 ++-- ...features-and-secrecy-crate-in-worker-rs.md | 7 +- docs/execplans/issue-50-async-support.md | 12 ++-- ...-59-make-pg-worker-a-first-class-binary.md | 17 +++-- ...e-60-3-environment-mutation-abstraction.md | 30 ++++----- .../issue-66-raii-fixture-cleanup.md | 7 +- docs/ortho-config-users-guide.md | 29 ++++---- ...esting-in-rust-via-dependency-injection.md | 4 +- docs/roadmap.md | 67 +++++++++++++++++++ docs/rust-doctest-dry-guide.md | 20 +++--- docs/rust-testing-with-rstest-fixtures.md | 28 ++++---- 15 files changed, 173 insertions(+), 94 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 3910cd6c..ef801a7b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -204,8 +204,8 @@ project: ### Dependency Management - **Mandate caret requirements for all dependencies.** All crate versions - specified in `Cargo.toml` must use SemVer-compatible caret requirements - (e.g., `some-crate = "1.2.3"`). This is Cargo's default and allows for safe, + specified in `Cargo.toml` must use SemVer-compatible caret requirements (e.g., + `some-crate = "1.2.3"`). This is Cargo's default and allows for safe, non-breaking updates to minor and patch versions while preventing breaking changes from new major versions. This approach is critical for ensuring build stability and reproducibility. diff --git a/docs/complexity-antipatterns-and-refactoring-strategies.md b/docs/complexity-antipatterns-and-refactoring-strategies.md index 83b1e845..2d223161 100644 --- a/docs/complexity-antipatterns-and-refactoring-strategies.md +++ b/docs/complexity-antipatterns-and-refactoring-strategies.md @@ -46,8 +46,8 @@ the number of edges, N is the number of nodes, and P is the number of connected components (typically 1 for a single program or method).3 A simpler formulation for a single subroutine is -M = number of decision points + 1, where decision points include constructs -like `if` statements and conditional loops.3 +M = number of decision points + 1, where decision points include constructs like +`if` statements and conditional loops.3 Thresholds and Implications: diff --git a/docs/developers-guide.md b/docs/developers-guide.md index a161b2dc..f1f6facc 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -23,6 +23,24 @@ also runs a Linux matrix for unprivileged and root execution. The root variant invokes the test suite under `sudo` so root-only privilege paths execute, while the unprivileged variant continues to collect coverage. +## Lint and formatting toolchain + +The repository pins its Rust toolchain in `rust-toolchain.toml` to a dated +nightly release because `rustfmt.toml` uses unstable formatter options imported +from the shared Rust agent template. Run formatting through the Makefile so the +pinned `rustfmt` and workspace settings are used consistently: + +```sh +make check-fmt +``` + +`make lint` is the authoritative local lint gate. It requires 100% Python +docstring coverage through `interrogate --fail-under 100 .`, builds Rust +documentation with warnings denied, and runs Clippy with warnings denied across +all targets and features. CI installs the pinned Interrogate tool before this +gate runs, so contributors should keep local Interrogate versions aligned with +the Makefile and workflow when updating the lint policy. + ## Release process Tagging a release with `v*` triggers `.github/workflows/release.yml`. The diff --git a/docs/execplans/configure-pg-worker-count.md b/docs/execplans/configure-pg-worker-count.md index 2a8b98b2..04d55b54 100644 --- a/docs/execplans/configure-pg-worker-count.md +++ b/docs/execplans/configure-pg-worker-count.md @@ -1,9 +1,8 @@ # Configure postgres-embedded worker counts at cluster setup -This execution plan (ExecPlan) is a living document. The sections -`Constraints`, `Tolerances`, `Risks`, `Progress`, `Surprises & Discoveries`, -`Decision Log`, and `Outcomes & Retrospective` must be kept up to date as work -proceeds. +This execution plan (ExecPlan) is a living document. The sections `Constraints`, +`Tolerances`, `Risks`, `Progress`, `Surprises & Discoveries`, `Decision Log`, +and `Outcomes & Retrospective` must be kept up to date as work proceeds. Status: COMPLETE diff --git a/docs/execplans/data-directory-recovery-logic.md b/docs/execplans/data-directory-recovery-logic.md index 26d17e5d..d017bf8c 100644 --- a/docs/execplans/data-directory-recovery-logic.md +++ b/docs/execplans/data-directory-recovery-logic.md @@ -1,9 +1,8 @@ # Data Directory Recovery Logic for pg_worker -This Execution Plan (ExecPlan) is a living document. The sections -`Constraints`, `Tolerances`, `Risks`, `Progress`, `Surprises & Discoveries`, -`Decision Log`, and `Outcomes & Retrospective` must be kept up to date as work -proceeds. +This Execution Plan (ExecPlan) is a living document. The sections `Constraints`, +`Tolerances`, `Risks`, `Progress`, `Surprises & Discoveries`, `Decision Log`, +and `Outcomes & Retrospective` must be kept up to date as work proceeds. Status: COMPLETED @@ -298,8 +297,8 @@ Validation for Stage C: Run quality gates to ensure code meets project standards. 1. Run `make test` (equivalent to `cargo test --workspace`) -2. Run `make lint` (equivalent to `cargo clippy --workspace - --all-targets --all-features -- -D warnings`) +2. Run `make lint` (equivalent to + `cargo clippy --workspace --all-targets --all-features -- -D warnings`) 3. Run `make check-fmt` (equivalent to `cargo fmt --workspace -- --check`) ## Concrete Steps @@ -330,7 +329,7 @@ Step 2: Remove dead_code expectation Command: Edit `tests/support/pg_worker.rs` and remove the `#[expect(dead_code, reason = "variant reserved for future data directory recovery errors")]` - line and the closing `]` from the `WorkerError::DataDirRecovery` variant. + line and the closing `]` from the `WorkerError::DataDirRecovery` variant. Expected result: No change to behaviour, clippy no longer expects the variant to be unused. diff --git a/docs/execplans/issue-20-leverage-serde-features-and-secrecy-crate-in-worker-rs.md b/docs/execplans/issue-20-leverage-serde-features-and-secrecy-crate-in-worker-rs.md index bfcbb5de..37cdb41e 100644 --- a/docs/execplans/issue-20-leverage-serde-features-and-secrecy-crate-in-worker-rs.md +++ b/docs/execplans/issue-20-leverage-serde-features-and-secrecy-crate-in-worker-rs.md @@ -1,9 +1,8 @@ # Refactor worker payload serde via secrecy -This Execution Plan (ExecPlan) is a living document. The sections -`Constraints`, `Tolerances`, `Risks`, `Progress`, `Surprises & Discoveries`, -`Decision Log`, and `Outcomes & Retrospective` must be kept up to date as work -proceeds. +This Execution Plan (ExecPlan) is a living document. The sections `Constraints`, +`Tolerances`, `Risks`, `Progress`, `Surprises & Discoveries`, `Decision Log`, +and `Outcomes & Retrospective` must be kept up to date as work proceeds. Status: COMPLETE diff --git a/docs/execplans/issue-50-async-support.md b/docs/execplans/issue-50-async-support.md index 9b93e9f9..995696f3 100644 --- a/docs/execplans/issue-50-async-support.md +++ b/docs/execplans/issue-50-async-support.md @@ -1,9 +1,8 @@ # Add async API for TestCluster -This Execution Plan (ExecPlan) is a living document. The sections -`Constraints`, `Tolerances`, `Risks`, `Progress`, `Surprises & discoveries`, -`Decision log`, and `Outcomes & retrospective` must be kept up to date as work -proceeds. +This Execution Plan (ExecPlan) is a living document. The sections `Constraints`, +`Tolerances`, `Risks`, `Progress`, `Surprises & discoveries`, `Decision log`, +and `Outcomes & retrospective` must be kept up to date as work proceeds. Status: COMPLETE @@ -190,8 +189,9 @@ Successfully implemented async API for `TestCluster`: - `src/cluster/runtime.rs` (22 lines) - Builds the single-threaded Tokio runtime via `build_runtime()`. -- `Cargo.toml` - Line 43: `tokio = { version = "1", features = ["rt", "macros"] - }`. Line 70-77: Feature flags section. +- `Cargo.toml` - Line 43: + `tokio = { version = "1", features = ["rt", "macros"] }`. Line 70-77: Feature + flags section. ### Current architecture diff --git a/docs/execplans/issue-59-make-pg-worker-a-first-class-binary.md b/docs/execplans/issue-59-make-pg-worker-a-first-class-binary.md index 491ac4f6..12f2ef58 100644 --- a/docs/execplans/issue-59-make-pg-worker-a-first-class-binary.md +++ b/docs/execplans/issue-59-make-pg-worker-a-first-class-binary.md @@ -1,9 +1,8 @@ # Promote pg_worker to first-class binary -This Execution Plan (ExecPlan) is a living document. The sections -`Constraints`, `Tolerances`, `Risks`, `Progress`, `Surprises & discoveries`, -`Decision log`, and `Outcomes & retrospective` must be kept up to date as work -proceeds. +This Execution Plan (ExecPlan) is a living document. The sections `Constraints`, +`Tolerances`, `Risks`, `Progress`, `Surprises & discoveries`, `Decision log`, +and `Outcomes & retrospective` must be kept up to date as work proceeds. Status: COMPLETED @@ -110,7 +109,7 @@ After this change: `CapabilityTempDir` are only used in test code (under `tests/`), which documentation builds don't see. Solution: Add `#[cfg(any(doc, test, feature = "cluster-unit-tests", feature = "dev-worker"))]` - attributes to align with how they're exported in `mod.rs`. + attributes to align with how they're exported in `mod.rs`. ## Decision log @@ -714,10 +713,10 @@ These are independent and serve different purposes: *Table 1: Discovery mechanisms by context.* -| Context | Discovery mechanism | Purpose | -| ---------- | ------------------------------- | ------------------------- | -| Tests | Build directory search | Find freshly built binary | -| Production | Explicit config → PATH search | Find installed binary | +| Context | Discovery mechanism | Purpose | +| ---------- | ----------------------------- | ------------------------- | +| Tests | Build directory search | Find freshly built binary | +| Production | Explicit config → PATH search | Find installed binary | ### Binary location structure diff --git a/docs/execplans/issue-60-3-environment-mutation-abstraction.md b/docs/execplans/issue-60-3-environment-mutation-abstraction.md index 6f032ad9..5a8fdc90 100644 --- a/docs/execplans/issue-60-3-environment-mutation-abstraction.md +++ b/docs/execplans/issue-60-3-environment-mutation-abstraction.md @@ -1,9 +1,8 @@ # Issue 60.3: Environment Mutation Abstraction -This execution plan (ExecPlan) is a living document. The sections -`Constraints`, `Tolerances`, `Risks`, `Progress`, `Surprises & Discoveries`, -`Decision Log`, and `Outcomes & Retrospective` must be kept up to date as work -proceeds. +This execution plan (ExecPlan) is a living document. The sections `Constraints`, +`Tolerances`, `Risks`, `Progress`, `Surprises & Discoveries`, `Decision Log`, +and `Outcomes & Retrospective` must be kept up to date as work proceeds. Status: COMPLETE @@ -106,8 +105,8 @@ actual current state of the work. and pass to `apply_worker_environment`. - [x] (2026-01-21 21:22Z) Add unit test for `TestEnvStore` demonstrating `get`, `set`, and `remove`. -- [x] (2026-01-21 21:28Z) Run `make check-fmt`, `make lint`, and `make - test` to validate all changes. +- [x] (2026-01-21 21:28Z) Run `make check-fmt`, `make lint`, and `make test` to + validate all changes. - [x] (2026-01-21 21:29Z) Commit the changes with a descriptive message. ## Surprises & discoveries @@ -138,8 +137,8 @@ All objectives achieved: with `set` and `remove` methods. - `ProcessEnvStore` wraps real `env::set_var` and `env::remove_var` with explicit SAFETY comments. -- `TestEnvStore` provides in-memory storage with `HashMap>` and a `get` method for test assertions. +- `TestEnvStore` provides in-memory storage with + `HashMap>` and a `get` method for test assertions. - `apply_worker_environment` refactored to accept `&mut dyn EnvStore` parameter. - `run_worker` updated to create `ProcessEnvStore` and pass to @@ -213,8 +212,8 @@ Existing tests use an `EnvironmentOperations` trait from This plan introduces a new `EnvStore` trait that: -- Defines `set(&mut self, key: &str, value: &str)` and `remove(&mut self, key: - &str)` methods +- Defines `set(&mut self, key: &str, value: &str)` and + `remove(&mut self, key: &str)` methods - Is placed directly in `tests/support/pg_worker.rs` alongside the production code - Provides two implementations: `ProcessEnvStore` for production and @@ -283,9 +282,10 @@ Validation: The code should compile with In `tests/support/pg_worker.rs`, modify the `apply_worker_environment` function (currently lines 208-221): -1. Change the function signature from `fn apply_worker_environment(environment: - &[(String, Option)])` to `fn apply_worker_environment(store: - &mut dyn EnvStore, environment: &[(String, Option)])` + to + `fn apply_worker_environment(store: &mut dyn EnvStore, environment: &[(String, Option`, an alias -for `Result>`. `OrthoError` wraps errors from `clap`, file -I/O and `figment`. Failures during the final merge of CLI values over -configuration sources surface as the `Merge` variant, providing clearer -diagnostics when the combined data is invalid. When multiple sources fail, the -errors are collected into the `Aggregate` variant so callers can inspect each -individual failure. Consumers should handle these errors appropriately, for -example by printing them to stderr and exiting. If required fields are missing -after merging, the crate returns `OrthoError::MissingRequiredValues` with a -user‑friendly list of missing paths and hints on how to provide them. For -example: +`load` and `load_and_merge_subcommand_for` return `OrthoResult`, an alias for +`Result>`. `OrthoError` wraps errors from `clap`, file I/O +and `figment`. Failures during the final merge of CLI values over configuration +sources surface as the `Merge` variant, providing clearer diagnostics when the +combined data is invalid. When multiple sources fail, the errors are collected +into the `Aggregate` variant so callers can inspect each individual failure. +Consumers should handle these errors appropriately, for example by printing +them to stderr and exiting. If required fields are missing after merging, the +crate returns `OrthoError::MissingRequiredValues` with a user‑friendly list of +missing paths and hints on how to provide them. For example: ```plaintext Missing required values: diff --git a/docs/reliable-testing-in-rust-via-dependency-injection.md b/docs/reliable-testing-in-rust-via-dependency-injection.md index 33d4329f..f26d8718 100644 --- a/docs/reliable-testing-in-rust-via-dependency-injection.md +++ b/docs/reliable-testing-in-rust-via-dependency-injection.md @@ -2,8 +2,8 @@ Writing robust, reliable, and parallelisable tests requires an intentional approach to handling external dependencies such as environment variables, the -filesystem, or the system clock. Functions that directly call `std::env::var` -or `SystemTime::now()` are difficult to test because they depend on global, +filesystem, or the system clock. Functions that directly call `std::env::var` or +`SystemTime::now()` are difficult to test because they depend on global, non-deterministic state. This leads to several problems: diff --git a/docs/roadmap.md b/docs/roadmap.md index 7ba4e95e..098e9810 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -20,6 +20,25 @@ ## 1. Stabilize zero-configuration bootstrap + +### 1.1. Detect and adapt to execution privileges + +- [x] 1.1.1. Ship runtime privilege detection that branches between root and + unprivileged flows, verified by integration tests covering both contexts. +- [x] 1.1.2. Introduce mandatory privilege-dropping logic for root executions + that provisions directories under the `nobody` user and proves idempotence + across two consecutive cluster initializations. + + +### 1.2. Harden bootstrap orchestration + +- [x] 1.2.1. Deliver a `bootstrap_for_tests()` helper that wraps configuration + discovery, handles privilege checks, and prepares directories, returning a + structured settings object exercised in sample tests. +- [x] 1.2.2. Automate environment variable preparation (for example `TZDIR` and + `PGPASSFILE`) with traceable defaults validated through assertions in the + bootstrap tests. + ### 1.1. Detect and adapt to execution privileges - [x] 1.1.1. Ship runtime privilege detection that branches between root and @@ -39,6 +58,24 @@ ## 2. Provide an ergonomic RAII fixture + +### 2.1. Implement the `TestCluster` lifecycle + +- [x] 2.1.1. Create the `TestCluster` RAII struct with `new` and `Drop` + implementations that guarantee cluster startup and teardown, evidenced by + tests ensuring no processes remain after scope exit. +- [x] 2.1.2. Expose connection metadata and helper methods (for example a + Diesel connection constructor) with documentation examples and smoke tests + that execute simple SQL statements. + + +### 2.2. Integrate with test frameworks + +- [x] 2.2.1. Publish an `rstest` fixture (or equivalent) that yields a ready + `TestCluster`, demonstrated in example tests showing zero explicit setup. +- [x] 2.2.2. Document usage patterns in README excerpts and doctests, so the + fixture becomes the default path for new integration tests. + ### 2.1. Implement the `TestCluster` lifecycle - [x] 2.1.1. Create the `TestCluster` RAII struct with `new` and `Drop` @@ -57,6 +94,36 @@ ## 3. Enhance visibility and platform coverage + +### 3.1. Instrument setup for observability + +- [x] 3.1.1. Add `tracing` spans and logs around privilege changes, directory + mutations, environment variable injection, and PostgreSQL lifecycle events, + verified by log assertions in integration tests. +- [x] 3.1.2. Surface the chosen settings (ports, directories, version) via + debug logging with sanitized output, ensuring sensitive values are redacted. + + +### 3.2. Strengthen concurrency guarantees + +- [x] 3.2.1. Add Loom-based concurrency tests for the `ScopedEnv` mutex, gated + behind a feature flag, and document how to run them. + + +### 3.3. Validate cross-platform behaviour + +- [x] 3.3.1. Confirm Linux root and unprivileged paths through Continuous + Integration (CI) matrix jobs, and document expected outcomes for macOS and + Windows in the roadmap appendix. +- [ ] 3.3.2. Establish guardrails that fail fast on unsupported root scenarios + on non-Linux systems, including unit coverage for the error messaging. + +### 3.4 Maintain contributor quality gates + +- [x] 3.4.1. Pin the Rust formatting and linting toolchain, import the shared + rustfmt and Clippy policy, and require 100% Python docstring coverage in the + local and CI lint gates. + ### 3.1. Instrument setup for observability - [x] 3.1.1. Add `tracing` spans and logs around privilege changes, directory diff --git a/docs/rust-doctest-dry-guide.md b/docs/rust-doctest-dry-guide.md index 998c1c67..70912c7b 100644 --- a/docs/rust-doctest-dry-guide.md +++ b/docs/rust-doctest-dry-guide.md @@ -14,8 +14,8 @@ the power and the inherent limitations of doctests. ### 1.1 The "Separate Crate" Paradigm At its heart, `rustdoc` treats each documentation test not as a snippet of code -running within the library's own context, but as an entirely separate, -temporary crate.[^1] When a developer executes +running within the library's own context, but as an entirely separate, temporary +crate.[^1] When a developer executes `cargo test --doc`, `rustdoc` initiates a multi-stage process for every code block found in the documentation comments[^3]: @@ -404,10 +404,10 @@ builds.[^13] pub struct UnixSocket; ``` -This `any` directive ensures the struct is compiled either when the target OS -is `unix` OR when `rustdoc` is running. This correctly makes the item visible -in the generated HTML. However, it is crucial to understand that this **does -not** make the doctest for `UnixSocket` pass on non-Unix platforms. +This `any` directive ensures the struct is compiled either when the target OS is +`unix` OR when `rustdoc` is running. This correctly makes the item visible in +the generated HTML. However, it is crucial to understand that this **does not** +make the doctest for `UnixSocket` pass on non-Unix platforms. This distinction highlights the "cfg duality." The `#[cfg(doc)]` attribute controls the *table of contents* of the documentation; it determines which @@ -579,8 +579,8 @@ real-world challenges when working with doctests. `#[test]` function in a temporary file or test module. This allows the developer to leverage the full power of the IDE. Once the code is working - correctly, it can be copied into the doc comment, and the necessary - formatting (`///`, `#`, etc.) can be applied.[^15] + correctly, it can be copied into the doc comment, and the necessary formatting + (`///`, `#`, etc.) can be applied.[^15] ## Conclusion and Recommendations @@ -645,7 +645,7 @@ July 15, 2025, [^11]: Compile_fail doc test ignored in cfg(test) - help - The Rust Programming Language Forum, accessed on July 15, 2025, ; - “Test setup for doctests”, accessed on July 15, 2025, +“Test setup for doctests”, accessed on July 15, 2025, [^12]: quote_doctest - Rust - [Docs.rs](http://Docs.rs), accessed on July 15, 2025, @@ -654,7 +654,7 @@ Language Forum, accessed on July 15, 2025, [^14]: rust - How can I conditionally execute a module-level doctest based …, accessed on July 15, 2025, - have doctests?, accessed on July 15, 2025, +have doctests?, accessed on July 15, 2025, [^15]: How do you write your doc tests? : r/rust - Reddit, accessed on July 15, 2025, diff --git a/docs/rust-testing-with-rstest-fixtures.md b/docs/rust-testing-with-rstest-fixtures.md index 38cc0a2a..ea56e07c 100644 --- a/docs/rust-testing-with-rstest-fixtures.md +++ b/docs/rust-testing-with-rstest-fixtures.md @@ -1368,20 +1368,20 @@ provided by `rstest`: **Table 2: Key** `rstest` **Attributes Quick Reference** -| Attribute | Core Purpose | -| ---------------------------- | -------------------------------------------------------------------------------------------- | -| #[rstest] | Marks a function as an rstest test; enables fixture injection and parameterization. | -| #[fixture] | Defines a function that provides a test fixture (setup data or services). | -| #[case(…)] | Defines a single parameterized test case with specific input values. | -| #[values(…)] | Defines a list of values for an argument, generating tests for each value or combination. | -| #[once] | Marks a fixture to be initialized only once and shared (as a static reference) across tests. | -| #[future] | Simplifies async argument types by removing impl Future boilerplate. | -| #[awt] | (Function or argument level) Automatically .awaits future arguments in async tests. | -| #[from(original_name)] | Allows renaming an injected fixture argument in the test function. | -| #[with(…)] | Overrides default arguments of a fixture for a specific test. | -| #[default(…)] | Provides default values for arguments within a fixture function. | -| #[timeout(…)] | Sets a timeout for an asynchronous test. | -| #[files("glob_pattern",…)] | Injects file paths (or contents, with mode=) matching a glob pattern as test arguments. | +| Attribute | Core Purpose | +| -------------------------- | -------------------------------------------------------------------------------------------- | +| #[rstest] | Marks a function as an rstest test; enables fixture injection and parameterization. | +| #[fixture] | Defines a function that provides a test fixture (setup data or services). | +| #[case(…)] | Defines a single parameterized test case with specific input values. | +| #[values(…)] | Defines a list of values for an argument, generating tests for each value or combination. | +| #[once] | Marks a fixture to be initialized only once and shared (as a static reference) across tests. | +| #[future] | Simplifies async argument types by removing impl Future boilerplate. | +| #[awt] | (Function or argument level) Automatically .awaits future arguments in async tests. | +| #[from(original_name)] | Allows renaming an injected fixture argument in the test function. | +| #[with(…)] | Overrides default arguments of a fixture for a specific test. | +| #[default(…)] | Provides default values for arguments within a fixture function. | +| #[timeout(…)] | Sets a timeout for an asynchronous test. | +| #[files("glob_pattern",…)] | Injects file paths (or contents, with mode=) matching a glob pattern as test arguments. | By mastering `rstest`, Rust developers can significantly elevate the quality and efficiency of their testing practices, leading to more reliable and From 500342763036812f5e26810b87d408e19323f066 Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 3 Jul 2026 00:52:28 +0200 Subject: [PATCH 4/8] Record enhanced lint policy decision Document the `make lint` tier ordering with Interrogate as the 100% Python docstring-coverage gate. Record the decision to pin the nightly rustfmt toolchain and import the shared Rust lint template in an ADR. --- docs/adr-004-enhanced-lint-requirements.md | 94 ++++++++++++++++++++++ docs/developers-guide.md | 29 ++++--- 2 files changed, 113 insertions(+), 10 deletions(-) create mode 100644 docs/adr-004-enhanced-lint-requirements.md diff --git a/docs/adr-004-enhanced-lint-requirements.md b/docs/adr-004-enhanced-lint-requirements.md new file mode 100644 index 00000000..845403b6 --- /dev/null +++ b/docs/adr-004-enhanced-lint-requirements.md @@ -0,0 +1,94 @@ +# ADR 004: enhanced lint and formatting requirements + +## Status + +Accepted on 2026-07-03. The repository imports the shared Rust formatting and +Clippy policy, pins a dated nightly toolchain for unstable rustfmt options, and +adds `interrogate` as a Python docstring-coverage tier in the lint gate. + +## Date + +2026-07-03. + +## Context and problem statement + +The project already requires Rust formatting, Rust documentation checks, +Clippy, and tests before changes are accepted. The previous policy left two +gaps: + +- Rust formatting and Clippy settings could drift from the shared agent + template used across Leynos Rust projects. +- Python helper files could pass linting even when documentable functions, + fixtures, or test utilities lacked docstrings. + +The shared template rustfmt configuration uses unstable rustfmt options, so +adopting it requires a nightly toolchain. Using the floating `nightly` channel +would make formatting results change as upstream nightly builds change. + +## Decision drivers + +- Keep Rust formatting and Clippy policy aligned with the shared Leynos Rust + agent template. +- Make formatting reproducible by pinning the nightly toolchain to a dated + release. +- Enforce complete Python docstring coverage through an objective gate. +- Keep the new checks inside the existing `make lint` and CI workflow rather + than introducing a parallel contributor process. + +## Options considered + +### Option A: keep the existing stable formatter and lint policy + +Continue using the previous stable rustfmt behaviour and omit Python docstring +coverage checks. + +This avoids toolchain churn, but it leaves the repository out of sync with the +shared Rust template and keeps Python documentation coverage subjective. + +### Option B: import the template policy and pin nightly rustfmt + +Add the template `rustfmt.toml` and Clippy policy, pin `rust-toolchain.toml` to +`nightly-2026-04-25`, and run `interrogate --fail-under 100 .` as the first +`make lint` tier. + +This provides reproducible formatting, keeps Rust lint policy aligned with the +template, and makes Python docstring coverage complete and objective. The cost +is that contributors must use the pinned nightly toolchain for formatting. + +### Option C: use a floating nightly channel + +Adopt the template configuration but set `rust-toolchain.toml` to `nightly`. + +This keeps the configuration small, but rustfmt and Clippy results may drift as +nightly changes. That drift makes CI failures harder to reproduce and upgrades +less intentional. + +## Decision outcome + +Choose Option B. + +The repository pins `rust-toolchain.toml` to `nightly-2026-04-25` so unstable +rustfmt features from the imported template remain reproducible. `make lint` +runs these tiers in order: + +1. `interrogate --fail-under 100 .` for Python docstring coverage. +2. `cargo doc --workspace --no-deps` with warnings denied. +3. `cargo clippy --all-targets --all-features -- -D warnings`. + +CI installs `interrogate==1.7.0` as a uv tool before the lint step, keeping the +docstring-coverage gate pinned with the workflow. + +## Consequences + +### Positive + +- Rust formatting and Clippy policy match the shared Leynos template. +- Nightly rustfmt output is reproducible because the channel is date-pinned. +- Python docstring coverage is enforced at 100% by a dedicated tool. + +### Negative + +- Contributors need the pinned nightly toolchain for formatting checks. +- New or changed Python helpers must document every documentable node. +- Future rustfmt or Clippy template upgrades require an intentional toolchain + update rather than an implicit nightly drift. diff --git a/docs/developers-guide.md b/docs/developers-guide.md index f1f6facc..bd9a9f71 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -25,21 +25,30 @@ the unprivileged variant continues to collect coverage. ## Lint and formatting toolchain -The repository pins its Rust toolchain in `rust-toolchain.toml` to a dated -nightly release because `rustfmt.toml` uses unstable formatter options imported -from the shared Rust agent template. Run formatting through the Makefile so the -pinned `rustfmt` and workspace settings are used consistently: +The repository pins `rust-toolchain.toml` to `nightly-2026-04-25` because the +imported `rustfmt.toml` template uses unstable rustfmt options. Use the +Makefile targets rather than invoking Cargo directly so local checks and CI +exercise the same nightly formatter and Clippy policy: ```sh make check-fmt ``` -`make lint` is the authoritative local lint gate. It requires 100% Python -docstring coverage through `interrogate --fail-under 100 .`, builds Rust -documentation with warnings denied, and runs Clippy with warnings denied across -all targets and features. CI installs the pinned Interrogate tool before this -gate runs, so contributors should keep local Interrogate versions aligned with -the Makefile and workflow when updating the lint policy. +Run the complete lint gate before committing changes: + +```sh +make lint +``` + +`make lint` runs three tiers in order, each gating the next: + +1. `interrogate --fail-under 100 .` — Python docstring coverage at 100%. +2. `cargo doc --workspace --no-deps` with `RUSTDOCFLAGS` set to deny warnings. +3. `cargo clippy --all-targets --all-features -- -D warnings`. + +CI installs the pinned `interrogate==1.7.0` uv tool before running `make lint`. +Keep the Makefile and workflow versions aligned when updating the +docstring-coverage policy. ## Release process From df32254f63877b997fbe27b6e733be3f8b7d0c78 Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 3 Jul 2026 23:56:35 +0200 Subject: [PATCH 5/8] Address workflow and doctest review notes Disable persisted checkout credentials in CI because the job does not push or perform authenticated fetches after checkout. Correct the Unix cfg example in the doctest guide so the code and prose use the valid `unix` cfg. --- .github/workflows/ci.yml | 2 ++ docs/rust-doctest-dry-guide.md | 15 +++++++-------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d10afc45..f39b07db 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -21,6 +21,8 @@ jobs: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} steps: - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 + with: + persist-credentials: false - name: Setup Rust uses: leynos/shared-actions/.github/actions/setup-rust@73f02134f6f1c2335326d1b9cdefaddd287bf23e - name: Format diff --git a/docs/rust-doctest-dry-guide.md b/docs/rust-doctest-dry-guide.md index 70912c7b..88983747 100644 --- a/docs/rust-doctest-dry-guide.md +++ b/docs/rust-doctest-dry-guide.md @@ -400,12 +400,12 @@ builds.[^13] ```Rust /// A socket that is only available on Unix platforms. -#[cfg(any(target_os = "unix", doc))] +#[cfg(any(unix, doc))] pub struct UnixSocket; ``` -This `any` directive ensures the struct is compiled either when the target OS is -`unix` OR when `rustdoc` is running. This correctly makes the item visible in +This `any` directive ensures the struct is compiled either when the `unix` cfg +is set OR when `rustdoc` is running. This correctly makes the item visible in the generated HTML. However, it is crucial to understand that this **does not** make the doctest for `UnixSocket` pass on non-Unix platforms. @@ -413,11 +413,10 @@ This distinction highlights the "cfg duality." The `#[cfg(doc)]` attribute controls the *table of contents* of the documentation; it determines which items are parsed and rendered. The actual compilation of a doctest, however, happens in a separate, later stage. In that stage, the `doc` cfg is *not* -passed to the compiler.[^13] The compiler only sees the host - -`cfg` (e.g., `target_os = "windows"`), so the `UnixSocket` type is not -available, and the test fails to compile. `#[cfg(doc)]` affects what is -documented, not what is testable. +passed to the compiler.[^13] The compiler only sees the host cfg (e.g., the +absence of `unix` on Windows), so the `UnixSocket` type is not available, and +the test fails to compile. `#[cfg(doc)]` affects what is documented, not what +is testable. ### 5.2 Executing Doctests Conditionally: Feature Flags From 84bd9f4471cbb89624001985b6fe66698c13c8e9 Mon Sep 17 00:00:00 2001 From: leynos Date: Sat, 4 Jul 2026 00:01:33 +0200 Subject: [PATCH 6/8] Fix roadmap Markdown lint failures Remove duplicated roadmap sections introduced during the rebase and normalize blank lines so the Markdown quality gates pass. --- docs/roadmap.md | 61 ------------------------------------------------- 1 file changed, 61 deletions(-) diff --git a/docs/roadmap.md b/docs/roadmap.md index 098e9810..fed50a7f 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -20,25 +20,6 @@ ## 1. Stabilize zero-configuration bootstrap - -### 1.1. Detect and adapt to execution privileges - -- [x] 1.1.1. Ship runtime privilege detection that branches between root and - unprivileged flows, verified by integration tests covering both contexts. -- [x] 1.1.2. Introduce mandatory privilege-dropping logic for root executions - that provisions directories under the `nobody` user and proves idempotence - across two consecutive cluster initializations. - - -### 1.2. Harden bootstrap orchestration - -- [x] 1.2.1. Deliver a `bootstrap_for_tests()` helper that wraps configuration - discovery, handles privilege checks, and prepares directories, returning a - structured settings object exercised in sample tests. -- [x] 1.2.2. Automate environment variable preparation (for example `TZDIR` and - `PGPASSFILE`) with traceable defaults validated through assertions in the - bootstrap tests. - ### 1.1. Detect and adapt to execution privileges - [x] 1.1.1. Ship runtime privilege detection that branches between root and @@ -58,24 +39,6 @@ ## 2. Provide an ergonomic RAII fixture - -### 2.1. Implement the `TestCluster` lifecycle - -- [x] 2.1.1. Create the `TestCluster` RAII struct with `new` and `Drop` - implementations that guarantee cluster startup and teardown, evidenced by - tests ensuring no processes remain after scope exit. -- [x] 2.1.2. Expose connection metadata and helper methods (for example a - Diesel connection constructor) with documentation examples and smoke tests - that execute simple SQL statements. - - -### 2.2. Integrate with test frameworks - -- [x] 2.2.1. Publish an `rstest` fixture (or equivalent) that yields a ready - `TestCluster`, demonstrated in example tests showing zero explicit setup. -- [x] 2.2.2. Document usage patterns in README excerpts and doctests, so the - fixture becomes the default path for new integration tests. - ### 2.1. Implement the `TestCluster` lifecycle - [x] 2.1.1. Create the `TestCluster` RAII struct with `new` and `Drop` @@ -94,7 +57,6 @@ ## 3. Enhance visibility and platform coverage - ### 3.1. Instrument setup for observability - [x] 3.1.1. Add `tracing` spans and logs around privilege changes, directory @@ -103,13 +65,11 @@ - [x] 3.1.2. Surface the chosen settings (ports, directories, version) via debug logging with sanitized output, ensuring sensitive values are redacted. - ### 3.2. Strengthen concurrency guarantees - [x] 3.2.1. Add Loom-based concurrency tests for the `ScopedEnv` mutex, gated behind a feature flag, and document how to run them. - ### 3.3. Validate cross-platform behaviour - [x] 3.3.1. Confirm Linux root and unprivileged paths through Continuous @@ -123,24 +83,3 @@ - [x] 3.4.1. Pin the Rust formatting and linting toolchain, import the shared rustfmt and Clippy policy, and require 100% Python docstring coverage in the local and CI lint gates. - -### 3.1. Instrument setup for observability - -- [x] 3.1.1. Add `tracing` spans and logs around privilege changes, directory - mutations, environment variable injection, and PostgreSQL lifecycle events, - verified by log assertions in integration tests. -- [x] 3.1.2. Surface the chosen settings (ports, directories, version) via - debug logging with sanitized output, ensuring sensitive values are redacted. - -### 3.2. Strengthen concurrency guarantees - -- [x] 3.2.1. Add Loom-based concurrency tests for the `ScopedEnv` mutex, gated - behind a feature flag, and document how to run them. - -### 3.3. Validate cross-platform behaviour - -- [x] 3.3.1. Confirm Linux root and unprivileged paths through Continuous - Integration (CI) matrix jobs, and document expected outcomes for macOS and - Windows in the roadmap appendix. -- [ ] 3.3.2. Establish guardrails that fail fast on unsupported root scenarios - on non-Linux systems, including unit coverage for the error messaging. From 49c226f40bb5222a31559edd17d6e1bcec6c2d9f Mon Sep 17 00:00:00 2001 From: leynos Date: Sat, 4 Jul 2026 00:31:06 +0200 Subject: [PATCH 7/8] Address review formatting findings Fix the documented worker environment signature and replace long backslash-continued strings with stable concat-based forms. --- .../issue-60-3-environment-mutation-abstraction.md | 2 +- src/bin/pg_worker.rs | 7 +++++-- tests/settings.rs | 12 +++++++----- tests/worker_process.rs | 9 +++++++-- 4 files changed, 20 insertions(+), 10 deletions(-) diff --git a/docs/execplans/issue-60-3-environment-mutation-abstraction.md b/docs/execplans/issue-60-3-environment-mutation-abstraction.md index 5a8fdc90..90b85f65 100644 --- a/docs/execplans/issue-60-3-environment-mutation-abstraction.md +++ b/docs/execplans/issue-60-3-environment-mutation-abstraction.md @@ -285,7 +285,7 @@ In `tests/support/pg_worker.rs`, modify the `apply_worker_environment` function 1. Change the function signature from `fn apply_worker_environment(environment: &[(String, Option)])` to - `fn apply_worker_environment(store: &mut dyn EnvStore, environment: &[(String, Option)])`. 2. Update the function body to use the store parameter instead of calling `env::set_var` and `env::remove_var` directly. Replace the unsafe calls with diff --git a/src/bin/pg_worker.rs b/src/bin/pg_worker.rs index ed9ec097..e4c64a63 100644 --- a/src/bin/pg_worker.rs +++ b/src/bin/pg_worker.rs @@ -77,8 +77,11 @@ impl Operation { "cleanup" => Ok(Self::Cleanup), "cleanup-full" => Ok(Self::CleanupFull), other => Err(WorkerError::InvalidArgs(format!( - "unknown operation '{other}'; expected setup, start, stop, cleanup, or \ - cleanup-full" + concat!( + "unknown operation '{}'; expected setup, start, stop, cleanup, ", + "or cleanup-full" + ), + other ))), } } diff --git a/tests/settings.rs b/tests/settings.rs index a786d436..6baab5e9 100644 --- a/tests/settings.rs +++ b/tests/settings.rs @@ -98,10 +98,9 @@ fn to_settings_default_config() -> color_eyre::Result<()> { } #[fixture] +#[rustfmt::skip] fn default_pg_env() -> PgEnvCfg { - PgEnvCfg { - ..PgEnvCfg::default() - } + PgEnvCfg::default() } #[rstest] @@ -177,8 +176,11 @@ fn with_temp_euid_changes_uid() -> color_eyre::Result<()> { /// Stub variant ensuring the suite reports skipped when privilege drops are unavailable. fn with_temp_euid_changes_uid() -> color_eyre::Result<()> { tracing::warn!( - "skipping root-dependent test: enable the privileged-tests feature to exercise privilege \ - drops", + "{}", + concat!( + "skipping root-dependent test: enable the privileged-tests feature to exercise ", + "privilege drops", + ), ); Ok(()) } diff --git a/tests/worker_process.rs b/tests/worker_process.rs index 7866cdf8..3e60f72f 100644 --- a/tests/worker_process.rs +++ b/tests/worker_process.rs @@ -131,8 +131,13 @@ fn run_truncates_stdout_and_stderr_on_failure() -> BootstrapResult<()> { let env_vars = Vec::new(); let long_output = "A".repeat(5_000); let script_body = format!( - "#!/bin/sh\ncat <<'EOF'\n{long_output}\nEOF\ncat <<'EOF' \ - >&2\n{long_output}\nEOF\nexit 1\n" + concat!( + "#!/bin/sh\n", + "cat <<'EOF'\n{0}\nEOF\n", + "cat <<'EOF' >&2\n{0}\nEOF\n", + "exit 1\n", + ), + long_output ); let worker_path = write_script(sandbox.path(), "fail.sh", &script_body)?; let request = request( From 7148db4a174fdf6e909d0ee5dbe12a4f25440fff Mon Sep 17 00:00:00 2001 From: leynos Date: Sat, 4 Jul 2026 00:47:35 +0200 Subject: [PATCH 8/8] Narrow shared cluster failure support Move raw environment mutation helpers into a small support module so the failure-cache test no longer compiles unused shared helpers or hides dead-code warnings. --- tests/shared_cluster_handle_failure.rs | 10 +++------- tests/support/env_mutation.rs | 22 ++++++++++++++++++++++ 2 files changed, 25 insertions(+), 7 deletions(-) create mode 100644 tests/support/env_mutation.rs diff --git a/tests/shared_cluster_handle_failure.rs b/tests/shared_cluster_handle_failure.rs index e5b1524e..9e31d7b0 100644 --- a/tests/shared_cluster_handle_failure.rs +++ b/tests/shared_cluster_handle_failure.rs @@ -17,14 +17,10 @@ use pg_embedded_setup_unpriv::{ }; use tracing::warn; -#[expect(dead_code, reason = "required by env_isolation module")] -#[path = "support/env.rs"] -mod env; -#[expect(dead_code, reason = "only set_env_var and remove_env_var are used")] -#[path = "support/env_isolation.rs"] -mod env_isolation; +#[path = "support/env_mutation.rs"] +mod env_mutation; -use env_isolation::{remove_env_var, set_env_var}; +use env_mutation::{remove_env_var, set_env_var}; /// Sets up the environment to force bootstrap failure. /// diff --git a/tests/support/env_mutation.rs b/tests/support/env_mutation.rs new file mode 100644 index 00000000..bb31fd3c --- /dev/null +++ b/tests/support/env_mutation.rs @@ -0,0 +1,22 @@ +//! Low-level process environment mutation helpers for isolated tests. + +use std::ffi::OsStr; + +/// Sets an environment variable whilst bypassing nightly's lint. +pub unsafe fn set_env_var(key: K, value: V) +where + K: AsRef, + V: AsRef, +{ + // SAFETY: callers must serialise environment mutations; enforced at call sites. + unsafe { std::env::set_var(key, value) }; +} + +/// Removes an environment variable whilst bypassing nightly's lint. +pub unsafe fn remove_env_var(key: K) +where + K: AsRef, +{ + // SAFETY: callers must serialise environment mutations; enforced at call sites. + unsafe { std::env::remove_var(key) }; +}