diff --git a/cli/src/main.rs b/cli/src/main.rs index 80227220..b1188186 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -3,30 +3,30 @@ mod shutdown; use std::{ env, path::{Path, PathBuf}, - sync::{Arc, mpsc::Sender}, + sync::mpsc::Sender, thread::JoinHandle, }; -use clap::{Args, Parser, Subcommand}; +use clap::{Args, Parser, Subcommand, ValueEnum}; use clap_verbosity_flag::{InfoLevel, Verbosity, log::LevelFilter}; use display::{ message::{DisplayMessage, send_message}, render::spin_up_renderer, }; use sentry::ClientInitGuard; +use superconsole::Line; use tracing_subscriber::{filter::FilterFn, prelude::*}; use trunk_analytics_cli::{ context::gather_debug_props, error_report::ErrorReport, exit_code::ExitCode, summary_output::{ - ErrorSummary, UploadCommand, UploadReport, ValidateReport, github, - json::{self, Report}, - text::{SummaryFileText, TextOptions, UploadText}, + ErrorSummary, UploadCommand, UploadReport, ValidateReport, github, json, + text::{self, SummaryFileText, TextOptions}, }, test_command::{TestArgs, run_test}, - upload_command::{RunUploadOptions, UploadArgs, UploadRunResult, run_upload}, - validate_command::{ValidateArgs, ValidateRunResult, run_validate}, + upload_command::{RunUploadOptions, UploadArgs, run_upload}, + validate_command::{ValidateArgs, run_validate}, }; #[derive(Debug, Parser)] @@ -46,8 +46,8 @@ impl Cli { pub fn debug_props(&self) -> String { let token = match &self.command { Commands::Test(args) => args.token(), - Commands::Upload(args) => args.token.clone(), - Commands::Validate(..) + Commands::Upload { args, .. } => args.token.clone(), + Commands::Validate { .. } | Commands::SummaryOutputSchema | Commands::GithubAnnotations(..) => None, }; @@ -61,8 +61,8 @@ impl Cli { pub fn command_name(&self) -> &str { match &self.command { Commands::Test(..) => "test", - Commands::Upload(..) => "upload", - Commands::Validate(..) => "validate", + Commands::Upload { .. } => "upload", + Commands::Validate { .. } => "validate", Commands::SummaryOutputSchema => "summary-output-schema", Commands::GithubAnnotations(..) => "github-annotations", } @@ -76,8 +76,8 @@ impl Cli { pub fn org_url_slug(&self) -> String { match &self.command { Commands::Test(args) => args.org_url_slug(), - Commands::Upload(args) => args.org_url_slug.clone(), - Commands::Validate(..) + Commands::Upload { args, .. } => args.org_url_slug.clone(), + Commands::Validate { .. } | Commands::SummaryOutputSchema | Commands::GithubAnnotations(..) => String::from("not used"), } @@ -86,18 +86,29 @@ impl Cli { pub fn summary_output_file(&self) -> Option { match &self.command { Commands::Test(args) => args.summary_output_file(), - Commands::Upload(args) => args.summary_output_file.clone(), - Commands::Validate(..) + Commands::Upload { args, .. } => args.summary_output_file.clone(), + Commands::Validate { .. } | Commands::SummaryOutputSchema | Commands::GithubAnnotations(..) => None, } } + pub fn output_format(&self) -> OutputFormat { + match &self.command { + Commands::Upload { output, .. } | Commands::Validate { output, .. } => { + output.output_format + } + Commands::Test(..) + | Commands::SummaryOutputSchema + | Commands::GithubAnnotations(..) => OutputFormat::Text, + } + } + pub fn repo_root(&self) -> String { let explicit_root = match &self.command { Commands::Test(args) => args.repo_root(), - Commands::Upload(args) => args.repo_root.clone(), - Commands::Validate(..) + Commands::Upload { args, .. } => args.repo_root.clone(), + Commands::Validate { .. } | Commands::SummaryOutputSchema | Commands::GithubAnnotations(..) => None, }; @@ -117,11 +128,21 @@ enum Commands { Test(TestArgs), /// Upload test results to Trunk Flaky Tests. Use this when you've already run tests and have result files. #[command(after_long_help = ExitCode::help())] - Upload(UploadArgs), + Upload { + #[command(flatten)] + args: UploadArgs, + #[command(flatten)] + output: OutputFormatArgs, + }, /// Validate test report files for compatibility with Trunk Flaky Tests. Runs locally without uploading data. #[command(after_long_help = ExitCode::help())] - Validate(ValidateArgs), - /// Print the JSON Schema of the summary that `--summary-output-file` writes. + Validate { + #[command(flatten)] + args: ValidateArgs, + #[command(flatten)] + output: OutputFormatArgs, + }, + /// Print the JSON Schema of the summary that `--summary-output-file` writes and `--output-format json` prints. SummaryOutputSchema, /// Print GitHub Actions annotations for why a run failed, from its `--summary-output-file`. /// Exits 0 only when it printed one. Used by the analytics-uploader action. @@ -129,6 +150,25 @@ enum Commands { GithubAnnotations(GithubAnnotationsArgs), } +/// Not in `UploadArgs`, because `test` includes those and its test command writes to stdout. +#[derive(Debug, Args)] +struct OutputFormatArgs { + #[arg( + long, + env = constants::TRUNK_OUTPUT_FORMAT_ENV, + value_enum, + default_value_t = OutputFormat::Text, + help = "Print the summary on stdout as text, or as the JSON that --summary-output-file writes. Logs and progress stay on stderr." + )] + output_format: OutputFormat, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, ValueEnum)] +enum OutputFormat { + Text, + Json, +} + #[derive(Debug, Args)] struct GithubAnnotationsArgs { /// The summary that `upload` or `test` wrote to `--summary-output-file`. @@ -136,8 +176,11 @@ struct GithubAnnotationsArgs { summary_file: PathBuf, } +/// `json` is the summary for `--output-format json`, printed once the renderer has finished so +/// nothing else is written to stdout around it. fn close_out_and_exit( exit_code: ExitCode, + json: Option, guard: ClientInitGuard, render_sender: Sender, render_handle: JoinHandle<()>, @@ -145,6 +188,12 @@ fn close_out_and_exit( guard.flush(None); drop(render_sender); let _ = render_handle.join(); + if let Some(report) = json { + match report.to_json() { + Ok(json) => println!("{json}"), + Err(error) => tracing::error!("Failed to print the summary as JSON: {error:?}"), + } + } std::process::exit(exit_code.code()) } @@ -166,6 +215,7 @@ fn main() -> anyhow::Result<()> { .block_on(async { let org_url_slug = cli.org_url_slug(); let summary_output_file = cli.summary_output_file(); + let output_format = cli.output_format(); let log_level_filter = cli.verbose.log_level_filter(); setup_logger( log_level_filter, @@ -193,92 +243,110 @@ fn main() -> anyhow::Result<()> { tracing::warn!( "Received termination signal; aborting in-flight work and shutting down." ); - close_out_and_exit(exit_code, guard, render_sender, render_handle) + close_out_and_exit(exit_code, None, guard, render_sender, render_handle) } - MainOutcome::Finished(Ok(RunResult::Upload(run_result))) => { - let text = Arc::new(UploadText { - report: UploadReport::new(&run_result, UploadCommand::Upload), - options: TextOptions::from(&run_result), - }); - send_message( - DisplayMessage::Final(text.clone(), String::from("upload display")), + MainOutcome::Finished(Ok(Report::Upload(report, options))) => { + show( + DisplayMessage::Notice, + text::render_notices(&report.notices), + "notices", &render_sender, ); + let json = match output_format { + OutputFormat::Text => { + show( + DisplayMessage::Report, + text::render_upload(&report, &options), + "upload summary", + &render_sender, + ); + None + } + OutputFormat::Json => Some(json::Report::Upload(&report)), + }; write_summary_file( summary_output_file.as_deref(), - Report::Upload(&text.report), + json::Report::Upload(&report), &render_sender, ); close_out_and_exit( - text.report.cli_exit_code(), + report.cli_exit_code(), + json, guard, render_sender, render_handle, ) } - MainOutcome::Finished(Ok(RunResult::Test(run_result))) => { - let text = Arc::new(UploadText { - report: UploadReport::new(&run_result, UploadCommand::Test), - options: TextOptions::from(&run_result), - }); - send_message( - DisplayMessage::Final(text.clone(), String::from("test display")), + MainOutcome::Finished(Ok(Report::Validate(report))) => { + show( + DisplayMessage::Notice, + text::render_notices(&report.notices), + "notices", &render_sender, ); - write_summary_file( - summary_output_file.as_deref(), - Report::Upload(&text.report), - &render_sender, - ); - close_out_and_exit( - text.report.cli_exit_code(), - guard, - render_sender, - render_handle, - ) - } - MainOutcome::Finished(Ok(RunResult::Validate(run_result))) => { - let report = ValidateReport::new(&run_result); - let exit_code = report.exit_code; - send_message( - DisplayMessage::Final(Arc::new(report), String::from("validate display")), - &render_sender, - ); - close_out_and_exit(exit_code, guard, render_sender, render_handle) + let json = match output_format { + OutputFormat::Text => { + show( + DisplayMessage::Report, + text::render_validate(&report), + "validate summary", + &render_sender, + ); + None + } + OutputFormat::Json => Some(json::Report::Validate(&report)), + }; + close_out_and_exit(report.exit_code, json, guard, render_sender, render_handle) } - MainOutcome::Finished(Ok(RunResult::SummaryOutputSchema)) => { + MainOutcome::Finished(Ok(Report::SummaryOutputSchema)) => { println!("{}", serde_json::to_string_pretty(&json::Report::schema())?); - close_out_and_exit(ExitCode::Success, guard, render_sender, render_handle) + close_out_and_exit(ExitCode::Success, None, guard, render_sender, render_handle) } MainOutcome::Finished(Err(error)) => { - let error = Arc::new(ErrorSummary::new(&ErrorReport::new( - error, - org_url_slug, - None, - ))); + let error = ErrorSummary::new(&ErrorReport::new(error, org_url_slug, None)); let exit_code = error.exit_code.unwrap_or(ExitCode::Success); - send_message( - DisplayMessage::Final(error.clone(), String::from("error display")), + show( + DisplayMessage::Notice, + text::render_error(&error), + "error", &render_sender, ); write_summary_file( summary_output_file.as_deref(), - Report::Error(&error), + json::Report::Error(&error), &render_sender, ); - close_out_and_exit(exit_code, guard, render_sender, render_handle) + let json = (output_format == OutputFormat::Json) + .then_some(json::Report::Error(&error)); + close_out_and_exit(exit_code, json, guard, render_sender, render_handle) } } }) } -fn write_summary_file(path: Option<&Path>, report: Report, render_sender: &Sender) { +fn show( + message: fn(Vec, String) -> DisplayMessage, + lines: anyhow::Result>, + description: &str, + render_sender: &Sender, +) { + match lines { + Ok(lines) => send_message(message(lines, String::from(description)), render_sender), + Err(error) => tracing::error!("Failed to render {description}: {error:?}"), + } +} + +fn write_summary_file( + path: Option<&Path>, + report: json::Report, + render_sender: &Sender, +) { let Some(path) = path else { return; }; let text = SummaryFileText::new(path, report.write_file(path)); send_message( - DisplayMessage::Final(Arc::new(text), String::from("summary file display")), + DisplayMessage::Notice(text.render(), String::from("summary file display")), render_sender, ); } @@ -286,19 +354,19 @@ fn write_summary_file(path: Option<&Path>, report: Report, render_sender: &Sende /// Outcome of racing the command against termination signals in `main`. enum MainOutcome { /// The command ran to completion (successfully or with an error). - Finished(anyhow::Result), + Finished(anyhow::Result), /// A termination signal was received first; the payload is the exit code to use. Interrupted(ExitCode), } -enum RunResult { - Upload(UploadRunResult), - Test(UploadRunResult), - Validate(ValidateRunResult), +/// What a command found, to show as its summary and take the exit code from. +enum Report { + Upload(Box, TextOptions), + Validate(ValidateReport), SummaryOutputSchema, } -async fn run(cli: Cli, render_sender: Sender) -> anyhow::Result { +async fn run(cli: Cli, render_sender: Sender) -> anyhow::Result { tracing::info!( "Starting trunk-analytics-cli {} (git={}) rustc={}", env!("CARGO_PKG_VERSION"), @@ -306,7 +374,9 @@ async fn run(cli: Cli, render_sender: Sender) -> anyhow::Result< env!("VERGEN_RUSTC_SEMVER") ); match cli.command { - Commands::Upload(upload_args) => { + Commands::Upload { + args: upload_args, .. + } => { let result = run_upload( upload_args, RunUploadOptions { @@ -315,31 +385,40 @@ async fn run(cli: Cli, render_sender: Sender) -> anyhow::Result< }, ) .await?; - Ok(RunResult::Upload(result)) + Ok(Report::Upload( + Box::new(UploadReport::new(&result, UploadCommand::Upload)), + TextOptions::from(&result), + )) } Commands::Test(test_args) => { let result = run_test(test_args, render_sender).await?; - Ok(RunResult::Test(result)) + Ok(Report::Upload( + Box::new(UploadReport::new(&result, UploadCommand::Test)), + TextOptions::from(&result), + )) } - Commands::Validate(validate_args) => { + Commands::Validate { + args: validate_args, + .. + } => { let result = run_validate(validate_args).await?; - Ok(RunResult::Validate(result)) + Ok(Report::Validate(ValidateReport::new(&result))) } - Commands::SummaryOutputSchema => Ok(RunResult::SummaryOutputSchema), + Commands::SummaryOutputSchema => Ok(Report::SummaryOutputSchema), Commands::GithubAnnotations(..) => { anyhow::bail!("github-annotations runs before the async runtime starts") } } } -fn to_trace_filter(filter: log::LevelFilter) -> tracing::Level { +fn to_trace_filter(filter: log::LevelFilter) -> tracing::level_filters::LevelFilter { match filter { - log::LevelFilter::Debug => tracing::Level::DEBUG, - log::LevelFilter::Error => tracing::Level::ERROR, - log::LevelFilter::Info => tracing::Level::INFO, - log::LevelFilter::Off => tracing::Level::TRACE, - log::LevelFilter::Trace => tracing::Level::TRACE, - log::LevelFilter::Warn => tracing::Level::WARN, + log::LevelFilter::Debug => tracing::level_filters::LevelFilter::DEBUG, + log::LevelFilter::Error => tracing::level_filters::LevelFilter::ERROR, + log::LevelFilter::Info => tracing::level_filters::LevelFilter::INFO, + log::LevelFilter::Off => tracing::level_filters::LevelFilter::OFF, + log::LevelFilter::Trace => tracing::level_filters::LevelFilter::TRACE, + log::LevelFilter::Warn => tracing::level_filters::LevelFilter::WARN, } } @@ -351,9 +430,12 @@ fn setup_logger( ) -> anyhow::Result<()> { let command_string = String::from(command_name); let sentry_layer = sentry_tracing::layer().event_mapper(move |event, context| { + // deprecation warnings are sent as events so we can see who still uses deprecated flags + let is_deprecation = *event.metadata().level() == tracing::Level::WARN + && event.metadata().fields().field("deprecated_flag").is_some(); // trunk-ignore(clippy/match_ref_pats) match event.metadata().level() { - &tracing::Level::ERROR => { + level if *level == tracing::Level::ERROR || is_deprecation => { let mut event = sentry_tracing::event_from_event(event, context); event .tags @@ -380,16 +462,19 @@ fn setup_logger( }); // make console layer toggle based on vebosity + // logs go to stderr so stdout is left for the command's output + let trace_filter = to_trace_filter(log_level_filter); let console_layer = tracing_subscriber::fmt::Layer::new() .with_target(true) .with_level(true) - .with_writer(std::io::stdout.with_max_level(to_trace_filter(log_level_filter))) + .with_writer(std::io::stderr) .with_filter(FilterFn::new(move |metadata| { !metadata .fields() .iter() .any(|field| field.name() == "hidden_in_console") - && to_trace_filter(log_level_filter) > tracing::Level::INFO + && trace_filter > tracing::Level::INFO + && *metadata.level() <= trace_filter })); tracing_subscriber::registry() diff --git a/cli/src/summary_output/error.rs b/cli/src/summary_output/error.rs index 2b208e09..09338204 100644 --- a/cli/src/summary_output/error.rs +++ b/cli/src/summary_output/error.rs @@ -21,7 +21,7 @@ pub struct ErrorSummary { pub explanation: Option, /// Where to check the org slug and token. Only set for `UNAUTHORIZED` errors. pub settings_url: Option, - /// A run that fails outright exits with this code, or 0 when it's null. Upload reports exit with their own `exitCode`. + /// A run that fails outright exits with this code, or 0 when it's null. Upload and validate reports exit with their own `exitCode`. pub exit_code: Option, } diff --git a/cli/src/summary_output/json.rs b/cli/src/summary_output/json.rs index c7915bed..b9b4b1ce 100644 --- a/cli/src/summary_output/json.rs +++ b/cli/src/summary_output/json.rs @@ -4,7 +4,7 @@ use anyhow::Context; use schemars::{JsonSchema, Schema, generate::SchemaSettings}; use serde::{Deserialize, Serialize}; -use super::{ErrorSummary, UploadReport}; +use super::{ErrorSummary, UploadReport, ValidateReport}; pub const SCHEMA_VERSION: u32 = 1; @@ -13,6 +13,8 @@ pub const SCHEMA_VERSION: u32 = 1; pub enum Report<'a> { /// The run finished and summarized its tests. Upload(&'a UploadReport), + /// `validate` finished and summarized the files it checked. + Validate(&'a ValidateReport), /// The run failed before it could summarize any tests. Error(&'a ErrorSummary), } @@ -21,7 +23,7 @@ pub enum Report<'a> { #[serde(rename_all = "camelCase")] #[schemars( title = "Trunk Analytics CLI summary", - description = "What `upload` and `test` write to `--summary-output-file`, and the RSpec gem to `TRUNK_SUMMARY_OUTPUT_FILE`." + description = "What `upload` and `test` write to `--summary-output-file`, the RSpec gem to `TRUNK_SUMMARY_OUTPUT_FILE`, and `upload` and `validate` print with `--output-format json`." )] struct Versioned<'a> { /// Goes up when a change could break a reader, such as renaming or removing a field. diff --git a/cli/src/summary_output/text.rs b/cli/src/summary_output/text.rs index b3795556..6763f439 100644 --- a/cli/src/summary_output/text.rs +++ b/cli/src/summary_output/text.rs @@ -1,6 +1,5 @@ use std::path::{Path, PathBuf}; -use display::end_output::EndOutput; use pluralizer::pluralize; use superconsole::{ Hyperlink, Line, Lines, Span, SuperConsole, @@ -8,7 +7,7 @@ use superconsole::{ }; use super::{ - ErrorSummary, FailureGroup, FileValidation, GroupKey, NoticeKind, Outcome, ParseError, + ErrorSummary, FailureGroup, FileValidation, GroupKey, Notice, NoticeKind, Outcome, ParseError, Quarantining, TestCounts, UploadReport, ValidateReport, ValidationCounts, }; use crate::{ @@ -16,7 +15,6 @@ use crate::{ exit_code::ExitCode, report_limiting::ValidationReport, upload_command::{UploadRunResult, get_bundle_upload_id_message}, - validate_command::ValidateRunResult, }; /// Flags that change how much of the summary to show, not what it says. @@ -38,23 +36,6 @@ impl From<&UploadRunResult> for TextOptions { } } -pub struct UploadText { - pub report: UploadReport, - pub options: TextOptions, -} - -impl EndOutput for UploadText { - fn output(&self) -> anyhow::Result> { - render_upload(&self.report, &self.options) - } -} - -impl EndOutput for ErrorSummary { - fn output(&self) -> anyhow::Result> { - render_error(self) - } -} - pub struct SummaryFileText { path: PathBuf, error: Option, @@ -83,14 +64,12 @@ impl SummaryFileText { Some(error) => format!("⚠️ Failed to write the summary to {path}: {error}"), } } -} -impl EndOutput for SummaryFileText { - fn output(&self) -> anyhow::Result> { - Ok(vec![ + pub fn render(&self) -> Vec { + vec![ Line::default(), Line::from_iter([Span::new_unstyled_lossy(self.message())]), - ]) + ] } } @@ -181,19 +160,6 @@ pub fn render_upload(report: &UploadReport, options: &TextOptions) -> anyhow::Re output.push(Line::default()); } - // The console shows tracing only when verbose, so these are shown here or not at all. - for notice in &report.notices { - if matches!( - notice.kind, - NoticeKind::Deprecation | NoticeKind::UploadSkipped - ) { - output.push(Line::from_iter([Span::new_styled_lossy( - style(format!("⚠️ {}", notice.message)).with(Color::Yellow), - )])); - output.push(Line::default()); - } - } - let bold = |text: String| Span::new_styled(style(text).attribute(Attribute::Bold)); let upload_line = match (&report.upload_url, &report.bundle_upload_id) { (Some(url), _) => Some(Line::from_iter([ @@ -567,35 +533,28 @@ fn url_span(url: String, attribute: Attribute, clickable: bool) -> anyhow::Resul }) } -impl EndOutput for ValidateReport { - fn output(&self) -> anyhow::Result> { - render_validate(self) +/// Shown on stderr, apart from the summary. +pub fn render_notices(notices: &[Notice]) -> anyhow::Result> { + let mut output: Vec = Vec::new(); + for notice in notices { + let prefix = match notice.kind { + NoticeKind::Deprecation | NoticeKind::UploadSkipped => "⚠️ ", + NoticeKind::BepParseError => "⚠️ BEP file had a parse error: ", + }; + output.push(Line::from_iter([ + Span::new_unstyled(prefix)?, + Span::new_unstyled_lossy(¬ice.message), + ])); } -} - -impl EndOutput for ValidateRunResult { - fn output(&self) -> anyhow::Result> { - render_validate(&ValidateReport::new(self)) + if !output.is_empty() { + output.push(Line::default()); } + Ok(output) } pub fn render_validate(report: &ValidateReport) -> anyhow::Result> { let mut output: Vec = Vec::new(); - let bep_parse_errors: Vec<&String> = report - .notices - .iter() - .filter(|notice| notice.kind == NoticeKind::BepParseError) - .map(|notice| ¬ice.message) - .collect(); - if !bep_parse_errors.is_empty() { - output.push(Line::from_iter([ - Span::new_unstyled("⚠️ BEP file had parse errors: ")?, - Span::new_unstyled_lossy(format!("{:?}", bep_parse_errors)), - ])); - output.push(Line::default()); - } - let num_files: usize = report .matched_files .iter() diff --git a/cli/src/summary_output/validate.rs b/cli/src/summary_output/validate.rs index e2d732c7..35bbc1d2 100644 --- a/cli/src/summary_output/validate.rs +++ b/cli/src/summary_output/validate.rs @@ -1,6 +1,7 @@ use std::collections::HashSet; use context::junit::validator::JunitReportValidationFlatIssue; +use schemars::JsonSchema; use serde::Serialize; use super::{FileValidation, Notice, ParseError}; @@ -9,32 +10,37 @@ use crate::{ validate_command::{ParseIssues, TestIssues, ValidateRunResult}, }; -#[derive(Debug, Clone, Serialize)] +#[derive(Debug, Clone, Serialize, JsonSchema)] #[serde(rename_all = "camelCase")] pub struct ValidateReport { + /// The files each glob matched. pub matched_files: Vec, + /// One entry per test report file. pub validations: Vec, + /// The CODEOWNERS file, when one was found. pub codeowners: Option, pub counts: ValidationCounts, + /// The code the CLI exits with. pub exit_code: ExitCode, + /// Warnings that didn't stop the run, such as a Bazel BEP parse error. pub notices: Vec, } -#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize, JsonSchema)] #[serde(rename_all = "camelCase")] pub struct MatchedFiles { pub glob: String, pub files: Vec, } -#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize, JsonSchema)] #[serde(rename_all = "camelCase")] pub struct Codeowners { pub path: String, pub warnings: Vec, } -#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, JsonSchema)] #[serde(rename_all = "camelCase")] pub struct ValidationCounts { pub files: usize, diff --git a/cli/src/test_command.rs b/cli/src/test_command.rs index 8f64bb4d..7d7d8ef3 100644 --- a/cli/src/test_command.rs +++ b/cli/src/test_command.rs @@ -2,15 +2,12 @@ use std::{ env, path::PathBuf, process::{Command, Stdio}, - sync::{Arc, mpsc::Sender}, + sync::mpsc::Sender, time::SystemTime, }; use clap::Args; -use display::{ - end_output::EndOutput, - message::{DisplayMessage, send_message}, -}; +use display::message::{DisplayMessage, send_message}; use superconsole::{ Line, Span, style::{Attribute, Stylize}, @@ -23,22 +20,6 @@ use crate::{ upload_command::{RunUploadOptions, UploadArgs, UploadRunResult, run_upload}, }; -enum RunOutput { - Title, -} -impl EndOutput for RunOutput { - fn output(&self) -> anyhow::Result> { - match self { - RunOutput::Title => Ok(vec![ - Line::from_iter([Span::new_styled( - String::from("📒 Test command outputs").attribute(Attribute::Bold), - )?]), - Line::default(), - ]), - } - } -} - #[derive(Args, Clone, Debug)] pub struct TestArgs { #[command(flatten)] @@ -79,43 +60,6 @@ pub struct TestRunResult { pub command_stderr: String, } -fn lines_of(output: String) -> impl Iterator { - output - .lines() - .flat_map(|lf_line| { - lf_line - .split('\r') - .map(|crlf_line| crlf_line.replace('\t', " ")) - }) - .collect::>() - .into_iter() -} - -impl EndOutput for TestRunResult { - fn output(&self) -> anyhow::Result> { - let mut output: Vec = Vec::new(); - - output.extend(vec![ - Line::from_iter([Span::new_styled( - String::from("📒 Test command outputs").attribute(Attribute::Bold), - )?]), - Line::default(), - ]); - - for stderr_line in lines_of(self.command_stderr.clone()) { - output.push(Line::from_iter([Span::new_unstyled_lossy(stderr_line)])); - } - - output.push(Line::default()); - - for stdout_line in lines_of(self.command_stdout.clone()) { - output.push(Line::from_iter([Span::new_unstyled_lossy(stdout_line)])); - } - - Ok(output) - } -} - pub async fn run_test( TestArgs { upload_args, @@ -142,6 +86,7 @@ pub async fn run_test( RunUploadOptions { pre_test_context: Some(test_context), test_run_result: Some(test_run_result.clone()), + render_sender: Some(render_sender), ..Default::default() }, ) @@ -164,9 +109,14 @@ pub async fn run_test_command>( render_sender: Sender, ) -> anyhow::Result { let exec_start = SystemTime::now(); - let title_ptr = Arc::new(RunOutput::Title); + let title = vec![ + Line::from_iter([Span::new_styled( + String::from("📒 Test command outputs").attribute(Attribute::Bold), + )?]), + Line::default(), + ]; send_message( - DisplayMessage::Final(title_ptr, String::from("test command title")), + DisplayMessage::Notice(title, String::from("test command title")), &render_sender, ); let child = Command::new(command.first().map(|s| s.as_ref()).unwrap_or_default()) diff --git a/cli/src/upload_command.rs b/cli/src/upload_command.rs index 0cace3f7..039c2af7 100644 --- a/cli/src/upload_command.rs +++ b/cli/src/upload_command.rs @@ -543,28 +543,28 @@ pub async fn run_upload( if upload_args.print_files.is_some() { let message = "The --print-files flag is deprecated and will be removed in a future version."; - tracing::error!("{message}"); + tracing::warn!(deprecated_flag = "--print-files", "{message}"); notices.push(Notice::deprecation(message)); } if let Some(team) = &upload_args.team { if !team.is_empty() { let message = "The --team flag is deprecated and will be removed in a future version."; - tracing::error!("{message}"); + tracing::warn!(deprecated_flag = "--team", "{message}"); notices.push(Notice::deprecation(message)); } } if upload_args.hide_banner { let message = "The --hide-banner flag is deprecated and does nothing. It will be removed in a future version."; - tracing::error!("{message}"); + tracing::warn!(deprecated_flag = "--hide-banner", "{message}"); notices.push(Notice::deprecation(message)); } if upload_args.use_quarantining.is_some() { let message = "The --use-quarantining flag is deprecated. Use --disable-quarantining instead."; - tracing::warn!("{message}"); + tracing::warn!(deprecated_flag = "--use-quarantining", "{message}"); notices.push(Notice::deprecation(message)); } diff --git a/cli/tests/summary/upload.rs b/cli/tests/summary/upload.rs index 0e4b35cf..8a84a1d2 100644 --- a/cli/tests/summary/upload.rs +++ b/cli/tests/summary/upload.rs @@ -227,20 +227,33 @@ fn prints_a_passing_run() { } #[test] -fn prints_deprecation_notices_first() { +fn prints_notices_apart_from_the_summary() { let mut result = upload_result(5); result.notices = vec![ Notice::deprecation( "Uploading to test collection zQ8nP2mW, inferred from the repository. Pass --test-collection-id zQ8nP2mW: analytics-cli 1.0.0 requires it.", ), - Notice::bep_parse_error("not shown here"), + Notice::upload_skipped("Test results were not uploaded."), + Notice::bep_parse_error("unexpected EOF"), ]; + let report = UploadReport::new(&result, UploadCommand::Upload); assert_eq!( - upload_summary(&result), + text::render_notices(&report.notices) + .unwrap() + .iter() + .map(Line::to_unstyled) + .collect::>(), [ "⚠️ Uploading to test collection zQ8nP2mW, inferred from the repository. Pass --test-collection-id zQ8nP2mW: analytics-cli 1.0.0 requires it.", + "⚠️ Test results were not uploaded.", + "⚠️ BEP file had a parse error: unexpected EOF", "", + ] + ); + assert_eq!( + upload_summary(&result), + [ "🏷️ Bundle Upload ID: 4b1d2c3e-5f60-4a7b-8c9d-0e1f2a3b4c5d", "", "📚 Test Report", diff --git a/cli/tests/summary/validate.rs b/cli/tests/summary/validate.rs index f5ff3cbb..aab17ee3 100644 --- a/cli/tests/summary/validate.rs +++ b/cli/tests/summary/validate.rs @@ -1,8 +1,10 @@ use clap::Parser; -use display::end_output::EndOutput; use pretty_assertions::assert_eq; use superconsole::Line; -use trunk_analytics_cli::validate_command::{ValidateArgs, run_validate}; +use trunk_analytics_cli::{ + summary_output::{ValidateReport, text}, + validate_command::{ValidateArgs, run_validate}, +}; #[derive(Parser)] struct Validate { @@ -10,12 +12,13 @@ struct Validate { args: ValidateArgs, } -async fn validate_summary(args: &[&str]) -> Vec { +async fn validate_report(args: &[&str]) -> ValidateReport { let Validate { args } = Validate::parse_from(["validate"].iter().chain(args)); - run_validate(args) - .await - .unwrap() - .output() + ValidateReport::new(&run_validate(args).await.unwrap()) +} + +fn unstyled(lines: anyhow::Result>) -> Vec { + lines .unwrap() .iter() .map(Line::to_unstyled) @@ -24,6 +27,10 @@ async fn validate_summary(args: &[&str]) -> Vec { .collect() } +async fn validate_summary(args: &[&str]) -> Vec { + unstyled(text::render_validate(&validate_report(args).await)) +} + #[tokio::test] async fn prints_validation_errors_warnings_and_codeowners() { assert_eq!( @@ -79,16 +86,22 @@ async fn prints_validation_errors_warnings_and_codeowners() { } #[tokio::test] -async fn prints_bep_parse_errors_without_a_codeowners_file() { +async fn prints_bep_parse_errors_as_notices_without_a_codeowners_file() { + let report = validate_report(&[ + "--bazel-bep-path", + "test_fixtures/summary/bep/bep_with_parse_error", + ]) + .await; assert_eq!( - validate_summary(&[ - "--bazel-bep-path", - "test_fixtures/summary/bep/bep_with_parse_error" - ]) - .await, + unstyled(text::render_notices(&report.notices)), [ - "⚠️ BEP file had parse errors: [\"Error parsing build event: EOF while parsing a value at line 3 column 0\"]", + "⚠️ BEP file had a parse error: Error parsing build event: EOF while parsing a value at line 3 column 0", "", + ] + ); + assert_eq!( + unstyled(text::render_validate(&report)), + [ "📚 Validating 1 files:", " Files matching: test_fixtures/summary/bep/test.xml:", " test_fixtures/summary/bep/test.xml", diff --git a/cli/tests/test.rs b/cli/tests/test.rs index 0a02628e..5f01847b 100644 --- a/cli/tests/test.rs +++ b/cli/tests/test.rs @@ -153,10 +153,10 @@ async fn test_command_fails_with_no_junit_files_no_quarantine_successful_upload( .assert() .failure() .code(128) - .stderr(predicate::str::contains( + .stdout(predicate::str::contains( "No tests were found in the provided test results, but non zero exit code provided: ", )) - .stderr(predicate::str::contains( + .stdout(predicate::str::contains( "The test step appears to have failed without running tests", )); @@ -614,3 +614,25 @@ async fn test_command_fails_when_the_command_cannot_be_started() { assert!(state.requests.lock().unwrap().is_empty()); } + +#[tokio::test(flavor = "multi_thread")] +async fn test_command_writes_summary_to_stdout_and_its_header_to_stderr() { + let temp_dir = tempdir().unwrap(); + generate_mock_git_repo(&temp_dir); + generate_mock_valid_junit_xmls(&temp_dir); + + let state = MockServerBuilder::new().spawn_mock_server().await; + + CommandBuilder::test( + temp_dir.path(), + state.host.clone(), + vec![String::from("true")], + ) + .command() + .assert() + .success() + .stdout(predicate::str::contains("📚 Test Report")) + .stderr(predicate::str::contains("📚 Test Report").not()) + .stderr(predicate::str::contains("📒 Test command outputs")) + .stdout(predicate::str::contains("📒 Test command outputs").not()); +} diff --git a/cli/tests/upload.rs b/cli/tests/upload.rs index ee3730a8..67693004 100644 --- a/cli/tests/upload.rs +++ b/cli/tests/upload.rs @@ -310,7 +310,7 @@ async fn upload_bundle() { println!("{assert}"); // A collection id was passed, so the upload surfaces as a link rather than the bare id // (the bare-id form is covered by the links-hidden test below). - assert.stderr(predicate::str::contains( + assert.stdout(predicate::str::contains( "/test-org/flaky-tests/collections/tc_123/uploads/eyJpZCI6IjgyYzZhNmU1LWY4ZWEtNGQ5My05YTI2LWI4YWI2ZmY4ZjZiYyIsImNyZWF0ZWRBdCI6MTc3ODQxNjQ5NjAwMH0", )); } @@ -332,11 +332,11 @@ async fn upload_bundle_prints_test_collection_links() { .failure(); assert - .stderr(predicate::str::contains( + .stdout(predicate::str::contains( "/test-org/flaky-tests/collections/tc_123/tests/", )) - .stderr(predicate::str::contains("/collections/tc_123/t/").not()) - .stderr(predicate::str::contains("?repo=trunk-io%2Fanalytics-cli").not()); + .stdout(predicate::str::contains("/collections/tc_123/t/").not()) + .stdout(predicate::str::contains("?repo=trunk-io%2Fanalytics-cli").not()); } // NOTE: must be multi threaded to start a mock server @@ -376,11 +376,11 @@ async fn upload_bundle_falls_back_to_short_links_without_collection_ids() { .failure(); assert - .stderr(predicate::str::contains( + .stdout(predicate::str::contains( "/test-org/flaky-tests/collections/tc_123/t/", )) - .stderr(predicate::str::contains("?repo=trunk-io%2Fanalytics-cli")) - .stderr(predicate::str::contains("/collections/tc_123/tests/").not()); + .stdout(predicate::str::contains("?repo=trunk-io%2Fanalytics-cli")) + .stdout(predicate::str::contains("/collections/tc_123/tests/").not()); } // NOTE: must be multi threaded to start a mock server @@ -402,10 +402,10 @@ async fn upload_bundle_prints_test_collection_upload_link() { // The canonical form: a base64url `{id, createdAt}` key from both halves the server // returned, not the bundle upload id. Needs no ?repo= — the short id scopes the upload. assert - .stderr(predicate::str::contains( + .stdout(predicate::str::contains( "/test-org/flaky-tests/collections/tc_123/uploads/eyJpZCI6IjgyYzZhNmU1LWY4ZWEtNGQ5My05YTI2LWI4YWI2ZmY4ZjZiYyIsImNyZWF0ZWRBdCI6MTc3ODQxNjQ5NjAwMH0", )) - .stderr(predicate::str::contains("Bundle Upload ID:").not()); + .stdout(predicate::str::contains("Bundle Upload ID:").not()); } // NOTE: must be multi threaded to start a mock server @@ -426,10 +426,10 @@ async fn upload_bundle_prints_bare_upload_id_when_links_hidden() { .failure(); assert - .stderr(predicate::str::contains( + .stdout(predicate::str::contains( get_bundle_upload_id_message("test-bundle-upload-id").as_str(), )) - .stderr(predicate::str::contains("/flaky-tests/collections/tc_123/uploads/").not()); + .stdout(predicate::str::contains("/flaky-tests/collections/tc_123/uploads/").not()); } // NOTE: must be multi threaded to start a mock server @@ -450,8 +450,8 @@ async fn upload_bundle_hides_test_collection_links_when_env_set() { .failure(); assert - .stderr(predicate::str::contains("/test-org/flaky-tests/test/")) - .stderr(predicate::str::contains("/flaky-tests/collections/").not()); + .stdout(predicate::str::contains("/test-org/flaky-tests/test/")) + .stdout(predicate::str::contains("/flaky-tests/collections/").not()); } // NOTE: must be multi threaded to start a mock server @@ -472,8 +472,8 @@ async fn upload_bundle_without_test_collection_id_prints_repo_links() { .failure(); assert - .stderr(predicate::str::contains("/test-org/flaky-tests/test/")) - .stderr(predicate::str::contains("/flaky-tests/collections/").not()); + .stdout(predicate::str::contains("/test-org/flaky-tests/test/")) + .stdout(predicate::str::contains("/flaky-tests/collections/").not()); } // NOTE: must be multi threaded to start a mock server @@ -876,7 +876,7 @@ async fn upload_bundle_using_xcresult(#[case] extra_args: &[&str]) { .command() .assert() .success() - .stderr(predicate::str::contains("1 file found, 1 with issues").not()); + .stdout(predicate::str::contains("1 file found, 1 with issues").not()); let requests = state.requests.lock().unwrap().clone(); assert_eq!(requests.len(), 3); @@ -1248,7 +1248,7 @@ async fn falls_back_to_binary_file() { let meta_json = fs::File::open(tar_extract_directory.join("meta.json")).unwrap(); assert!(serde_json::from_reader::(meta_json).is_ok()); - assert.stdout(predicate::str::contains( + assert.stderr(predicate::str::contains( "Attempting to parse bep file as binary", )); } @@ -1523,9 +1523,12 @@ async fn upload_bundle_invalid_repo_root() { .command() .assert() .failure() + .stderr(predicate::str::contains("Error Encountered")) .stderr(predicate::str::contains( "\"../\" does not appear to be a git repository", - )); + )) + // A run that fails outright has no summary + .stdout(predicate::str::is_empty()); let requests = state.requests.lock().unwrap().clone(); assert_eq!(requests.len(), 0); @@ -1714,6 +1717,64 @@ async fn upload_keeps_its_exit_code_when_the_summary_output_file_cannot_be_writt assert!(leftovers.is_empty(), "left behind {leftovers:?}"); } +#[tokio::test(flavor = "multi_thread")] +async fn upload_prints_only_the_json_summary_on_stdout() { + let temp_dir = tempdir().unwrap(); + generate_mock_git_repo(&temp_dir); + generate_mock_valid_junit_xmls_with_failures(&temp_dir); + let summary_path = temp_dir.path().join("summary.json"); + + let mut mock_server_builder = MockServerBuilder::new(); + // Slow enough to show a progress message. + mock_server_builder.set_get_quarantining_config_handler( + |Json(_): Json| async move { + tokio::time::sleep(Duration::from_secs(3)).await; + Json(GetQuarantineConfigResponse::default()) + }, + ); + let state = mock_server_builder.spawn_mock_server().await; + let assert = CommandBuilder::upload(temp_dir.path(), state.host.clone()) + .verbose(true) + .command() + .args(["--output-format", "json", "--hide-banner"]) + .arg("--summary-output-file") + .arg(&summary_path) + .assert() + .code(ExitCode::Failure.code()) + .stderr(predicate::str::contains("Starting trunk-analytics-cli")) + .stderr(predicate::str::contains( + "⚠️ The --hide-banner flag is deprecated", + )) + .stderr(predicate::str::contains("taking longer than expected")) + .stderr(predicate::str::contains("Summary written to")); + + let summary: serde_json::Value = serde_json::from_slice(&assert.get_output().stdout).unwrap(); + assert_eq!(summary, read_summary_output_file(&summary_path)); + assert_eq!(summary["type"], "UPLOAD"); + assert_eq!(summary["exitCode"], ExitCode::Failure.code()); +} + +#[tokio::test(flavor = "multi_thread")] +async fn upload_prints_a_json_error_on_stdout() { + let temp_dir = tempdir().unwrap(); + generate_mock_git_repo(&temp_dir); + + let state = MockServerBuilder::new().spawn_mock_server().await; + let assert = CommandBuilder::upload(temp_dir.path(), state.host.clone()) + .repo_root("../") + .command() + .env(constants::TRUNK_OUTPUT_FORMAT_ENV, "json") + .assert() + .code(ExitCode::Error.code()) + .stderr(predicate::str::contains("Error Encountered")); + + let summary: serde_json::Value = serde_json::from_slice(&assert.get_output().stdout).unwrap(); + assert_eq!(summary["schemaVersion"], 1); + assert_eq!(summary["type"], "ERROR"); + assert_eq!(summary["kind"], "GIT_REPOSITORY"); + assert_eq!(summary["exitCode"], ExitCode::Error.code()); +} + #[tokio::test(flavor = "multi_thread")] async fn upload_bundle_valid_repo_root_implicit() { let temp_dir = tempdir().unwrap(); @@ -1758,10 +1819,10 @@ async fn upload_bundle_with_no_junit_files_no_quarantine_successful_upload() { .assert() .code(0) .success() - .stderr(predicate::str::contains( + .stdout(predicate::str::contains( "No tests were found in the provided test results", )) - .stderr(predicate::str::contains("without running tests").not()); + .stdout(predicate::str::contains("without running tests").not()); // HINT: View CLI output with `cargo test -- --nocapture` println!("{assert}"); @@ -1976,7 +2037,7 @@ async fn is_not_ok_on_bad_request() { command .assert() .failure() - .stderr(predicate::str::contains("error")); + .stdout(predicate::str::contains("error")); } #[tokio::test(flavor = "multi_thread")] @@ -2765,7 +2826,7 @@ async fn presents_error_message_if_you_dont_have_permission_for_s3_upload() { .command() .assert() .failure() - .stderr(predicate::str::contains("Error Encountered")); + .stdout(predicate::str::contains("Error Encountered")); // HINT: View CLI output with `cargo test -- --nocapture` println!("{assert}"); @@ -2807,7 +2868,7 @@ async fn presents_error_message_if_you_dont_have_permission_for_upload_endpoint( .command() .assert() .failure() - .stderr(predicate::str::contains("Error Encountered")); + .stdout(predicate::str::contains("Error Encountered")); // HINT: View CLI output with `cargo test -- --nocapture` println!("{assert}"); @@ -2895,7 +2956,7 @@ async fn does_not_print_exit_code_with_validation_reports_none() { .command() .assert() .failure() - .stderr(predicate::str::contains("File Validation").not()); + .stdout(predicate::str::contains("File Validation").not()); println!("{assert}"); } @@ -2919,7 +2980,7 @@ async fn reports_failures_even_if_cannot_get_quarantine_context() { let mut command = CommandBuilder::upload(temp_dir.path(), state.host.clone()).command(); - let assert = command.assert().failure().stderr(predicate::str::contains( + let assert = command.assert().failure().stdout(predicate::str::contains( "We were unable to determine the quarantine status for tests.", )); @@ -2963,7 +3024,7 @@ async fn reports_failing_tests_but_succeeds_when_quarantine_disabled() { let assert = command .assert() .success() - .stderr(predicate::str::contains("Fail: 0").not()); + .stdout(predicate::str::contains("Fail: 0").not()); println!("{assert}"); } @@ -3865,3 +3926,73 @@ async fn upload_on_v1_when_trunk_chooses_v2_but_names_no_collection() { "the run must have uploaded on v1" ); } + +// A debug log emitted while setting up the API client, so it only shows up with `--verbose`. +const DEBUG_LOG_MARKER: &str = "Using public api address"; + +// NOTE: must be multi threaded to start a mock server +#[tokio::test(flavor = "multi_thread")] +async fn upload_verbose_logs_go_to_stderr() { + let temp_dir = tempdir().unwrap(); + generate_mock_git_repo(&temp_dir); + generate_mock_valid_junit_xmls(&temp_dir); + + let state = MockServerBuilder::new().spawn_mock_server().await; + + let assert = CommandBuilder::upload(temp_dir.path(), state.host.clone()) + .verbose(true) + .command() + .arg("--team") + .arg("some-team") + .assert(); + + println!("{assert}"); + // stdout is kept free of logs so it can carry machine-readable output + assert + .stdout(predicate::str::contains(DEBUG_LOG_MARKER).not()) + .stderr(predicate::str::contains(DEBUG_LOG_MARKER)) + .stderr(predicate::str::contains( + "The --team flag is deprecated and will be removed in a future version.", + )) + .stderr(predicate::str::contains("WARN")); +} + +// NOTE: must be multi threaded to start a mock server +#[tokio::test(flavor = "multi_thread")] +async fn upload_quietest_verbosity_logs_nothing() { + let temp_dir = tempdir().unwrap(); + generate_mock_git_repo(&temp_dir); + generate_mock_valid_junit_xmls(&temp_dir); + + let state = MockServerBuilder::new().spawn_mock_server().await; + + let assert = CommandBuilder::upload(temp_dir.path(), state.host.clone()) + .extra_args(&["-qqq"]) + .command() + .assert(); + + println!("{assert}"); + assert + .stdout(predicate::str::contains(DEBUG_LOG_MARKER).not()) + .stderr(predicate::str::contains(DEBUG_LOG_MARKER).not()); +} + +#[tokio::test(flavor = "multi_thread")] +async fn upload_writes_summary_to_stdout_and_notices_to_stderr() { + let temp_dir = tempdir().unwrap(); + generate_mock_git_repo(&temp_dir); + generate_mock_valid_junit_xmls(&temp_dir); + + let state = MockServerBuilder::new().spawn_mock_server().await; + + CommandBuilder::upload(temp_dir.path(), state.host.clone()) + .extra_args(&["--hide-banner"]) + .command() + .assert() + .stdout(predicate::str::contains("📚 Test Report")) + .stderr(predicate::str::contains("📚 Test Report").not()) + .stderr(predicate::str::contains( + "The --hide-banner flag is deprecated", + )) + .stdout(predicate::str::contains("deprecated").not()); +} diff --git a/cli/tests/validate.rs b/cli/tests/validate.rs index 82f9f2e3..627b7c2c 100644 --- a/cli/tests/validate.rs +++ b/cli/tests/validate.rs @@ -25,14 +25,14 @@ fn validate_success() { .command() .assert() .success() - .stderr(predicate::str::contains( + .stdout(predicate::str::contains( Line::from_iter([ Span::new_styled(style(String::from("0")).with(Color::Red)).unwrap(), Span::new_unstyled(String::from(" errors")).unwrap(), ]) .render(), )) - .stderr(predicate::str::contains( + .stdout(predicate::str::contains( Line::from_iter([ Span::new_styled(style(String::from("0")).with(Color::Green)).unwrap(), Span::new_unstyled(String::from(" valid files, ")).unwrap(), @@ -43,8 +43,8 @@ fn validate_success() { ]) .render(), )) - .stderr(predicate::str::contains("Checking for codeowners file...")) - .stderr(predicate::str::contains("Found codeowners path:")); + .stdout(predicate::str::contains("Checking for codeowners file...")) + .stdout(predicate::str::contains("Found codeowners path:")); println!("{assert}"); } @@ -105,7 +105,7 @@ fn validate_invalid_junits_no_codeowners() { .command() .assert() .failure() - .stderr(predicate::str::contains( + .stdout(predicate::str::contains( Line::from_iter([ Span::new_styled(style(String::from("0")).with(Color::Yellow)).unwrap(), Span::new_unstyled(String::from(" warnings, and ")).unwrap(), @@ -114,9 +114,9 @@ fn validate_invalid_junits_no_codeowners() { ]) .render(), )) - .stderr(predicate::str::contains("test suite names are missing")) - .stderr(predicate::str::contains("Checking for codeowners file...")) - .stderr(predicate::str::contains("No codeowners file found")); + .stdout(predicate::str::contains("test suite names are missing")) + .stdout(predicate::str::contains("Checking for codeowners file...")) + .stdout(predicate::str::contains("No codeowners file found")); println!("{assert}"); } @@ -131,7 +131,7 @@ fn validate_empty_xml() { .command() .assert() .success() - .stderr(predicate::str::contains( + .stdout(predicate::str::contains( Line::from_iter([ Span::new_styled(style(String::from("1")).with(Color::Yellow)).unwrap(), Span::new_unstyled(String::from(" warning, and ")).unwrap(), @@ -140,7 +140,7 @@ fn validate_empty_xml() { ]) .render(), )) - .stderr(predicate::str::contains("no reports found")); + .stdout(predicate::str::contains("no reports found")); println!("{assert}"); } @@ -155,7 +155,7 @@ fn validate_invalid_xml() { .command() .assert() .failure() - .stderr(predicate::str::contains( + .stdout(predicate::str::contains( Line::from_iter([ Span::new_styled(style(String::from("0")).with(Color::Yellow)).unwrap(), Span::new_unstyled(String::from(" warnings, and ")).unwrap(), @@ -164,7 +164,7 @@ fn validate_invalid_xml() { ]) .render(), )) - .stderr(predicate::str::contains("syntax error: tag not closed")); + .stdout(predicate::str::contains("syntax error: tag not closed")); println!("{assert}"); } @@ -178,7 +178,7 @@ fn validate_suboptimal_junits() { .command() .assert() .success() - .stderr(predicate::str::contains( + .stdout(predicate::str::contains( Line::from_iter([ Span::new_styled(style(String::from("1")).with(Color::Yellow)).unwrap(), Span::new_unstyled(String::from(" warning, and ")).unwrap(), @@ -187,7 +187,7 @@ fn validate_suboptimal_junits() { ]) .render(), )) - .stderr(predicate::str::contains( + .stdout(predicate::str::contains( "report has stale (> 1 hour(s)) timestamps", )); @@ -204,7 +204,7 @@ fn validate_missing_filepath_suboptimal_junits() { .command() .assert() .success() - .stderr(predicate::str::contains( + .stdout(predicate::str::contains( Line::from_iter([ Span::new_styled(style(String::from("2")).with(Color::Yellow)).unwrap(), Span::new_unstyled(String::from(" warnings, and ")).unwrap(), @@ -212,12 +212,72 @@ fn validate_missing_filepath_suboptimal_junits() { Span::new_unstyled(String::from(" errors")).unwrap(), ]).render(), )) - .stderr(predicate::str::contains( + .stdout(predicate::str::contains( "report has test cases with missing file or filepath", )) - .stderr(predicate::str::contains( + .stdout(predicate::str::contains( "CODEOWNERS found but test cases are missing filepaths. We will not be able to correlate flaky tests with owners.", )); println!("{assert}"); } + +#[test] +fn validate_writes_summary_to_stdout_and_notices_to_stderr() { + let temp_dir = tempdir().unwrap(); + generate_mock_valid_junit_xmls(&temp_dir); + + let assert = CommandBuilder::validate(temp_dir.path()) + .extra_args(&["--hide-banner"]) + .command() + .assert() + .success() + .stdout(predicate::str::contains("Checking for codeowners file...")) + .stderr(predicate::str::contains("Checking for codeowners file...").not()) + .stderr(predicate::str::contains( + "The --hide-banner flag is deprecated", + )) + .stdout(predicate::str::contains("deprecated").not()); + + println!("{assert}"); +} + +#[test] +fn validate_prints_only_the_json_summary_on_stdout() { + let temp_dir = tempdir().unwrap(); + generate_mock_valid_junit_xmls(&temp_dir); + generate_mock_codeowners(&temp_dir); + + let assert = CommandBuilder::validate(temp_dir.path()) + .command() + .args(["--output-format", "json", "--hide-banner", "--verbose"]) + .assert() + .success() + .stderr(predicate::str::contains("Starting trunk-analytics-cli")) + .stderr(predicate::str::contains( + "⚠️ The --hide-banner flag is deprecated", + )); + + let summary: serde_json::Value = serde_json::from_slice(&assert.get_output().stdout).unwrap(); + assert_eq!(summary["schemaVersion"], 1); + assert_eq!(summary["type"], "VALIDATE"); + assert_eq!(summary["counts"]["files"], 1); + assert_eq!(summary["exitCode"], 0); +} + +#[test] +fn validate_prints_json_with_the_same_exit_code() { + let temp_dir = tempdir().unwrap(); + generate_mock_invalid_junit_xmls(&temp_dir); + + let assert = CommandBuilder::validate(temp_dir.path()) + .command() + .args(["--output-format", "json"]) + .assert() + .code(1); + + let summary: serde_json::Value = serde_json::from_slice(&assert.get_output().stdout).unwrap(); + assert_eq!(summary["type"], "VALIDATE"); + assert_eq!(summary["counts"]["errors"], 1); + assert_eq!(summary["exitCode"], 1); +} diff --git a/constants/src/lib.rs b/constants/src/lib.rs index 9ada3b06..12816eb6 100644 --- a/constants/src/lib.rs +++ b/constants/src/lib.rs @@ -56,6 +56,7 @@ pub const TRUNK_VALIDATION_REPORT_ENV: &str = "TRUNK_VALIDATION_REPORT"; pub const TRUNK_SHOW_FAILURE_MESSAGES_ENV: &str = "TRUNK_SHOW_FAILURE_MESSAGES"; pub const TRUNK_HIDE_TEST_COLLECTION_LINKS_ENV: &str = "TRUNK_HIDE_TEST_COLLECTION_LINKS"; pub const TRUNK_SUMMARY_OUTPUT_FILE_ENV: &str = "TRUNK_SUMMARY_OUTPUT_FILE"; +pub const TRUNK_OUTPUT_FORMAT_ENV: &str = "TRUNK_OUTPUT_FORMAT"; pub const TRUNK_DEBUG_ENV: &str = "TRUNK_DEBUG"; pub const TRUNK_USE_EXPERIMENTAL_XCRESULT_TEST_LOCATIONS_ENV: &str = "TRUNK_USE_EXPERIMENTAL_XCRESULT_TEST_LOCATIONS"; @@ -110,6 +111,7 @@ pub const TRUNK_ENVS_TO_CAPTURE: &[&str] = &[ TRUNK_SHOW_FAILURE_MESSAGES_ENV, TRUNK_HIDE_TEST_COLLECTION_LINKS_ENV, TRUNK_SUMMARY_OUTPUT_FILE_ENV, + TRUNK_OUTPUT_FORMAT_ENV, TRUNK_DEBUG_ENV, TRUNK_USE_EXPERIMENTAL_XCRESULT_TEST_LOCATIONS_ENV, ]; diff --git a/display/src/end_output.rs b/display/src/end_output.rs deleted file mode 100644 index cedcbe1e..00000000 --- a/display/src/end_output.rs +++ /dev/null @@ -1,37 +0,0 @@ -use std::convert::Infallible; - -use anyhow::Result; -use superconsole::{Component, Dimensions, DrawMode, Line, Lines, SuperConsole}; - -pub trait EndOutput { - fn output(&self) -> Result>; -} - -enum EmptyComponent { - Base, -} -impl Component for EmptyComponent { - type Error = Infallible; - fn draw_unchecked( - &self, - _dimensions: Dimensions, - _mode: DrawMode, - ) -> Result { - Ok(Lines(vec![])) - } -} - -// Clears out any renders, and outputs the entire output of result, scrolling the terminal and wrapping long lines (you know, normal terminal output) -pub fn display_end( - superconsole: &mut SuperConsole, - result: &(impl EndOutput + ?Sized), -) -> Result<()> { - let lines = result.output()?; - // Superconsole truncates line emission if you emit more lines than the console has rows in a single render loop. - // We could batch these based on tty rows, but the performance is fine without doing so, and we're not exactly running - // a reactive ui here. - for line in lines { - superconsole.emit_now(Lines(vec![line]), &EmptyComponent::Base)?; - } - Ok(()) -} diff --git a/display/src/lib.rs b/display/src/lib.rs index cf9c93ae..d4a7f6fd 100644 --- a/display/src/lib.rs +++ b/display/src/lib.rs @@ -1,3 +1,2 @@ -pub mod end_output; pub mod message; pub mod render; diff --git a/display/src/message.rs b/display/src/message.rs index 7a05a929..a6176f3e 100644 --- a/display/src/message.rs +++ b/display/src/message.rs @@ -1,13 +1,17 @@ -use std::sync::{Arc, mpsc::Sender}; +use std::{ + convert::Infallible, + sync::{Arc, mpsc::Sender}, +}; use anyhow::Result; use superconsole::{Component, Dimensions, DrawMode, Line, Lines, Span, SpanError, SuperConsole}; -use crate::end_output::{EndOutput, display_end}; - pub enum DisplayMessage { Progress(Arc, String), - Final(Arc, String), + /// The command's summary, on stdout so it can be captured apart from logs and progress. + Report(Vec, String), + /// Anything else to show the user, such as warnings or why a run failed, on stderr. + Notice(Vec, String), } impl DisplayMessage { @@ -16,7 +20,10 @@ impl DisplayMessage { DisplayMessage::Progress(progress_message, _) => superconsole .render(&**progress_message) .map_err(anyhow::Error::new), - DisplayMessage::Final(final_message, _) => display_end(superconsole, &**final_message), + DisplayMessage::Report(lines, _) => { + emit_lines(superconsole, lines, SuperConsole::emit_aux) + } + DisplayMessage::Notice(lines, _) => emit_lines(superconsole, lines, SuperConsole::emit), }; if let Err(e) = render_result { tracing::error!("Failed to render {}: {:?}", self.description(), e); @@ -26,11 +33,41 @@ impl DisplayMessage { fn description(&self) -> String { match self { DisplayMessage::Progress(_, description) => description.to_string(), - DisplayMessage::Final(_, description) => description.to_string(), + DisplayMessage::Report(_, description) => description.to_string(), + DisplayMessage::Notice(_, description) => description.to_string(), } } } +struct EmptyComponent; + +impl Component for EmptyComponent { + type Error = Infallible; + fn draw_unchecked( + &self, + _dimensions: Dimensions, + _mode: DrawMode, + ) -> Result { + Ok(Lines(vec![])) + } +} + +// Clears the progress line, and writes the lines as normal terminal output that scrolls and wraps. +fn emit_lines( + superconsole: &mut SuperConsole, + lines: &[Line], + emit: fn(&mut SuperConsole, Lines), +) -> Result<()> { + // Superconsole truncates line emission if you emit more lines than the console has rows in a single render loop. + // We could batch these based on tty rows, but the performance is fine without doing so, and we're not exactly running + // a reactive ui here. + for line in lines { + emit(superconsole, Lines(vec![line.clone()])); + superconsole.render(&EmptyComponent)?; + } + Ok(()) +} + pub fn send_message(message: DisplayMessage, sender: &Sender) { let send_result = sender.send(message); if let Err(e) = send_result {