Build the validate summary as data, separate from rendering

cli/src/main.rs

@@ -18,7 +18,7 @@ use trunk_analytics_cli::{
     context::gather_debug_props,
     error_report::ErrorReport,
     summary_output::{
-        ErrorSummary, UploadCommand, UploadReport,
+        ErrorSummary, UploadCommand, UploadReport, ValidateReport,
         text::{TextOptions, UploadText},
     },
     test_command::{TestArgs, run_test},
@@ -176,12 +176,13 @@ fn main() -> anyhow::Result<()> {
                     close_out_and_exit(exit_code, guard, render_sender, render_handle)
                 }
                 MainOutcome::Finished(Ok(RunResult::Validate(run_result))) => {
-                    let result_ptr = Arc::new(run_result);
+                    let report = ValidateReport::new(&run_result);
+                    let exit_code = report.exit_code;
                     send_message(
-                        DisplayMessage::Final(result_ptr.clone(), String::from("validate display")),
+                        DisplayMessage::Final(Arc::new(report), String::from("validate display")),
                         &render_sender,
                     );
-                    close_out_and_exit(result_ptr.exit_code(), guard, render_sender, render_handle)
+                    close_out_and_exit(exit_code, guard, render_sender, render_handle)
                 }
                 MainOutcome::Finished(Err(error)) => {
                     let error = ErrorSummary::new(&ErrorReport::new(error, org_url_slug, None));

cli/src/summary_output/mod.rs

@@ -2,10 +2,12 @@ mod error;
 mod notice;
 pub mod text;
 mod upload;
+mod validate;
 
 pub use error::ErrorSummary;
 pub use notice::{Notice, NoticeKind};
 pub use upload::{
-    FailedTest, FailureGroup, FileValidation, GroupKey, Outcome, TestCounts, UploadCommand,
-    UploadReport,
+    FailedTest, FailureGroup, FileValidation, GroupKey, Outcome, ParseError, TestCounts,
+    UploadCommand, UploadReport,
 };
+pub use validate::{Codeowners, MatchedFiles, ValidateReport, ValidationCounts};

cli/src/summary_output/text.rs

@@ -7,12 +7,14 @@ use superconsole::{
 };
 
 use super::{
-    ErrorSummary, FailureGroup, FileValidation, GroupKey, Outcome, TestCounts, UploadReport,
+    ErrorSummary, FailureGroup, FileValidation, GroupKey, NoticeKind, Outcome, ParseError,
+    TestCounts, UploadReport, ValidateReport, ValidationCounts,
 };
 use crate::{
     error_report::{CONNECTION_REFUSED_CONTEXT, ErrorKind, HELP_TEXT, UNAUTHORIZED_CONTEXT},
     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.
@@ -485,11 +487,243 @@ 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)
+    }
+}
+
+impl EndOutput for ValidateRunResult {
+    fn output(&self) -> anyhow::Result<Vec<Line>> {
+        render_validate(&ValidateReport::new(self))
+    }
+}
+
+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()
+        .map(|matched| matched.files.len())
+        .sum();
+    output.push(Line::from_iter([Span::new_unstyled(format!(
+        "📚 Validating {} files:",
+        num_files
+    ))?]));
+    for matched in report.matched_files.iter() {
+        output.push(Line::from_iter([Span::new_unstyled(format!(
+            "  Files matching: {}:",
+            matched.glob
+        ))?]));
+        for file in matched.files.iter() {
+            output.push(Line::from_iter([Span::new_unstyled(format!(
+                "    {}",
+                file
+            ))?]))
+        }
+        output.push(Line::default());
+    }
+
+    // Files that weren't read as exactly one report have no test counts.
+    output.push(Line::from_iter([Span::new_styled(
+        style(String::from("File parse issues")).attribute(Attribute::Bold),
+    )?]));
+    for validation in report
+        .validations
+        .iter()
+        .filter(|validation| validation.test_case_count.is_none())
+    {
+        let num_errors = validation.errors.len() + usize::from(validation.parse_error.is_some());
+        output.push(file_title(validation, num_errors)?);
+        output.push(Line::from_iter([Span::new_styled(
+            style(format!(
+                "{} errors, {} warnings",
+                num_errors,
+                validation.warnings.len()
+            ))
+            .attribute(Attribute::Italic),
+        )?]));
+        let parse_error = validation.parse_error.as_ref().map(anyhow_debug);
+        output.extend(render_issues(
+            parse_error.iter().chain(validation.errors.iter()),
+            &validation.warnings,
+        )?);
+        output.push(Line::default());
+    }
+
+    output.push(Line::from_iter([Span::new_styled(
+        style(String::from("Test Validation Issues")).attribute(Attribute::Bold),
+    )?]));
+    for validation in report.validations.iter() {
+        let (Some(num_test_suites), Some(num_test_cases)) =
+            (validation.test_suite_count, validation.test_case_count)
+        else {
+            continue;
+        };
+        output.push(file_title(validation, validation.errors.len())?);
+        output.push(Line::from_iter([Span::new_styled(
+            style(format!(
+                "{} test suites, {} test cases, {} errors, {} warnings",
+                num_test_suites,
+                num_test_cases,
+                validation.errors.len(),
+                validation.warnings.len(),
+            ))
+            .attribute(Attribute::Italic),
+        )?]));
+        output.extend(render_issues(
+            validation.errors.iter(),
+            &validation.warnings,
+        )?);
+    }
+
+    output.push(Line::default());
+    output.push(Line::from_iter([Span::new_styled(
+        style(String::from("Checking for codeowners file...")).attribute(Attribute::Bold),
+    )?]));
+    match &report.codeowners {
+        None => {
+            output.push(Line::from_iter([Span::new_unstyled(
+                "  No codeowners file found",
+            )?]));
+        }
+        Some(codeowners) => {
+            output.push(Line::from_iter([Span::new_styled_lossy(
+                style(format!("  Found codeowners path: {:?}", codeowners.path))
+                    .attribute(Attribute::Italic),
+            )]));
+            for warning in codeowners.warnings.iter() {
+                output.push(Line::from_iter([Span::new_unstyled_lossy(format!(
+                    "  {}",
+                    warning
+                ))]));
+            }
+        }
+    }
+    output.push(Line::default());
+
+    let ValidationCounts {
+        files,
+        valid_files,
+        files_with_warnings,
+        files_with_errors,
+        warnings,
+        errors,
+    } = report.counts;
+    output.push(Line::from_iter([
+        Span::new_styled(style(format!("{}", valid_files)).with(Color::Green))?,
+        Span::new_unstyled(format!(
+            " {}, ",
+            pluralize("valid file", valid_files as isize, false)
+        ))?,
+        Span::new_styled(style(format!("{}", files_with_warnings)).with(Color::Yellow))?,
+        Span::new_unstyled(format!(
+            " {} with warnings, and ",
+            pluralize("file", files_with_warnings as isize, false)
+        ))?,
+        Span::new_styled(style(format!("{}", files_with_errors)).with(Color::Red))?,
+        Span::new_unstyled(format!(
+            " {} with errors, ",
+            pluralize("file", files_with_errors as isize, false)
+        ))?,
+        Span::new_unstyled(format!(
+            "with {} total (a file is double counted if it has both errors and warnings)",
+            pluralize("file", files as isize, true)
+        ))?,
+    ]));
+    output.push(Line::from_iter([
+        Span::new_styled(style(format!("{}", warnings)).with(Color::Yellow))?,
+        Span::new_unstyled(format!(
+            " {}, and ",
+            pluralize("warning", warnings as isize, false)
+        ))?,
+        Span::new_styled(style(format!("{}", errors)).with(Color::Red))?,
+        Span::new_unstyled(format!(" {}", pluralize("error", errors as isize, false)))?,
+    ]));
+
+    Ok(output)
+}
+
+fn file_title(validation: &FileValidation, num_errors: usize) -> anyhow::Result<Line> {
+    let color = if num_errors > 0 {
+        Color::Red
+    } else if !validation.warnings.is_empty() {
+        Color::Yellow
+    } else {
+        Color::Green
+    };
+    Ok(Line::from_iter([Span::new_styled_lossy(
+        style(validation.file.clone()).with(color),
+    )]))
+}
+
+fn render_issues<'a>(
+    errors: impl Iterator<Item = &'a String>,
+    warnings: &[String],
+) -> anyhow::Result<Vec<Line>> {
+    let mut output = Vec::new();
+    let mut errors = errors.peekable();
+    if errors.peek().is_some() {
+        output.push(Line::from_iter([Span::new_unstyled(" ❌ Errors:")?]));
+        for error in errors {
+            output.push(Line::from_iter([Span::new_unstyled_lossy(format!(
+                "  {}",
+                error
+            ))]));
+        }
+    }
+    if !warnings.is_empty() {
+        output.push(Line::from_iter([Span::new_unstyled(" ⚠️  Warnings:")?]));
+        for warning in warnings {
+            output.push(Line::from_iter([Span::new_unstyled_lossy(format!(
+                "  {}",
+                warning
+            ))]));
+        }
+    }
+    Ok(output)
+}
+
+/// The error as `{:?}` on an `anyhow::Error` prints it, which `validate` has always shown:
+/// the error, then its causes, indented (and numbered when there's more than one).
+fn anyhow_debug(parse_error: &ParseError) -> String {
+    let mut text = parse_error.error.clone();
+    if parse_error.causes.is_empty() {
+        return text;
+    }
+    text.push_str("\n\nCaused by:");
+    let numbered = parse_error.causes.len() > 1;
+    for (n, cause) in parse_error.causes.iter().enumerate() {
+        text.push('\n');
+        let (first, rest) = if numbered {
+            (format!("{n: >5}: "), "       ")
+        } else {
+            (String::from("    "), "    ")
+        };
+        text.push_str(&first);
+        text.push_str(&cause.replace('\n', &format!("\n{rest}")));
+    }
+    text
+}
+
 #[cfg(test)]
 mod tests {
-    use superconsole::{Line, style::Attribute};
-
-    use super::url_span;
+    use super::*;
 
     const URL: &str = "https://app.trunk.io/my-org/flaky-tests/test/1";
 
@@ -504,4 +738,18 @@ mod tests {
         assert!(!plain.contains("\x1b]8;;"));
         assert!(plain.contains(URL));
     }
+
+    #[test]
+    fn anyhow_debug_matches_anyhow() {
+        let errors = [
+            anyhow::anyhow!("no causes"),
+            anyhow::anyhow!("cause").context("error"),
+            anyhow::anyhow!("root\nspans lines")
+                .context("middle")
+                .context("error\nspans lines"),
+        ];
+        for error in errors {
+            assert_eq!(anyhow_debug(&ParseError::new(&error)), format!("{error:?}"));
+        }
+    }
 }

cli/src/summary_output/upload.rs

@@ -80,9 +80,18 @@ pub struct FailedTest {
 #[serde(rename_all = "camelCase")]
 pub struct FileValidation {
     pub file: String,
-    pub parse_error: Option<String>,
+    pub parse_error: Option<ParseError>,
     pub errors: Vec<String>,
     pub warnings: Vec<String>,
+    pub test_suite_count: Option<usize>,
+    pub test_case_count: Option<usize>,
+}
+
+#[derive(Debug, Clone, PartialEq, Eq, Serialize)]
+#[serde(rename_all = "camelCase")]
+pub struct ParseError {
+    pub error: String,
+    pub causes: Vec<String>,
 }
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)]
@@ -163,6 +172,15 @@ impl FileValidation {
     }
 }
 
+impl ParseError {
+    pub fn new(error: &anyhow::Error) -> Self {
+        Self {
+            error: error.to_string(),
+            causes: error.chain().skip(1).map(ToString::to_string).collect(),
+        }
+    }
+}
+
 impl Outcome {
     fn new(counts: &TestCounts, test_run_exit_code: i32) -> Self {
         if counts.total == 0 {
@@ -251,9 +269,11 @@ fn file_validations(validations: &JunitReportValidations) -> Vec<FileValidation>
         .map(|(file, validation)| match validation {
             Err(error) => FileValidation {
                 file: file.clone(),
-                parse_error: Some(format!("{error:#}")),
+                parse_error: Some(ParseError::new(error)),
                 errors: Vec::new(),
                 warnings: Vec::new(),
+                test_suite_count: None,
+                test_case_count: None,
             },
             Ok(validation) => {
                 let issues = validation.all_issues_flat();
@@ -269,6 +289,8 @@ fn file_validations(validations: &JunitReportValidations) -> Vec<FileValidation>
                     parse_error: None,
                     errors: messages(JunitValidationLevel::Invalid),
                     warnings: messages(JunitValidationLevel::SubOptimal),
+                    test_suite_count: Some(validation.test_suites().len()),
+                    test_case_count: Some(validation.test_cases().len()),
                 }
             }
         })