trunk-merge/pr-1277/a69f58bc-9e84-4ee0-a1a2-1c7876d1afb4

cli/src/summary_output/error.rs

@@ -21,7 +21,7 @@ pub struct ErrorSummary {
     pub explanation: Option<String>,
     /// Where to check the org slug and token. Only set for `UNAUTHORIZED` errors.
     pub settings_url: Option<String>,
-    /// 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<ExitCode>,
 }
 

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.

cli/src/summary_output/text.rs

@@ -1,22 +1,20 @@
 use std::path::{Path, PathBuf};
 
-use display::end_output::EndOutput;
 use pluralizer::pluralize;
 use superconsole::{
     Hyperlink, Line, Lines, Span, SuperConsole,
     style::{Attribute, Color, Stylize, style},
 };
 
 use super::{
-    ErrorSummary, FailureGroup, FileValidation, GroupKey, NoticeKind, Outcome, ParseError,
+    ErrorSummary, FailureGroup, FileValidation, GroupKey, Notice, NoticeKind, Outcome, ParseError,
     Quarantining, TestCounts, UploadReport, ValidateReport, ValidationCounts,
 };
 use crate::{
     error_report::{CONNECTION_REFUSED_CONTEXT, ErrorKind, HELP_TEXT, UNAUTHORIZED_CONTEXT},
     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<Vec<Line>> {
-        render_upload(&self.report, &self.options)
-    }
-}
-
-impl EndOutput for ErrorSummary {
-    fn output(&self) -> anyhow::Result<Vec<Line>> {
-        render_error(self)
-    }
-}
-
 pub struct SummaryFileText {
     path: PathBuf,
     error: Option<String>,
@@ -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<Vec<Line>> {
-        Ok(vec![
+    pub fn render(&self) -> Vec<Line> {
+        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<Vec<Line>> {
-        render_validate(self)
+/// Shown on stderr, apart from the summary.
+pub fn render_notices(notices: &[Notice]) -> anyhow::Result<Vec<Line>> {
+    let mut output: Vec<Line> = 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(&notice.message),
+        ]));
     }
-}
-
-impl EndOutput for ValidateRunResult {
-    fn output(&self) -> anyhow::Result<Vec<Line>> {
-        render_validate(&ValidateReport::new(self))
+    if !output.is_empty() {
+        output.push(Line::default());
     }
+    Ok(output)
 }
 
 pub fn render_validate(report: &ValidateReport) -> anyhow::Result<Vec<Line>> {
     let mut output: Vec<Line> = Vec::new();
 
-    let bep_parse_errors: Vec<&String> = report
-        .notices
-        .iter()
-        .filter(|notice| notice.kind == NoticeKind::BepParseError)
-        .map(|notice| &notice.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()

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<MatchedFiles>,
+    /// One entry per test report file.
     pub validations: Vec<FileValidation>,
+    /// The CODEOWNERS file, when one was found.
     pub codeowners: Option<Codeowners>,
     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<Notice>,
 }
 
-#[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<String>,
 }
 
-#[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<String>,
 }
 
-#[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,

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<Vec<Line>> {
-        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<Item = String> {
-    output
-        .lines()
-        .flat_map(|lf_line| {
-            lf_line
-                .split('\r')
-                .map(|crlf_line| crlf_line.replace('\t', "    "))
-        })
-        .collect::<Vec<String>>()
-        .into_iter()
-}
-
-impl EndOutput for TestRunResult {
-    fn output(&self) -> anyhow::Result<Vec<Line>> {
-        let mut output: Vec<Line> = 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<T: AsRef<str>>(
     render_sender: Sender<DisplayMessage>,
 ) -> anyhow::Result<TestRunResult> {
     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())

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));
     }