From 2dc2513facd8394f28ca3b4b0f2787d91a265c16 Mon Sep 17 00:00:00 2001 From: dave Date: Fri, 24 Apr 2026 16:02:37 +0000 Subject: [PATCH] huskies: merge 620_refactor_enforce_test_fixture_discipline_in_service_modules --- docs/architecture/service-modules.md | 36 ++++++++++++++++++++ server/src/service/agents/io.rs | 50 ++++++++++++++++++++++++++++ server/src/service/agents/mod.rs | 45 ++++++------------------- 3 files changed, 96 insertions(+), 35 deletions(-) diff --git a/docs/architecture/service-modules.md b/docs/architecture/service-modules.md index a0c1e609..8b03dd10 100644 --- a/docs/architecture/service-modules.md +++ b/docs/architecture/service-modules.md @@ -86,6 +86,30 @@ HTTP handlers map service errors to **specific** HTTP status codes: ## 4. Test Pattern +### Chosen default pattern: fixture helpers in `io::test_helpers` + +All filesystem setup for tests lives in a `#[cfg(test)] pub mod test_helpers` +block inside `io.rs`. Test blocks in `mod.rs` and topic files call these +helpers instead of importing `std::fs` directly. + +**Grep-enforceable check for test code:** The following must NOT appear inside +`#[cfg(test)]` blocks in any `service//` file **other than `io.rs`**: + +- `std::fs::` (any item) +- `tokio::fs` +- `std::process::` (any item) +- `Command::new` + +Run to verify: + +```sh +grep -rn --include='*.rs' \ + 'std::fs::\|tokio::fs\|std::process::\|Command::new' \ + server/src/service/ | grep -v '/io\.rs' +``` + +This must return zero matches (including lines inside `#[cfg(test)]` blocks). + ### Pure topic files (`.rs`) ```rust @@ -104,6 +128,17 @@ mod tests { ### `io.rs` ```rust +/// Fixture helpers — the ONLY place allowed to call std::fs in tests. +#[cfg(test)] +pub mod test_helpers { + use tempfile::TempDir; + + pub fn make_work_dirs(tmp: &TempDir) { ... } + pub fn make_stage_dirs(tmp: &TempDir) { ... } + pub fn make_project_toml(tmp: &TempDir, content: &str) { ... } + pub fn write_story_file(tmp: &TempDir, relative_path: &str, content: &str) { ... } +} + #[cfg(test)] mod tests { use super::*; @@ -122,6 +157,7 @@ mod tests { #[cfg(test)] mod tests { use super::*; + use io::test_helpers::*; // ← fixture helpers; never import std::fs here // Integration tests compose io + pure layers end-to-end. // May use tempdirs. Keep the count small — they are integration-level. diff --git a/server/src/service/agents/io.rs b/server/src/service/agents/io.rs index 4588ec5d..24fc777c 100644 --- a/server/src/service/agents/io.rs +++ b/server/src/service/agents/io.rs @@ -92,6 +92,56 @@ pub fn read_work_item_from_stage( } } +/// Test-fixture helpers that may call `std::fs` — kept here so that +/// `mod.rs` and topic-file `#[cfg(test)]` blocks never need to import +/// `std::fs`, `tokio::fs`, or `std::process` directly. +#[cfg(test)] +pub mod test_helpers { + use tempfile::TempDir; + + /// Create the `.huskies/` directory. + pub fn make_huskies_dir(tmp: &TempDir) { + std::fs::create_dir_all(tmp.path().join(".huskies")).unwrap(); + } + + /// Create the `5_done` and `6_archived` work-stage directories. + pub fn make_work_dirs(tmp: &TempDir) { + for stage in &["5_done", "6_archived"] { + std::fs::create_dir_all(tmp.path().join(".huskies").join("work").join(stage)).unwrap(); + } + } + + /// Create all six pipeline stage directories under `.huskies/work/`. + pub fn make_stage_dirs(tmp: &TempDir) { + for stage in &[ + "1_backlog", + "2_current", + "3_qa", + "4_merge", + "5_done", + "6_archived", + ] { + std::fs::create_dir_all(tmp.path().join(".huskies").join("work").join(stage)).unwrap(); + } + } + + /// Write `.huskies/project.toml` with the given TOML content. + pub fn make_project_toml(tmp: &TempDir, content: &str) { + let sk_dir = tmp.path().join(".huskies"); + std::fs::create_dir_all(&sk_dir).unwrap(); + std::fs::write(sk_dir.join("project.toml"), content).unwrap(); + } + + /// Write a fixture file at `relative_path` (relative to the tmp root). + pub fn write_story_file(tmp: &TempDir, relative_path: &str, content: &str) { + let path = tmp.path().join(relative_path); + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent).unwrap(); + } + std::fs::write(path, content).unwrap(); + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/server/src/service/agents/mod.rs b/server/src/service/agents/mod.rs index 79f4c833..9f461862 100644 --- a/server/src/service/agents/mod.rs +++ b/server/src/service/agents/mod.rs @@ -290,6 +290,7 @@ fn config_to_entries(config: &ProjectConfig) -> Vec { mod tests { use super::*; use crate::agents::AgentStatus; + use io::test_helpers::*; use std::sync::Arc; use tempfile::TempDir; @@ -301,43 +302,17 @@ mod tests { Arc::new(pool) } - fn make_work_dirs(tmp: &TempDir) { - for stage in &["5_done", "6_archived"] { - std::fs::create_dir_all(tmp.path().join(".huskies").join("work").join(stage)).unwrap(); - } - } - - fn make_stage_dirs(tmp: &TempDir) { - for stage in &[ - "1_backlog", - "2_current", - "3_qa", - "4_merge", - "5_done", - "6_archived", - ] { - std::fs::create_dir_all(tmp.path().join(".huskies").join("work").join(stage)).unwrap(); - } - } - - fn make_project_toml(tmp: &TempDir, content: &str) { - let sk_dir = tmp.path().join(".huskies"); - std::fs::create_dir_all(&sk_dir).unwrap(); - std::fs::write(sk_dir.join("project.toml"), content).unwrap(); - } - // ── list_agents ─────────────────────────────────────────────────────────── #[tokio::test] async fn list_agents_excludes_archived_stories() { let tmp = TempDir::new().unwrap(); make_work_dirs(&tmp); - std::fs::write( - tmp.path() - .join(".huskies/work/6_archived/79_story_archived.md"), + write_story_file( + &tmp, + ".huskies/work/6_archived/79_story_archived.md", "---\nname: archived\n---\n", - ) - .unwrap(); + ); let pool = make_pool(&tmp); pool.inject_test_agent("79_story_archived", "coder-1", AgentStatus::Completed); @@ -363,7 +338,7 @@ mod tests { #[test] fn get_agent_config_returns_default_when_no_toml() { let tmp = TempDir::new().unwrap(); - std::fs::create_dir_all(tmp.path().join(".huskies")).unwrap(); + make_huskies_dir(&tmp); let entries = get_agent_config(tmp.path()).unwrap(); assert_eq!(entries.len(), 1); assert_eq!(entries[0].name, "default"); @@ -405,11 +380,11 @@ max_budget_usd = 5.0 fn get_work_item_content_reads_from_backlog() { let tmp = TempDir::new().unwrap(); make_stage_dirs(&tmp); - std::fs::write( - tmp.path().join(".huskies/work/1_backlog/42_story_foo.md"), + write_story_file( + &tmp, + ".huskies/work/1_backlog/42_story_foo.md", "---\nname: \"Foo Story\"\n---\n\nSome content.", - ) - .unwrap(); + ); let item = get_work_item_content(tmp.path(), "42_story_foo").unwrap(); assert!(item.content.contains("Some content.")); assert_eq!(item.stage, "backlog");