From df80219a891531252483cdbe0cee7b3e45a36ecf Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Sun, 5 Apr 2026 13:15:59 -0400 Subject: [PATCH] test: speed up slow default-profile tests and tighten nextest thresholds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove redundant config_change_after_submission test (1.67s avg) from fabro-server — already covered by start_run_persists_full_settings_snapshot and architectural guarantees. Defer reqwest::Client init past validation in web_search tool so missing-key/missing-query tests skip macOS proxy discovery (1.56s → 9ms). Move telemetry panic event tests to a CLI IT via a new cfg(debug_assertions) __test_panic subcommand. Lower default nextest SLOW threshold from 3s to 1.5s with 2x headroom over the new worst-case (0.84s). Co-Authored-By: Claude Opus 4.6 (1M context) --- .config/nextest.toml | 2 +- lib/crates/fabro-agent/src/tools.rs | 2 +- lib/crates/fabro-cli/src/args.rs | 9 +++++ lib/crates/fabro-cli/src/main.rs | 6 +++ lib/crates/fabro-cli/tests/it/cmd/mod.rs | 1 + .../fabro-cli/tests/it/cmd/test_panic.rs | 33 ++++++++++++++++ lib/crates/fabro-server/src/server.rs | 39 ------------------- lib/crates/fabro-telemetry/src/panic.rs | 38 ------------------ 8 files changed, 51 insertions(+), 79 deletions(-) create mode 100644 lib/crates/fabro-cli/tests/it/cmd/test_panic.rs diff --git a/.config/nextest.toml b/.config/nextest.toml index 2863a87b2..3478d369a 100644 --- a/.config/nextest.toml +++ b/.config/nextest.toml @@ -1,6 +1,6 @@ [profile.default] # Unit tests: flag SLOW after 5s, hard-kill after 15s -slow-timeout = { period = "3s", terminate-after = 2 } +slow-timeout = { period = "1.5s", terminate-after = 2 } [[profile.default.overrides]] filter = "package(fabro-cli) & kind(test)" diff --git a/lib/crates/fabro-agent/src/tools.rs b/lib/crates/fabro-agent/src/tools.rs index 43c808956..0377ef2bf 100644 --- a/lib/crates/fabro-agent/src/tools.rs +++ b/lib/crates/fabro-agent/src/tools.rs @@ -481,7 +481,6 @@ fn make_web_search_tool_with_api_key(api_key: Option) -> RegisteredTool }), }, executor: Arc::new(move |args, _ctx| { - let client = CLIENT.get_or_init(reqwest::Client::new).clone(); let api_key = api_key.clone(); Box::pin(async move { let api_key = api_key.ok_or_else(|| { @@ -489,6 +488,7 @@ fn make_web_search_tool_with_api_key(api_key: Option) -> RegisteredTool })?; let query = required_str(&args, "query")?; + let client = CLIENT.get_or_init(reqwest::Client::new).clone(); let count = args .get("max_results") .and_then(serde_json::Value::as_u64) diff --git a/lib/crates/fabro-cli/src/args.rs b/lib/crates/fabro-cli/src/args.rs index d010d4846..cf15026e1 100644 --- a/lib/crates/fabro-cli/src/args.rs +++ b/lib/crates/fabro-cli/src/args.rs @@ -819,6 +819,13 @@ pub(crate) enum Commands { /// Path to the JSON event file path: PathBuf, }, + /// Build a panic event and write JSON to stdout (internal testing) + #[cfg(debug_assertions)] + #[command(name = "__test_panic", hide = true)] + TestPanic { + /// Panic message + message: String, + }, } impl Commands { @@ -894,6 +901,8 @@ impl Commands { }, Self::SendAnalytics { .. } => "__send_analytics", Self::SendPanic { .. } => "__send_panic", + #[cfg(debug_assertions)] + Self::TestPanic { .. } => "__test_panic", } } } diff --git a/lib/crates/fabro-cli/src/main.rs b/lib/crates/fabro-cli/src/main.rs index 8e969b878..748f4146c 100644 --- a/lib/crates/fabro-cli/src/main.rs +++ b/lib/crates/fabro-cli/src/main.rs @@ -253,6 +253,12 @@ async fn main_inner() -> (String, Result<()>) { let _ = std::fs::remove_file(&path); result?; } + #[cfg(debug_assertions)] + Commands::TestPanic { message } => { + let event = tel_panic::build_event(&message); + let json = serde_json::to_string_pretty(&event)?; + println!("{json}"); + } } Ok(()) diff --git a/lib/crates/fabro-cli/tests/it/cmd/mod.rs b/lib/crates/fabro-cli/tests/it/cmd/mod.rs index 84875457a..fa8683437 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/mod.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/mod.rs @@ -61,6 +61,7 @@ pub(crate) mod support; mod system; mod system_df; mod system_prune; +mod test_panic; mod top_level; mod upgrade; mod validate; diff --git a/lib/crates/fabro-cli/tests/it/cmd/test_panic.rs b/lib/crates/fabro-cli/tests/it/cmd/test_panic.rs new file mode 100644 index 000000000..77e5fc235 --- /dev/null +++ b/lib/crates/fabro-cli/tests/it/cmd/test_panic.rs @@ -0,0 +1,33 @@ +use fabro_test::test_context; + +#[test] +fn builds_event_and_roundtrips_through_json() { + let context = test_context!(); + let mut cmd = context.command(); + cmd.args(["__test_panic", "test panic message"]); + + let output = cmd.output().expect("command should execute"); + assert!(output.status.success(), "command failed: {output:?}"); + + let stdout = String::from_utf8(output.stdout).unwrap(); + let event: serde_json::Value = serde_json::from_str(&stdout).unwrap(); + + assert_eq!(event["level"], "fatal"); + + let exception = &event["exception"]["values"][0]; + assert_eq!(exception["type"], "panic"); + assert_eq!(exception["value"], "test panic message"); + + let mechanism = &exception["mechanism"]; + assert_eq!(mechanism["type"], "panic"); + assert_eq!(mechanism["handled"], false); + + assert!( + exception["stacktrace"].is_object(), + "stacktrace should be present" + ); + assert!( + event["contexts"]["os"].is_object(), + "OS context should be present" + ); +} diff --git a/lib/crates/fabro-server/src/server.rs b/lib/crates/fabro-server/src/server.rs index aea866e06..97af17332 100644 --- a/lib/crates/fabro-server/src/server.rs +++ b/lib/crates/fabro-server/src/server.rs @@ -2268,18 +2268,6 @@ mod tests { } } - fn command_dot(command: &str) -> String { - format!( - r#"digraph Test {{ - graph [goal="Test"] - start [shape=Mdiamond] - exit [shape=Msquare] - command [shape=parallelogram, tool_command="{command}"] - start -> command -> exit - }}"# - ) - } - fn test_app_with() -> Router { let state = create_app_state(); build_router(state, AuthMode::Disabled) @@ -3387,33 +3375,6 @@ mod tests { assert_eq!(run_record.settings, expected_settings); } - #[tokio::test] - async fn config_change_after_submission_does_not_affect_execution() { - let output_dir = tempfile::tempdir().unwrap(); - let output_path = output_dir.path().join("executed.txt"); - let dot = command_dot(&format!("printf snapshot > {}", output_path.display())); - let initial_settings = dry_run_settings(); - let state = create_app_state_with_options(initial_settings.clone(), 5); - let app = build_router(Arc::clone(&state), AuthMode::Disabled); - - let run_id_str = create_and_start_run(&app, &dot).await; - let run_id = run_id_str.parse::().unwrap(); - - *state.settings.write().unwrap() = Settings::default(); - - execute_run(Arc::clone(&state), run_id).await; - - let runs = state.runs.lock().expect("runs lock poisoned"); - let managed_run = runs.get(&run_id).expect("run should still exist"); - assert_eq!(managed_run.status, RunStatus::Completed); - drop(runs); - - assert!( - !output_path.exists(), - "run should still use snapshotted dry-run settings" - ); - } - #[tokio::test] async fn cancel_queued_run_succeeds() { let state = create_app_state(); diff --git a/lib/crates/fabro-telemetry/src/panic.rs b/lib/crates/fabro-telemetry/src/panic.rs index a2889c18e..b49b06452 100644 --- a/lib/crates/fabro-telemetry/src/panic.rs +++ b/lib/crates/fabro-telemetry/src/panic.rs @@ -130,31 +130,6 @@ pub fn capture(path: &Path) -> anyhow::Result<()> { mod tests { use super::*; - #[test] - fn build_event_structure() { - let event = build_event("test panic message"); - - assert_eq!(event.level, sentry::Level::Fatal); - assert_eq!(event.exception.values.len(), 1); - - let exc = &event.exception.values[0]; - assert_eq!(exc.ty, "panic"); - assert_eq!(exc.value.as_deref(), Some("test panic message")); - - let mech = exc.mechanism.as_ref().unwrap(); - assert_eq!(mech.ty, "panic"); - assert_eq!(mech.handled, Some(false)); - - // Stacktrace should be present. - assert!(exc.stacktrace.is_some()); - - // OS context should be present. - assert!(event.contexts.contains_key("os")); - - // Release should be set. - assert!(event.release.is_some()); - } - #[test] fn broken_pipe_is_filtered() { assert!(is_broken_pipe("Broken pipe (os error 32)")); @@ -179,17 +154,4 @@ mod tests { let err_msg = result.unwrap_err().to_string(); assert!(err_msg.contains("SENTRY_DSN not set")); } - - #[test] - fn event_round_trips_through_json() { - let event = build_event("roundtrip test"); - let json = serde_json::to_vec(&event).unwrap(); - let deserialized: Event<'static> = serde_json::from_slice(&json).unwrap(); - assert_eq!(deserialized.level, sentry::Level::Fatal); - assert_eq!(deserialized.exception.values.len(), 1); - assert_eq!( - deserialized.exception.values[0].value.as_deref(), - Some("roundtrip test") - ); - } }