fix(test): reduce shared-daemon cli overhead

This commit is contained in:
Bryan Helmkamp 2026-04-05 00:56:08 -04:00
parent 3642858c46
commit 8d8c3e3637
15 changed files with 135 additions and 52 deletions

View file

@ -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::<Result<Vec<_>>>()?;
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<String>,
last_seq: u32,
initial_exit_code: Option<ExitCode>,
kill_on_detach: bool,
styles: &'static Styles,
engine_child: Option<std::process::Child>,
@ -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<u32> = 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<ExitCode> {
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::*;

View file

@ -108,6 +108,7 @@ pub(crate) async fn connect_server(storage_dir: &Path) -> Result<ServerStoreClie
let http_client = reqwest::ClientBuilder::new()
.unix_socket(socket_path)
.no_proxy()
.build()
.context("Failed to build Unix-socket HTTP client for fabro server")?;
wait_for_server_ready(&http_client).await?;

View file

@ -1,6 +1,6 @@
use fabro_test::{fabro_snapshot, test_context};
use super::support::{read_text, setup_artifact_run, setup_completed_dry_run, text_tree};
use super::support::{read_text, setup_artifact_run, setup_completed_fast_dry_run, text_tree};
#[test]
fn help() {
@ -38,7 +38,7 @@ fn help() {
#[test]
fn artifact_cp_empty_run_reports_no_artifacts() {
let context = test_context!();
let run = setup_completed_dry_run(&context);
let run = setup_completed_fast_dry_run(&context);
let dest = context.temp_dir.join("artifact-dest");
let mut cmd = context.command();
cmd.args(["artifact", "cp", &run.run_id, dest.to_str().unwrap()]);

View file

@ -1,6 +1,6 @@
use fabro_test::{fabro_snapshot, test_context};
use super::support::{setup_artifact_run, setup_completed_dry_run};
use super::support::{setup_artifact_run, setup_completed_fast_dry_run};
#[test]
fn help() {
@ -36,7 +36,7 @@ fn help() {
#[test]
fn artifact_list_empty_run_reports_no_artifacts() {
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(["artifact", "list", &run.run_id]);

View file

@ -3,8 +3,8 @@ use insta::assert_snapshot;
use fabro_test::{fabro_snapshot, test_context};
use super::support::{
compact_git_inspect, compact_inspect, run_success, setup_completed_dry_run,
setup_created_dry_run, setup_git_backed_changed_run,
compact_git_inspect, compact_inspect, run_success, setup_completed_fast_dry_run,
setup_created_fast_dry_run, setup_git_backed_changed_run,
};
#[test]
@ -39,7 +39,7 @@ fn help() {
#[test]
fn inspect_created_run_shows_run_record_without_start_or_conclusion() {
let context = test_context!();
let run = setup_created_dry_run(&context);
let run = setup_created_fast_dry_run(&context);
let output = run_success(&context, &["inspect", &run.run_id]);
assert_snapshot!(serde_json::to_string_pretty(&compact_inspect(&output)).unwrap(), @r###"
@ -66,7 +66,7 @@ fn inspect_created_run_shows_run_record_without_start_or_conclusion() {
#[test]
fn inspect_completed_run_shows_run_start_conclusion_checkpoint() {
let context = test_context!();
let run = setup_completed_dry_run(&context);
let run = setup_completed_fast_dry_run(&context);
let output = run_success(&context, &["inspect", &run.run_id]);
assert_snapshot!(serde_json::to_string_pretty(&compact_inspect(&output)).unwrap(), @r#"
@ -109,7 +109,7 @@ fn inspect_completed_run_shows_run_start_conclusion_checkpoint() {
#[test]
fn inspect_completed_run_reads_store_without_disk_metadata_files() {
let context = test_context!();
let run = setup_completed_dry_run(&context);
let run = setup_completed_fast_dry_run(&context);
for name in [
"run.json",
"start.json",

View file

@ -3,7 +3,7 @@ use std::process::Command;
use fabro_test::test_context;
use serde_json::Value;
use super::support::{fixture, output_stderr, output_stdout, setup_completed_dry_run};
use super::support::{fixture, output_stderr, output_stdout, setup_completed_fast_dry_run};
fn dot_is_available() -> 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()

View file

@ -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"));

View file

@ -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<T>(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| {

View file

@ -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()]);

View file

@ -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()));

View file

@ -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();

View file

@ -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()

View file

@ -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",

View file

@ -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]);

View file

@ -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 {