From 0dcec5af77ddfcee84615192f2b5f2f76412414b Mon Sep 17 00:00:00 2001 From: Scott Werner Date: Sun, 13 Sep 2026 09:16:31 -0600 Subject: [PATCH] Drop redundant create tests and reuse the production client in fixtures --- lib/apps/fabro-cli/tests/it/cmd/support.rs | 124 +++++------ lib/apps/fabro-server/src/server/tests.rs | 199 ------------------ .../fabro-server/tests/it/scenario/dry_run.rs | 12 -- 3 files changed, 57 insertions(+), 278 deletions(-) diff --git a/lib/apps/fabro-cli/tests/it/cmd/support.rs b/lib/apps/fabro-cli/tests/it/cmd/support.rs index 0256417a5..5527027b7 100644 --- a/lib/apps/fabro-cli/tests/it/cmd/support.rs +++ b/lib/apps/fabro-cli/tests/it/cmd/support.rs @@ -9,6 +9,7 @@ reason = "These CLI integration test helpers shell out to real git and fabro binaries while constructing fixtures." )] +use std::collections::BTreeMap; use std::path::{Path, PathBuf}; use std::process::Output; use std::sync::atomic::{AtomicU64, Ordering}; @@ -17,13 +18,17 @@ use std::time::{Duration, Instant}; use base64::Engine as _; use base64::engine::general_purpose::STANDARD as BASE64_STANDARD; +use fabro_client::Client; use fabro_config::bind::Bind; use fabro_config::daemon::ServerDaemon; use fabro_config::{Storage, envfile}; use fabro_store::EventEnvelope; use fabro_test::{TestContext, expect_reqwest_status}; use fabro_types::test_support::test_principal; -use fabro_types::{RunId, StageId}; +use fabro_types::{ + GitRunTarget, RunId, RunIntent, RunIntentArgs, RunTarget, StageId, WorkflowPath, + WorkflowVersion, +}; use httpmock::{HttpMockResponse, Mock, MockServer}; use serde_json::Value; use shlex::try_quote; @@ -791,6 +796,10 @@ pub(crate) fn server_endpoint(storage_dir: &Path) -> Option<(fabro_http::HttpCli let runtime_directory = Storage::new(storage_dir).runtime_directory(); let daemon = ServerDaemon::read(&runtime_directory).ok().flatten()?; let mut headers = fabro_http::HeaderMap::new(); + headers.insert( + fabro_http::header::USER_AGENT, + fabro_http::HeaderValue::from_static("fabro-cli/test"), + ); if let Some(token) = local_dev_token(storage_dir) { headers.insert( fabro_http::header::AUTHORIZATION, @@ -934,11 +943,12 @@ async fn seed_dry_run(context: &TestContext, state: SeededRunState) -> RunSetup context, "simple.fabro", fast_simple_workflow_source(), - serde_json::json!({ - "dry_run": true, - "auto_approve": true, - "labels": test_label_map(context), - }), + RunIntentArgs { + dry_run: Some(true), + auto_approve: Some(true), + labels: test_label_map(context), + ..Default::default() + }, false, ) .await; @@ -959,10 +969,11 @@ async fn seed_git_backed_changed_run(context: &TestContext) -> SeededGitRunSetup context, "flow.fabro", changed_git_workflow_source(), - serde_json::json!({ - "provider": "openai", - "labels": test_label_map(context), - }), + RunIntentArgs { + provider: Some("openai".to_string()), + labels: test_label_map(context), + ..Default::default() + }, true, ) .await; @@ -993,10 +1004,11 @@ async fn seed_git_backed_noop_run(context: &TestContext) -> RunSetup { context, "flow.fabro", noop_git_workflow_source(), - serde_json::json!({ - "provider": "openai", - "labels": test_label_map(context), - }), + RunIntentArgs { + provider: Some("openai".to_string()), + labels: test_label_map(context), + ..Default::default() + }, true, ) .await; @@ -1014,9 +1026,10 @@ async fn seed_artifact_run(context: &TestContext) -> RunSetup { context, "artifact_run.fabro", artifact_workflow_source(), - serde_json::json!({ - "labels": test_label_map(context), - }), + RunIntentArgs { + labels: test_label_map(context), + ..Default::default() + }, false, ) .await; @@ -1052,7 +1065,7 @@ async fn create_seeded_run( context: &TestContext, target_path: &str, source: &str, - args: serde_json::Value, + args: RunIntentArgs, git: bool, ) -> RunSetup { let target = if git { @@ -1063,64 +1076,41 @@ async fn create_seeded_run( "origin", "https://github.com/fabro-sh/seeded-fixture.git", ]); - serde_json::json!({"kind": "git", "repo": "fabro-sh/seeded-fixture", "branch": "main", "sha": sha}) + RunTarget::Git(GitRunTarget { + repo: "fabro-sh/seeded-fixture".to_string(), + branch: "main".to_string(), + sha: Some(sha), + tag: None, + }) } else { - serde_json::json!({"kind": "none"}) + RunTarget::None {} }; let (client, base_url) = server_endpoint(&context.storage_dir) .expect("test server endpoint should be available for seeded run creation"); - let path = - fabro_types::WorkflowPath::new(target_path).expect("seeded workflow path should be valid"); - let version = fabro_types::WorkflowVersion::new( + let client = Client::from_http_client(base_url, client); + let path = WorkflowPath::new(target_path).expect("seeded workflow path should be valid"); + let version = WorkflowVersion::new( path.clone(), - std::collections::BTreeMap::from([(path, source.to_string())]), - std::collections::BTreeMap::new(), + BTreeMap::from([(path, source.to_string())]), + BTreeMap::new(), ) - .expect("seeded workflow registration should succeed"); - let registered = client - .post(format!("{base_url}/api/v1/workflow-versions")) - .json(&version) - .send() + .expect("seeded workflow should be valid"); + let workflow_version_id = client + .create_workflow_version(&version) .await .expect("seeded workflow registration should succeed"); - let registered = expect_reqwest_status( - registered, - fabro_http::StatusCode::CREATED, - "register seeded workflow", - ) - .await; - let registered: serde_json::Value = registered - .json() + let run_id = client + .create_run_from_intent(RunIntent { + workflow_version_id, + target, + environment_id: Some("default".to_string()), + args, + parent_id: None, + title: None, + goal: None, + }) .await - .expect("workflow registration response should be JSON"); - let intent = serde_json::json!({ - "workflow_version_id": registered["workflow_version_id"], - "target": target, - "environment_id": "default", - "args": args, - }); - let response = client - .post(format!("{base_url}/api/v1/runs")) - .header("user-agent", "fabro-cli/test") - .json(&intent) - .send() - .await - .expect("seeded run create request should execute"); - let response = expect_reqwest_status( - response, - fabro_http::StatusCode::CREATED, - "POST /api/v1/runs for seeded fixture", - ) - .await; - let body: serde_json::Value = response - .json() - .await - .expect("seeded run create response should parse"); - let run_id = body["id"] - .as_str() - .expect("seeded run create response should contain an id") - .parse::() - .expect("seeded run create response id should be valid") + .expect("seeded run creation should succeed") .to_string(); RunSetup { diff --git a/lib/apps/fabro-server/src/server/tests.rs b/lib/apps/fabro-server/src/server/tests.rs index 57f3442da..066c9de5a 100644 --- a/lib/apps/fabro-server/src/server/tests.rs +++ b/lib/apps/fabro-server/src/server/tests.rs @@ -4951,32 +4951,6 @@ enabled = false assert_run_intent_targets_unavailable(&daytona_state).await; } -#[tokio::test] -async fn post_runs_create_regression_keeps_api_behavior_without_automation_metadata() { - let state = TestAppStateBuilder::new() - .env_lookup(|_| None) - .vault_entries([(EnvVars::OPENAI_API_KEY, "test-openai-api-key")]) - .build(); - let app = crate::test_support::build_test_router(Arc::clone(&state)); - let mut intent = test_intent(&app, MINIMAL_DOT).await; - intent["title"] = json!("API title"); - - let body = post_run_intent(&app, intent).await; - let run_id: RunId = body["id"].as_str().unwrap().parse().unwrap(); - - assert_eq!(body["title"], "API title"); - assert!(body["automation"].is_null()); - assert_eq!(body["lifecycle"]["status"]["kind"], "submitted"); - let summary = state - .stores - .run_summaries - .get(&run_id, Utc::now()) - .await - .unwrap() - .unwrap(); - assert!(summary.automation.is_none()); -} - #[tokio::test] async fn create_run_from_intent_helper_persists_automation_version_and_exact_target() { let state = TestAppStateBuilder::new() @@ -5050,69 +5024,6 @@ async fn create_run_from_intent_helper_persists_automation_version_and_exact_tar ); } -#[tokio::test] -async fn create_run_from_intent_snapshots_variables_before_allocating_id() { - let state = TestAppStateBuilder::new() - .env_lookup(|_| None) - .vault_entries([(EnvVars::OPENAI_API_KEY, "test-openai-api-key")]) - .build(); - let variable = state - .stores - .variables - .set("OWNER", "payments", None) - .await - .unwrap(); - let version = store_workflow_version( - &state, - r#"digraph Test { - graph [goal="Test"] - start [shape=Mdiamond] - work [prompt="Ship {{ vars.OWNER }}"] - exit [shape=Msquare] - start -> work -> exit - }"#, - None, - ) - .await; - let intent = serde_json::from_value( - json!({"workflow_version_id": version, "target": {"kind":"none"}, "args":{}}), - ) - .unwrap(); - let response = Box::pin(handler::runs::create_run_from_intent( - Arc::clone(&state), - handler::runs::CreateRunFromIntentRequest { - intent, - explicit_run_id: None, - actor: Principal::System { - system_kind: SystemActorKind::Engine, - }, - headers: HeaderMap::new(), - automation: None, - }, - )) - .await; - let body = response_json!(response, StatusCode::CREATED).await; - let run_id = body["id"].as_str().unwrap().parse::().unwrap(); - assert!(run_id.created_at().timestamp_millis() >= variable.updated_at.timestamp_millis()); - let projection = state - .stores - .runs - .open_run_reader(&run_id) - .await - .unwrap() - .state() - .await - .unwrap(); - assert_eq!( - projection.spec.graph.nodes["work"] - .attrs - .get("prompt") - .and_then(AttrValue::as_str), - Some("Ship payments") - ); - assert_eq!(projection.status, RunStatus::Submitted); -} - #[tokio::test] async fn fake_automation_materializer_injection_captures_input_and_returns_version() { let fake = TestAutomationRunMaterializer::succeed(GitRunTarget { @@ -9566,31 +9477,6 @@ async fn static_favicon_is_served() { ); } -#[tokio::test] -async fn post_runs_creates_submitted_run_and_returns_id() { - let app = test_app_with(); - - let req = Request::builder() - .method("POST") - .uri(api("/runs")) - .header("content-type", "application/json") - .body(intent_body(&app, MINIMAL_DOT).await) - .unwrap(); - - let response = app.oneshot(req).await.unwrap(); - let body = response_json!(response, StatusCode::CREATED).await; - assert!(body["id"].is_string()); - assert!(!body["id"].as_str().unwrap().is_empty()); -} - -#[tokio::test] -async fn workflow_registration_rejects_invalid_dot_before_run_creation() { - let app = test_app_with(); - let response = app.oneshot(Request::builder().method("POST").uri(api("/workflow-versions")).header("content-type", "application/json").body(Body::from(json!({"entrypoint":"workflow.fabro","files":{"workflow.fabro":"not a graph"},"workflow_dependencies":{}}).to_string())).unwrap()).await.unwrap(); - let body = response_json!(response, StatusCode::UNPROCESSABLE_ENTITY).await; - assert_eq!(body["errors"][0]["code"], "workflow_version_invalid"); -} - #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn get_run_status_returns_status() { let state = test_app_state(); @@ -12153,45 +12039,6 @@ async fn dev_token_web_login_authorizes_cookie_backed_api_requests() { assert_eq!(state_body["spec"]["provenance"]["subject"]["login"], "dev"); } -#[tokio::test] -async fn create_run_persists_definition_and_spec_blobs() { - let state = test_app_state(); - let app = crate::test_support::build_test_router(Arc::clone(&state)); - let raw_intent = serde_json::to_string_pretty(&test_intent(&app, MINIMAL_DOT).await).unwrap(); - - let req = Request::builder() - .method("POST") - .uri(api("/runs")) - .header("content-type", "application/json") - .body(Body::from(raw_intent)) - .unwrap(); - - let response = app.clone().oneshot(req).await.unwrap(); - let body = response_json!(response, StatusCode::CREATED).await; - let run_id = body["id"].as_str().unwrap().parse::().unwrap(); - - let run_store = state.stores.runs.open_run_reader(&run_id).await.unwrap(); - let events = run_store.list_events().await.unwrap(); - let created = events[0].event.to_value().unwrap(); - let submitted = events[1].event.to_value().unwrap(); - assert!(created["properties"]["spec_blob"].is_string()); - let definition_blob = submitted["properties"]["definition_blob"] - .as_str() - .expect("run.submitted should carry definition_blob") - .parse::() - .unwrap(); - - let accepted_definition_bytes = run_store - .read_blob(&definition_blob) - .await - .unwrap() - .expect("accepted definition blob should exist"); - let accepted_definition: serde_json::Value = - serde_json::from_slice(&accepted_definition_bytes).unwrap(); - assert_eq!(accepted_definition["workflow_path"], "workflow.fabro"); - assert!(accepted_definition["workflows"]["workflow.fabro"].is_object()); -} - #[tokio::test] async fn list_run_events_returns_paginated_json() { let state = test_app_state(); @@ -13672,24 +13519,6 @@ async fn stage_artifacts_multipart_requires_manifest_first() { assert_status!(response, StatusCode::BAD_REQUEST).await; } -#[tokio::test] -async fn create_run_returns_submitted() { - let state = test_app_state(); - let app = crate::test_support::build_test_router(Arc::clone(&state)); - - let req = Request::builder() - .method("POST") - .uri(api("/runs")) - .header("content-type", "application/json") - .body(intent_body(&app, MINIMAL_DOT).await) - .unwrap(); - - let response = app.oneshot(req).await.unwrap(); - let body = response_json!(response, StatusCode::CREATED).await; - assert_eq!(run_json_status(&body)["kind"], "submitted"); - assert_eq!(body["title"], "Test"); -} - #[tokio::test] async fn create_run_accepts_explicit_title() { let state = test_app_state(); @@ -16635,34 +16464,6 @@ fn aggregate_billing_counts_projection_rollup_usage_visits() { ); } -#[tokio::test] -async fn post_runs_returns_submitted_status() { - let state = test_app_state(); - let app = crate::test_support::build_test_router(state); - - let req = Request::builder() - .method("POST") - .uri(api("/runs")) - .header("content-type", "application/json") - .body(intent_body(&app, MINIMAL_DOT).await) - .unwrap(); - - let response = app.clone().oneshot(req).await.unwrap(); - let body = response_json!(response, StatusCode::CREATED).await; - let run_id = body["id"].as_str().unwrap().parse::().unwrap(); - - // Check status is submitted (no start, no scheduler running) - let req = Request::builder() - .method("GET") - .uri(api(&format!("/runs/{run_id}"))) - .body(Body::empty()) - .unwrap(); - - let response = app.oneshot(req).await.unwrap(); - let body = body_json(response.into_body()).await; - assert_eq!(run_json_status(&body)["kind"], "submitted"); -} - #[expect( clippy::disallowed_methods, reason = "test asserts the raw template source" diff --git a/lib/apps/fabro-server/tests/it/scenario/dry_run.rs b/lib/apps/fabro-server/tests/it/scenario/dry_run.rs index b13a98c0c..9ec6aedec 100644 --- a/lib/apps/fabro-server/tests/it/scenario/dry_run.rs +++ b/lib/apps/fabro-server/tests/it/scenario/dry_run.rs @@ -102,18 +102,6 @@ async fn test_model_unknown_via_full_router() { .await; } -#[tokio::test] -async fn registration_rejects_invalid_dot_without_a_provider() { - let app = test_app_with_no_providers(); - let req = Request::builder().method("POST").uri(api("/workflow-versions")).header("content-type", "application/json").body(Body::from(serde_json::json!({"entrypoint":"workflow.fabro","files":{"workflow.fabro":"not valid dot"},"workflow_dependencies":{}}).to_string())).unwrap(); - response_status( - app.oneshot(req).await.unwrap(), - StatusCode::UNPROCESSABLE_ENTITY, - "register invalid workflow", - ) - .await; -} - #[tokio::test] async fn completion_no_provider_non_streaming_returns_400() { let app = test_app_with_no_providers();