mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-10-08 03:10:26 +00:00
perf(run): skip implicit preflight
This commit is contained in:
parent
fa2cb2a839
commit
93034e2ec5
4 changed files with 64 additions and 37 deletions
|
|
@ -5,7 +5,7 @@ use fabro_util::terminal::Styles;
|
|||
use crate::args::PreflightArgs;
|
||||
use crate::command_context::CommandContext;
|
||||
use crate::commands::run::output::{
|
||||
api_check_report_to_local, api_diagnostics_to_local, print_preflight_workflow_summary,
|
||||
api_check_report_to_local, api_diagnostics_to_local, print_workflow_summary,
|
||||
};
|
||||
use crate::commands::run::overrides::preflight_args_overrides;
|
||||
use crate::manifest_builder::{ManifestBuildInput, build_run_manifest, preflight_manifest_args};
|
||||
|
|
@ -59,7 +59,7 @@ pub(crate) async fn execute(
|
|||
if ctx.json_output() {
|
||||
print_json_pretty(&response)?;
|
||||
} else {
|
||||
print_preflight_workflow_summary(
|
||||
print_workflow_summary(
|
||||
&response.workflow,
|
||||
Some(&manifest.target_path),
|
||||
styles,
|
||||
|
|
|
|||
|
|
@ -1,8 +1,11 @@
|
|||
use anyhow::bail;
|
||||
use fabro_config::RunLayer;
|
||||
use fabro_config::user::active_settings_path;
|
||||
use fabro_server::manifest_validation;
|
||||
use fabro_types::RunId;
|
||||
use fabro_util::terminal::Styles;
|
||||
|
||||
use super::output::{api_diagnostics_to_local, print_preflight_workflow_summary};
|
||||
use super::output::{api_diagnostics_to_local, print_workflow_summary};
|
||||
use super::overrides::run_args_overrides;
|
||||
use crate::args::RunArgs;
|
||||
use crate::command_context::CommandContext;
|
||||
|
|
@ -44,24 +47,26 @@ pub(crate) async fn create_run(
|
|||
run_id,
|
||||
user_settings_path: Some(active_settings_path(None)),
|
||||
})?;
|
||||
let client = ctx.server().await?;
|
||||
if !quiet {
|
||||
let printer = ctx.printer();
|
||||
let preflight = client.run_preflight(built.manifest.clone()).await?;
|
||||
let diagnostics = api_diagnostics_to_local(&preflight.workflow.diagnostics);
|
||||
if !diagnostics
|
||||
let validation =
|
||||
manifest_validation::validate_manifest(&RunLayer::default(), &built.manifest)?;
|
||||
let diagnostics = api_diagnostics_to_local(&validation.workflow.diagnostics);
|
||||
print_workflow_summary(
|
||||
&validation.workflow,
|
||||
Some(&built.target_path),
|
||||
styles,
|
||||
printer,
|
||||
);
|
||||
if diagnostics
|
||||
.iter()
|
||||
.any(|diagnostic| diagnostic.severity == fabro_validate::Severity::Error)
|
||||
{
|
||||
print_preflight_workflow_summary(
|
||||
&preflight.workflow,
|
||||
Some(&built.target_path),
|
||||
styles,
|
||||
printer,
|
||||
);
|
||||
bail!("Validation failed");
|
||||
}
|
||||
}
|
||||
|
||||
let client = ctx.server().await?;
|
||||
let created_run_id = client.create_run_from_manifest(built.manifest).await?;
|
||||
|
||||
Ok(CreatedRun {
|
||||
|
|
|
|||
|
|
@ -19,7 +19,7 @@ use indicatif::HumanDuration;
|
|||
use crate::server_client;
|
||||
use crate::shared::{format_tokens_human, format_usd_micros, print_diagnostics, relative_path};
|
||||
|
||||
pub(crate) fn print_preflight_workflow_summary(
|
||||
pub(crate) fn print_workflow_summary(
|
||||
workflow: &types::PreflightWorkflowSummary,
|
||||
graph_path_override: Option<&Path>,
|
||||
styles: &Styles,
|
||||
|
|
|
|||
|
|
@ -27,24 +27,6 @@ fn run_status_response(run_id: &str, status: &str) -> serde_json::Value {
|
|||
})
|
||||
}
|
||||
|
||||
fn preflight_response() -> serde_json::Value {
|
||||
serde_json::json!({
|
||||
"ok": true,
|
||||
"workflow": {
|
||||
"name": "Simple",
|
||||
"graph_path": null,
|
||||
"nodes": 4,
|
||||
"edges": 3,
|
||||
"goal": "Run tests and report results",
|
||||
"diagnostics": []
|
||||
},
|
||||
"checks": {
|
||||
"title": "Preflight",
|
||||
"sections": []
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
fn remote_run_state_response() -> serde_json::Value {
|
||||
serde_json::json!({
|
||||
"spec": null,
|
||||
|
|
@ -472,11 +454,11 @@ fn remote_foreground_run_consumes_paginated_events_and_prints_server_backed_summ
|
|||
let context = test_context!();
|
||||
let server = MockServer::start();
|
||||
let run_id = unique_run_id();
|
||||
server.mock(|when, then| {
|
||||
let preflight = server.mock(|when, then| {
|
||||
when.method("POST").path("/api/v1/preflight");
|
||||
then.status(200)
|
||||
then.status(500)
|
||||
.header("Content-Type", "application/json")
|
||||
.body(preflight_response().to_string());
|
||||
.body(serde_json::json!({ "error": "preflight should not run" }).to_string());
|
||||
});
|
||||
server.mock(|when, then| {
|
||||
when.method("POST").path("/api/v1/runs");
|
||||
|
|
@ -568,6 +550,7 @@ fn remote_foreground_run_consumes_paginated_events_and_prints_server_backed_summ
|
|||
String::from_utf8_lossy(&output.stdout),
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
preflight.assert_calls(0);
|
||||
first_page.assert();
|
||||
second_page.assert();
|
||||
|
||||
|
|
@ -585,6 +568,45 @@ fn remote_foreground_run_consumes_paginated_events_and_prints_server_backed_summ
|
|||
assert!(!stderr.contains("=== Artifacts ==="), "{stderr}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn foreground_run_rejects_invalid_workflow_before_creating_remote_run() {
|
||||
let context = test_context!();
|
||||
let server = MockServer::start();
|
||||
let create = server.mock(|when, then| {
|
||||
when.method("POST").path("/api/v1/runs");
|
||||
then.status(500)
|
||||
.header("Content-Type", "application/json")
|
||||
.body(serde_json::json!({ "error": "run should not be created" }).to_string());
|
||||
});
|
||||
|
||||
let workflow = context.install_fixture("invalid.fabro");
|
||||
let output = context
|
||||
.run_cmd()
|
||||
.args([
|
||||
"--server",
|
||||
&format!("{}/api/v1", server.base_url()),
|
||||
workflow.to_str().unwrap(),
|
||||
])
|
||||
.output()
|
||||
.expect("command should execute");
|
||||
|
||||
assert!(
|
||||
!output.status.success(),
|
||||
"invalid run should fail:\nstdout:\n{}\nstderr:\n{}",
|
||||
String::from_utf8_lossy(&output.stdout),
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
create.assert_calls(0);
|
||||
|
||||
let stderr = output_stderr(&output);
|
||||
assert!(stderr.contains("Workflow: Invalid"), "{stderr}");
|
||||
assert!(
|
||||
stderr.contains("Pipeline must have exactly one start node"),
|
||||
"{stderr}"
|
||||
);
|
||||
assert!(stderr.contains("Validation failed"), "{stderr}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn local_foreground_run_prints_artifact_paths_from_server_artifact_list() {
|
||||
let context = test_context!();
|
||||
|
|
@ -693,7 +715,7 @@ fn dry_run_simple() {
|
|||
fn dry_run_with_goal_file_reads_contents_into_goal() {
|
||||
// Regression test for the `--goal-file` flag that was previously
|
||||
// being silently ignored in the v2 path. The file content must end
|
||||
// up in the effective goal displayed in the preflight summary.
|
||||
// up in the effective goal displayed in the workflow summary.
|
||||
let context = test_context!();
|
||||
|
||||
let goal_dir = tempfile::tempdir().unwrap();
|
||||
|
|
@ -715,7 +737,7 @@ fn dry_run_with_goal_file_reads_contents_into_goal() {
|
|||
let stderr = String::from_utf8_lossy(&output.stderr);
|
||||
assert!(
|
||||
stderr.contains("Ship the rate-limiting feature end to end."),
|
||||
"goal file content should appear in preflight summary, got:\n{stderr}"
|
||||
"goal file content should appear in workflow summary, got:\n{stderr}"
|
||||
);
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue