From 19aa5940ea3cec65bcc1b4aac4220dac2463afc0 Mon Sep 17 00:00:00 2001 From: Scott Werner Date: Wed, 19 Aug 2026 16:50:37 -0400 Subject: [PATCH] Add a shared RunSpec test fixture and adopt it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `RunSpec` has 13 fields and no `Default`, so every test that needed one spelled out all 13 even when it cared about one or two. That put 64 hand-rolled `RunSpec { .. }` literals in `lib/`, and made a single additive field cost a mechanical edit at roughly 30 sites. Add `test_run_spec()` to `fabro-types`' feature-gated `test_support` module: fixed `fixtures::RUN_1`, default settings, a minimal `test` graph, `test_run_provenance()`, and every optional field unset. Tests now spread it and only spell out what they assert on. Adopt it at the 13 literals where the spread removes real duplication, including the crate-local `test_run_spec` helpers in `fabro-store` and `fabro-workflow`, which are now defined in terms of the shared fixture. Tests that populate every field on purpose — the exhaustive `RunSpec` serde round-trip in particular — keep spelling it out. No production code and no behavior changes. Co-Authored-By: Claude Opus 5 (1M context) --- lib/components/fabro-dump/src/lib.rs | 21 ++---- lib/components/fabro-store/src/run_state.rs | 44 ++----------- .../tests/serializable_projection.rs | 19 ++---- .../fabro-workflow/src/billing_rollup.rs | 17 +---- .../fabro-workflow/src/run_lookup.rs | 19 ++---- .../fabro-workflow/src/runtime_store.rs | 18 +---- .../tests/run_projection_round_trip.rs | 17 +---- .../fabro-types/src/run_projection.rs | 65 ++----------------- .../fabro-types/src/test_support.rs | 36 +++++++++- .../fabro-types/tests/run_spec_methods.rs | 10 +-- 10 files changed, 76 insertions(+), 190 deletions(-) diff --git a/lib/components/fabro-dump/src/lib.rs b/lib/components/fabro-dump/src/lib.rs index 33f3185f6..43cf278eb 100644 --- a/lib/components/fabro-dump/src/lib.rs +++ b/lib/components/fabro-dump/src/lib.rs @@ -486,8 +486,7 @@ mod tests { use fabro_types::{ Checkpoint, CheckpointRecord, Conclusion, RunDiff, RunSandbox, RunSandboxInstance, RunSandboxPlan, RunStatus, SandboxProviderKind, StageCompletion, StageModelUsage, - StageOutcome, StartRecord, SuccessReason, WorkflowSettings, first_event_seq, fixtures, - test_support, + StageOutcome, StartRecord, SuccessReason, first_event_seq, fixtures, test_support, }; use futures::executor; @@ -495,24 +494,18 @@ mod tests { fn sample_run_spec() -> RunSpec { RunSpec { - run_id: fixtures::RUN_1, - settings: WorkflowSettings::default(), - graph: Graph::new("ship"), - graph_source: Some("digraph Ship {}".to_string()), - workflow_slug: Some("demo".to_string()), - automation: None, + graph: Graph::new("ship"), + graph_source: Some("digraph Ship {}".to_string()), + workflow_slug: Some("demo".to_string()), source_directory: Some("/tmp/project".to_string()), - git: Some(fabro_types::GitContext { + git: Some(fabro_types::GitContext { origin_url: "https://github.com/fabro-sh/fabro.git".to_string(), branch: "main".to_string(), sha: None, dirty: fabro_types::DirtyStatus::Clean, }), - labels: HashMap::from([("team".to_string(), "platform".to_string())]), - provenance: test_support::test_run_provenance(), - manifest_blob: None, - definition_blob: None, - fork_source_ref: None, + labels: HashMap::from([("team".to_string(), "platform".to_string())]), + ..test_support::test_run_spec() } } diff --git a/lib/components/fabro-store/src/run_state.rs b/lib/components/fabro-store/src/run_state.rs index c89587e59..1b8138fbd 100644 --- a/lib/components/fabro-store/src/run_state.rs +++ b/lib/components/fabro-store/src/run_state.rs @@ -2281,19 +2281,8 @@ mod tests { fn test_run_spec() -> RunSpec { RunSpec { - run_id: fixtures::RUN_1, - settings: WorkflowSettings::default(), - graph: Graph::new("test"), - graph_source: Some("digraph test {}".to_string()), - workflow_slug: None, - automation: None, - source_directory: None, - labels: HashMap::new(), - provenance: test_support::test_run_provenance(), - manifest_blob: None, - definition_blob: None, - git: None, - fork_source_ref: None, + graph_source: Some("digraph test {}".to_string()), + ..test_support::test_run_spec() } } @@ -4086,19 +4075,9 @@ mod tests { fn summary_synthesizes_submitted_when_run_exists_without_status() { let mut state = initialized_projection(); state.spec = fabro_types::RunSpec { - run_id: fixtures::RUN_1, - settings: WorkflowSettings::default(), - graph: fabro_types::Graph::new("test"), - graph_source: None, - workflow_slug: Some("test".to_string()), - automation: None, + workflow_slug: Some("test".to_string()), source_directory: Some("/tmp/repo".to_string()), - git: None, - labels: HashMap::new(), - provenance: test_support::test_run_provenance(), - manifest_blob: None, - definition_blob: None, - fork_source_ref: None, + ..test_support::test_run_spec() }; let summary_json = serde_json::to_value(build_summary(&state, &fixtures::RUN_1)).unwrap(); @@ -4112,19 +4091,10 @@ mod tests { fn summary_preserves_absent_workflow_name_and_reports_graph_name() { let mut state = initialized_projection(); state.spec = fabro_types::RunSpec { - run_id: fixtures::RUN_1, - settings: WorkflowSettings::default(), - graph: fabro_types::Graph::new("GraphName"), - graph_source: None, - workflow_slug: Some("release-flow".to_string()), - automation: None, + graph: fabro_types::Graph::new("GraphName"), + workflow_slug: Some("release-flow".to_string()), source_directory: Some("/tmp/repo".to_string()), - git: None, - labels: HashMap::new(), - provenance: test_support::test_run_provenance(), - manifest_blob: None, - definition_blob: None, - fork_source_ref: None, + ..test_support::test_run_spec() }; let summary = build_summary(&state, &fixtures::RUN_1); diff --git a/lib/components/fabro-store/tests/serializable_projection.rs b/lib/components/fabro-store/tests/serializable_projection.rs index ef0ed067b..d6fdcc682 100644 --- a/lib/components/fabro-store/tests/serializable_projection.rs +++ b/lib/components/fabro-store/tests/serializable_projection.rs @@ -8,30 +8,23 @@ use fabro_types::{ BilledModelUsage, BilledTokenCounts, Checkpoint, CheckpointRecord, InterviewQuestionRecord, ParallelBranchResult, QuestionType, RunDiff, RunSandbox, RunSandboxInstance, RunSandboxPlan, RunSandboxRuntime, RunStatus, SandboxProviderKind, StageCompletion, StageModelUsage, - StageOutcome, StartRecord, WorkflowSettings, first_event_seq, fixtures, test_support, + StageOutcome, StartRecord, first_event_seq, fixtures, test_support, }; use serde_json::json; fn sample_run_spec() -> RunSpec { RunSpec { - run_id: fixtures::RUN_1, - settings: WorkflowSettings::default(), - graph: Graph::new("ship"), - graph_source: None, - workflow_slug: Some("demo".to_string()), - automation: None, + graph: Graph::new("ship"), + workflow_slug: Some("demo".to_string()), source_directory: Some("/tmp/project".to_string()), - labels: HashMap::from([("team".to_string(), "platform".to_string())]), - provenance: test_support::test_run_provenance(), - manifest_blob: None, - definition_blob: None, - git: Some(fabro_types::GitContext { + labels: HashMap::from([("team".to_string(), "platform".to_string())]), + git: Some(fabro_types::GitContext { origin_url: "https://github.com/fabro-sh/fabro.git".to_string(), branch: "main".to_string(), sha: None, dirty: fabro_types::DirtyStatus::Clean, }), - fork_source_ref: None, + ..test_support::test_run_spec() } } diff --git a/lib/components/fabro-workflow/src/billing_rollup.rs b/lib/components/fabro-workflow/src/billing_rollup.rs index 986b541d4..1b6487f36 100644 --- a/lib/components/fabro-workflow/src/billing_rollup.rs +++ b/lib/components/fabro-workflow/src/billing_rollup.rs @@ -126,12 +126,10 @@ pub fn billing_rollup_from_projection( #[cfg(test)] mod tests { - use std::collections::HashMap; - use fabro_model::{Catalog, ModelRef, ProviderId}; use fabro_types::{ AttrValue, BilledTokenCounts, Graph, Node, RunProjection, RunSpec, StageCompletion, - StageOutcome, WorkflowSettings, first_event_seq, fixtures, test_support, + StageOutcome, first_event_seq, test_support, }; use super::billing_rollup_from_projection; @@ -311,19 +309,8 @@ mod tests { }); RunSpec { - run_id: fixtures::RUN_1, - settings: WorkflowSettings::default(), graph, - graph_source: None, - workflow_slug: None, - automation: None, - source_directory: None, - labels: HashMap::new(), - provenance: test_support::test_run_provenance(), - manifest_blob: None, - definition_blob: None, - git: None, - fork_source_ref: None, + ..test_support::test_run_spec() } } } diff --git a/lib/components/fabro-workflow/src/run_lookup.rs b/lib/components/fabro-workflow/src/run_lookup.rs index 99e9825fe..1edd36290 100644 --- a/lib/components/fabro-workflow/src/run_lookup.rs +++ b/lib/components/fabro-workflow/src/run_lookup.rs @@ -445,13 +445,11 @@ fn run_id_matches(run_id: RunId, prefix: &str) -> bool { #[cfg(test)] mod tests { - use std::collections::HashMap; use std::sync::Arc; use std::time::Duration; - use fabro_graphviz::graph::Graph; use fabro_store::Database; - use fabro_types::{RunStatus, WorkflowSettings, fixtures, test_support}; + use fabro_types::{RunStatus, fixtures, test_support}; use object_store::memory::InMemory; use super::scan_runs_combined; @@ -470,24 +468,15 @@ mod tests { fn sample_run_spec() -> RunSpec { RunSpec { - run_id: fixtures::RUN_1, - settings: WorkflowSettings::default(), - graph: Graph::new("test"), - graph_source: None, - workflow_slug: Some("test".to_string()), - automation: None, + workflow_slug: Some("test".to_string()), source_directory: Some("/tmp/project".to_string()), - git: Some(fabro_types::GitContext { + git: Some(fabro_types::GitContext { origin_url: String::new(), branch: "main".to_string(), sha: None, dirty: fabro_types::DirtyStatus::Clean, }), - labels: HashMap::new(), - provenance: test_support::test_run_provenance(), - manifest_blob: None, - definition_blob: None, - fork_source_ref: None, + ..test_support::test_run_spec() } } diff --git a/lib/components/fabro-workflow/src/runtime_store.rs b/lib/components/fabro-workflow/src/runtime_store.rs index af4eae425..63d55ca64 100644 --- a/lib/components/fabro-workflow/src/runtime_store.rs +++ b/lib/components/fabro-workflow/src/runtime_store.rs @@ -112,15 +112,13 @@ impl RunStoreBackend for LocalRunStoreBackend { #[cfg(test)] mod tests { - use std::collections::HashMap; use std::sync::Arc; use std::time::Duration; use chrono::Utc; - use fabro_graphviz::graph::Graph; use fabro_store::Database; use fabro_types::run_event::RunSubmittedProps; - use fabro_types::{EventBody, RunEvent, WorkflowSettings, fixtures, test_support}; + use fabro_types::{EventBody, RunEvent, fixtures, test_support}; use object_store::memory::InMemory; use super::RunStoreHandle; @@ -139,19 +137,9 @@ mod tests { fn test_run_spec() -> RunSpec { RunSpec { - run_id: fixtures::RUN_1, - settings: WorkflowSettings::default(), - graph: Graph::new("test"), - graph_source: None, - workflow_slug: Some("test".to_string()), - automation: None, + workflow_slug: Some("test".to_string()), source_directory: Some("/tmp/test".to_string()), - git: None, - labels: HashMap::new(), - provenance: test_support::test_run_provenance(), - manifest_blob: None, - definition_blob: None, - fork_source_ref: None, + ..test_support::test_run_spec() } } diff --git a/lib/foundation/fabro-api/tests/run_projection_round_trip.rs b/lib/foundation/fabro-api/tests/run_projection_round_trip.rs index 77d28178b..f1a00a5a5 100644 --- a/lib/foundation/fabro-api/tests/run_projection_round_trip.rs +++ b/lib/foundation/fabro-api/tests/run_projection_round_trip.rs @@ -1,7 +1,7 @@ use std::any::{TypeId, type_name}; use fabro_api::types::RunProjection as ApiRunProjection; -use fabro_types::{Graph, RunProjection, RunSpec, WorkflowSettings, test_support}; +use fabro_types::{RunProjection, RunSpec, test_support}; use serde_json::json; #[test] fn run_projection_reuses_canonical_type() { @@ -129,19 +129,8 @@ fn run_projection_round_trips_with_pending_control_unset() { fn run_spec_json() -> serde_json::Value { serde_json::to_value(RunSpec { - run_id: fabro_types::fixtures::RUN_1, - settings: WorkflowSettings::default(), - graph: Graph::new("test"), - graph_source: Some("digraph test {}".to_string()), - workflow_slug: None, - automation: None, - source_directory: None, - labels: std::collections::HashMap::new(), - provenance: test_support::test_run_provenance(), - manifest_blob: None, - definition_blob: None, - git: None, - fork_source_ref: None, + graph_source: Some("digraph test {}".to_string()), + ..test_support::test_run_spec() }) .unwrap() } diff --git a/lib/foundation/fabro-types/src/run_projection.rs b/lib/foundation/fabro-types/src/run_projection.rs index 3a6be70d4..d41258298 100644 --- a/lib/foundation/fabro-types/src/run_projection.rs +++ b/lib/foundation/fabro-types/src/run_projection.rs @@ -1061,11 +1061,9 @@ impl RunProjection { #[cfg(test)] mod title_tests { - use std::collections::HashMap; - use chrono::Utc; - use crate::{AttrValue, Graph, RunId, RunProjection, RunSpec, WorkflowSettings, test_support}; + use crate::{AttrValue, Graph, RunProjection, RunSpec, test_support}; fn projection_with_goal(goal: Option<&str>) -> RunProjection { let mut graph = Graph::new("test"); @@ -1076,19 +1074,8 @@ mod title_tests { } let spec = RunSpec { - run_id: RunId::new(), - settings: WorkflowSettings::default(), graph, - graph_source: None, - workflow_slug: None, - automation: None, - source_directory: None, - labels: HashMap::new(), - provenance: test_support::test_run_provenance(), - manifest_blob: None, - definition_blob: None, - git: None, - fork_source_ref: None, + ..test_support::test_run_spec() }; RunProjection::new(String::new(), spec, Utc::now()) } @@ -1129,7 +1116,6 @@ mod title_tests { #[cfg(test)] mod iter_stages_tests { - use std::collections::HashMap; use std::num::NonZeroU32; use chrono::Utc; @@ -1137,10 +1123,7 @@ mod iter_stages_tests { use serde_json::json; use super::RunProjection; - use crate::{ - AgentControlState, BilledTokenCounts, Graph, RunId, RunSpec, StageProjection, - WorkflowSettings, test_support, - }; + use crate::{AgentControlState, BilledTokenCounts, StageProjection, test_support}; fn seq(n: u32) -> NonZeroU32 { NonZeroU32::new(n).unwrap() @@ -1149,21 +1132,7 @@ mod iter_stages_tests { fn projection() -> RunProjection { RunProjection::new( "Test run".to_string(), - RunSpec { - run_id: RunId::new(), - settings: WorkflowSettings::default(), - graph: Graph::new("test"), - graph_source: None, - workflow_slug: None, - automation: None, - source_directory: None, - labels: HashMap::default(), - provenance: test_support::test_run_provenance(), - manifest_blob: None, - definition_blob: None, - git: None, - fork_source_ref: None, - }, + test_support::test_run_spec(), Utc::now(), ) } @@ -1336,14 +1305,12 @@ mod iter_stages_tests { #[cfg(test)] mod live_timing_tests { - use std::collections::HashMap; - use chrono::{DateTime, TimeZone, Utc}; use super::{RunProjection, StageToolBatchProjection}; use crate::{ - Graph, ModelRef, RunId, RunSpec, StageHandler, StageInferenceProjection, StageProjection, - StageState, StageTiming, StartRecord, WorkflowSettings, first_event_seq, test_support, + ModelRef, StageHandler, StageInferenceProjection, StageProjection, StageState, StageTiming, + StartRecord, first_event_seq, test_support, }; fn at(seconds: i64) -> DateTime { @@ -1351,25 +1318,7 @@ mod live_timing_tests { } fn projection() -> RunProjection { - RunProjection::new( - "Test run".to_string(), - RunSpec { - run_id: RunId::new(), - settings: WorkflowSettings::default(), - graph: Graph::new("test"), - graph_source: None, - workflow_slug: None, - automation: None, - source_directory: None, - labels: HashMap::default(), - provenance: test_support::test_run_provenance(), - manifest_blob: None, - definition_blob: None, - git: None, - fork_source_ref: None, - }, - at(0), - ) + RunProjection::new("Test run".to_string(), test_support::test_run_spec(), at(0)) } /// In-flight stage that started at `at(0)`. diff --git a/lib/foundation/fabro-types/src/test_support.rs b/lib/foundation/fabro-types/src/test_support.rs index 994813974..dfd0d7e7d 100644 --- a/lib/foundation/fabro-types/src/test_support.rs +++ b/lib/foundation/fabro-types/src/test_support.rs @@ -1,4 +1,8 @@ -use crate::{AuthMethod, IdpIdentity, Principal, RunProvenance}; +use std::collections::HashMap; + +use crate::{ + AuthMethod, Graph, IdpIdentity, Principal, RunProvenance, RunSpec, WorkflowSettings, fixtures, +}; #[must_use] pub fn test_principal() -> Principal { @@ -17,3 +21,33 @@ pub fn test_run_provenance() -> RunProvenance { subject: test_principal(), } } + +/// Neutral [`RunSpec`] for tests: a fixed run id, default settings, a minimal +/// `test` graph, and every optional field unset. +/// +/// Spread it so a test only spells out the fields it actually asserts on: +/// +/// ```ignore +/// let spec = RunSpec { +/// workflow_slug: Some("release-flow".to_string()), +/// ..test_run_spec() +/// }; +/// ``` +#[must_use] +pub fn test_run_spec() -> RunSpec { + RunSpec { + run_id: fixtures::RUN_1, + settings: WorkflowSettings::default(), + graph: Graph::new("test"), + graph_source: None, + workflow_slug: None, + automation: None, + source_directory: None, + labels: HashMap::new(), + provenance: test_run_provenance(), + manifest_blob: None, + definition_blob: None, + git: None, + fork_source_ref: None, + } +} diff --git a/lib/foundation/fabro-types/tests/run_spec_methods.rs b/lib/foundation/fabro-types/tests/run_spec_methods.rs index f6f76fecf..6f6aefaaf 100644 --- a/lib/foundation/fabro-types/tests/run_spec_methods.rs +++ b/lib/foundation/fabro-types/tests/run_spec_methods.rs @@ -3,7 +3,7 @@ use std::collections::HashMap; use fabro_types::graph::Graph; use fabro_types::run::{DirtyStatus, GitContext, RunSpec}; use fabro_types::settings::{ProjectNamespace, WorkflowNamespace}; -use fabro_types::test_support::test_run_provenance; +use fabro_types::test_support::test_run_spec; use fabro_types::{WorkflowSettings, fixtures}; fn sample_run_spec() -> RunSpec { @@ -20,24 +20,18 @@ fn sample_run_spec() -> RunSpec { }; RunSpec { - run_id: fixtures::RUN_1, settings, graph: Graph::new("ship"), - graph_source: None, workflow_slug: Some("demo".to_string()), - automation: None, source_directory: Some("/Users/client/project".to_string()), labels: HashMap::from([("team".to_string(), "platform".to_string())]), - provenance: test_run_provenance(), - manifest_blob: None, - definition_blob: None, git: Some(GitContext { origin_url: "https://github.com/fabro-sh/fabro.git".to_string(), branch: "main".to_string(), sha: Some("abc123".to_string()), dirty: DirtyStatus::Dirty, }), - fork_source_ref: None, + ..test_run_spec() } }