diff --git a/packages/rust/src/cli/run.rs b/packages/rust/src/cli/run.rs index b9db5f050..4a0b6f2d4 100644 --- a/packages/rust/src/cli/run.rs +++ b/packages/rust/src/cli/run.rs @@ -15,11 +15,16 @@ //! The `dirsql` binary is a shim over [`run_cli`], so `cargo install dirsql //! --features cli` and every other entry path run the same code. -use std::io::IsTerminal; +use std::future::Future; +use std::io::{IsTerminal, Write}; use std::path::PathBuf; +use std::pin::Pin; use super::{ - AppState, ServerConfig, execute::execute_query, init::InitOptions, repl::run_repl, + AppState, ServerConfig, + execute::{QueryFailure, execute_query}, + init::InitOptions, + repl::run_repl, serve_with_state, table, }; use crate::{DirSQL, Extension, Row, Table}; @@ -398,26 +403,58 @@ async fn run_default(cli: Cli) -> u8 { /// message — the same string the HTTP `{"error": …}` body carries — to /// stderr with a non-zero exit. async fn run_query(args: QueryArgs) -> u8 { - let parser = match resolve_on_file(&args.on_file) { + let terminal = std::io::stdout().is_terminal(); + let mut out = std::io::stdout(); + let mut err = std::io::stderr(); + run_query_with( + args, + resolve_on_file, + load_state, + |state, body| Box::pin(execute_query(state, body, None)), + terminal, + &mut out, + &mut err, + ) + .await +} + +async fn run_query_with( + args: QueryArgs, + resolve: R, + load: L, + execute: E, + terminal: bool, + out: &mut dyn Write, + err: &mut dyn Write, +) -> u8 +where + R: FnOnce(&[String]) -> std::result::Result, String>, + L: FnOnce(&ConfigArgs, Option) -> AppState, + E: for<'a> FnOnce( + &'a AppState, + String, + ) -> Pin> + 'a>>, +{ + let parser = match resolve(&args.on_file) { Ok(parser) => parser, Err(message) => { - eprintln!("dirsql query: {message}"); + let _ = writeln!(err, "dirsql query: {message}"); return 1; } }; - let state = load_state(&args.common, parser); + let state = load(&args.common, parser); // Unbounded: the process IS the query, so `timeout(1)` expresses any cap // natively; only the long-lived server enforces `query_timeout` (408). - let format = args.format.resolve(std::io::stdout().is_terminal()); + let format = args.format.resolve(terminal); - match execute_query(&state, query_body(&args.sql), None).await { + match execute(&state, query_body(&args.sql)).await { Ok(value) => { - print!("{}", render_rows(&value, format)); + let _ = write!(out, "{}", render_rows(&value, format)); 0 } Err(failure) => { - eprintln!("dirsql query: {}", failure.message()); + let _ = writeln!(err, "dirsql query: {}", failure.message()); 1 } } @@ -664,6 +701,57 @@ mod tests { assert_eq!(query_body(" "), r#"{"sql":" "}"#); } + #[tokio::test] + async fn run_query_with_writes_rows_and_returns_success() { + let args = match Cli::parse_from(["dirsql", "query", "SELECT 1"]).command { + Some(Command::Query(args)) => args, + other => panic!("expected query args, got {other:?}"), + }; + let mut out = Vec::new(); + let mut err = Vec::new(); + let code = run_query_with( + args, + |_| Ok(None), + |_, _| AppState::Unavailable("test state".to_owned()), + |_, body| { + Box::pin(async move { + assert_eq!(body, r#"{"sql":"SELECT 1"}"#); + Ok::<_, QueryFailure>(one_row()) + }) + }, + false, + &mut out, + &mut err, + ) + .await; + assert_eq!(code, 0); + assert!(!out.is_empty()); + assert!(err.is_empty()); + } + + #[tokio::test] + async fn run_query_with_reports_query_failure_and_returns_error() { + let args = match Cli::parse_from(["dirsql", "query", "SELECT 1"]).command { + Some(Command::Query(args)) => args, + other => panic!("expected query args, got {other:?}"), + }; + let mut out = Vec::new(); + let mut err = Vec::new(); + let code = run_query_with( + args, + |_| Ok(None), + |_, _| AppState::Unavailable("test state".to_owned()), + |_, _| Box::pin(async { Err(QueryFailure::BadRequest("invalid SQL".to_owned())) }), + false, + &mut out, + &mut err, + ) + .await; + assert_eq!(code, 1); + assert!(out.is_empty()); + assert_eq!(err, b"dirsql query: invalid SQL\n"); + } + /// The `ConfigArgs` parsed from a `query` subcommand invocation (#609: /// config flags are subcommand-local, so they live on the Query variant). fn query_common(argv: &[&str]) -> ConfigArgs { @@ -1089,6 +1177,18 @@ mod tests { assert_eq!(run_cli(vec!["dirsql".into(), "--help".into()]), 0); } + #[test] + fn run_cli_returns_the_query_exit_code() { + assert_eq!( + run_cli(vec!["dirsql".into(), "query".into(), "SELECT 1".into()]), + 0 + ); + assert_eq!( + run_cli(vec!["dirsql".into(), "query".into(), "SELECT FROM".into()]), + 1 + ); + } + #[test] fn context_parses_as_its_own_subcommand_not_as_sql() { let cli = Cli::try_parse_from(["dirsql", "context"]).unwrap(); diff --git a/packages/ts/e2e-attestations/claude-1330-cli-exit.json b/packages/ts/e2e-attestations/claude-1330-cli-exit.json new file mode 100644 index 000000000..ca3dea682 --- /dev/null +++ b/packages/ts/e2e-attestations/claude-1330-cli-exit.json @@ -0,0 +1,7 @@ +{ + "command": "pnpm test:e2e", + "ran_at": 1791270342, + "exit_code": 0, + "commit": "5076e5ad0909a14bb7dde96058de54bb3aec05e3", + "branch": "claude/1330-cli-exit" +} diff --git a/testing-conventions.toml b/testing-conventions.toml index 7b3f00b89..64cc26416 100644 --- a/testing-conventions.toml +++ b/testing-conventions.toml @@ -141,12 +141,6 @@ rules = ["mutation"] lines = [360, 371] reason = "rows walks and stats a real tree and column hands a cell to SQLite; the cell choice is unit-tested in `cell`, and tests/vtab.rs stat_columns_carry_real_values and content_is_excluded_from_star_but_selectable_by_name assert both through a real connection" -[[rust.exempt]] -path = "src/cli/run.rs" -rules = ["mutation"] -lines = [351, 392] -reason = "run_default and run_query build a real index and drive stdout, stderr and the process exit; tests/on_file_failure_e2e.rs (a_clean_scan_exits_zero, a_failed_command_fails_the_run) and tests/repl_pipe_e2e.rs assert their codes through the built binary" - [[rust.exempt]] path = "src/vtab.rs" rules = ["mutation"]