Skip to content

Commit b4cccf0

Browse files
committed
Put test differences into a <details> section and add better explanation of the post merge report
1 parent 232be86 commit b4cccf0

File tree

3 files changed

+56
-34
lines changed

3 files changed

+56
-34
lines changed

src/ci/citool/src/analysis.rs

+35-28
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,6 @@ fn report_test_diffs(diff: AggregatedTestDiffs) {
246246
println!("No test diffs found");
247247
return;
248248
}
249-
println!("\n{} test {} found\n", diff.diffs.len(), pluralize("difference", diff.diffs.len()));
250249

251250
fn format_outcome(outcome: &TestOutcome) -> String {
252251
match outcome {
@@ -320,34 +319,42 @@ fn report_test_diffs(diff: AggregatedTestDiffs) {
320319
// Sort diffs by job group and test name
321320
grouped_diffs.sort_by(|(d1, g1), (d2, g2)| g1.cmp(&g2).then(d1.test.name.cmp(&d2.test.name)));
322321

323-
for (diff, job_group) in grouped_diffs {
324-
println!(
325-
"- `{}`: {} ({})",
326-
diff.test.name,
327-
format_diff(&diff.diff),
328-
format_job_group(job_group)
329-
);
330-
}
322+
output_details(
323+
&format!("Show {} test {}\n", original_diff_count, pluralize("diff", original_diff_count)),
324+
|| {
325+
for (diff, job_group) in grouped_diffs {
326+
println!(
327+
"- `{}`: {} ({})",
328+
diff.test.name,
329+
format_diff(&diff.diff),
330+
format_job_group(job_group)
331+
);
332+
}
331333

332-
let extra_diffs = diffs.len().saturating_sub(max_diff_count);
333-
if extra_diffs > 0 {
334-
println!("\n(and {extra_diffs} additional {})", pluralize("test diff", extra_diffs));
335-
}
334+
let extra_diffs = diffs.len().saturating_sub(max_diff_count);
335+
if extra_diffs > 0 {
336+
println!(
337+
"\n(and {extra_diffs} additional {})",
338+
pluralize("test diff", extra_diffs)
339+
);
340+
}
336341

337-
if doctest_count > 0 {
338-
println!(
339-
"\nAdditionally, {doctest_count} doctest {} were found. These are ignored, as they are noisy.",
340-
pluralize("diff", doctest_count)
341-
);
342-
}
342+
if doctest_count > 0 {
343+
println!(
344+
"\nAdditionally, {doctest_count} doctest {} were found. These are ignored, as they are noisy.",
345+
pluralize("diff", doctest_count)
346+
);
347+
}
343348

344-
// Now print the job group index
345-
println!("\n**Job group index**\n");
346-
for (group, jobs) in job_index.into_iter().enumerate() {
347-
println!(
348-
"- {}: {}",
349-
format_job_group(group as u64),
350-
jobs.iter().map(|j| format!("`{j}`")).collect::<Vec<_>>().join(", ")
351-
);
352-
}
349+
// Now print the job group index
350+
println!("\n**Job group index**\n");
351+
for (group, jobs) in job_index.into_iter().enumerate() {
352+
println!(
353+
"- {}: {}",
354+
format_job_group(group as u64),
355+
jobs.iter().map(|j| format!("`{j}`")).collect::<Vec<_>>().join(", ")
356+
);
357+
}
358+
},
359+
);
353360
}

src/ci/citool/src/main.rs

+18-5
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ use crate::cpu_usage::load_cpu_usage;
1919
use crate::datadog::upload_datadog_metric;
2020
use crate::jobs::RunType;
2121
use crate::metrics::{JobMetrics, download_auto_job_metrics, download_job_metrics, load_metrics};
22-
use crate::utils::load_env_var;
22+
use crate::utils::{load_env_var, output_details};
2323
use analysis::output_bootstrap_stats;
2424

2525
const CI_DIRECTORY: &str = concat!(env!("CARGO_MANIFEST_DIR"), "/..");
@@ -159,6 +159,22 @@ fn postprocess_metrics(
159159
Ok(())
160160
}
161161

162+
fn post_merge_report(db: JobDatabase, current: String, parent: String) -> anyhow::Result<()> {
163+
let metrics = download_auto_job_metrics(&db, &parent, &current)?;
164+
165+
output_details("What is this?", || {
166+
println!(
167+
r#"This is an experimental post-merge analysis report that shows differences in
168+
test outcomes between the merged PR and its parent PR."#
169+
);
170+
});
171+
172+
println!("\nComparing {parent} (parent) -> {current} (this PR)\n");
173+
output_test_diffs(metrics);
174+
175+
Ok(())
176+
}
177+
162178
#[derive(clap::Parser)]
163179
enum Args {
164180
/// Calculate a list of jobs that should be executed on CI.
@@ -243,10 +259,7 @@ fn main() -> anyhow::Result<()> {
243259
postprocess_metrics(metrics_path, parent, job_name)?;
244260
}
245261
Args::PostMergeReport { current, parent } => {
246-
let db = load_db(default_jobs_file)?;
247-
let metrics = download_auto_job_metrics(&db, &parent, &current)?;
248-
println!("Comparing {parent} (base) -> {current} (this PR)\n");
249-
output_test_diffs(metrics);
262+
post_merge_report(load_db(default_jobs_file)?, current, parent)?;
250263
}
251264
}
252265

src/ci/citool/src/utils.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,9 @@ where
2222
{
2323
println!(
2424
r"<details>
25-
<summary>{summary}</summary>"
25+
<summary>{summary}</summary>
26+
27+
"
2628
);
2729
func();
2830
println!("</details>\n");

0 commit comments

Comments
 (0)