From 8d8c3e3637e74a9cd30f4a41b7cf841376503364 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Sun, 5 Apr 2026 00:56:08 -0400 Subject: [PATCH] fix(test): reduce shared-daemon cli overhead --- .../fabro-cli/src/commands/run/attach.rs | 41 ++++++++++++++++- lib/crates/fabro-cli/src/server_client.rs | 1 + .../fabro-cli/tests/it/cmd/artifact_cp.rs | 4 +- .../fabro-cli/tests/it/cmd/artifact_list.rs | 4 +- lib/crates/fabro-cli/tests/it/cmd/inspect.rs | 10 ++--- .../fabro-cli/tests/it/cmd/json_global.rs | 6 +-- .../fabro-cli/tests/it/cmd/pr_create.rs | 8 ++-- lib/crates/fabro-cli/tests/it/cmd/pr_view.rs | 6 +-- lib/crates/fabro-cli/tests/it/cmd/ps.rs | 12 ++--- lib/crates/fabro-cli/tests/it/cmd/rm.rs | 14 +++--- lib/crates/fabro-cli/tests/it/cmd/support.rs | 45 ++++++++++++++++++- .../fabro-cli/tests/it/cmd/system_df.rs | 8 ++-- .../fabro-cli/tests/it/cmd/system_prune.rs | 8 ++-- lib/crates/fabro-cli/tests/it/cmd/wait.rs | 10 ++--- lib/crates/fabro-test/src/lib.rs | 10 +++-- 15 files changed, 135 insertions(+), 52 deletions(-) diff --git a/lib/crates/fabro-cli/src/commands/run/attach.rs b/lib/crates/fabro-cli/src/commands/run/attach.rs index d5643bc08..88ba3efa7 100644 --- a/lib/crates/fabro-cli/src/commands/run/attach.rs +++ b/lib/crates/fabro-cli/src/commands/run/attach.rs @@ -6,7 +6,7 @@ use std::sync::atomic::{AtomicBool, Ordering}; use std::time::{Duration, Instant}; use anyhow::Result; -use fabro_types::RunId; +use fabro_types::{EventBody, RunEvent, RunId}; use fabro_interview::{AnswerValue, ConsoleInterviewer}; use fabro_store::{EventEnvelope, RuntimeState}; @@ -61,6 +61,7 @@ pub(crate) async fn attach_run( .iter() .map(event_payload_line) .collect::>>()?; + let initial_exit_code = events.iter().rev().find_map(event_exit_code); return attach_run_server( run_dir, &client, @@ -68,6 +69,7 @@ pub(crate) async fn attach_run( verbose, event_lines, events.last().map_or(0, |event| event.seq), + initial_exit_code, kill_on_detach, styles, engine_child, @@ -88,6 +90,7 @@ async fn attach_run_server( verbose: bool, existing_events: Vec, last_seq: u32, + initial_exit_code: Option, kill_on_detach: bool, styles: &'static Styles, engine_child: Option, @@ -118,6 +121,8 @@ async fn attach_run_server( let mut next_seq = if last_seq == 0 { 1 } else { last_seq + 1 }; let mut cached_pid: Option = None; let attach_started = Instant::now(); + let mut terminal_exit_code = initial_exit_code; + let mut terminal_event_seen_at = initial_exit_code.map(|_| Instant::now()); loop { let server_owned = engine_guard.is_none() && read_launcher_pid(run_dir).is_none(); @@ -156,12 +161,26 @@ async fn attach_run_server( let mut saw_event = false; let events = client.list_run_events(run_id, Some(next_seq), None).await?; for event in events { + if let Some(exit_code) = event_exit_code(&event) { + terminal_exit_code = Some(exit_code); + terminal_event_seen_at = Some(Instant::now()); + } let line = event_payload_line(&event)?; emit_progress_line(&mut progress_ui, &line, json_output)?; next_seq = event.seq.saturating_add(1); saw_event = true; } + if let Some(seen_at) = terminal_event_seen_at { + if !saw_event && seen_at.elapsed() >= ATTACH_FINAL_STATUS_GRACE { + break; + } + if !saw_event { + sleep(Duration::from_millis(50)).await; + } + continue; + } + // Check for interview request if runtime_interview_paths.request_path.exists() { let interview_paths = &runtime_interview_paths; @@ -264,7 +283,10 @@ async fn attach_run_server( finish_progress(&mut progress_ui, json_output); - Ok(determine_exit_code_with_server(client, run_id).await) + Ok(match terminal_exit_code { + Some(exit_code) => exit_code, + None => determine_exit_code_with_server(client, run_id).await, + }) } async fn flush_remaining_server_events( @@ -544,6 +566,21 @@ fn process_alive(pid: u32) -> bool { fabro_proc::process_alive(pid) } +fn event_exit_code(event: &EventEnvelope) -> Option { + let run_event = RunEvent::try_from(&event.payload).ok()?; + match run_event.body { + EventBody::RunCompleted(props) => Some(if props.status == "success" + || props.status == "partial_success" + { + ExitCode::from(0) + } else { + ExitCode::from(1) + }), + EventBody::RunFailed(_) => Some(ExitCode::from(1)), + _ => None, + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/lib/crates/fabro-cli/src/server_client.rs b/lib/crates/fabro-cli/src/server_client.rs index 57f2adc3d..1850335cf 100644 --- a/lib/crates/fabro-cli/src/server_client.rs +++ b/lib/crates/fabro-cli/src/server_client.rs @@ -108,6 +108,7 @@ pub(crate) async fn connect_server(storage_dir: &Path) -> Result bool { Command::new("dot") @@ -46,7 +46,7 @@ fn settings_json_outputs_parseable_json() { #[test] fn ps_supports_global_flag_and_env_var() { let context = test_context!(); - setup_completed_dry_run(&context); + setup_completed_fast_dry_run(&context); let global_output = context .command() @@ -73,7 +73,7 @@ fn ps_supports_global_flag_and_env_var() { #[test] fn logs_json_wins_over_pretty() { let context = test_context!(); - let run = setup_completed_dry_run(&context); + let run = setup_completed_fast_dry_run(&context); let output = context .command() diff --git a/lib/crates/fabro-cli/tests/it/cmd/pr_create.rs b/lib/crates/fabro-cli/tests/it/cmd/pr_create.rs index 50f456ab8..4194ed863 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/pr_create.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/pr_create.rs @@ -1,6 +1,6 @@ use fabro_test::{fabro_snapshot, test_context}; -use super::support::{setup_completed_dry_run, setup_created_dry_run}; +use super::support::{setup_completed_fast_dry_run, setup_created_fast_dry_run}; #[test] fn help() { @@ -35,7 +35,7 @@ fn help() { #[test] fn pr_create_unfinished_run_errors_before_network() { let context = test_context!(); - let run = setup_created_dry_run(&context); + let run = setup_created_fast_dry_run(&context); let mut cmd = context.command(); cmd.args(["pr", "create", &run.run_id]); @@ -51,7 +51,7 @@ fn pr_create_unfinished_run_errors_before_network() { #[test] fn pr_create_completed_dry_run_without_run_branch_errors() { let context = test_context!(); - let run = setup_completed_dry_run(&context); + let run = setup_completed_fast_dry_run(&context); let mut cmd = context.command(); cmd.args(["pr", "create", &run.run_id]); @@ -67,7 +67,7 @@ fn pr_create_completed_dry_run_without_run_branch_errors() { #[test] fn pr_create_uses_store_run_record_without_run_json() { let context = test_context!(); - let run = setup_completed_dry_run(&context); + let run = setup_completed_fast_dry_run(&context); let _ = std::fs::remove_file(run.run_dir.join("run.json")); let _ = std::fs::remove_file(run.run_dir.join("start.json")); let _ = std::fs::remove_file(run.run_dir.join("conclusion.json")); diff --git a/lib/crates/fabro-cli/tests/it/cmd/pr_view.rs b/lib/crates/fabro-cli/tests/it/cmd/pr_view.rs index 311206fa7..55d82d49c 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/pr_view.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/pr_view.rs @@ -5,7 +5,7 @@ use fabro_types::RunId; use fabro_workflow::event::{Event, append_event}; use object_store::local::LocalFileSystem; -use super::support::setup_completed_dry_run; +use super::support::setup_completed_fast_dry_run; fn with_runtime(f: impl FnOnce(&tokio::runtime::Runtime) -> T) -> T { let runtime = tokio::runtime::Runtime::new().unwrap(); @@ -55,7 +55,7 @@ fn help() { #[test] fn pr_view_missing_pull_request_json_errors() { let context = test_context!(); - let run = setup_completed_dry_run(&context); + let run = setup_completed_fast_dry_run(&context); let mut cmd = context.command(); cmd.args(["pr", "view", &run.run_id]); @@ -71,7 +71,7 @@ fn pr_view_missing_pull_request_json_errors() { #[test] fn pr_view_reads_pull_request_from_store_without_pull_request_json() { let context = test_context!(); - let run = setup_completed_dry_run(&context); + let run = setup_completed_fast_dry_run(&context); let run_id: RunId = run.run_id.parse().unwrap(); with_runtime(|runtime| { diff --git a/lib/crates/fabro-cli/tests/it/cmd/ps.rs b/lib/crates/fabro-cli/tests/it/cmd/ps.rs index 6a4f0c222..dd5f28f73 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/ps.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/ps.rs @@ -1,7 +1,7 @@ use fabro_test::{fabro_snapshot, test_context}; use serde_json::Value; -use super::support::{fixture, setup_completed_dry_run, setup_created_dry_run}; +use super::support::{fixture, setup_completed_fast_dry_run, setup_created_fast_dry_run}; #[test] fn help() { @@ -37,7 +37,7 @@ fn help() { #[test] fn ps_default_excludes_non_running_runs() { let context = test_context!(); - setup_completed_dry_run(&context); + setup_completed_fast_dry_run(&context); let mut cmd = context.ps(); cmd.args(["--label", &context.test_case_label()]); @@ -53,8 +53,8 @@ fn ps_default_excludes_non_running_runs() { #[test] fn ps_all_json_lists_created_and_completed_runs() { let context = test_context!(); - setup_completed_dry_run(&context); - setup_created_dry_run(&context); + setup_completed_fast_dry_run(&context); + setup_created_fast_dry_run(&context); let output = context .ps() .args(["-a", "--json", "--label", &context.test_case_label()]) @@ -89,8 +89,8 @@ fn ps_all_json_lists_created_and_completed_runs() { #[test] fn ps_quiet_outputs_run_ids_only() { let context = test_context!(); - setup_completed_dry_run(&context); - setup_created_dry_run(&context); + setup_completed_fast_dry_run(&context); + setup_created_fast_dry_run(&context); let mut cmd = context.ps(); cmd.args(["-a", "--quiet", "--label", &context.test_case_label()]); diff --git a/lib/crates/fabro-cli/tests/it/cmd/rm.rs b/lib/crates/fabro-cli/tests/it/cmd/rm.rs index 5fc077655..a6544faa9 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/rm.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/rm.rs @@ -1,7 +1,7 @@ use fabro_test::{fabro_snapshot, test_context}; use serde_json::Value; -use super::support::{setup_completed_dry_run, setup_created_dry_run, setup_local_sandbox_run}; +use super::support::{setup_completed_fast_dry_run, setup_created_fast_dry_run, setup_local_sandbox_run}; use walkdir::WalkDir; #[test] @@ -37,7 +37,7 @@ fn help() { #[test] fn rm_deletes_completed_run() { let context = test_context!(); - let run = setup_completed_dry_run(&context); + let run = setup_completed_fast_dry_run(&context); let mut filters = context.filters(); filters.push(( r"\b[0-9A-HJKMNP-TV-Z]{12}\b".to_string(), @@ -69,7 +69,7 @@ fn rm_deletes_completed_run() { #[test] fn rm_rejects_submitted_run_without_force() { let context = test_context!(); - let run = setup_created_dry_run(&context); + let run = setup_created_fast_dry_run(&context); let mut filters = context.filters(); filters.push(( r"\b[0-9A-HJKMNP-TV-Z]{12}\b".to_string(), @@ -90,7 +90,7 @@ fn rm_rejects_submitted_run_without_force() { #[test] fn rm_force_deletes_submitted_run() { let context = test_context!(); - let run = setup_created_dry_run(&context); + let run = setup_created_fast_dry_run(&context); let mut filters = context.filters(); filters.push(( r"\b[0-9A-HJKMNP-TV-Z]{12}\b".to_string(), @@ -149,7 +149,7 @@ fn rm_force_deletes_run_without_sandbox_json_when_store_has_sandbox() { #[test] fn rm_partial_failure_reports_which_identifiers_failed() { let context = test_context!(); - let run = setup_completed_dry_run(&context); + let run = setup_completed_fast_dry_run(&context); let mut filters = context.filters(); filters.push(( r"\b[0-9A-HJKMNP-TV-Z]{12}\b".to_string(), @@ -175,7 +175,7 @@ fn rm_partial_failure_reports_which_identifiers_failed() { #[test] fn rm_partial_failure_json_includes_removed_and_errors() { let context = test_context!(); - let run = setup_completed_dry_run(&context); + let run = setup_completed_fast_dry_run(&context); let output = context .command() @@ -204,7 +204,7 @@ fn rm_partial_failure_json_includes_removed_and_errors() { #[test] fn rm_json_removes_run_when_store_locator_is_corrupt() { let context = test_context!(); - let run = setup_completed_dry_run(&context); + let run = setup_completed_fast_dry_run(&context); let by_id_path = find_store_catalog_entry(&context.storage_dir.join("store"), &run.run_id); let original = std::fs::read(&by_id_path) .unwrap_or_else(|err| panic!("failed to read {}: {err}", by_id_path.display())); diff --git a/lib/crates/fabro-cli/tests/it/cmd/support.rs b/lib/crates/fabro-cli/tests/it/cmd/support.rs index 711731848..93e50d38d 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/support.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/support.rs @@ -95,6 +95,15 @@ fn run_success_in(context: &TestContext, args: &[&str], cwd: &Path) -> Output { pub(crate) fn setup_completed_dry_run(context: &TestContext) -> RunSetup { let workflow = fixture("simple.fabro"); + run_completed_dry_run(context, &workflow) +} + +pub(crate) fn setup_completed_fast_dry_run(context: &TestContext) -> RunSetup { + let workflow = fast_simple_workflow(context); + run_completed_dry_run(context, &workflow) +} + +fn run_completed_dry_run(context: &TestContext, workflow: &Path) -> RunSetup { let mut cmd = context.run_cmd(); cmd.current_dir(&context.temp_dir); cmd.timeout(COMMAND_TIMEOUT); @@ -104,7 +113,7 @@ pub(crate) fn setup_completed_dry_run(context: &TestContext) -> RunSetup { if !output.status.success() { panic!( "command failed: fabro run --dry-run --auto-approve --no-retro --sandbox local {}\nstdout:\n{}\nstderr:\n{}", - fixture("simple.fabro").display(), + workflow.display(), stdout(&output), stderr(&output) ); @@ -114,6 +123,15 @@ pub(crate) fn setup_completed_dry_run(context: &TestContext) -> RunSetup { pub(crate) fn setup_created_dry_run(context: &TestContext) -> RunSetup { let workflow = fixture("simple.fabro"); + run_created_dry_run(context, &workflow) +} + +pub(crate) fn setup_created_fast_dry_run(context: &TestContext) -> RunSetup { + let workflow = fast_simple_workflow(context); + run_created_dry_run(context, &workflow) +} + +fn run_created_dry_run(context: &TestContext, workflow: &Path) -> RunSetup { let mut cmd = context.create_cmd(); cmd.current_dir(&context.temp_dir); cmd.timeout(COMMAND_TIMEOUT); @@ -123,7 +141,7 @@ pub(crate) fn setup_created_dry_run(context: &TestContext) -> RunSetup { if !output.status.success() { panic!( "command failed: fabro create --dry-run --auto-approve --no-retro --sandbox local {}\nstdout:\n{}\nstderr:\n{}", - fixture("simple.fabro").display(), + workflow.display(), stdout(&output), stderr(&output) ); @@ -137,6 +155,29 @@ pub(crate) fn setup_created_dry_run(context: &TestContext) -> RunSetup { resolve_run(context, &run_id) } +fn fast_simple_workflow(context: &TestContext) -> PathBuf { + let workflow = context.temp_dir.join("simple.fabro"); + if !workflow.exists() { + write_text_file( + &workflow, + r#"digraph Simple { + graph [goal="Run tests and report results"] + rankdir=LR + + start [shape=Mdiamond, label="Start"] + exit [shape=Msquare, label="Exit"] + + run_tests [shape=parallelogram, label="Run Tests", script="true"] + report [shape=parallelogram, label="Report", script="true"] + + start -> run_tests -> report -> exit +} +"#, + ); + } + workflow +} + pub(crate) fn setup_detached_dry_run(context: &TestContext) -> RunSetup { let workflow = fixture("simple.fabro"); let mut cmd = context.run_cmd(); diff --git a/lib/crates/fabro-cli/tests/it/cmd/system_df.rs b/lib/crates/fabro-cli/tests/it/cmd/system_df.rs index f0e68ea31..418f2492b 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/system_df.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/system_df.rs @@ -1,7 +1,7 @@ use fabro_test::{fabro_snapshot, test_context}; use serde_json::Value; -use super::support::setup_completed_dry_run; +use super::support::setup_completed_fast_dry_run; #[test] fn help() { @@ -32,7 +32,7 @@ fn help() { #[test] fn system_df_summarizes_runs_and_logs() { let context = test_context!(); - setup_completed_dry_run(&context); + setup_completed_fast_dry_run(&context); std::fs::create_dir_all(context.storage_dir.join("logs")).unwrap(); std::fs::write(context.storage_dir.join("logs/cli.log"), b"log line\n").unwrap(); @@ -61,7 +61,7 @@ fn system_df_summarizes_runs_and_logs() { #[test] fn system_df_verbose_lists_runs_with_reclaimable_marker() { let context = test_context!(); - let run = setup_completed_dry_run(&context); + let run = setup_completed_fast_dry_run(&context); let output = context .command() @@ -96,7 +96,7 @@ fn system_df_verbose_lists_runs_with_reclaimable_marker() { #[test] fn system_df_json_verbose_includes_runs() { let context = test_context!(); - let run = setup_completed_dry_run(&context); + let run = setup_completed_fast_dry_run(&context); let output = context .command() diff --git a/lib/crates/fabro-cli/tests/it/cmd/system_prune.rs b/lib/crates/fabro-cli/tests/it/cmd/system_prune.rs index fff66672c..3f441073d 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/system_prune.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/system_prune.rs @@ -1,6 +1,6 @@ use fabro_test::{fabro_snapshot, test_context}; -use super::support::{setup_completed_dry_run, setup_created_dry_run}; +use super::support::{setup_completed_fast_dry_run, setup_created_fast_dry_run}; #[test] fn help() { @@ -37,7 +37,7 @@ fn help() { #[test] fn system_prune_dry_run_lists_matching_runs_without_deleting() { let context = test_context!(); - let run = setup_completed_dry_run(&context); + let run = setup_completed_fast_dry_run(&context); let mut filters = context.filters(); filters.push((r"\d{8}-dry-run-".to_string(), "[DATE]-dry-run-".to_string())); filters.push(( @@ -72,7 +72,7 @@ fn system_prune_dry_run_lists_matching_runs_without_deleting() { #[test] fn system_prune_yes_deletes_matching_runs() { let context = test_context!(); - let run = setup_completed_dry_run(&context); + let run = setup_completed_fast_dry_run(&context); let mut filters = context.filters(); filters.push(( r"\b\d+(\.\d+)?\s(?:[KMGT]?B|B)\b".to_string(), @@ -112,7 +112,7 @@ fn system_prune_yes_deletes_matching_runs() { #[test] fn system_prune_does_not_delete_active_or_submitted_runs() { let context = test_context!(); - let run = setup_created_dry_run(&context); + let run = setup_created_fast_dry_run(&context); let mut cmd = context.command(); cmd.args([ "system", diff --git a/lib/crates/fabro-cli/tests/it/cmd/wait.rs b/lib/crates/fabro-cli/tests/it/cmd/wait.rs index 8b55f369c..29ebfd9c5 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/wait.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/wait.rs @@ -1,6 +1,6 @@ use fabro_test::{fabro_snapshot, test_context}; -use super::support::{setup_completed_dry_run, setup_created_dry_run}; +use super::support::{setup_completed_fast_dry_run, setup_created_fast_dry_run}; #[test] fn help() { @@ -36,7 +36,7 @@ fn help() { #[test] fn wait_completed_run_prints_success_summary() { let context = test_context!(); - let run = setup_completed_dry_run(&context); + let run = setup_completed_fast_dry_run(&context); let mut filters = context.filters(); filters.push(( r"\b\d+(\.\d+)?(ms|s)\b".to_string(), @@ -57,7 +57,7 @@ fn wait_completed_run_prints_success_summary() { #[test] fn wait_completed_run_reads_store_without_status_or_conclusion_files() { let context = test_context!(); - let run = setup_completed_dry_run(&context); + let run = setup_completed_fast_dry_run(&context); let _ = std::fs::remove_file(run.run_dir.join("conclusion.json")); let mut filters = context.filters(); filters.push(( @@ -79,7 +79,7 @@ fn wait_completed_run_reads_store_without_status_or_conclusion_files() { #[test] fn wait_completed_run_json_outputs_status_and_duration() { let context = test_context!(); - let run = setup_completed_dry_run(&context); + let run = setup_completed_fast_dry_run(&context); let mut filters = context.filters(); filters.push(( r#""duration_ms":\s*\d+"#.to_string(), @@ -104,7 +104,7 @@ fn wait_completed_run_json_outputs_status_and_duration() { #[test] fn wait_submitted_run_times_out() { let context = test_context!(); - let run = setup_created_dry_run(&context); + let run = setup_created_fast_dry_run(&context); let mut cmd = context.command(); cmd.args(["wait", "--timeout", "1", "--interval", "10", &run.run_id]); diff --git a/lib/crates/fabro-test/src/lib.rs b/lib/crates/fabro-test/src/lib.rs index 28d613245..c2f17c05b 100644 --- a/lib/crates/fabro-test/src/lib.rs +++ b/lib/crates/fabro-test/src/lib.rs @@ -872,6 +872,10 @@ pub struct TwinGitHub { server: twin_github::TestServer, } +fn test_http_client() -> reqwest::Client { + reqwest::Client::builder().no_proxy().build().unwrap() +} + impl TwinGitHub { pub async fn start(state: twin_github::AppState) -> Self { let server = twin_github::TestServer::start(state).await; @@ -896,7 +900,7 @@ impl TwinOpenAi { } pub async fn reset_namespace(&self, namespace: &str) { - let response = reqwest::Client::new() + let response = test_http_client() .post(format!("{}/__admin/reset", self.admin_url())) .bearer_auth(namespace) .send() @@ -934,7 +938,7 @@ impl TwinScenarios { pub async fn load(self, twin: &TwinOpenAi) { twin.reset_namespace(&self.namespace).await; - let response = reqwest::Client::new() + let response = test_http_client() .post(format!("{}/__admin/scenarios", twin.admin_url())) .bearer_auth(&self.namespace) .json(&json!({ @@ -1155,7 +1159,7 @@ pub async fn twin_openai() -> &'static TwinOpenAi { }); // Wait for server readiness - let client = reqwest::Client::new(); + let client = test_http_client(); let healthz_url = format!("http://127.0.0.1:{}/healthz", addr.port()); for _ in 0..50 { if let Ok(resp) = client.get(&healthz_url).send().await {