From 0b91c71514b0485ce221ba85eb2ebd825eec34ef Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Sun, 19 Apr 2026 21:01:25 -0400 Subject: [PATCH] test(unwrap): clean shared support and workflow fixtures --- lib/crates/fabro-llm/tests/integration.rs | 5 ++- lib/crates/fabro-test/src/lib.rs | 37 ++++++++++--------- .../tests/it/daytona_integration.rs | 8 +++- .../tests/it/git_integration.rs | 24 +++++++----- .../fabro-workflow/tests/it/integration.rs | 12 +++--- test/twin/github/src/handlers/git.rs | 6 +-- test/twin/github/src/server.rs | 9 ++++- test/twin/github/src/state.rs | 10 +++-- 8 files changed, 67 insertions(+), 44 deletions(-) diff --git a/lib/crates/fabro-llm/tests/integration.rs b/lib/crates/fabro-llm/tests/integration.rs index 28de85a83..24199cf1f 100644 --- a/lib/crates/fabro-llm/tests/integration.rs +++ b/lib/crates/fabro-llm/tests/integration.rs @@ -173,7 +173,10 @@ async fn run_multi_turn_cache_test( provider_options: None, }; - let response = adapter.complete(&request).await.unwrap(); + let response = adapter + .complete(&request) + .await + .expect("provider adapter should return a response"); let text = response.text(); assert!( !text.is_empty(), diff --git a/lib/crates/fabro-test/src/lib.rs b/lib/crates/fabro-test/src/lib.rs index 9228c9ce2..67ad787d8 100644 --- a/lib/crates/fabro-test/src/lib.rs +++ b/lib/crates/fabro-test/src/lib.rs @@ -1000,32 +1000,32 @@ impl TestContext { &session_paths.server.socket_path, false, ); + let temp_dir_str = temp_dir + .to_str() + .expect("temp_dir should be valid UTF-8 for snapshot filtering"); + let home_dir_str = home_dir + .to_str() + .expect("home_dir should be valid UTF-8 for snapshot filtering"); + let storage_dir_str = storage_dir + .to_str() + .expect("storage_dir should be valid UTF-8 for snapshot filtering"); let filters = vec![ ( - regex::escape(&format!("/private{}", temp_dir.to_str().unwrap())), + regex::escape(&format!("/private{temp_dir_str}")), "[TEMP_DIR]".to_string(), ), + (regex::escape(temp_dir_str), "[TEMP_DIR]".to_string()), ( - regex::escape(temp_dir.to_str().unwrap()), - "[TEMP_DIR]".to_string(), - ), - ( - regex::escape(&format!("/private{}", home_dir.to_str().unwrap())), + regex::escape(&format!("/private{home_dir_str}")), "[HOME_DIR]".to_string(), ), + (regex::escape(home_dir_str), "[HOME_DIR]".to_string()), ( - regex::escape(home_dir.to_str().unwrap()), - "[HOME_DIR]".to_string(), - ), - ( - regex::escape(&format!("/private{}", storage_dir.to_str().unwrap())), - "[STORAGE_DIR]".to_string(), - ), - ( - regex::escape(storage_dir.to_str().unwrap()), + regex::escape(&format!("/private{storage_dir_str}")), "[STORAGE_DIR]".to_string(), ), + (regex::escape(storage_dir_str), "[STORAGE_DIR]".to_string()), (regex::escape(&test_case_id), "[TEST_CASE]".to_string()), (regex::escape(&test_run_id), "[TEST_RUN]".to_string()), ]; @@ -1419,7 +1419,10 @@ impl TestContext { self.test_case_id(), scratch_dir.display() ); - entries.into_iter().next().unwrap() + entries + .into_iter() + .next() + .expect("exactly one run directory should exist after the length check") } } @@ -1584,7 +1587,7 @@ pub struct TwinGitHub { } pub fn test_http_client() -> fabro_http::HttpClient { - fabro_http::test_http_client().unwrap() + fabro_http::test_http_client().expect("test HTTP client should build") } impl TwinGitHub { diff --git a/lib/crates/fabro-workflow/tests/it/daytona_integration.rs b/lib/crates/fabro-workflow/tests/it/daytona_integration.rs index e9bf711f5..25db32e37 100644 --- a/lib/crates/fabro-workflow/tests/it/daytona_integration.rs +++ b/lib/crates/fabro-workflow/tests/it/daytona_integration.rs @@ -1001,7 +1001,9 @@ async fn run_daytona_cli_test(provider: Provider, model: &str, install_command: let env = DaytonaSandbox::new(config, Some(creds), None, None, None) .await .expect("Failed to create Daytona client — is DAYTONA_API_KEY set?"); - env.initialize().await.unwrap(); + env.initialize() + .await + .expect("Daytona sandbox should initialize"); let env: Arc = Arc::new(env); // Install prerequisites (bash, curl, Node 20 via nodesource) if not available @@ -1079,7 +1081,9 @@ async fn run_daytona_cli_test(provider: Provider, model: &str, install_command: Err(e) => panic!("{provider}/{model} on Daytona failed: {e}"), } - env.cleanup().await.unwrap(); + env.cleanup() + .await + .expect("Daytona sandbox cleanup should succeed"); } #[fabro_macros::e2e_test(live("DAYTONA_API_KEY"), live("GITHUB_APP_PRIVATE_KEY"))] diff --git a/lib/crates/fabro-workflow/tests/it/git_integration.rs b/lib/crates/fabro-workflow/tests/it/git_integration.rs index 72b508206..caf03567a 100644 --- a/lib/crates/fabro-workflow/tests/it/git_integration.rs +++ b/lib/crates/fabro-workflow/tests/it/git_integration.rs @@ -32,12 +32,12 @@ fn assert_success(output: &Output, context: &str) { } fn init_repo(dir: &Path) { - std::fs::create_dir_all(dir).unwrap(); + std::fs::create_dir_all(dir).expect("failed to create repo dir"); let init = Command::new("git") .args(["init"]) .current_dir(dir) .output() - .unwrap(); + .expect("git init should run"); assert_success(&init, "git init"); let commit = Command::new("git") .args([ @@ -52,17 +52,21 @@ fn init_repo(dir: &Path) { ]) .current_dir(dir) .output() - .unwrap(); + .expect("git commit --allow-empty should run"); assert_success(&commit, "git commit --allow-empty"); } fn init_bare_remote(dir: &Path) { - std::fs::create_dir_all(dir.parent().unwrap()).unwrap(); + std::fs::create_dir_all( + dir.parent() + .expect("bare remote path should have a parent directory"), + ) + .expect("failed to create bare remote parent dir"); let init = Command::new("git") .args(["init", "--bare"]) .arg(dir) .output() - .unwrap(); + .expect("git init --bare should run"); assert_success(&init, "git init --bare"); } @@ -72,7 +76,7 @@ fn add_origin(repo_dir: &Path, remote_dir: &Path) { .arg(remote_dir) .current_dir(repo_dir) .output() - .unwrap(); + .expect("git remote add origin should run"); assert_success(&output, "git remote add origin"); } @@ -81,7 +85,7 @@ fn rename_branch(repo_dir: &Path, branch: &str) { .args(["branch", "-M", branch]) .current_dir(repo_dir) .output() - .unwrap(); + .expect("git branch -M should run"); assert_success(&output, "git branch -M"); } @@ -99,7 +103,7 @@ fn empty_commit(repo_dir: &Path, message: &str) { ]) .current_dir(repo_dir) .output() - .unwrap(); + .expect("git commit --allow-empty should run"); assert_success(&output, "git commit --allow-empty"); } @@ -108,9 +112,9 @@ fn list_branch(repo_dir: &Path, branch: &str) -> String { .args(["branch", "--list", branch]) .current_dir(repo_dir) .output() - .unwrap(); + .expect("git branch --list should run"); assert_success(&output, "git branch --list"); - String::from_utf8(output.stdout).unwrap() + String::from_utf8(output.stdout).expect("git branch --list output should be UTF-8") } fn local_env(repo: &Path) -> Arc { diff --git a/lib/crates/fabro-workflow/tests/it/integration.rs b/lib/crates/fabro-workflow/tests/it/integration.rs index 2752c8eee..b2fb8d21b 100644 --- a/lib/crates/fabro-workflow/tests/it/integration.rs +++ b/lib/crates/fabro-workflow/tests/it/integration.rs @@ -188,7 +188,9 @@ fn load_run_checkpoint(run_dir: &Path) -> Result ArtifactStore { @@ -344,7 +346,7 @@ async fn end_to_end_linear_pipeline() { let graph = parse(input).expect("parse should succeed"); validate_or_raise(&graph, &[]).expect("validation should pass"); - let dir = tempfile::tempdir().unwrap(); + let dir = tempfile::tempdir().expect("temporary run dir should be created"); let engine = WorkflowRunner::new( make_linear_registry(), Arc::new(Emitter::default()), @@ -591,7 +593,7 @@ async fn end_to_end_human_gate_pipeline() { }]); let interviewer = Arc::new(QueueInterviewer::new(answers)); - let dir = tempfile::tempdir().unwrap(); + let dir = tempfile::tempdir().expect("temporary run dir should be created"); let mut registry = HandlerRegistry::new(Box::new(StartHandler)); registry.register("start", Box::new(StartHandler)); registry.register("exit", Box::new(ExitHandler)); @@ -686,7 +688,7 @@ async fn human_gate_interrupted_input_fails_closed_without_fail_route() { let interviewer = Arc::new(CallbackInterviewer::new(|_| Answer::interrupted())); - let dir = tempfile::tempdir().unwrap(); + let dir = tempfile::tempdir().expect("temporary run dir should be created"); let mut registry = HandlerRegistry::new(Box::new(StartHandler)); registry.register("start", Box::new(StartHandler)); registry.register("exit", Box::new(ExitHandler)); @@ -8384,7 +8386,7 @@ async fn run_fidelity_prompt_pipeline(fidelity: &str) -> String { graph.edges.push(Edge::new("run_tests", "report")); graph.edges.push(Edge::new("report", "exit")); - let dir = tempfile::tempdir().unwrap(); + let dir = tempfile::tempdir().expect("temporary run dir should be created"); let mut registry = HandlerRegistry::new(Box::new(StartHandler)); registry.register("start", Box::new(StartHandler)); registry.register("exit", Box::new(ExitHandler)); diff --git a/test/twin/github/src/handlers/git.rs b/test/twin/github/src/handlers/git.rs index 1676ce8bd..4035e5d05 100644 --- a/test/twin/github/src/handlers/git.rs +++ b/test/twin/github/src/handlers/git.rs @@ -132,7 +132,7 @@ async fn handle_git_cgi( .status(StatusCode::UNAUTHORIZED) .header("WWW-Authenticate", "Basic realm=\"twin-github\"") .body(Body::from("Authentication required")) - .unwrap(); + .expect("unauthorized response should build"); } Some((_username, password)) => { let token_info = state.validate_token(&password); @@ -250,7 +250,7 @@ fn parse_cgi_response(raw: &[u8]) -> Response { return Response::builder() .status(StatusCode::OK) .body(Body::from(raw.to_vec())) - .unwrap(); + .expect("fallback CGI response should build"); }; let header_bytes = &raw[..header_end]; @@ -277,7 +277,7 @@ fn parse_cgi_response(raw: &[u8]) -> Response { builder .status(status) .body(Body::from(body_bytes.to_vec())) - .unwrap() + .expect("CGI response should build") } fn find_subsequence(haystack: &[u8], needle: &[u8]) -> Option { diff --git a/test/twin/github/src/server.rs b/test/twin/github/src/server.rs index 89fc93416..a4a2b36ff 100644 --- a/test/twin/github/src/server.rs +++ b/test/twin/github/src/server.rs @@ -27,8 +27,13 @@ impl TestServer { let shared_state: SharedState = Arc::new(RwLock::new(state)); let router = build_router(shared_state); - let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); - let port = listener.local_addr().unwrap().port(); + let listener = TcpListener::bind("127.0.0.1:0") + .await + .expect("test server should bind an ephemeral port"); + let port = listener + .local_addr() + .expect("bound test server should have a local address") + .port(); let url = format!("http://127.0.0.1:{port}"); let (shutdown_tx, shutdown_rx) = tokio::sync::oneshot::channel::<()>(); diff --git a/test/twin/github/src/state.rs b/test/twin/github/src/state.rs index 2ebb3a156..136dfc62d 100644 --- a/test/twin/github/src/state.rs +++ b/test/twin/github/src/state.rs @@ -276,12 +276,14 @@ pub fn derive_public_key_pem(private_key_pem: &str) -> String { child .stdin .take() - .unwrap() + .expect("openssl child should expose stdin") .write_all(private_key_pem.as_bytes()) - .unwrap(); - let output = child.wait_with_output().unwrap(); + .expect("private key should be written to openssl stdin"); + let output = child + .wait_with_output() + .expect("openssl child should exit successfully"); assert!(output.status.success(), "openssl rsa -pubout failed"); - String::from_utf8(output.stdout).unwrap() + String::from_utf8(output.stdout).expect("openssl public key output should be UTF-8") } impl AppState {