Skip to content

Commit 6ba58b0

Browse files
authored
Unrolled build for rust-lang#138013
Rollup merge of rust-lang#138013 - Kobzol:ci-post-merge-analysis, r=marcoieni Add post-merge analysis CI workflow This PR adds a post-merge analysis workflow, which was discussed [here](https://rust-lang.zulipchat.com/#narrow/channel/242791-t-infra/topic/Reporting.20test.20suite.20statistics.20after.20merge). The workflow currently analyzes test suite results from bootstrap metrics. It downloads metrics for all known jobs in the parent and current (HEAD) commit, compares them and prints a truncated diff. It then posts this diff to the merged PR as a comment. Later I also want to add other statistics to the analysis, e.g. changes in CI/bootstrap step durations. It can be tested locally e.g. using this: ``` cargo run --release --manifest-path src/ci/citool/Cargo.toml post-merge-report 3cb0272 fd17dea ``` This uses a slightly older commit as a parent, to have more results in the diff (normally the diff won't be so large). CC `@jieyouxu` r? `@marcoieni`
2 parents f5a1ef7 + d5da6b7 commit 6ba58b0

File tree

5 files changed

+319
-10
lines changed

5 files changed

+319
-10
lines changed

.github/workflows/post-merge.yml

+36
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
# Workflow that runs after a merge to master, analyses changes in test executions
2+
# and posts the result to the merged PR.
3+
4+
name: Post merge analysis
5+
6+
on:
7+
push:
8+
branches:
9+
- master
10+
11+
jobs:
12+
analysis:
13+
runs-on: ubuntu-24.04
14+
if: github.repository == 'rust-lang/rust'
15+
permissions:
16+
pull-requests: write
17+
steps:
18+
- uses: actions/checkout@v4
19+
- name: Perform analysis and send PR
20+
run: |
21+
# Get closest bors merge commit
22+
PARENT_COMMIT=`git rev-list --author='bors <[email protected]>' -n1 --first-parent HEAD^1`
23+
24+
# Find PR for the current commit
25+
HEAD_PR=`gh pr list --search "${{ github.sha }}" --state merged --json number --jq '.[0].number'`
26+
27+
echo "Parent: ${PARENT_COMMIT}"
28+
echo "HEAD: ${{ github.sha }} (#${HEAD_PR})"
29+
30+
cd src/ci/citool
31+
32+
echo "Post-merge analysis result" > output.log
33+
cargo run --release post-merge-analysis ${PARENT_COMMIT} ${{ github.sha }} >> output.log
34+
cat output.log
35+
36+
gh pr comment ${HEAD_PR} -F output.log

src/build_helper/src/metrics.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ pub struct Test {
7474
pub outcome: TestOutcome,
7575
}
7676

77-
#[derive(Serialize, Deserialize)]
77+
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, Hash)]
7878
#[serde(tag = "outcome", rename_all = "snake_case")]
7979
pub enum TestOutcome {
8080
Passed,

src/ci/citool/src/main.rs

+12
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
mod cpu_usage;
22
mod datadog;
3+
mod merge_report;
34
mod metrics;
45
mod utils;
56

@@ -13,6 +14,7 @@ use serde_yaml::Value;
1314

1415
use crate::cpu_usage::load_cpu_usage;
1516
use crate::datadog::upload_datadog_metric;
17+
use crate::merge_report::post_merge_report;
1618
use crate::metrics::postprocess_metrics;
1719
use crate::utils::load_env_var;
1820

@@ -373,6 +375,13 @@ enum Args {
373375
/// Path to a CSV containing the CI job CPU usage.
374376
cpu_usage_csv: PathBuf,
375377
},
378+
/// Generate a report of test execution changes between two rustc commits.
379+
PostMergeReport {
380+
/// Parent commit to use as a base of the comparison.
381+
parent: String,
382+
/// Current commit that will be compared to `parent`.
383+
current: String,
384+
},
376385
}
377386

378387
#[derive(clap::ValueEnum, Clone)]
@@ -410,6 +419,9 @@ fn main() -> anyhow::Result<()> {
410419
Args::PostprocessMetrics { metrics_path, summary_path } => {
411420
postprocess_metrics(&metrics_path, &summary_path)?;
412421
}
422+
Args::PostMergeReport { current: commit, parent } => {
423+
post_merge_report(load_db(default_jobs_file)?, parent, commit)?;
424+
}
413425
}
414426

415427
Ok(())

src/ci/citool/src/merge_report.rs

+257
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,257 @@
1+
use std::cmp::Reverse;
2+
use std::collections::HashMap;
3+
4+
use anyhow::Context;
5+
use build_helper::metrics::{JsonRoot, TestOutcome};
6+
7+
use crate::JobDatabase;
8+
use crate::metrics::get_test_suites;
9+
10+
type Sha = String;
11+
type JobName = String;
12+
13+
/// Computes a post merge CI analysis report between the `parent` and `current` commits.
14+
pub fn post_merge_report(job_db: JobDatabase, parent: Sha, current: Sha) -> anyhow::Result<()> {
15+
let jobs = download_all_metrics(&job_db, &parent, &current)?;
16+
let diffs = aggregate_test_diffs(&jobs)?;
17+
report_test_changes(diffs);
18+
19+
Ok(())
20+
}
21+
22+
struct JobMetrics {
23+
parent: Option<JsonRoot>,
24+
current: JsonRoot,
25+
}
26+
27+
/// Download before/after metrics for all auto jobs in the job database.
28+
fn download_all_metrics(
29+
job_db: &JobDatabase,
30+
parent: &str,
31+
current: &str,
32+
) -> anyhow::Result<HashMap<JobName, JobMetrics>> {
33+
let mut jobs = HashMap::default();
34+
35+
for job in &job_db.auto_jobs {
36+
eprintln!("Downloading metrics of job {}", job.name);
37+
let metrics_parent = match download_job_metrics(&job.name, parent) {
38+
Ok(metrics) => Some(metrics),
39+
Err(error) => {
40+
eprintln!(
41+
r#"Did not find metrics for job `{}` at `{}`: {error:?}.
42+
Maybe it was newly added?"#,
43+
job.name, parent
44+
);
45+
None
46+
}
47+
};
48+
let metrics_current = download_job_metrics(&job.name, current)?;
49+
jobs.insert(
50+
job.name.clone(),
51+
JobMetrics { parent: metrics_parent, current: metrics_current },
52+
);
53+
}
54+
Ok(jobs)
55+
}
56+
57+
fn download_job_metrics(job_name: &str, sha: &str) -> anyhow::Result<JsonRoot> {
58+
let url = get_metrics_url(job_name, sha);
59+
let mut response = ureq::get(&url).call()?;
60+
if !response.status().is_success() {
61+
return Err(anyhow::anyhow!(
62+
"Cannot fetch metrics from {url}: {}\n{}",
63+
response.status(),
64+
response.body_mut().read_to_string()?
65+
));
66+
}
67+
let data: JsonRoot = response
68+
.body_mut()
69+
.read_json()
70+
.with_context(|| anyhow::anyhow!("cannot deserialize metrics from {url}"))?;
71+
Ok(data)
72+
}
73+
74+
fn get_metrics_url(job_name: &str, sha: &str) -> String {
75+
let suffix = if job_name.ends_with("-alt") { "-alt" } else { "" };
76+
format!("https://ci-artifacts.rust-lang.org/rustc-builds{suffix}/{sha}/metrics-{job_name}.json")
77+
}
78+
79+
fn aggregate_test_diffs(
80+
jobs: &HashMap<JobName, JobMetrics>,
81+
) -> anyhow::Result<Vec<AggregatedTestDiffs>> {
82+
let mut job_diffs = vec![];
83+
84+
// Aggregate test suites
85+
for (name, metrics) in jobs {
86+
if let Some(parent) = &metrics.parent {
87+
let tests_parent = aggregate_tests(parent);
88+
let tests_current = aggregate_tests(&metrics.current);
89+
let test_diffs = calculate_test_diffs(tests_parent, tests_current);
90+
if !test_diffs.is_empty() {
91+
job_diffs.push((name.clone(), test_diffs));
92+
}
93+
}
94+
}
95+
96+
// Aggregate jobs with the same diff, as often the same diff will appear in many jobs
97+
let job_diffs: HashMap<Vec<(Test, TestOutcomeDiff)>, Vec<String>> =
98+
job_diffs.into_iter().fold(HashMap::new(), |mut acc, (job, diffs)| {
99+
acc.entry(diffs).or_default().push(job);
100+
acc
101+
});
102+
103+
Ok(job_diffs
104+
.into_iter()
105+
.map(|(test_diffs, jobs)| AggregatedTestDiffs { jobs, test_diffs })
106+
.collect())
107+
}
108+
109+
fn calculate_test_diffs(
110+
reference: TestSuiteData,
111+
current: TestSuiteData,
112+
) -> Vec<(Test, TestOutcomeDiff)> {
113+
let mut diffs = vec![];
114+
for (test, outcome) in &current.tests {
115+
match reference.tests.get(test) {
116+
Some(before) => {
117+
if before != outcome {
118+
diffs.push((
119+
test.clone(),
120+
TestOutcomeDiff::ChangeOutcome {
121+
before: before.clone(),
122+
after: outcome.clone(),
123+
},
124+
));
125+
}
126+
}
127+
None => diffs.push((test.clone(), TestOutcomeDiff::Added(outcome.clone()))),
128+
}
129+
}
130+
for (test, outcome) in &reference.tests {
131+
if !current.tests.contains_key(test) {
132+
diffs.push((test.clone(), TestOutcomeDiff::Missing { before: outcome.clone() }));
133+
}
134+
}
135+
136+
diffs
137+
}
138+
139+
/// Represents a difference in the outcome of tests between a base and a current commit.
140+
#[derive(Debug)]
141+
struct AggregatedTestDiffs {
142+
/// All jobs that had the exact same test diffs.
143+
jobs: Vec<String>,
144+
test_diffs: Vec<(Test, TestOutcomeDiff)>,
145+
}
146+
147+
#[derive(Eq, PartialEq, Hash, Debug)]
148+
enum TestOutcomeDiff {
149+
ChangeOutcome { before: TestOutcome, after: TestOutcome },
150+
Missing { before: TestOutcome },
151+
Added(TestOutcome),
152+
}
153+
154+
/// Aggregates test suite executions from all bootstrap invocations in a given CI job.
155+
#[derive(Default)]
156+
struct TestSuiteData {
157+
tests: HashMap<Test, TestOutcome>,
158+
}
159+
160+
#[derive(Hash, PartialEq, Eq, Debug, Clone)]
161+
struct Test {
162+
name: String,
163+
}
164+
165+
/// Extracts all tests from the passed metrics and map them to their outcomes.
166+
fn aggregate_tests(metrics: &JsonRoot) -> TestSuiteData {
167+
let mut tests = HashMap::new();
168+
let test_suites = get_test_suites(&metrics);
169+
for suite in test_suites {
170+
for test in &suite.tests {
171+
let test_entry = Test { name: normalize_test_name(&test.name) };
172+
tests.insert(test_entry, test.outcome.clone());
173+
}
174+
}
175+
TestSuiteData { tests }
176+
}
177+
178+
/// Normalizes Windows-style path delimiters to Unix-style paths.
179+
fn normalize_test_name(name: &str) -> String {
180+
name.replace('\\', "/")
181+
}
182+
183+
/// Prints test changes in Markdown format to stdout.
184+
fn report_test_changes(mut diffs: Vec<AggregatedTestDiffs>) {
185+
println!("## Test differences");
186+
if diffs.is_empty() {
187+
println!("No test diffs found");
188+
return;
189+
}
190+
191+
// Sort diffs in decreasing order by diff count
192+
diffs.sort_by_key(|entry| Reverse(entry.test_diffs.len()));
193+
194+
fn format_outcome(outcome: &TestOutcome) -> String {
195+
match outcome {
196+
TestOutcome::Passed => "pass".to_string(),
197+
TestOutcome::Failed => "fail".to_string(),
198+
TestOutcome::Ignored { ignore_reason } => {
199+
let reason = match ignore_reason {
200+
Some(reason) => format!(" ({reason})"),
201+
None => String::new(),
202+
};
203+
format!("ignore{reason}")
204+
}
205+
}
206+
}
207+
208+
fn format_diff(diff: &TestOutcomeDiff) -> String {
209+
match diff {
210+
TestOutcomeDiff::ChangeOutcome { before, after } => {
211+
format!("{} -> {}", format_outcome(before), format_outcome(after))
212+
}
213+
TestOutcomeDiff::Missing { before } => {
214+
format!("{} -> [missing]", format_outcome(before))
215+
}
216+
TestOutcomeDiff::Added(outcome) => {
217+
format!("[missing] -> {}", format_outcome(outcome))
218+
}
219+
}
220+
}
221+
222+
let max_diff_count = 10;
223+
let max_job_count = 5;
224+
let max_test_count = 10;
225+
226+
for diff in diffs.iter().take(max_diff_count) {
227+
let mut jobs = diff.jobs.clone();
228+
jobs.sort();
229+
230+
let jobs = jobs.iter().take(max_job_count).map(|j| format!("`{j}`")).collect::<Vec<_>>();
231+
232+
let extra_jobs = diff.jobs.len().saturating_sub(max_job_count);
233+
let suffix = if extra_jobs > 0 {
234+
format!(" (and {extra_jobs} {})", pluralize("other", extra_jobs))
235+
} else {
236+
String::new()
237+
};
238+
println!("- {}{suffix}", jobs.join(","));
239+
240+
let extra_tests = diff.test_diffs.len().saturating_sub(max_test_count);
241+
for (test, outcome_diff) in diff.test_diffs.iter().take(max_test_count) {
242+
println!(" - {}: {}", test.name, format_diff(&outcome_diff));
243+
}
244+
if extra_tests > 0 {
245+
println!(" - (and {extra_tests} additional {})", pluralize("tests", extra_tests));
246+
}
247+
}
248+
249+
let extra_diffs = diffs.len().saturating_sub(max_diff_count);
250+
if extra_diffs > 0 {
251+
println!("\n(and {extra_diffs} additional {})", pluralize("diff", extra_diffs));
252+
}
253+
}
254+
255+
fn pluralize(text: &str, count: usize) -> String {
256+
if count == 1 { text.to_string() } else { format!("{text}s") }
257+
}

src/ci/citool/src/metrics.rs

+13-9
Original file line numberDiff line numberDiff line change
@@ -105,17 +105,21 @@ struct TestSuiteRecord {
105105
failed: u64,
106106
}
107107

108+
fn test_metadata_name(metadata: &TestSuiteMetadata) -> String {
109+
match metadata {
110+
TestSuiteMetadata::CargoPackage { crates, stage, .. } => {
111+
format!("{} (stage {stage})", crates.join(", "))
112+
}
113+
TestSuiteMetadata::Compiletest { suite, stage, .. } => {
114+
format!("{suite} (stage {stage})")
115+
}
116+
}
117+
}
118+
108119
fn aggregate_test_suites(suites: &[&TestSuite]) -> BTreeMap<String, TestSuiteRecord> {
109120
let mut records: BTreeMap<String, TestSuiteRecord> = BTreeMap::new();
110121
for suite in suites {
111-
let name = match &suite.metadata {
112-
TestSuiteMetadata::CargoPackage { crates, stage, .. } => {
113-
format!("{} (stage {stage})", crates.join(", "))
114-
}
115-
TestSuiteMetadata::Compiletest { suite, stage, .. } => {
116-
format!("{suite} (stage {stage})")
117-
}
118-
};
122+
let name = test_metadata_name(&suite.metadata);
119123
let record = records.entry(name).or_default();
120124
for test in &suite.tests {
121125
match test.outcome {
@@ -134,7 +138,7 @@ fn aggregate_test_suites(suites: &[&TestSuite]) -> BTreeMap<String, TestSuiteRec
134138
records
135139
}
136140

137-
fn get_test_suites(metrics: &JsonRoot) -> Vec<&TestSuite> {
141+
pub fn get_test_suites(metrics: &JsonRoot) -> Vec<&TestSuite> {
138142
fn visit_test_suites<'a>(nodes: &'a [JsonNode], suites: &mut Vec<&'a TestSuite>) {
139143
for node in nodes {
140144
match node {

0 commit comments

Comments
 (0)