From b4cccf01587ac672dee7a92df7e3e21728f5b7aa Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= <berykubik@gmail.com>
Date: Sat, 15 Mar 2025 10:00:11 +0100
Subject: [PATCH] Put test differences into a `<details>` section and add
 better explanation of the post merge report

---
 src/ci/citool/src/analysis.rs | 63 +++++++++++++++++++----------------
 src/ci/citool/src/main.rs     | 23 ++++++++++---
 src/ci/citool/src/utils.rs    |  4 ++-
 3 files changed, 56 insertions(+), 34 deletions(-)

diff --git a/src/ci/citool/src/analysis.rs b/src/ci/citool/src/analysis.rs
index 8a3754435770..6469bc251be6 100644
--- a/src/ci/citool/src/analysis.rs
+++ b/src/ci/citool/src/analysis.rs
@@ -246,7 +246,6 @@ fn report_test_diffs(diff: AggregatedTestDiffs) {
         println!("No test diffs found");
         return;
     }
-    println!("\n{} test {} found\n", diff.diffs.len(), pluralize("difference", diff.diffs.len()));
 
     fn format_outcome(outcome: &TestOutcome) -> String {
         match outcome {
@@ -320,34 +319,42 @@ fn report_test_diffs(diff: AggregatedTestDiffs) {
     // Sort diffs by job group and test name
     grouped_diffs.sort_by(|(d1, g1), (d2, g2)| g1.cmp(&g2).then(d1.test.name.cmp(&d2.test.name)));
 
-    for (diff, job_group) in grouped_diffs {
-        println!(
-            "- `{}`: {} ({})",
-            diff.test.name,
-            format_diff(&diff.diff),
-            format_job_group(job_group)
-        );
-    }
+    output_details(
+        &format!("Show {} test {}\n", original_diff_count, pluralize("diff", original_diff_count)),
+        || {
+            for (diff, job_group) in grouped_diffs {
+                println!(
+                    "- `{}`: {} ({})",
+                    diff.test.name,
+                    format_diff(&diff.diff),
+                    format_job_group(job_group)
+                );
+            }
 
-    let extra_diffs = diffs.len().saturating_sub(max_diff_count);
-    if extra_diffs > 0 {
-        println!("\n(and {extra_diffs} additional {})", pluralize("test diff", extra_diffs));
-    }
+            let extra_diffs = diffs.len().saturating_sub(max_diff_count);
+            if extra_diffs > 0 {
+                println!(
+                    "\n(and {extra_diffs} additional {})",
+                    pluralize("test diff", extra_diffs)
+                );
+            }
 
-    if doctest_count > 0 {
-        println!(
-            "\nAdditionally, {doctest_count} doctest {} were found. These are ignored, as they are noisy.",
-            pluralize("diff", doctest_count)
-        );
-    }
+            if doctest_count > 0 {
+                println!(
+                    "\nAdditionally, {doctest_count} doctest {} were found. These are ignored, as they are noisy.",
+                    pluralize("diff", doctest_count)
+                );
+            }
 
-    // Now print the job group index
-    println!("\n**Job group index**\n");
-    for (group, jobs) in job_index.into_iter().enumerate() {
-        println!(
-            "- {}: {}",
-            format_job_group(group as u64),
-            jobs.iter().map(|j| format!("`{j}`")).collect::<Vec<_>>().join(", ")
-        );
-    }
+            // Now print the job group index
+            println!("\n**Job group index**\n");
+            for (group, jobs) in job_index.into_iter().enumerate() {
+                println!(
+                    "- {}: {}",
+                    format_job_group(group as u64),
+                    jobs.iter().map(|j| format!("`{j}`")).collect::<Vec<_>>().join(", ")
+                );
+            }
+        },
+    );
 }
diff --git a/src/ci/citool/src/main.rs b/src/ci/citool/src/main.rs
index fb0639367bd0..01483a2633d0 100644
--- a/src/ci/citool/src/main.rs
+++ b/src/ci/citool/src/main.rs
@@ -19,7 +19,7 @@ use crate::cpu_usage::load_cpu_usage;
 use crate::datadog::upload_datadog_metric;
 use crate::jobs::RunType;
 use crate::metrics::{JobMetrics, download_auto_job_metrics, download_job_metrics, load_metrics};
-use crate::utils::load_env_var;
+use crate::utils::{load_env_var, output_details};
 use analysis::output_bootstrap_stats;
 
 const CI_DIRECTORY: &str = concat!(env!("CARGO_MANIFEST_DIR"), "/..");
@@ -159,6 +159,22 @@ fn postprocess_metrics(
     Ok(())
 }
 
+fn post_merge_report(db: JobDatabase, current: String, parent: String) -> anyhow::Result<()> {
+    let metrics = download_auto_job_metrics(&db, &parent, &current)?;
+
+    output_details("What is this?", || {
+        println!(
+            r#"This is an experimental post-merge analysis report that shows differences in
+test outcomes between the merged PR and its parent PR."#
+        );
+    });
+
+    println!("\nComparing {parent} (parent) -> {current} (this PR)\n");
+    output_test_diffs(metrics);
+
+    Ok(())
+}
+
 #[derive(clap::Parser)]
 enum Args {
     /// Calculate a list of jobs that should be executed on CI.
@@ -243,10 +259,7 @@ fn main() -> anyhow::Result<()> {
             postprocess_metrics(metrics_path, parent, job_name)?;
         }
         Args::PostMergeReport { current, parent } => {
-            let db = load_db(default_jobs_file)?;
-            let metrics = download_auto_job_metrics(&db, &parent, &current)?;
-            println!("Comparing {parent} (base) -> {current} (this PR)\n");
-            output_test_diffs(metrics);
+            post_merge_report(load_db(default_jobs_file)?, current, parent)?;
         }
     }
 
diff --git a/src/ci/citool/src/utils.rs b/src/ci/citool/src/utils.rs
index bbe24c3633bc..b9b1bf4d4550 100644
--- a/src/ci/citool/src/utils.rs
+++ b/src/ci/citool/src/utils.rs
@@ -22,7 +22,9 @@ where
 {
     println!(
         r"<details>
-<summary>{summary}</summary>"
+<summary>{summary}</summary>
+
+"
     );
     func();
     println!("</details>\n");