From c65f9828fe39cee82d79c3ff430b62762fb33132 Mon Sep 17 00:00:00 2001 From: Leynos Date: Thu, 31 Jul 2025 22:48:33 +0100 Subject: [PATCH 1/8] Document wait_for_file re-export --- test-support/src/lib.rs | 3 +++ tests/steps/listener_steps.rs | 10 +++++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/test-support/src/lib.rs b/test-support/src/lib.rs index 3775971..595ab1c 100644 --- a/test-support/src/lib.rs +++ b/test-support/src/lib.rs @@ -2,4 +2,7 @@ pub mod util; +/// Wait for a file to appear, retrying with a fixed delay. +/// +/// This is re-exported from [`util`] for convenience in tests. pub use util::wait_for_file; diff --git a/tests/steps/listener_steps.rs b/tests/steps/listener_steps.rs index d15309e..a666683 100644 --- a/tests/steps/listener_steps.rs +++ b/tests/steps/listener_steps.rs @@ -17,6 +17,9 @@ use comenqd::config::Config; use comenqd::daemon::{queue_writer, run_listener}; use yaque::channel; +const SOCKET_RETRY_COUNT: u32 = 10; +const SOCKET_RETRY_DELAY_MS: u64 = 10; + #[derive(Default, World)] pub struct ListenerWorld { dir: Option, @@ -67,7 +70,12 @@ async fn running_listener(world: &mut ListenerWorld) { .expect("config not initialised in ListenerWorld") .socket_path; assert!( - wait_for_file(socket_path, 10, Duration::from_millis(10)).await, + wait_for_file( + socket_path, + SOCKET_RETRY_COUNT, + Duration::from_millis(SOCKET_RETRY_DELAY_MS), + ) + .await, "socket file {} not created within timeout", socket_path.display() ); From 94ec09a72d5c0b19e7abb8a7c4ab9c0164a1253a Mon Sep 17 00:00:00 2001 From: Leynos Date: Sun, 3 Aug 2025 01:12:58 +0100 Subject: [PATCH 2/8] Document wait_for_file and expose retries --- test-support/src/lib.rs | 20 ++++++++++++++++++++ test-support/src/util.rs | 8 ++++++++ tests/steps/listener_steps.rs | 12 ++---------- 3 files changed, 30 insertions(+), 10 deletions(-) diff --git a/test-support/src/lib.rs b/test-support/src/lib.rs index 595ab1c..8c4eb7d 100644 --- a/test-support/src/lib.rs +++ b/test-support/src/lib.rs @@ -2,7 +2,27 @@ pub mod util; +pub use util::{SOCKET_RETRY_COUNT, SOCKET_RETRY_DELAY}; + /// Wait for a file to appear, retrying with a fixed delay. /// /// This is re-exported from [`util`] for convenience in tests. +/// +/// # Arguments +/// * `path` – Path to the file that is expected to be created. +/// * `tries` – Maximum number of polling attempts. +/// * `delay` – Pause between attempts as a [`std::time::Duration`]. +/// +/// # Returns +/// `true` if the file appears within `tries` attempts, otherwise `false`. +/// +/// # Examples +/// ```rust,ignore +/// use std::path::Path; +/// use test_support::{wait_for_file, SOCKET_RETRY_COUNT, SOCKET_RETRY_DELAY}; +/// +/// let path = Path::new("/tmp/example.sock"); +/// let found = wait_for_file(path, SOCKET_RETRY_COUNT, SOCKET_RETRY_DELAY).await; +/// assert!(found); +/// ``` pub use util::wait_for_file; diff --git a/test-support/src/util.rs b/test-support/src/util.rs index ee5092a..0bf9eac 100644 --- a/test-support/src/util.rs +++ b/test-support/src/util.rs @@ -6,6 +6,14 @@ use std::path::Path; use std::time::Duration; use tokio::time::sleep; +/// Maximum number of times to poll for an expected file. +pub const SOCKET_RETRY_COUNT: u32 = 10; + +/// Delay between polls when waiting for a file to appear. +/// +/// The value is ten milliseconds. +pub const SOCKET_RETRY_DELAY: Duration = Duration::from_millis(10); + /// Wait for a file to appear within the given number of tries. /// /// # Examples diff --git a/tests/steps/listener_steps.rs b/tests/steps/listener_steps.rs index a666683..0e130b7 100644 --- a/tests/steps/listener_steps.rs +++ b/tests/steps/listener_steps.rs @@ -7,7 +7,7 @@ use std::time::Duration; use cucumber::{World, given, then, when}; use tempfile::TempDir; -use test_support::wait_for_file; +use test_support::{SOCKET_RETRY_COUNT, SOCKET_RETRY_DELAY, wait_for_file}; use tokio::io::AsyncWriteExt; use tokio::net::UnixStream; use tokio::sync::{mpsc, watch}; @@ -17,9 +17,6 @@ use comenqd::config::Config; use comenqd::daemon::{queue_writer, run_listener}; use yaque::channel; -const SOCKET_RETRY_COUNT: u32 = 10; -const SOCKET_RETRY_DELAY_MS: u64 = 10; - #[derive(Default, World)] pub struct ListenerWorld { dir: Option, @@ -70,12 +67,7 @@ async fn running_listener(world: &mut ListenerWorld) { .expect("config not initialised in ListenerWorld") .socket_path; assert!( - wait_for_file( - socket_path, - SOCKET_RETRY_COUNT, - Duration::from_millis(SOCKET_RETRY_DELAY_MS), - ) - .await, + wait_for_file(socket_path, SOCKET_RETRY_COUNT, SOCKET_RETRY_DELAY).await, "socket file {} not created within timeout", socket_path.display() ); From 9f5157c7588c695636c1cd2a08c042e0b81754b7 Mon Sep 17 00:00:00 2001 From: Leynos Date: Sun, 3 Aug 2025 14:19:12 +0100 Subject: [PATCH 3/8] Clarify wait_for_file API and test constants --- test-support/src/lib.rs | 20 +++++++++++++++----- test-support/src/util.rs | 2 ++ tests/steps/listener_steps.rs | 3 ++- 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/test-support/src/lib.rs b/test-support/src/lib.rs index 8c4eb7d..8843a60 100644 --- a/test-support/src/lib.rs +++ b/test-support/src/lib.rs @@ -2,7 +2,13 @@ pub mod util; -pub use util::{SOCKET_RETRY_COUNT, SOCKET_RETRY_DELAY}; +/// Maximum number of times to poll for an expected file. +pub use util::SOCKET_RETRY_COUNT; + +/// Delay between polls when waiting for a file to appear. +/// +/// Multiply by [`SOCKET_RETRY_COUNT`] for the worst-case wait duration. +pub use util::SOCKET_RETRY_DELAY; /// Wait for a file to appear, retrying with a fixed delay. /// @@ -12,17 +18,21 @@ pub use util::{SOCKET_RETRY_COUNT, SOCKET_RETRY_DELAY}; /// * `path` – Path to the file that is expected to be created. /// * `tries` – Maximum number of polling attempts. /// * `delay` – Pause between attempts as a [`std::time::Duration`]. +/// The total wait time is `tries * delay`. /// /// # Returns /// `true` if the file appears within `tries` attempts, otherwise `false`. /// /// # Examples -/// ```rust,ignore +/// ```rust,no_run /// use std::path::Path; /// use test_support::{wait_for_file, SOCKET_RETRY_COUNT, SOCKET_RETRY_DELAY}; /// -/// let path = Path::new("/tmp/example.sock"); -/// let found = wait_for_file(path, SOCKET_RETRY_COUNT, SOCKET_RETRY_DELAY).await; -/// assert!(found); +/// #[tokio::main] +/// async fn main() { +/// let path = Path::new("/tmp/example.sock"); +/// let found = wait_for_file(path, SOCKET_RETRY_COUNT, SOCKET_RETRY_DELAY).await; +/// assert!(found); +/// } /// ``` pub use util::wait_for_file; diff --git a/test-support/src/util.rs b/test-support/src/util.rs index 0bf9eac..91c34be 100644 --- a/test-support/src/util.rs +++ b/test-support/src/util.rs @@ -11,6 +11,8 @@ pub const SOCKET_RETRY_COUNT: u32 = 10; /// Delay between polls when waiting for a file to appear. /// +/// Each attempt sleeps for this duration; multiply by +/// [`SOCKET_RETRY_COUNT`] to obtain the worst-case total wait. /// The value is ten milliseconds. pub const SOCKET_RETRY_DELAY: Duration = Duration::from_millis(10); diff --git a/tests/steps/listener_steps.rs b/tests/steps/listener_steps.rs index 0e130b7..bb38ea8 100644 --- a/tests/steps/listener_steps.rs +++ b/tests/steps/listener_steps.rs @@ -5,7 +5,8 @@ use std::sync::Arc; use std::time::Duration; -use cucumber::{World, given, then, when}; +use cucumber::World; +use cucumber::{given, then, when}; use tempfile::TempDir; use test_support::{SOCKET_RETRY_COUNT, SOCKET_RETRY_DELAY, wait_for_file}; use tokio::io::AsyncWriteExt; From 9115c225c733a342e60f7102b757737699313d55 Mon Sep 17 00:00:00 2001 From: Leynos Date: Sun, 3 Aug 2025 14:41:20 +0100 Subject: [PATCH 4/8] Add final new-line --- test-support/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test-support/src/lib.rs b/test-support/src/lib.rs index fe1130a..e879eb7 100644 --- a/test-support/src/lib.rs +++ b/test-support/src/lib.rs @@ -40,4 +40,4 @@ pub use util::SOCKET_RETRY_DELAY; /// assert!(found); /// } /// ``` -pub use util::wait_for_file; \ No newline at end of file +pub use util::wait_for_file; From d02547468c880c814c8214475b48617d912f8d7a Mon Sep 17 00:00:00 2001 From: Leynos Date: Sun, 3 Aug 2025 15:58:15 +0100 Subject: [PATCH 5/8] Restrict release tag pattern (#42) --- .github/workflows/release.yml | 2 +- Cargo.lock | 2 ++ Cargo.toml | 1 + crates/comenq/Cargo.toml | 4 ++++ crates/comenqd/Cargo.toml | 4 ++-- 5 files changed, 10 insertions(+), 3 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 87b2816..9c9850b 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -4,7 +4,7 @@ on: push: tags: # Match semantic version tags (e.g. v1.2.3, v10.11.12, v12.3.7-beta7) - - 'v*.*.*' + - 'v[0-9]*.[0-9]*.[0-9]*' jobs: goreleaser: diff --git a/Cargo.lock b/Cargo.lock index b221715..68660db 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -305,8 +305,10 @@ version = "0.1.0" dependencies = [ "clap", "comenq-lib", + "rstest", "serde", "serde_json", + "tempfile", "thiserror 1.0.69", "tokio", "tracing", diff --git a/Cargo.toml b/Cargo.toml index 2609a02..0fdfd2d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -51,6 +51,7 @@ thiserror = "1.0" ortho_config = { git = "https://github.com/leynos/ortho-config.git", tag = "v0.4.0" } serde_yaml = "0.9" tempfile = "3.10" +rstest = "0.18.0" [lints.clippy] pedantic = { level = "warn", priority = -1 } diff --git a/crates/comenq/Cargo.toml b/crates/comenq/Cargo.toml index 1781a6e..4503182 100644 --- a/crates/comenq/Cargo.toml +++ b/crates/comenq/Cargo.toml @@ -14,3 +14,7 @@ serde_json = { workspace = true } comenq-lib = { path = "../.." } thiserror = { workspace = true } tracing = { workspace = true } + +[dev-dependencies] +rstest = { workspace = true } +tempfile = { workspace = true } diff --git a/crates/comenqd/Cargo.toml b/crates/comenqd/Cargo.toml index 116f04d..181c610 100644 --- a/crates/comenqd/Cargo.toml +++ b/crates/comenqd/Cargo.toml @@ -20,8 +20,8 @@ ortho_config = { workspace = true } figment = { version = "0.10", default-features = false, features = ["env", "toml"] } [dev-dependencies] -rstest = "0.18.0" -tempfile = "3.10" # latest 3.x at time of writing; update as new patch versions release +rstest = { workspace = true } +tempfile = { workspace = true } # latest 3.x at time of writing; update as new patch versions release serial_test = "2" test-support = { path = "../../test-support" } test-utils = { path = "../test-utils" } From 80ac2b9e3aabaa8ba23bc942e6d368cb7fafceee Mon Sep 17 00:00:00 2001 From: Leynos Date: Sun, 3 Aug 2025 16:02:40 +0100 Subject: [PATCH 6/8] Remove unneeded import --- crates/comenq/src/client.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/comenq/src/client.rs b/crates/comenq/src/client.rs index 2c80570..b54f595 100644 --- a/crates/comenq/src/client.rs +++ b/crates/comenq/src/client.rs @@ -91,7 +91,6 @@ mod tests { use super::{ClientError, parse_slug, run}; use crate::Args; use comenq_lib::CommentRequest; - use rstest::rstest; use tempfile::tempdir; use tokio::io::AsyncReadExt; use tokio::net::UnixListener; From 6010207bf93442eec729dae6364629acc90bdb9d Mon Sep 17 00:00:00 2001 From: Leynos Date: Sun, 3 Aug 2025 16:31:16 +0100 Subject: [PATCH 7/8] Fix test compilation errors (#44) --- Cargo.lock | 3 +- crates/comenqd/Cargo.toml | 3 +- crates/comenqd/src/config.rs | 53 +++++++++++++++++++++++++++++------ crates/comenqd/src/daemon.rs | 44 +++++++++++++++++++++++++---- crates/comenqd/src/logging.rs | 3 +- 5 files changed, 86 insertions(+), 20 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 68660db..8e1ec7e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -349,12 +349,11 @@ dependencies = [ "serde_json", "serial_test", "tempfile", - "test-support", - "test-utils", "thiserror 1.0.69", "tokio", "tracing", "tracing-subscriber", + "wiremock", "yaque", ] diff --git a/crates/comenqd/Cargo.toml b/crates/comenqd/Cargo.toml index 181c610..a54dd01 100644 --- a/crates/comenqd/Cargo.toml +++ b/crates/comenqd/Cargo.toml @@ -23,5 +23,4 @@ figment = { version = "0.10", default-features = false, features = ["env", "toml rstest = { workspace = true } tempfile = { workspace = true } # latest 3.x at time of writing; update as new patch versions release serial_test = "2" -test-support = { path = "../../test-support" } -test-utils = { path = "../test-utils" } +wiremock = "0.6" diff --git a/crates/comenqd/src/config.rs b/crates/comenqd/src/config.rs index 8ff9b63..6f0bb6d 100644 --- a/crates/comenqd/src/config.rs +++ b/crates/comenqd/src/config.rs @@ -20,7 +20,7 @@ const DEFAULT_QUEUE_PATH: &str = "/var/lib/comenq/queue"; const DEFAULT_COOLDOWN: u64 = 960; /// Runtime configuration for the daemon. -#[derive(Debug, Deserialize, Serialize, PartialEq, Eq)] +#[derive(Debug, Deserialize, Serialize, PartialEq, Eq, Clone)] pub struct Config { /// GitHub Personal Access Token. pub github_token: String, @@ -119,17 +119,52 @@ mod tests { use tempfile::tempdir; mod env_guard { - include!(concat!( - env!("CARGO_MANIFEST_DIR"), - "/../../tests/support/env_guard.rs" - )); - } + //! Test helpers for managing environment variables. + + #[derive(Debug)] + pub struct EnvVarGuard { + key: String, + original: Option, + } + + impl EnvVarGuard { + /// Set an environment variable for the lifetime of the returned guard. + pub fn set(key: &str, value: &str) -> Self { + let original = std::env::var(key).ok(); + set_env_var(key, value); + Self { + key: key.to_string(), + original, + } + } + } - pub mod support { - pub use super::env_guard::{EnvVarGuard, remove_env_var, set_env_var}; + impl Drop for EnvVarGuard { + fn drop(&mut self) { + match &self.original { + Some(v) => set_env_var(&self.key, v), + None => remove_env_var(&self.key), + } + } + } + + /// Set an environment variable for tests. + /// + /// The nightly compiler marks `std::env::set_var` as `unsafe`. + /// Tests run serially so using it is acceptable here. + pub fn set_env_var(key: &str, value: &str) { + unsafe { std::env::set_var(key, value) }; + } + + /// Remove an environment variable for tests. + /// + /// `std::env::remove_var` is also `unsafe` on nightly. + pub fn remove_env_var(key: &str) { + unsafe { std::env::remove_var(key) }; + } } - use support::{EnvVarGuard, remove_env_var}; + use env_guard::{EnvVarGuard, remove_env_var}; #[rstest] #[serial_test::serial] diff --git a/crates/comenqd/src/daemon.rs b/crates/comenqd/src/daemon.rs index 24fd3f2..90b8018 100644 --- a/crates/comenqd/src/daemon.rs +++ b/crates/comenqd/src/daemon.rs @@ -281,15 +281,27 @@ pub async fn run_worker( mod tests { //! Tests for the daemon tasks. use super::*; - use tempfile::tempdir; - use test_support::wait_for_file; - use test_utils::{octocrab_for, temp_config}; + use octocrab::Octocrab; + use std::fs as stdfs; + use std::path::Path; + use std::sync::Arc; + use tempfile::{TempDir, tempdir}; use tokio::io::AsyncWriteExt; - use tokio::net::{UnixListener, UnixStream}; + use tokio::net::UnixStream; use tokio::sync::{mpsc, watch}; use tokio::time::{Duration, sleep}; use wiremock::matchers::{method, path}; use wiremock::{Mock, MockServer, ResponseTemplate}; + use yaque::Receiver; + fn temp_config(tmp: &TempDir) -> Config { + Config { + github_token: String::from("t"), + socket_path: tmp.path().join("sock"), + queue_path: tmp.path().join("q"), + cooldown_period_seconds: 1, + } + } + fn cfg_with_cooldown(dir: &TempDir, secs: u64) -> Config { Config { cooldown_period_seconds: secs, @@ -297,6 +309,28 @@ mod tests { } } + #[expect(clippy::expect_used, reason = "simplify test helper setup")] + fn octocrab_for(server: &MockServer) -> Arc { + Arc::new( + Octocrab::builder() + .personal_token("t".to_string()) + .base_uri(server.uri()) + .expect("base_uri") + .build() + .expect("build octocrab"), + ) + } + + async fn wait_for_file(path: &Path, tries: u32, delay: Duration) -> bool { + for _ in 0..tries { + if path.exists() { + return true; + } + sleep(delay).await; + } + path.exists() + } + async fn setup_run_worker(status: u16) -> (MockServer, Arc, Receiver, Arc) { let dir = tempdir().expect("tempdir"); let cfg = Arc::new(Config { @@ -364,7 +398,7 @@ mod tests { let dir = tempdir().expect("tempdir"); let queue_path = dir.path().join("q"); let (sender, mut receiver) = channel(&queue_path).expect("channel"); - let (client_tx, mut writer_rx) = mpsc::unbounded_channel(); + let (client_tx, writer_rx) = mpsc::unbounded_channel(); let writer = tokio::spawn(queue_writer(sender, writer_rx)); let (mut client, server) = UnixStream::pair().expect("pair"); diff --git a/crates/comenqd/src/logging.rs b/crates/comenqd/src/logging.rs index 14162c0..3c0ea55 100644 --- a/crates/comenqd/src/logging.rs +++ b/crates/comenqd/src/logging.rs @@ -20,7 +20,6 @@ where fmt() .with_env_filter(EnvFilter::from_default_env()) .with_writer(writer) - .json() .init(); } @@ -66,7 +65,7 @@ mod tests { #[test] fn init_logging() { let buf = Arc::new(Mutex::new(Vec::new())); - std::env::set_var("RUST_LOG", "info"); + unsafe { std::env::set_var("RUST_LOG", "info") }; init_with_writer(BufMakeWriter { buf: buf.clone() }); info!("captured"); let output = String::from_utf8(buf.lock().expect("Failed to lock log buffer").clone()) From 181cb6e1d20871699427804c9b76f0c4464b9c6c Mon Sep 17 00:00:00 2001 From: Leynos Date: Sun, 3 Aug 2025 17:35:21 +0100 Subject: [PATCH 8/8] Fix test setup sender mutability (#45) --- crates/comenqd/src/daemon.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/comenqd/src/daemon.rs b/crates/comenqd/src/daemon.rs index 90b8018..4df9ac4 100644 --- a/crates/comenqd/src/daemon.rs +++ b/crates/comenqd/src/daemon.rs @@ -337,7 +337,7 @@ mod tests { cooldown_period_seconds: 0, ..temp_config(&dir) }); - let (sender, rx) = channel(&cfg.queue_path).expect("channel"); + let (mut sender, rx) = channel(&cfg.queue_path).expect("channel"); let req = CommentRequest { owner: "o".into(), repo: "r".into(),