From 2e8608c83676cd99a3c664dc9ce18ada21266cd6 Mon Sep 17 00:00:00 2001 From: Leynos Date: Sat, 26 Jul 2025 10:43:17 +0100 Subject: [PATCH 1/8] Add daemon config loader with ortho-config --- Cargo.lock | 303 +++++++++++++++++++++++++++++++++- Cargo.toml | 4 +- crates/comenqd/Cargo.toml | 9 + crates/comenqd/src/config.rs | 85 ++++++++++ crates/comenqd/src/lib.rs | 3 + crates/comenqd/src/main.rs | 9 +- docs/comenq-design.md | 4 + docs/roadmap.md | 2 +- tests/cucumber.rs | 7 +- tests/features/config.feature | 11 ++ tests/steps/config_steps.rs | 57 +++++++ tests/steps/mod.rs | 4 + 12 files changed, 487 insertions(+), 11 deletions(-) create mode 100644 crates/comenqd/src/config.rs create mode 100644 crates/comenqd/src/lib.rs create mode 100644 tests/features/config.feature create mode 100644 tests/steps/config_steps.rs diff --git a/Cargo.lock b/Cargo.lock index af97bd2..577b603 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -114,6 +114,15 @@ dependencies = [ "syn", ] +[[package]] +name = "atomic" +version = "0.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a89cbf775b137e9b968e67227ef7f775587cde3fd31b0d8599dbd0f598a48340" +dependencies = [ + "bytemuck", +] + [[package]] name = "autocfg" version = "1.5.0" @@ -175,6 +184,12 @@ version = "0.6.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "175812e0be2bccb6abe50bb8d566126198344f707e304f45c648fd8f2cc0365e" +[[package]] +name = "bytemuck" +version = "1.23.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5c76a5792e44e4abe34d3abf15636779261d45a7450612059293d1d2cfc63422" + [[package]] name = "bytes" version = "1.10.1" @@ -219,6 +234,18 @@ dependencies = [ "clap_derive", ] +[[package]] +name = "clap-dispatch" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0a558b9547b590c876e46e301da15d3b0e93b0384fd50d2c7870f7ea86760df5" +dependencies = [ + "heck 0.5.0", + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "clap_builder" version = "4.5.41" @@ -275,7 +302,9 @@ version = "0.1.0" dependencies = [ "clap", "comenq", + "comenqd", "cucumber", + "ortho_config", "serde", "serde_json", "tempfile", @@ -289,9 +318,13 @@ dependencies = [ "anyhow", "clap", "comenq-lib", + "figment", "octocrab", + "ortho_config", + "rstest", "serde", "serde_json", + "tempfile", "thiserror 1.0.69", "tokio", "tracing", @@ -442,6 +475,27 @@ dependencies = [ "syn", ] +[[package]] +name = "directories" +version = "6.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "16f5094c54661b38d03bd7e50df373292118db60b585c08a411c6d840017fe7d" +dependencies = [ + "dirs-sys", +] + +[[package]] +name = "dirs-sys" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e01a3366d27ee9890022452ee61b2b63a67e6f13f58900b651ff5665f0bb1fab" +dependencies = [ + "libc", + "option-ext", + "redox_users", + "windows-sys 0.60.2", +] + [[package]] name = "displaydoc" version = "0.2.5" @@ -471,6 +525,12 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "34aa73646ffb006b8f5147f3dc182bd4bcb190227ce861fc4a4844bf8e3cb2c0" +[[package]] +name = "equivalent" +version = "1.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "877a4ace8713b0bcf2a4e7eec82529c029f1d0619886d18145fea96c3ffe5c0f" + [[package]] name = "errno" version = "0.3.13" @@ -478,7 +538,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "778e2ac28f6c47af28e4907f13ffd1e1ddbd400980a9abd7c8df189bf578a5ad" dependencies = [ "libc", - "windows-sys 0.60.2", + "windows-sys 0.52.0", ] [[package]] @@ -594,6 +654,12 @@ version = "0.3.31" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f90f7dce0722e95104fcb095585910c0977252f286e354b5e3bd38902cd99988" +[[package]] +name = "futures-timer" +version = "3.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f288b0a4f20f9a56b5d1da57e2227c661b7b16168e2f72365f57b63326e29b24" + [[package]] name = "futures-util" version = "0.3.31" @@ -660,6 +726,12 @@ version = "0.31.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "07e28edb80900c19c28f1072f2e8aeca7fa06b23cd4169cefe1af5aa3260783f" +[[package]] +name = "glob" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a8d1add55171497b4705a648c6b583acafb01d58050a51727785f0b2c8e0a2b2" + [[package]] name = "globset" version = "0.4.16" @@ -684,6 +756,12 @@ dependencies = [ "walkdir", ] +[[package]] +name = "hashbrown" +version = "0.15.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5971ac85611da7067dbfcabef3c70ebb5606018acd9e2a3903a0da507521e0d5" + [[package]] name = "heck" version = "0.4.1" @@ -961,12 +1039,28 @@ dependencies = [ "winapi-util", ] +[[package]] +name = "indexmap" +version = "2.10.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fe4cd85333e22411419a0bcae1297d25e58c9443848b11dc6a86fefe8c78a661" +dependencies = [ + "equivalent", + "hashbrown", +] + [[package]] name = "inflections" version = "1.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a257582fdcde896fd96463bf2d40eefea0580021c0712a0e2b028b60b47a837a" +[[package]] +name = "inlinable_string" +version = "0.1.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c8fae54786f62fb2918dcfae3d568594e50eb9b5c25bf04371af6fe7516452fb" + [[package]] name = "inotify" version = "0.9.6" @@ -1374,6 +1468,39 @@ version = "0.1.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d05e27ee213611ffe7d6348b942e8f942b37114c00cc03cec254295a4a17852e" +[[package]] +name = "option-ext" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "04744f49eae99ab78e0d5c0b603ab218f515ea8cfe5a456d7629ad883a3b6e7d" + +[[package]] +name = "ortho_config" +version = "0.3.0" +source = "git+https://github.com/leynos/ortho-config.git?tag=v0.4.0#5c27310a915795b9e7c7749c8e38111291aa6976" +dependencies = [ + "clap", + "clap-dispatch", + "directories", + "figment", + "ortho_config_macros", + "serde", + "thiserror 1.0.69", + "toml", + "uncased", + "xdg", +] + +[[package]] +name = "ortho_config_macros" +version = "0.3.0" +source = "git+https://github.com/leynos/ortho-config.git?tag=v0.4.0#5c27310a915795b9e7c7749c8e38111291aa6976" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "overload" version = "0.1.1" @@ -1403,6 +1530,29 @@ dependencies = [ "windows-targets 0.52.6", ] +[[package]] +name = "pear" +version = "0.2.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bdeeaa00ce488657faba8ebf44ab9361f9365a97bd39ffb8a60663f57ff4b467" +dependencies = [ + "inlinable_string", + "pear_codegen", + "yansi", +] + +[[package]] +name = "pear_codegen" +version = "0.2.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4bab5b985dc082b345f812b7df84e1bef27e7207b39e448439ba8bd69c93f147" +dependencies = [ + "proc-macro2", + "proc-macro2-diagnostics", + "quote", + "syn", +] + [[package]] name = "peg" version = "0.6.3" @@ -1511,6 +1661,19 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "proc-macro2-diagnostics" +version = "0.10.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "af066a9c399a26e020ada66a034357a868728e72cd426f3adcd35f80d88d88c8" +dependencies = [ + "proc-macro2", + "quote", + "syn", + "version_check", + "yansi", +] + [[package]] name = "quote" version = "1.0.40" @@ -1565,6 +1728,17 @@ dependencies = [ "bitflags 2.9.1", ] +[[package]] +name = "redox_users" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dd6f9d3d47bdd2ad6945c5015a226ec6155d0bcdfd8f7cd29f86b71f8de99d2b" +dependencies = [ + "getrandom 0.2.16", + "libredox", + "thiserror 2.0.12", +] + [[package]] name = "regex" version = "1.11.1" @@ -1615,6 +1789,12 @@ version = "0.8.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2b15c43186be67a4fd63bee50d0303afffcef381492ebe2c5d87f324e1b8815c" +[[package]] +name = "relative-path" +version = "1.9.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ba39f3699c378cd8970968dcbff9c43159ea4cfbd88d43c00b22f2ef10a435d2" + [[package]] name = "ring" version = "0.17.14" @@ -1629,12 +1809,50 @@ dependencies = [ "windows-sys 0.52.0", ] +[[package]] +name = "rstest" +version = "0.18.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "97eeab2f3c0a199bc4be135c36c924b6590b88c377d416494288c14f2db30199" +dependencies = [ + "futures", + "futures-timer", + "rstest_macros", + "rustc_version", +] + +[[package]] +name = "rstest_macros" +version = "0.18.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d428f8247852f894ee1be110b375111b586d4fa431f6c46e64ba5a0dcccbe605" +dependencies = [ + "cfg-if", + "glob", + "proc-macro2", + "quote", + "regex", + "relative-path", + "rustc_version", + "syn", + "unicode-ident", +] + [[package]] name = "rustc-demangle" version = "0.1.25" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "989e6739f80c4ad5b13e0fd7fe89531180375b18520cc8c82080e4dc4035b84f" +[[package]] +name = "rustc_version" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cfcb3a22ef46e85b45de6ee7e79d063319ebb6594faafcf1c225ea92ab6e9b92" +dependencies = [ + "semver", +] + [[package]] name = "rustix" version = "1.0.8" @@ -1645,7 +1863,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys", - "windows-sys 0.60.2", + "windows-sys 0.52.0", ] [[package]] @@ -1832,6 +2050,15 @@ dependencies = [ "serde", ] +[[package]] +name = "serde_spanned" +version = "0.6.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bf41e0cfaf7226dca15e8197172c295a782857fcb97fad1808a166870dee75a3" +dependencies = [ + "serde", +] + [[package]] name = "serde_urlencoded" version = "0.7.1" @@ -1924,7 +2151,7 @@ version = "0.8.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1961e2ef424c1424204d3a5d6975f934f56b6d50ff5732382d84ebf460e147f7" dependencies = [ - "heck 0.5.0", + "heck 0.4.1", "proc-macro2", "quote", "syn", @@ -2216,6 +2443,47 @@ dependencies = [ "tokio", ] +[[package]] +name = "toml" +version = "0.8.23" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dc1beb996b9d83529a9e75c17a1686767d148d70663143c7854d8b4a09ced362" +dependencies = [ + "serde", + "serde_spanned", + "toml_datetime", + "toml_edit", +] + +[[package]] +name = "toml_datetime" +version = "0.6.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "22cddaf88f4fbc13c51aebbf5f8eceb5c7c5a9da2ac40a13519eb5b0a0e8f11c" +dependencies = [ + "serde", +] + +[[package]] +name = "toml_edit" +version = "0.22.27" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "41fe8c660ae4257887cf66394862d21dbca4a6ddd26f04a3560410406a2f819a" +dependencies = [ + "indexmap", + "serde", + "serde_spanned", + "toml_datetime", + "toml_write", + "winnow", +] + +[[package]] +name = "toml_write" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5d99f8c9a7727884afe522e9bd5edbfc91a3312b36a77b5fb8926e4c31a41801" + [[package]] name = "tower" version = "0.4.13" @@ -2353,6 +2621,15 @@ dependencies = [ "syn", ] +[[package]] +name = "uncased" +version = "0.9.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e1b88fcfe09e89d3866a5c11019378088af2d24c3fbd4f0543f96b479ec90697" +dependencies = [ + "version_check", +] + [[package]] name = "unicode-ident" version = "1.0.18" @@ -2407,6 +2684,12 @@ version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ba73ea9cf16a25df0c8caa16c51acb937d5712a8429db78a3ee29d5dcacd3a65" +[[package]] +name = "version_check" +version = "0.9.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0b928f33d975fc6ad9f86c8f283853ad26bdd5b10b7f1542aa2fa15e2289105a" + [[package]] name = "walkdir" version = "2.5.0" @@ -2521,7 +2804,7 @@ version = "0.1.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cf221c93e13a30d793f7645a0e7762c55d169dbb0a49671918a2319d289b10bb" dependencies = [ - "windows-sys 0.59.0", + "windows-sys 0.48.0", ] [[package]] @@ -2891,6 +3174,18 @@ version = "0.6.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ea2f10b9bb0928dfb1b42b65e1f9e36f7f54dbdf08457afefb38afcdec4fa2bb" +[[package]] +name = "xdg" +version = "3.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2fb433233f2df9344722454bc7e96465c9d03bff9d77c248f9e7523fe79585b5" + +[[package]] +name = "yansi" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cfe53a6657fd280eaa890a3bc59152892ffa3e30101319d168b781ed6529b049" + [[package]] name = "yaque" version = "0.6.6" diff --git a/Cargo.toml b/Cargo.toml index 8b88b60..5f7f305 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,6 +15,8 @@ cucumber = "0.20" tokio = { workspace = true } clap = { workspace = true } comenq = { path = "crates/comenq" } +comenqd = { path = "crates/comenqd" } +ortho_config = { workspace = true } tempfile = { workspace = true } [[test]] @@ -39,7 +41,7 @@ tracing = "0.1" tracing-subscriber = { version = "0.3", features = ["env-filter"] } anyhow = "1.0" thiserror = "1.0" -tempfile = "3" +ortho_config = { git = "https://github.com/leynos/ortho-config.git", tag = "v0.4.0" } [lints.clippy] pedantic = { level = "warn", priority = -1 } diff --git a/crates/comenqd/Cargo.toml b/crates/comenqd/Cargo.toml index 5819566..65af17f 100644 --- a/crates/comenqd/Cargo.toml +++ b/crates/comenqd/Cargo.toml @@ -3,6 +3,9 @@ name = "comenqd" version = "0.1.0" edition = "2024" +[lib] +path = "src/lib.rs" + [dependencies] tokio = { workspace = true } clap = { workspace = true } @@ -15,3 +18,9 @@ tracing-subscriber = { workspace = true } anyhow = { workspace = true } thiserror = { workspace = true } comenq-lib = { path = "../.." } +ortho_config = { workspace = true } +figment = { version = "0.10", default-features = false, features = ["env", "toml"] } + +[dev-dependencies] +rstest = "0.18" +tempfile = "3" diff --git a/crates/comenqd/src/config.rs b/crates/comenqd/src/config.rs new file mode 100644 index 0000000..52466c0 --- /dev/null +++ b/crates/comenqd/src/config.rs @@ -0,0 +1,85 @@ +//! Configuration loading for the Comenqd daemon. +//! +//! The configuration is stored in `/etc/comenqd/config.toml`. Values may be +//! overridden by environment variables using the `COMENQD_` prefix. + +use figment::providers::Env; +use serde::Deserialize; +use std::io; +use std::path::{Path, PathBuf}; + +/// Runtime configuration for the daemon. +#[derive(Debug, Deserialize, PartialEq, Eq)] +pub struct Config { + /// GitHub Personal Access Token. + pub github_token: String, + /// Path to the Unix Domain Socket. + #[serde(default = "default_socket_path")] + pub socket_path: PathBuf, + /// Directory for the persistent queue. + #[serde(default = "default_queue_path")] + pub queue_path: PathBuf, +} + +fn default_socket_path() -> PathBuf { + PathBuf::from("/run/comenq/comenq.sock") +} + +fn default_queue_path() -> PathBuf { + PathBuf::from("/var/lib/comenq/queue") +} + +impl Config { + /// Default location of the daemon configuration file. + pub const DEFAULT_PATH: &'static str = "/etc/comenqd/config.toml"; + + /// Load the configuration from `DEFAULT_PATH`. + #[allow(clippy::result_large_err)] + pub fn load() -> Result { + Self::from_file(Path::new(Self::DEFAULT_PATH)) + } + + /// Load the configuration from the specified path, merging `COMENQD_*` + /// environment variables over file values. + #[allow(clippy::result_large_err)] + pub fn from_file(path: &Path) -> Result { + let mut fig = ortho_config::load_config_file(path)?.ok_or_else(|| { + ortho_config::OrthoError::File { + path: path.to_path_buf(), + source: Box::new(io::Error::from(io::ErrorKind::NotFound)), + } + })?; + fig = fig.merge(Env::prefixed("COMENQD_").split("__")); + fig.extract().map_err(ortho_config::OrthoError::from) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use rstest::rstest; + use std::fs; + use tempfile::tempdir; + + #[rstest] + fn loads_from_file() { + let dir = tempdir().unwrap(); + let path = dir.path().join("config.toml"); + fs::write( + &path, + "github_token='abc'\nsocket_path='/tmp/s.sock'\nqueue_path='/tmp/q'", + ) + .unwrap(); + let cfg = Config::from_file(&path).unwrap(); + assert_eq!(cfg.github_token, "abc"); + assert_eq!(cfg.socket_path, PathBuf::from("/tmp/s.sock")); + assert_eq!(cfg.queue_path, PathBuf::from("/tmp/q")); + } + + #[rstest] + fn error_when_missing_file() { + let path = PathBuf::from("/nonexistent/file.toml"); + let res = Config::from_file(&path); + assert!(res.is_err()); + } +} diff --git a/crates/comenqd/src/lib.rs b/crates/comenqd/src/lib.rs new file mode 100644 index 0000000..ad5200a --- /dev/null +++ b/crates/comenqd/src/lib.rs @@ -0,0 +1,3 @@ +//! Library components for the Comenqd daemon. + +pub mod config; diff --git a/crates/comenqd/src/main.rs b/crates/comenqd/src/main.rs index 367988a..d6ebb65 100644 --- a/crates/comenqd/src/main.rs +++ b/crates/comenqd/src/main.rs @@ -4,7 +4,12 @@ use tracing::info; -fn main() { +mod config; +use config::Config; + +fn main() -> Result<(), ortho_config::OrthoError> { tracing_subscriber::fmt::init(); - info!("Comenqd daemon started"); + let cfg = Config::load()?; + info!(socket = ?cfg.socket_path, queue = ?cfg.queue_path, "Comenqd daemon started"); + Ok(()) } diff --git a/docs/comenq-design.md b/docs/comenq-design.md index 43fb0ea..92232ed 100644 --- a/docs/comenq-design.md +++ b/docs/comenq-design.md @@ -513,6 +513,10 @@ at `/etc/comenqd/config.toml` is the conventional choice. | log_level | String | The minimum log level to record (e.g., "info", "debug", "trace"). | info | | cooldown_period_seconds | u64 | The cooling-off period in seconds after each comment post. | 900 | +Configuration is loaded using the `ortho_config` crate. The daemon calls +`Config::load()` which merges values from `/etc/comenqd/config.toml`, +`COMENQD_*` environment variables, and any supplied CLI arguments. + Robust logging is non-negotiable for a background process. The `tracing` crate with `tracing-subscriber` will be used to provide structured, asynchronous logging. Key events to be logged include: diff --git a/docs/roadmap.md b/docs/roadmap.md index 4624226..e5573af 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -30,7 +30,7 @@ ## Milestone 3: `comenqd` Daemon Core -- [ ] Implement configuration loading from a TOML file +- [x] Implement configuration loading from a TOML file (`/etc/comenqd/config.toml`) for parameters like `github_token`, `socket_path`, and `queue_path`. diff --git a/tests/cucumber.rs b/tests/cucumber.rs index 1d165c0..e0ff087 100644 --- a/tests/cucumber.rs +++ b/tests/cucumber.rs @@ -1,12 +1,13 @@ mod steps; use cucumber::World as _; -use steps::{CliWorld, ClientWorld, CommentWorld}; +use steps::{CliWorld, ClientWorld, CommentWorld, ConfigWorld}; #[tokio::main] async fn main() { tokio::join!( - CommentWorld::run("tests/features/comment_request.feature"), CliWorld::run("tests/features/cli.feature"), - ClientWorld::run("tests/features/client_main.feature") + ClientWorld::run("tests/features/client_main.feature"), + CommentWorld::run("tests/features/comment_request.feature"), + ConfigWorld::run("tests/features/config.feature"), ); } diff --git a/tests/features/config.feature b/tests/features/config.feature new file mode 100644 index 0000000..582b0a5 --- /dev/null +++ b/tests/features/config.feature @@ -0,0 +1,11 @@ +Feature: Daemon configuration + + Scenario: loading a valid configuration file + Given a configuration file with token "abc" + When the config is loaded + Then github token is "abc" + + Scenario: missing configuration file + Given a missing configuration file + When the config is loaded + Then config loading fails diff --git a/tests/steps/config_steps.rs b/tests/steps/config_steps.rs new file mode 100644 index 0000000..c9222e9 --- /dev/null +++ b/tests/steps/config_steps.rs @@ -0,0 +1,57 @@ +//! Behavioural steps for daemon configuration loading. +#![allow( + clippy::expect_used, + clippy::needless_pass_by_value, + clippy::uninlined_format_args, + reason = "simplify test failure output" +)] + +use cucumber::{World, given, then, when}; +use std::fs; +use std::path::PathBuf; +use tempfile::TempDir; + +use comenqd::config::Config; + +#[derive(Debug, Default, World)] +pub struct ConfigWorld { + dir: Option, + path: Option, + result: Option>, +} + +#[given(regex = r#"^a configuration file with token \"(.+)\"$"#)] +fn config_file_with_token(world: &mut ConfigWorld, token: String) { + let dir = TempDir::new().expect("create temp dir"); + let path = dir.path().join("config.toml"); + fs::write(&path, format!("github_token='{token}'")).expect("write file"); + world.dir = Some(dir); + world.path = Some(path); +} + +#[given("a missing configuration file")] +fn missing_configuration_file(world: &mut ConfigWorld) { + world.path = Some(PathBuf::from("/nonexistent/nowhere.toml")); +} + +#[when("the config is loaded")] +fn load_config(world: &mut ConfigWorld) { + let path = world.path.as_ref().expect("path set"); + world.result = Some(Config::from_file(path)); +} + +#[then(regex = r#"^github token is \"(.+)\"$"#)] +fn github_token_is(world: &mut ConfigWorld, expected: String) { + match world.result.take() { + Some(Ok(cfg)) => assert_eq!(cfg.github_token, expected), + other => panic!("expected success, got {other:?}"), + } +} + +#[then("config loading fails")] +fn config_loading_fails(world: &mut ConfigWorld) { + match world.result.take() { + Some(Err(_)) => {} + other => panic!("expected error, got {other:?}"), + } +} diff --git a/tests/steps/mod.rs b/tests/steps/mod.rs index 918d9ae..b40fa18 100644 --- a/tests/steps/mod.rs +++ b/tests/steps/mod.rs @@ -4,3 +4,7 @@ pub mod cli_steps; pub use cli_steps::CliWorld; pub mod comment_steps; pub use comment_steps::CommentWorld; +pub mod cli_steps; +pub use cli_steps::CliWorld; +pub mod config_steps; +pub use config_steps::ConfigWorld; From 4db5da569ad2a602fcac986d9129382cc4123205 Mon Sep 17 00:00:00 2001 From: Leynos Date: Sat, 26 Jul 2025 14:31:49 +0100 Subject: [PATCH 2/8] Add CLI and env merging to daemon config --- Cargo.lock | 55 ++++++++++++++++++++++++--- crates/comenqd/Cargo.toml | 1 + crates/comenqd/src/config.rs | 70 ++++++++++++++++++++++++++++++++--- tests/features/config.feature | 6 +++ tests/steps/config_steps.rs | 27 ++++++++++++++ 5 files changed, 148 insertions(+), 11 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 577b603..f456bed 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -324,6 +324,7 @@ dependencies = [ "rstest", "serde", "serde_json", + "serial_test", "tempfile", "thiserror 1.0.69", "tokio", @@ -455,6 +456,19 @@ dependencies = [ "regex-syntax 0.7.5", ] +[[package]] +name = "dashmap" +version = "5.5.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "978747c1d849a7d2ee5e8adc0159961c48fb7e5db2f06af6723b80123bb53856" +dependencies = [ + "cfg-if", + "hashbrown 0.14.5", + "lock_api", + "once_cell", + "parking_lot_core", +] + [[package]] name = "deranged" version = "0.4.0" @@ -538,7 +552,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "778e2ac28f6c47af28e4907f13ffd1e1ddbd400980a9abd7c8df189bf578a5ad" dependencies = [ "libc", - "windows-sys 0.52.0", + "windows-sys 0.60.2", ] [[package]] @@ -756,6 +770,12 @@ dependencies = [ "walkdir", ] +[[package]] +name = "hashbrown" +version = "0.14.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e5274423e17b7c9fc20b6e7e208532f9b19825d82dfd615708b70edd83df41f1" + [[package]] name = "hashbrown" version = "0.15.4" @@ -1046,7 +1066,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "fe4cd85333e22411419a0bcae1297d25e58c9443848b11dc6a86fefe8c78a661" dependencies = [ "equivalent", - "hashbrown", + "hashbrown 0.15.4", ] [[package]] @@ -1863,7 +1883,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys", - "windows-sys 0.52.0", + "windows-sys 0.60.2", ] [[package]] @@ -2071,6 +2091,31 @@ dependencies = [ "serde", ] +[[package]] +name = "serial_test" +version = "2.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0e56dd856803e253c8f298af3f4d7eb0ae5e23a737252cd90bb4f3b435033b2d" +dependencies = [ + "dashmap", + "futures", + "lazy_static", + "log", + "parking_lot", + "serial_test_derive", +] + +[[package]] +name = "serial_test_derive" +version = "2.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "91d129178576168c589c9ec973feedf7d3126c01ac2bf08795109aa35b69fb8f" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "sharded-slab" version = "0.1.7" @@ -2151,7 +2196,7 @@ version = "0.8.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1961e2ef424c1424204d3a5d6975f934f56b6d50ff5732382d84ebf460e147f7" dependencies = [ - "heck 0.4.1", + "heck 0.5.0", "proc-macro2", "quote", "syn", @@ -2804,7 +2849,7 @@ version = "0.1.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cf221c93e13a30d793f7645a0e7762c55d169dbb0a49671918a2319d289b10bb" dependencies = [ - "windows-sys 0.48.0", + "windows-sys 0.59.0", ] [[package]] diff --git a/crates/comenqd/Cargo.toml b/crates/comenqd/Cargo.toml index 65af17f..3308650 100644 --- a/crates/comenqd/Cargo.toml +++ b/crates/comenqd/Cargo.toml @@ -24,3 +24,4 @@ figment = { version = "0.10", default-features = false, features = ["env", "toml [dev-dependencies] rstest = "0.18" tempfile = "3" +serial_test = "2" diff --git a/crates/comenqd/src/config.rs b/crates/comenqd/src/config.rs index 52466c0..32f9523 100644 --- a/crates/comenqd/src/config.rs +++ b/crates/comenqd/src/config.rs @@ -3,13 +3,14 @@ //! The configuration is stored in `/etc/comenqd/config.toml`. Values may be //! overridden by environment variables using the `COMENQD_` prefix. +use clap::Parser; use figment::providers::Env; -use serde::Deserialize; +use serde::{Deserialize, Serialize}; use std::io; use std::path::{Path, PathBuf}; /// Runtime configuration for the daemon. -#[derive(Debug, Deserialize, PartialEq, Eq)] +#[derive(Debug, Deserialize, Serialize, PartialEq, Eq)] pub struct Config { /// GitHub Personal Access Token. pub github_token: String, @@ -21,6 +22,23 @@ pub struct Config { pub queue_path: PathBuf, } +/// Command-line overrides for configuration values. +#[derive(Debug, Default, Parser, Serialize)] +struct CliArgs { + /// Path to the configuration file. + #[arg(short, long, value_name = "FILE", default_value = Config::DEFAULT_PATH)] + config: PathBuf, + /// GitHub Personal Access Token. + #[arg(long)] + github_token: Option, + /// Override the Unix Domain Socket path. + #[arg(long)] + socket_path: Option, + /// Override the queue directory. + #[arg(long)] + queue_path: Option, +} + fn default_socket_path() -> PathBuf { PathBuf::from("/run/comenq/comenq.sock") } @@ -33,24 +51,43 @@ impl Config { /// Default location of the daemon configuration file. pub const DEFAULT_PATH: &'static str = "/etc/comenqd/config.toml"; - /// Load the configuration from `DEFAULT_PATH`. + /// Load the configuration using command-line overrides and environment + /// variables. #[allow(clippy::result_large_err)] pub fn load() -> Result { - Self::from_file(Path::new(Self::DEFAULT_PATH)) + let args = CliArgs::parse(); + Self::from_file_with_cli(&args.config, &args) } /// Load the configuration from the specified path, merging `COMENQD_*` - /// environment variables over file values. + /// environment variables and CLI arguments over file values. #[allow(clippy::result_large_err)] pub fn from_file(path: &Path) -> Result { + Self::from_file_with_cli(path, &CliArgs::default()) + } + + #[allow(clippy::result_large_err)] + fn from_file_with_cli(path: &Path, cli: &CliArgs) -> Result { let mut fig = ortho_config::load_config_file(path)?.ok_or_else(|| { ortho_config::OrthoError::File { path: path.to_path_buf(), source: Box::new(io::Error::from(io::ErrorKind::NotFound)), } })?; + fig = fig.merge(Env::prefixed("COMENQD_").split("__")); - fig.extract().map_err(ortho_config::OrthoError::from) + let mut cfg: Self = fig.extract().map_err(ortho_config::OrthoError::from)?; + + if let Some(token) = &cli.github_token { + cfg.github_token = token.clone(); + } + if let Some(socket) = &cli.socket_path { + cfg.socket_path = socket.clone(); + } + if let Some(queue) = &cli.queue_path { + cfg.queue_path = queue.clone(); + } + Ok(cfg) } } @@ -62,6 +99,7 @@ mod tests { use tempfile::tempdir; #[rstest] + #[serial_test::serial] fn loads_from_file() { let dir = tempdir().unwrap(); let path = dir.path().join("config.toml"); @@ -70,6 +108,9 @@ mod tests { "github_token='abc'\nsocket_path='/tmp/s.sock'\nqueue_path='/tmp/q'", ) .unwrap(); + unsafe { + std::env::remove_var("COMENQD_SOCKET_PATH"); + } let cfg = Config::from_file(&path).unwrap(); assert_eq!(cfg.github_token, "abc"); assert_eq!(cfg.socket_path, PathBuf::from("/tmp/s.sock")); @@ -77,9 +118,26 @@ mod tests { } #[rstest] + #[serial_test::serial] fn error_when_missing_file() { let path = PathBuf::from("/nonexistent/file.toml"); let res = Config::from_file(&path); assert!(res.is_err()); } + + #[rstest] + #[serial_test::serial] + fn env_vars_override_file() { + let dir = tempdir().unwrap(); + let path = dir.path().join("config.toml"); + fs::write(&path, "github_token='abc'\nsocket_path='/tmp/s.sock'").unwrap(); + unsafe { + std::env::set_var("COMENQD_SOCKET_PATH", "/tmp/override.sock"); + } + let cfg = Config::from_file(&path).unwrap(); + unsafe { + std::env::remove_var("COMENQD_SOCKET_PATH"); + } + assert_eq!(cfg.socket_path, PathBuf::from("/tmp/override.sock")); + } } diff --git a/tests/features/config.feature b/tests/features/config.feature index 582b0a5..38aef14 100644 --- a/tests/features/config.feature +++ b/tests/features/config.feature @@ -9,3 +9,9 @@ Feature: Daemon configuration Given a missing configuration file When the config is loaded Then config loading fails + + Scenario: environment variable overrides file + Given a configuration file with token "abc" + And environment variable "COMENQD_SOCKET_PATH" is "/tmp/env.sock" + When the config is loaded + Then socket path is "/tmp/env.sock" diff --git a/tests/steps/config_steps.rs b/tests/steps/config_steps.rs index c9222e9..ddb771c 100644 --- a/tests/steps/config_steps.rs +++ b/tests/steps/config_steps.rs @@ -18,6 +18,7 @@ pub struct ConfigWorld { dir: Option, path: Option, result: Option>, + env_key: Option, } #[given(regex = r#"^a configuration file with token \"(.+)\"$"#)] @@ -34,6 +35,14 @@ fn missing_configuration_file(world: &mut ConfigWorld) { world.path = Some(PathBuf::from("/nonexistent/nowhere.toml")); } +#[given(regex = r#"^environment variable \"(.+)\" is \"(.+)\"$"#)] +fn set_env_var(world: &mut ConfigWorld, key: String, value: String) { + unsafe { + std::env::set_var(&key, &value); + } + world.env_key = Some(key); +} + #[when("the config is loaded")] fn load_config(world: &mut ConfigWorld) { let path = world.path.as_ref().expect("path set"); @@ -55,3 +64,21 @@ fn config_loading_fails(world: &mut ConfigWorld) { other => panic!("expected error, got {other:?}"), } } + +#[then(regex = r#"^socket path is \"(.+)\"$"#)] +fn socket_path_is(world: &mut ConfigWorld, expected: String) { + match world.result.take() { + Some(Ok(cfg)) => assert_eq!(cfg.socket_path, PathBuf::from(expected)), + other => panic!("expected success, got {other:?}"), + } +} + +impl Drop for ConfigWorld { + fn drop(&mut self) { + if let Some(ref key) = self.env_key { + unsafe { + std::env::remove_var(key); + } + } + } +} From 786523c2973cea73ad42905873140d122d8fa853 Mon Sep 17 00:00:00 2001 From: Leynos Date: Sat, 26 Jul 2025 14:58:56 +0100 Subject: [PATCH 3/8] Add config error tests and serial scenarios --- Cargo.toml | 2 +- crates/comenqd/src/config.rs | 31 +++++++++++++++++++++++++++++ docs/comenq-design.md | 6 +++++- docs/roadmap.md | 2 +- tests/features/config.feature | 16 +++++++++++++++ tests/steps/config_steps.rs | 37 +++++++++++++++++++++++++++++++++++ 6 files changed, 91 insertions(+), 3 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 5f7f305..1e24f91 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,7 +17,7 @@ clap = { workspace = true } comenq = { path = "crates/comenq" } comenqd = { path = "crates/comenqd" } ortho_config = { workspace = true } -tempfile = { workspace = true } +tempfile = "3" [[test]] name = "cucumber" diff --git a/crates/comenqd/src/config.rs b/crates/comenqd/src/config.rs index 32f9523..d9ed62b 100644 --- a/crates/comenqd/src/config.rs +++ b/crates/comenqd/src/config.rs @@ -140,4 +140,35 @@ mod tests { } assert_eq!(cfg.socket_path, PathBuf::from("/tmp/override.sock")); } + + #[rstest] + #[serial_test::serial] + fn error_with_invalid_toml() { + let dir = tempdir().unwrap(); + let path = dir.path().join("config.toml"); + fs::write(&path, "github_token='abc' this is not toml").unwrap(); + let res = Config::from_file(&path); + assert!(res.is_err()); + } + + #[rstest] + #[serial_test::serial] + fn error_when_missing_token() { + let dir = tempdir().unwrap(); + let path = dir.path().join("config.toml"); + fs::write(&path, "socket_path='/tmp/s.sock'").unwrap(); + let res = Config::from_file(&path); + assert!(res.is_err()); + } + + #[rstest] + #[serial_test::serial] + fn defaults_are_applied() { + let dir = tempdir().unwrap(); + let path = dir.path().join("config.toml"); + fs::write(&path, "github_token='abc'").unwrap(); + let cfg = Config::from_file(&path).unwrap(); + assert_eq!(cfg.socket_path, PathBuf::from("/run/comenq/comenq.sock")); + assert_eq!(cfg.queue_path, PathBuf::from("/var/lib/comenq/queue")); + } } diff --git a/docs/comenq-design.md b/docs/comenq-design.md index 92232ed..e0d871e 100644 --- a/docs/comenq-design.md +++ b/docs/comenq-design.md @@ -515,7 +515,11 @@ at `/etc/comenqd/config.toml` is the conventional choice. Configuration is loaded using the `ortho_config` crate. The daemon calls `Config::load()` which merges values from `/etc/comenqd/config.toml`, -`COMENQD_*` environment variables, and any supplied CLI arguments. +`COMENQD_*` environment variables, and any supplied CLI arguments. CLI +arguments have the highest precedence, followed by environment variables, and +finally the configuration file. Missing optional fields are replaced with +defaults, while an absent `github_token` or invalid TOML results in a +configuration error. Robust logging is non-negotiable for a background process. The `tracing` crate with `tracing-subscriber` will be used to provide structured, asynchronous diff --git a/docs/roadmap.md b/docs/roadmap.md index e5573af..7d321d8 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -30,7 +30,7 @@ ## Milestone 3: `comenqd` Daemon Core -- [x] Implement configuration loading from a TOML file +- [x] Implement configuration loading from a TOML file (done) (`/etc/comenqd/config.toml`) for parameters like `github_token`, `socket_path`, and `queue_path`. diff --git a/tests/features/config.feature b/tests/features/config.feature index 38aef14..0e19579 100644 --- a/tests/features/config.feature +++ b/tests/features/config.feature @@ -1,3 +1,4 @@ +@serial Feature: Daemon configuration Scenario: loading a valid configuration file @@ -15,3 +16,18 @@ Feature: Daemon configuration And environment variable "COMENQD_SOCKET_PATH" is "/tmp/env.sock" When the config is loaded Then socket path is "/tmp/env.sock" + + Scenario: invalid TOML syntax + Given an invalid configuration file + When the config is loaded + Then config loading fails + + Scenario: missing required field + Given a configuration file without github_token + When the config is loaded + Then config loading fails + + Scenario: uses default socket path + Given a configuration file with token "abc" and no socket_path + When the config is loaded + Then socket path is "/run/comenq/comenq.sock" diff --git a/tests/steps/config_steps.rs b/tests/steps/config_steps.rs index ddb771c..efcfd93 100644 --- a/tests/steps/config_steps.rs +++ b/tests/steps/config_steps.rs @@ -28,6 +28,43 @@ fn config_file_with_token(world: &mut ConfigWorld, token: String) { fs::write(&path, format!("github_token='{token}'")).expect("write file"); world.dir = Some(dir); world.path = Some(path); + unsafe { + std::env::remove_var("COMENQD_SOCKET_PATH"); + } +} + +#[given("an invalid configuration file")] +fn invalid_configuration_file(world: &mut ConfigWorld) { + let dir = TempDir::new().expect("create temp dir"); + let path = dir.path().join("config.toml"); + fs::write(&path, "github_token='abc' this is not toml").expect("write file"); + world.dir = Some(dir); + world.path = Some(path); +} + +#[given("a configuration file without github_token")] +fn config_file_without_token(world: &mut ConfigWorld) { + let dir = TempDir::new().expect("create temp dir"); + let path = dir.path().join("config.toml"); + fs::write(&path, "socket_path='/tmp/s.sock'").expect("write file"); + world.dir = Some(dir); + world.path = Some(path); +} + +#[given(regex = r#"^a configuration file with token \"(.+)\" and no socket_path$"#)] +fn config_without_socket(world: &mut ConfigWorld, token: String) { + let dir = TempDir::new().expect("create temp dir"); + let path = dir.path().join("config.toml"); + fs::write( + &path, + format!("github_token='{token}'\nqueue_path='/tmp/q'"), + ) + .expect("write file"); + world.dir = Some(dir); + world.path = Some(path); + unsafe { + std::env::remove_var("COMENQD_SOCKET_PATH"); + } } #[given("a missing configuration file")] From 764a7fd6e7c99fe122f72f020f5e34c982bd495e Mon Sep 17 00:00:00 2001 From: Leynos Date: Sat, 26 Jul 2025 15:41:21 +0100 Subject: [PATCH 4/8] Fix config loader and tests --- Cargo.toml | 4 ++-- crates/comenqd/Cargo.toml | 6 ++---- crates/comenqd/src/config.rs | 20 ++++++++++++++------ crates/comenqd/src/lib.rs | 14 ++++++++++++++ tests/steps/config_steps.rs | 27 +++++++++++++++++++++------ tests/steps/mod.rs | 2 ++ 6 files changed, 55 insertions(+), 18 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 1e24f91..d88cede 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,8 +16,8 @@ tokio = { workspace = true } clap = { workspace = true } comenq = { path = "crates/comenq" } comenqd = { path = "crates/comenqd" } -ortho_config = { workspace = true } -tempfile = "3" +ortho_config = { git = "https://github.com/leynos/ortho-config.git", tag = "v0.4.0" } +tempfile = "3.10" # latest 3.x at time of writing; update as new patch versions release [[test]] name = "cucumber" diff --git a/crates/comenqd/Cargo.toml b/crates/comenqd/Cargo.toml index 3308650..d8738fc 100644 --- a/crates/comenqd/Cargo.toml +++ b/crates/comenqd/Cargo.toml @@ -3,8 +3,6 @@ name = "comenqd" version = "0.1.0" edition = "2024" -[lib] -path = "src/lib.rs" [dependencies] tokio = { workspace = true } @@ -22,6 +20,6 @@ ortho_config = { workspace = true } figment = { version = "0.10", default-features = false, features = ["env", "toml"] } [dev-dependencies] -rstest = "0.18" -tempfile = "3" +rstest = "0.18.0" +tempfile = "3.10" # latest 3.x at time of writing; update as new patch versions release serial_test = "2" diff --git a/crates/comenqd/src/config.rs b/crates/comenqd/src/config.rs index d9ed62b..917c0c4 100644 --- a/crates/comenqd/src/config.rs +++ b/crates/comenqd/src/config.rs @@ -9,6 +9,11 @@ use serde::{Deserialize, Serialize}; use std::io; use std::path::{Path, PathBuf}; +/// Default socket path when none is provided. +const DEFAULT_SOCKET_PATH: &str = "/run/comenq/comenq.sock"; +/// Default queue directory when none is provided. +const DEFAULT_QUEUE_PATH: &str = "/var/lib/comenq/queue"; + /// Runtime configuration for the daemon. #[derive(Debug, Deserialize, Serialize, PartialEq, Eq)] pub struct Config { @@ -40,11 +45,11 @@ struct CliArgs { } fn default_socket_path() -> PathBuf { - PathBuf::from("/run/comenq/comenq.sock") + PathBuf::from(DEFAULT_SOCKET_PATH) } fn default_queue_path() -> PathBuf { - PathBuf::from("/var/lib/comenq/queue") + PathBuf::from(DEFAULT_QUEUE_PATH) } impl Config { @@ -53,7 +58,7 @@ impl Config { /// Load the configuration using command-line overrides and environment /// variables. - #[allow(clippy::result_large_err)] + #[expect(clippy::result_large_err, reason = "propagate figment errors")] pub fn load() -> Result { let args = CliArgs::parse(); Self::from_file_with_cli(&args.config, &args) @@ -61,17 +66,20 @@ impl Config { /// Load the configuration from the specified path, merging `COMENQD_*` /// environment variables and CLI arguments over file values. - #[allow(clippy::result_large_err)] + #[expect(clippy::result_large_err, reason = "propagate figment errors")] pub fn from_file(path: &Path) -> Result { Self::from_file_with_cli(path, &CliArgs::default()) } - #[allow(clippy::result_large_err)] + #[expect(clippy::result_large_err, reason = "propagate figment errors")] fn from_file_with_cli(path: &Path, cli: &CliArgs) -> Result { let mut fig = ortho_config::load_config_file(path)?.ok_or_else(|| { ortho_config::OrthoError::File { path: path.to_path_buf(), - source: Box::new(io::Error::from(io::ErrorKind::NotFound)), + source: Box::new(io::Error::new( + io::ErrorKind::NotFound, + "Configuration file not found", + )), } })?; diff --git a/crates/comenqd/src/lib.rs b/crates/comenqd/src/lib.rs index ad5200a..2c7d8f0 100644 --- a/crates/comenqd/src/lib.rs +++ b/crates/comenqd/src/lib.rs @@ -1,3 +1,17 @@ //! Library components for the Comenqd daemon. +//! +//! # Overview +//! This crate exposes: +//! - [`config::Config`] — typed, validated daemon configuration loaded from +//! `/etc/comenqd/config.toml` with environment and CLI overrides. +//! - Further daemon-specific helpers (to be added). +//! +//! # Examples +//! ```rust,no_run +//! use comenqd::config::Config; +//! +//! let cfg = Config::load().expect("configuration must be valid"); +//! println!("socket: {}", cfg.socket_path.display()); +//! ``` pub mod config; diff --git a/tests/steps/config_steps.rs b/tests/steps/config_steps.rs index efcfd93..1e8dcd7 100644 --- a/tests/steps/config_steps.rs +++ b/tests/steps/config_steps.rs @@ -1,10 +1,4 @@ //! Behavioural steps for daemon configuration loading. -#![allow( - clippy::expect_used, - clippy::needless_pass_by_value, - clippy::uninlined_format_args, - reason = "simplify test failure output" -)] use cucumber::{World, given, then, when}; use std::fs; @@ -22,6 +16,11 @@ pub struct ConfigWorld { } #[given(regex = r#"^a configuration file with token \"(.+)\"$"#)] +#[expect(clippy::expect_used, reason = "test setup uses expect")] +#[expect( + clippy::needless_pass_by_value, + reason = "cucumber requires owned values" +)] fn config_file_with_token(world: &mut ConfigWorld, token: String) { let dir = TempDir::new().expect("create temp dir"); let path = dir.path().join("config.toml"); @@ -33,6 +32,7 @@ fn config_file_with_token(world: &mut ConfigWorld, token: String) { } } +#[expect(clippy::expect_used, reason = "test setup uses expect")] #[given("an invalid configuration file")] fn invalid_configuration_file(world: &mut ConfigWorld) { let dir = TempDir::new().expect("create temp dir"); @@ -42,6 +42,7 @@ fn invalid_configuration_file(world: &mut ConfigWorld) { world.path = Some(path); } +#[expect(clippy::expect_used, reason = "test setup uses expect")] #[given("a configuration file without github_token")] fn config_file_without_token(world: &mut ConfigWorld) { let dir = TempDir::new().expect("create temp dir"); @@ -52,6 +53,11 @@ fn config_file_without_token(world: &mut ConfigWorld) { } #[given(regex = r#"^a configuration file with token \"(.+)\" and no socket_path$"#)] +#[expect(clippy::expect_used, reason = "test setup uses expect")] +#[expect( + clippy::needless_pass_by_value, + reason = "cucumber requires owned values" +)] fn config_without_socket(world: &mut ConfigWorld, token: String) { let dir = TempDir::new().expect("create temp dir"); let path = dir.path().join("config.toml"); @@ -72,6 +78,10 @@ fn missing_configuration_file(world: &mut ConfigWorld) { world.path = Some(PathBuf::from("/nonexistent/nowhere.toml")); } +#[expect( + clippy::needless_pass_by_value, + reason = "cucumber requires owned values" +)] #[given(regex = r#"^environment variable \"(.+)\" is \"(.+)\"$"#)] fn set_env_var(world: &mut ConfigWorld, key: String, value: String) { unsafe { @@ -81,12 +91,17 @@ fn set_env_var(world: &mut ConfigWorld, key: String, value: String) { } #[when("the config is loaded")] +#[expect(clippy::expect_used, reason = "test assertions")] fn load_config(world: &mut ConfigWorld) { let path = world.path.as_ref().expect("path set"); world.result = Some(Config::from_file(path)); } #[then(regex = r#"^github token is \"(.+)\"$"#)] +#[expect( + clippy::needless_pass_by_value, + reason = "cucumber requires owned values" +)] fn github_token_is(world: &mut ConfigWorld, expected: String) { match world.result.take() { Some(Ok(cfg)) => assert_eq!(cfg.github_token, expected), diff --git a/tests/steps/mod.rs b/tests/steps/mod.rs index b40fa18..76d0329 100644 --- a/tests/steps/mod.rs +++ b/tests/steps/mod.rs @@ -6,5 +6,7 @@ pub mod comment_steps; pub use comment_steps::CommentWorld; pub mod cli_steps; pub use cli_steps::CliWorld; +pub mod comment_steps; +pub use comment_steps::CommentWorld; pub mod config_steps; pub use config_steps::ConfigWorld; From f4e80c26fd135b9c428a355dae28377ed16548d5 Mon Sep 17 00:00:00 2001 From: Leynos Date: Sat, 26 Jul 2025 16:29:46 +0100 Subject: [PATCH 5/8] Add daemon runtime and fix config tests --- Cargo.toml | 2 +- crates/comenqd/src/config.rs | 30 +++++++++---- crates/comenqd/src/daemon.rs | 86 ++++++++++++++++++++++++++++++++++++ crates/comenqd/src/lib.rs | 1 + crates/comenqd/src/main.rs | 7 ++- docs/comenq-design.md | 4 +- tests/steps/config_steps.rs | 26 ++++++----- tests/steps/mod.rs | 4 +- 8 files changed, 132 insertions(+), 28 deletions(-) create mode 100644 crates/comenqd/src/daemon.rs diff --git a/Cargo.toml b/Cargo.toml index d88cede..15843fb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,7 +16,7 @@ tokio = { workspace = true } clap = { workspace = true } comenq = { path = "crates/comenq" } comenqd = { path = "crates/comenqd" } -ortho_config = { git = "https://github.com/leynos/ortho-config.git", tag = "v0.4.0" } +ortho_config = { workspace = true } tempfile = "3.10" # latest 3.x at time of writing; update as new patch versions release [[test]] diff --git a/crates/comenqd/src/config.rs b/crates/comenqd/src/config.rs index 917c0c4..cae2744 100644 --- a/crates/comenqd/src/config.rs +++ b/crates/comenqd/src/config.rs @@ -13,6 +13,8 @@ use std::path::{Path, PathBuf}; const DEFAULT_SOCKET_PATH: &str = "/run/comenq/comenq.sock"; /// Default queue directory when none is provided. const DEFAULT_QUEUE_PATH: &str = "/var/lib/comenq/queue"; +/// Default cooldown in seconds between comment posts. +const DEFAULT_COOLDOWN: u64 = 900; /// Runtime configuration for the daemon. #[derive(Debug, Deserialize, Serialize, PartialEq, Eq)] @@ -25,6 +27,9 @@ pub struct Config { /// Directory for the persistent queue. #[serde(default = "default_queue_path")] pub queue_path: PathBuf, + /// Cooldown between comment posts in seconds. + #[serde(default = "default_cooldown")] + pub cooldown_period_seconds: u64, } /// Command-line overrides for configuration values. @@ -52,6 +57,10 @@ fn default_queue_path() -> PathBuf { PathBuf::from(DEFAULT_QUEUE_PATH) } +fn default_cooldown() -> u64 { + DEFAULT_COOLDOWN +} + impl Config { /// Default location of the daemon configuration file. pub const DEFAULT_PATH: &'static str = "/etc/comenqd/config.toml"; @@ -106,6 +115,14 @@ mod tests { use std::fs; use tempfile::tempdir; + fn set_env(key: &str, val: &str) { + unsafe { std::env::set_var(key, val); } + } + + fn remove_env(key: &str) { + unsafe { std::env::remove_var(key); } + } + #[rstest] #[serial_test::serial] fn loads_from_file() { @@ -116,9 +133,7 @@ mod tests { "github_token='abc'\nsocket_path='/tmp/s.sock'\nqueue_path='/tmp/q'", ) .unwrap(); - unsafe { - std::env::remove_var("COMENQD_SOCKET_PATH"); - } + remove_env("COMENQD_SOCKET_PATH"); let cfg = Config::from_file(&path).unwrap(); assert_eq!(cfg.github_token, "abc"); assert_eq!(cfg.socket_path, PathBuf::from("/tmp/s.sock")); @@ -139,13 +154,9 @@ mod tests { let dir = tempdir().unwrap(); let path = dir.path().join("config.toml"); fs::write(&path, "github_token='abc'\nsocket_path='/tmp/s.sock'").unwrap(); - unsafe { - std::env::set_var("COMENQD_SOCKET_PATH", "/tmp/override.sock"); - } + set_env("COMENQD_SOCKET_PATH", "/tmp/override.sock"); let cfg = Config::from_file(&path).unwrap(); - unsafe { - std::env::remove_var("COMENQD_SOCKET_PATH"); - } + remove_env("COMENQD_SOCKET_PATH"); assert_eq!(cfg.socket_path, PathBuf::from("/tmp/override.sock")); } @@ -178,5 +189,6 @@ mod tests { let cfg = Config::from_file(&path).unwrap(); assert_eq!(cfg.socket_path, PathBuf::from("/run/comenq/comenq.sock")); assert_eq!(cfg.queue_path, PathBuf::from("/var/lib/comenq/queue")); + assert_eq!(cfg.cooldown_period_seconds, DEFAULT_COOLDOWN); } } diff --git a/crates/comenqd/src/daemon.rs b/crates/comenqd/src/daemon.rs new file mode 100644 index 0000000..925d4ba --- /dev/null +++ b/crates/comenqd/src/daemon.rs @@ -0,0 +1,86 @@ +//! Asynchronous daemon tasks for comenqd. +//! +//! This module provides the run function used by `main` which spawns the +//! Unix socket listener and the queue worker. + +use crate::config::Config; +use anyhow::Result; +use comenq_lib::CommentRequest; +use octocrab::Octocrab; +use std::fs; +use std::os::unix::fs::PermissionsExt; +use std::sync::Arc; +use tokio::io::AsyncReadExt; +use tokio::net::{UnixListener, UnixStream}; +use yaque::{Receiver, Sender, channel}; + +/// Start the daemon with the provided configuration. +pub async fn run(config: Config) -> Result<()> { + let octocrab = Arc::new( + Octocrab::builder() + .personal_token(config.github_token.clone()) + .build()?, + ); + let (tx, rx) = channel(&config.queue_path)?; + let cfg = Arc::new(config); + + let listener = tokio::spawn(run_listener(cfg.clone(), tx)); + let worker = tokio::spawn(run_worker(cfg.clone(), rx, octocrab)); + + tokio::select! { + res = listener => res??, + res = worker => res??, + } + + Ok(()) +} + +async fn run_listener(config: Arc, mut tx: Sender) -> Result<()> { + if fs::metadata(&config.socket_path).is_ok() { + fs::remove_file(&config.socket_path)?; + } + let listener = UnixListener::bind(&config.socket_path)?; + fs::set_permissions(&config.socket_path, fs::Permissions::from_mode(0o660))?; + + loop { + let (stream, _) = listener.accept().await?; + if let Err(e) = handle_client(stream, &mut tx).await { + tracing::warn!(error = %e, "Client handling failed"); + } + } +} + +async fn handle_client(mut stream: UnixStream, tx: &mut Sender) -> Result<()> { + let mut buffer = Vec::new(); + stream.read_to_end(&mut buffer).await?; + let request: CommentRequest = serde_json::from_slice(&buffer)?; + let bytes = serde_json::to_vec(&request)?; + tx.send(bytes).await?; + Ok(()) +} + +async fn run_worker( + config: Arc, + mut rx: Receiver, + octocrab: Arc, +) -> Result<()> { + loop { + let guard = rx.recv().await?; + let request: CommentRequest = serde_json::from_slice(&guard)?; + let result = octocrab + .issues(&request.owner, &request.repo) + .create_comment(request.pr_number, &request.body) + .await; + + if result.is_ok() { + guard.commit()?; + } else { + tracing::error!(?result, "Failed to post comment"); + } + + tokio::time::sleep(std::time::Duration::from_secs( + config.cooldown_period_seconds, + )) + .await; + } +} diff --git a/crates/comenqd/src/lib.rs b/crates/comenqd/src/lib.rs index 2c7d8f0..e811536 100644 --- a/crates/comenqd/src/lib.rs +++ b/crates/comenqd/src/lib.rs @@ -15,3 +15,4 @@ //! ``` pub mod config; +pub mod daemon; diff --git a/crates/comenqd/src/main.rs b/crates/comenqd/src/main.rs index d6ebb65..03e11b7 100644 --- a/crates/comenqd/src/main.rs +++ b/crates/comenqd/src/main.rs @@ -5,11 +5,14 @@ use tracing::info; mod config; +mod daemon; use config::Config; +use daemon::run; -fn main() -> Result<(), ortho_config::OrthoError> { +#[tokio::main] +async fn main() -> anyhow::Result<()> { tracing_subscriber::fmt::init(); let cfg = Config::load()?; info!(socket = ?cfg.socket_path, queue = ?cfg.queue_path, "Comenqd daemon started"); - Ok(()) + run(cfg).await } diff --git a/docs/comenq-design.md b/docs/comenq-design.md index e0d871e..135ffc3 100644 --- a/docs/comenq-design.md +++ b/docs/comenq-design.md @@ -1012,8 +1012,8 @@ async fn run_worker(config: Arc, mut rx: Receiver, octoc The repository initialises the workspace with `comenq-lib` at the root and two binary crates under `crates/`. `CommentRequest` resides in the library and -derives both `Serialize` and `Deserialize`. The binaries currently contain stub -`main` functions awaiting further implementation. +derives both `Serialize` and `Deserialize`. The daemon now spawns a Unix +listener and queue worker as described above. ## Works cited diff --git a/tests/steps/config_steps.rs b/tests/steps/config_steps.rs index 1e8dcd7..90f6872 100644 --- a/tests/steps/config_steps.rs +++ b/tests/steps/config_steps.rs @@ -7,6 +7,16 @@ use tempfile::TempDir; use comenqd::config::Config; +fn set_env(key: &str, value: &str) { + // Safety: manipulating process environment is inherently unsafe but tests + // run serialised so concurrent mutation cannot occur. + unsafe { std::env::set_var(key, value); } +} + +fn remove_env(key: &str) { + unsafe { std::env::remove_var(key); } +} + #[derive(Debug, Default, World)] pub struct ConfigWorld { dir: Option, @@ -27,9 +37,7 @@ fn config_file_with_token(world: &mut ConfigWorld, token: String) { fs::write(&path, format!("github_token='{token}'")).expect("write file"); world.dir = Some(dir); world.path = Some(path); - unsafe { - std::env::remove_var("COMENQD_SOCKET_PATH"); - } + remove_env("COMENQD_SOCKET_PATH"); } #[expect(clippy::expect_used, reason = "test setup uses expect")] @@ -68,9 +76,7 @@ fn config_without_socket(world: &mut ConfigWorld, token: String) { .expect("write file"); world.dir = Some(dir); world.path = Some(path); - unsafe { - std::env::remove_var("COMENQD_SOCKET_PATH"); - } + remove_env("COMENQD_SOCKET_PATH"); } #[given("a missing configuration file")] @@ -84,9 +90,7 @@ fn missing_configuration_file(world: &mut ConfigWorld) { )] #[given(regex = r#"^environment variable \"(.+)\" is \"(.+)\"$"#)] fn set_env_var(world: &mut ConfigWorld, key: String, value: String) { - unsafe { - std::env::set_var(&key, &value); - } + set_env(&key, &value); world.env_key = Some(key); } @@ -128,9 +132,7 @@ fn socket_path_is(world: &mut ConfigWorld, expected: String) { impl Drop for ConfigWorld { fn drop(&mut self) { if let Some(ref key) = self.env_key { - unsafe { - std::env::remove_var(key); - } + remove_env(key); } } } diff --git a/tests/steps/mod.rs b/tests/steps/mod.rs index 76d0329..d58e4b0 100644 --- a/tests/steps/mod.rs +++ b/tests/steps/mod.rs @@ -6,7 +6,7 @@ pub mod comment_steps; pub use comment_steps::CommentWorld; pub mod cli_steps; pub use cli_steps::CliWorld; -pub mod comment_steps; -pub use comment_steps::CommentWorld; pub mod config_steps; pub use config_steps::ConfigWorld; +pub mod comment_steps; +pub use comment_steps::CommentWorld; From 16f633232916d838b38b57cb86b17a73d41cd327 Mon Sep 17 00:00:00 2001 From: Leynos Date: Sat, 26 Jul 2025 17:35:20 +0100 Subject: [PATCH 6/8] Add env var guard for tests --- crates/comenqd/src/config.rs | 34 +++++++++++++++++++++++---- tests/steps/config_steps.rs | 45 ++++++++++++++++++++++++++++-------- 2 files changed, 64 insertions(+), 15 deletions(-) diff --git a/crates/comenqd/src/config.rs b/crates/comenqd/src/config.rs index cae2744..07ddbca 100644 --- a/crates/comenqd/src/config.rs +++ b/crates/comenqd/src/config.rs @@ -115,12 +115,37 @@ mod tests { use std::fs; use tempfile::tempdir; - fn set_env(key: &str, val: &str) { - unsafe { std::env::set_var(key, val); } + struct EnvVarGuard { + key: String, + original: Option, + } + + impl EnvVarGuard { + fn set(key: &str, val: &str) -> Self { + let original = std::env::var(key).ok(); + unsafe { + std::env::set_var(key, val); + } + Self { + key: key.to_string(), + original, + } + } + } + + impl Drop for EnvVarGuard { + fn drop(&mut self) { + match &self.original { + Some(v) => unsafe { std::env::set_var(&self.key, v) }, + None => unsafe { std::env::remove_var(&self.key) }, + } + } } fn remove_env(key: &str) { - unsafe { std::env::remove_var(key); } + unsafe { + std::env::remove_var(key); + } } #[rstest] @@ -154,9 +179,8 @@ mod tests { let dir = tempdir().unwrap(); let path = dir.path().join("config.toml"); fs::write(&path, "github_token='abc'\nsocket_path='/tmp/s.sock'").unwrap(); - set_env("COMENQD_SOCKET_PATH", "/tmp/override.sock"); + let _guard = EnvVarGuard::set("COMENQD_SOCKET_PATH", "/tmp/override.sock"); let cfg = Config::from_file(&path).unwrap(); - remove_env("COMENQD_SOCKET_PATH"); assert_eq!(cfg.socket_path, PathBuf::from("/tmp/override.sock")); } diff --git a/tests/steps/config_steps.rs b/tests/steps/config_steps.rs index 90f6872..7edae50 100644 --- a/tests/steps/config_steps.rs +++ b/tests/steps/config_steps.rs @@ -7,14 +7,40 @@ use tempfile::TempDir; use comenqd::config::Config; -fn set_env(key: &str, value: &str) { - // Safety: manipulating process environment is inherently unsafe but tests - // run serialised so concurrent mutation cannot occur. - unsafe { std::env::set_var(key, value); } +/// RAII guard for temporarily setting an environment variable. +#[derive(Debug)] +struct EnvVarGuard { + key: String, + original: Option, +} + +impl EnvVarGuard { + fn set(key: &str, value: &str) -> Self { + let original = std::env::var(key).ok(); + // Safety: serial_test ensures these manipulations are single-threaded. + unsafe { + std::env::set_var(key, value); + } + Self { + key: key.to_string(), + original, + } + } +} + +impl Drop for EnvVarGuard { + fn drop(&mut self) { + match &self.original { + Some(val) => unsafe { std::env::set_var(&self.key, val) }, + None => unsafe { std::env::remove_var(&self.key) }, + } + } } fn remove_env(key: &str) { - unsafe { std::env::remove_var(key); } + unsafe { + std::env::remove_var(key); + } } #[derive(Debug, Default, World)] @@ -22,7 +48,7 @@ pub struct ConfigWorld { dir: Option, path: Option, result: Option>, - env_key: Option, + env_guard: Option, } #[given(regex = r#"^a configuration file with token \"(.+)\"$"#)] @@ -90,8 +116,7 @@ fn missing_configuration_file(world: &mut ConfigWorld) { )] #[given(regex = r#"^environment variable \"(.+)\" is \"(.+)\"$"#)] fn set_env_var(world: &mut ConfigWorld, key: String, value: String) { - set_env(&key, &value); - world.env_key = Some(key); + world.env_guard = Some(EnvVarGuard::set(&key, &value)); } #[when("the config is loaded")] @@ -131,8 +156,8 @@ fn socket_path_is(world: &mut ConfigWorld, expected: String) { impl Drop for ConfigWorld { fn drop(&mut self) { - if let Some(ref key) = self.env_key { - remove_env(key); + if let Some(_guard) = self.env_guard.take() { + // dropping the guard restores the previous state } } } From 7fc31205235a73900307541cbd794fb2ccce6fd0 Mon Sep 17 00:00:00 2001 From: Leynos Date: Sat, 26 Jul 2025 20:03:00 +0100 Subject: [PATCH 7/8] Refactor daemon run logic --- Cargo.toml | 2 +- crates/comenqd/src/daemon.rs | 72 +++++++++++++++++++++--------------- 2 files changed, 43 insertions(+), 31 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 15843fb..d88cede 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,7 +16,7 @@ tokio = { workspace = true } clap = { workspace = true } comenq = { path = "crates/comenq" } comenqd = { path = "crates/comenqd" } -ortho_config = { workspace = true } +ortho_config = { git = "https://github.com/leynos/ortho-config.git", tag = "v0.4.0" } tempfile = "3.10" # latest 3.x at time of writing; update as new patch versions release [[test]] diff --git a/crates/comenqd/src/daemon.rs b/crates/comenqd/src/daemon.rs index 925d4ba..4a9b226 100644 --- a/crates/comenqd/src/daemon.rs +++ b/crates/comenqd/src/daemon.rs @@ -9,18 +9,31 @@ use comenq_lib::CommentRequest; use octocrab::Octocrab; use std::fs; use std::os::unix::fs::PermissionsExt; +use std::path::Path; use std::sync::Arc; +use std::time::Duration; use tokio::io::AsyncReadExt; use tokio::net::{UnixListener, UnixStream}; use yaque::{Receiver, Sender, channel}; +fn build_octocrab(token: &str) -> Result { + Ok(Octocrab::builder() + .personal_token(token.to_string()) + .build()?) +} + +fn prepare_listener(path: &Path) -> Result { + if fs::metadata(path).is_ok() { + fs::remove_file(path)?; + } + let listener = UnixListener::bind(path)?; + fs::set_permissions(path, fs::Permissions::from_mode(0o660))?; + Ok(listener) +} + /// Start the daemon with the provided configuration. pub async fn run(config: Config) -> Result<()> { - let octocrab = Arc::new( - Octocrab::builder() - .personal_token(config.github_token.clone()) - .build()?, - ); + let octocrab = Arc::new(build_octocrab(&config.github_token)?); let (tx, rx) = channel(&config.queue_path)?; let cfg = Arc::new(config); @@ -28,19 +41,21 @@ pub async fn run(config: Config) -> Result<()> { let worker = tokio::spawn(run_worker(cfg.clone(), rx, octocrab)); tokio::select! { - res = listener => res??, - res = worker => res??, + res = listener => match res { + Ok(inner) => inner?, + Err(e) => return Err(e.into()), + }, + res = worker => match res { + Ok(inner) => inner?, + Err(e) => return Err(e.into()), + }, } Ok(()) } async fn run_listener(config: Arc, mut tx: Sender) -> Result<()> { - if fs::metadata(&config.socket_path).is_ok() { - fs::remove_file(&config.socket_path)?; - } - let listener = UnixListener::bind(&config.socket_path)?; - fs::set_permissions(&config.socket_path, fs::Permissions::from_mode(0o660))?; + let listener = prepare_listener(&config.socket_path)?; loop { let (stream, _) = listener.accept().await?; @@ -59,28 +74,25 @@ async fn handle_client(mut stream: UnixStream, tx: &mut Sender) -> Result<()> { Ok(()) } -async fn run_worker( - config: Arc, - mut rx: Receiver, - octocrab: Arc, -) -> Result<()> { +async fn run_worker(config: Arc, mut rx: Receiver, octocrab: Arc) -> Result<()> { loop { let guard = rx.recv().await?; let request: CommentRequest = serde_json::from_slice(&guard)?; - let result = octocrab - .issues(&request.owner, &request.repo) - .create_comment(request.pr_number, &request.body) - .await; - if result.is_ok() { - guard.commit()?; - } else { - tracing::error!(?result, "Failed to post comment"); - } + let issues = octocrab.issues(&request.owner, &request.repo); + let post = issues.create_comment(request.pr_number, &request.body); - tokio::time::sleep(std::time::Duration::from_secs( - config.cooldown_period_seconds, - )) - .await; + match tokio::time::timeout(Duration::from_secs(10), post).await { + Ok(Ok(_)) => { + guard.commit()?; + tokio::time::sleep(Duration::from_secs(config.cooldown_period_seconds)).await; + } + Ok(Err(e)) => { + tracing::error!(error = %e, owner = %request.owner, repo = %request.repo, pr = request.pr_number, "GitHub API call failed"); + } + Err(e) => { + tracing::error!(error = %e, "Timed out posting comment"); + } + } } } From 6a98db814fca75476d0d8ae6fdda0144a8a595b6 Mon Sep 17 00:00:00 2001 From: Leynos Date: Sat, 26 Jul 2025 20:25:04 +0100 Subject: [PATCH 8/8] Wrap env var ops for tests --- crates/comenqd/src/config.rs | 24 ++++++++++++++++-------- tests/steps/config_steps.rs | 23 ++++++++++++++--------- 2 files changed, 30 insertions(+), 17 deletions(-) diff --git a/crates/comenqd/src/config.rs b/crates/comenqd/src/config.rs index 07ddbca..47237b7 100644 --- a/crates/comenqd/src/config.rs +++ b/crates/comenqd/src/config.rs @@ -123,9 +123,7 @@ mod tests { impl EnvVarGuard { fn set(key: &str, val: &str) -> Self { let original = std::env::var(key).ok(); - unsafe { - std::env::set_var(key, val); - } + set_env_var(key, val); Self { key: key.to_string(), original, @@ -136,16 +134,26 @@ mod tests { impl Drop for EnvVarGuard { fn drop(&mut self) { match &self.original { - Some(v) => unsafe { std::env::set_var(&self.key, v) }, - None => unsafe { std::env::remove_var(&self.key) }, + Some(v) => set_env_var(&self.key, v), + None => remove_env_var(&self.key), } } } fn remove_env(key: &str) { - unsafe { - std::env::remove_var(key); - } + remove_env_var(key); + } + + /// Safely set an environment variable for tests. + fn set_env_var(key: &str, val: &str) { + // Safety: tests using `serial_test::serial` run single-threaded. + unsafe { std::env::set_var(key, val) }; + } + + /// Safely remove an environment variable for tests. + fn remove_env_var(key: &str) { + // Safety: tests using `serial_test::serial` run single-threaded. + unsafe { std::env::remove_var(key) }; } #[rstest] diff --git a/tests/steps/config_steps.rs b/tests/steps/config_steps.rs index 7edae50..51a10d6 100644 --- a/tests/steps/config_steps.rs +++ b/tests/steps/config_steps.rs @@ -17,10 +17,7 @@ struct EnvVarGuard { impl EnvVarGuard { fn set(key: &str, value: &str) -> Self { let original = std::env::var(key).ok(); - // Safety: serial_test ensures these manipulations are single-threaded. - unsafe { - std::env::set_var(key, value); - } + set_env_var_safe(key, value); Self { key: key.to_string(), original, @@ -31,16 +28,24 @@ impl EnvVarGuard { impl Drop for EnvVarGuard { fn drop(&mut self) { match &self.original { - Some(val) => unsafe { std::env::set_var(&self.key, val) }, - None => unsafe { std::env::remove_var(&self.key) }, + Some(val) => set_env_var_safe(&self.key, val), + None => remove_env_var_safe(&self.key), } } } fn remove_env(key: &str) { - unsafe { - std::env::remove_var(key); - } + remove_env_var_safe(key); +} + +fn set_env_var_safe(key: &str, value: &str) { + // Safety: each scenario runs under serial_test, so no concurrent access. + unsafe { std::env::set_var(key, value) }; +} + +fn remove_env_var_safe(key: &str) { + // Safety: each scenario runs under serial_test, so no concurrent access. + unsafe { std::env::remove_var(key) }; } #[derive(Debug, Default, World)]