Skip to content
This repository was archived by the owner on May 28, 2025. It is now read-only.

Commit f981a0a

Browse files
committed
Do not print doctest diffs in the report
When they are moved around in code, their name changes, which produces too noisy diffs.
1 parent d5d633d commit f981a0a

File tree

1 file changed

+44
-12
lines changed

1 file changed

+44
-12
lines changed

src/ci/citool/src/merge_report.rs

Lines changed: 44 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use std::collections::{HashMap, HashSet};
22
use std::path::PathBuf;
33

44
use anyhow::Context;
5-
use build_helper::metrics::{JsonRoot, TestOutcome};
5+
use build_helper::metrics::{JsonRoot, TestOutcome, TestSuiteMetadata};
66

77
use crate::jobs::JobDatabase;
88
use crate::metrics::get_test_suites;
@@ -177,6 +177,7 @@ struct TestSuiteData {
177177
#[derive(Hash, PartialEq, Eq, Debug, Clone)]
178178
struct Test {
179179
name: String,
180+
is_doctest: bool,
180181
}
181182

182183
/// Extracts all tests from the passed metrics and map them to their outcomes.
@@ -185,7 +186,10 @@ fn aggregate_tests(metrics: &JsonRoot) -> TestSuiteData {
185186
let test_suites = get_test_suites(&metrics);
186187
for suite in test_suites {
187188
for test in &suite.tests {
188-
let test_entry = Test { name: normalize_test_name(&test.name) };
189+
// Poor man's detection of doctests based on the "(line XYZ)" suffix
190+
let is_doctest = matches!(suite.metadata, TestSuiteMetadata::CargoPackage { .. })
191+
&& test.name.contains("(line");
192+
let test_entry = Test { name: normalize_test_name(&test.name), is_doctest };
189193
tests.insert(test_entry, test.outcome.clone());
190194
}
191195
}
@@ -198,7 +202,7 @@ fn normalize_test_name(name: &str) -> String {
198202
}
199203

200204
/// Prints test changes in Markdown format to stdout.
201-
fn report_test_diffs(mut diff: AggregatedTestDiffs) {
205+
fn report_test_diffs(diff: AggregatedTestDiffs) {
202206
println!("## Test differences");
203207
if diff.diffs.is_empty() {
204208
println!("No test diffs found");
@@ -233,6 +237,10 @@ fn report_test_diffs(mut diff: AggregatedTestDiffs) {
233237
}
234238
}
235239

240+
fn format_job_group(group: u64) -> String {
241+
format!("**J{group}**")
242+
}
243+
236244
// It would be quite noisy to repeat the jobs that contained the test changes after/next to
237245
// every test diff. At the same time, grouping the test diffs by
238246
// [unique set of jobs that contained them] also doesn't work well, because the test diffs
@@ -243,12 +251,20 @@ fn report_test_diffs(mut diff: AggregatedTestDiffs) {
243251
let mut job_list_to_group: HashMap<&[JobName], u64> = HashMap::new();
244252
let mut job_index: Vec<&[JobName]> = vec![];
245253

246-
for (_, jobs) in diff.diffs.iter_mut() {
247-
jobs.sort();
248-
}
254+
let original_diff_count = diff.diffs.len();
255+
let diffs = diff
256+
.diffs
257+
.into_iter()
258+
.filter(|(diff, _)| !diff.test.is_doctest)
259+
.map(|(diff, mut jobs)| {
260+
jobs.sort();
261+
(diff, jobs)
262+
})
263+
.collect::<Vec<_>>();
264+
let doctest_count = original_diff_count.saturating_sub(diffs.len());
249265

250266
let max_diff_count = 100;
251-
for (diff, jobs) in diff.diffs.iter().take(max_diff_count) {
267+
for (diff, jobs) in diffs.iter().take(max_diff_count) {
252268
let jobs = &*jobs;
253269
let job_group = match job_list_to_group.get(jobs.as_slice()) {
254270
Some(id) => *id,
@@ -266,18 +282,34 @@ fn report_test_diffs(mut diff: AggregatedTestDiffs) {
266282
grouped_diffs.sort_by(|(d1, g1), (d2, g2)| g1.cmp(&g2).then(d1.test.name.cmp(&d2.test.name)));
267283

268284
for (diff, job_group) in grouped_diffs {
269-
println!("- `{}`: {} (*J{job_group}*)", diff.test.name, format_diff(&diff.diff));
285+
println!(
286+
"- `{}`: {} ({})",
287+
diff.test.name,
288+
format_diff(&diff.diff),
289+
format_job_group(job_group)
290+
);
270291
}
271292

272-
let extra_diffs = diff.diffs.len().saturating_sub(max_diff_count);
293+
let extra_diffs = diffs.len().saturating_sub(max_diff_count);
273294
if extra_diffs > 0 {
274295
println!("\n(and {extra_diffs} additional {})", pluralize("test diff", extra_diffs));
275296
}
276297

277-
// Now print the job index
278-
println!("\n**Job index**\n");
298+
if doctest_count > 0 {
299+
println!(
300+
"\nAdditionally, {doctest_count} doctest {} were found.",
301+
pluralize("diff", doctest_count)
302+
);
303+
}
304+
305+
// Now print the job group index
306+
println!("\n**Job group index**\n");
279307
for (group, jobs) in job_index.into_iter().enumerate() {
280-
println!("- J{group}: `{}`", jobs.join(", "));
308+
println!(
309+
"- {}: {}",
310+
format_job_group(group as u64),
311+
jobs.iter().map(|j| format!("`{j}`")).collect::<Vec<_>>().join(", ")
312+
);
281313
}
282314
}
283315

0 commit comments

Comments
 (0)