From d5da6b71fa7f29946a41d624e038c1c0eed8a4d3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= <berykubik@gmail.com>
Date: Tue, 4 Mar 2025 18:26:11 +0100
Subject: [PATCH] Add post-merge analysis CI workflow
---
.github/workflows/post-merge.yml | 36 +++++
src/build_helper/src/metrics.rs | 2 +-
src/ci/citool/src/main.rs | 12 ++
src/ci/citool/src/merge_report.rs | 257 ++++++++++++++++++++++++++++++
src/ci/citool/src/metrics.rs | 22 +--
5 files changed, 319 insertions(+), 10 deletions(-)
create mode 100644 .github/workflows/post-merge.yml
create mode 100644 src/ci/citool/src/merge_report.rs
diff --git a/.github/workflows/post-merge.yml b/.github/workflows/post-merge.yml
new file mode 100644
index 00000000000..d3f42c5a905
--- /dev/null
+++ b/.github/workflows/post-merge.yml
@@ -0,0 +1,36 @@
+# Workflow that runs after a merge to master, analyses changes in test executions
+# and posts the result to the merged PR.
+
+name: Post merge analysis
+
+on:
+ push:
+ branches:
+ - master
+
+jobs:
+ analysis:
+ runs-on: ubuntu-24.04
+ if: github.repository == 'rust-lang/rust'
+ permissions:
+ pull-requests: write
+ steps:
+ - uses: actions/checkout@v4
+ - name: Perform analysis and send PR
+ run: |
+ # Get closest bors merge commit
+ PARENT_COMMIT=`git rev-list --author='bors <bors@rust-lang.org>' -n1 --first-parent HEAD^1`
+
+ # Find PR for the current commit
+ HEAD_PR=`gh pr list --search "${{ github.sha }}" --state merged --json number --jq '.[0].number'`
+
+ echo "Parent: ${PARENT_COMMIT}"
+ echo "HEAD: ${{ github.sha }} (#${HEAD_PR})"
+
+ cd src/ci/citool
+
+ echo "Post-merge analysis result" > output.log
+ cargo run --release post-merge-analysis ${PARENT_COMMIT} ${{ github.sha }} >> output.log
+ cat output.log
+
+ gh pr comment ${HEAD_PR} -F output.log
diff --git a/src/build_helper/src/metrics.rs b/src/build_helper/src/metrics.rs
index eb306550fc4..b6daac32a44 100644
--- a/src/build_helper/src/metrics.rs
+++ b/src/build_helper/src/metrics.rs
@@ -74,7 +74,7 @@ pub struct Test {
pub outcome: TestOutcome,
}
-#[derive(Serialize, Deserialize)]
+#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, Hash)]
#[serde(tag = "outcome", rename_all = "snake_case")]
pub enum TestOutcome {
Passed,
diff --git a/src/ci/citool/src/main.rs b/src/ci/citool/src/main.rs
index 16edd681886..52e7638d98b 100644
--- a/src/ci/citool/src/main.rs
+++ b/src/ci/citool/src/main.rs
@@ -1,5 +1,6 @@
mod cpu_usage;
mod datadog;
+mod merge_report;
mod metrics;
mod utils;
@@ -13,6 +14,7 @@ use serde_yaml::Value;
use crate::cpu_usage::load_cpu_usage;
use crate::datadog::upload_datadog_metric;
+use crate::merge_report::post_merge_report;
use crate::metrics::postprocess_metrics;
use crate::utils::load_env_var;
@@ -373,6 +375,13 @@ enum Args {
/// Path to a CSV containing the CI job CPU usage.
cpu_usage_csv: PathBuf,
},
+ /// Generate a report of test execution changes between two rustc commits.
+ PostMergeReport {
+ /// Parent commit to use as a base of the comparison.
+ parent: String,
+ /// Current commit that will be compared to `parent`.
+ current: String,
+ },
}
#[derive(clap::ValueEnum, Clone)]
@@ -410,6 +419,9 @@ fn main() -> anyhow::Result<()> {
Args::PostprocessMetrics { metrics_path, summary_path } => {
postprocess_metrics(&metrics_path, &summary_path)?;
}
+ Args::PostMergeReport { current: commit, parent } => {
+ post_merge_report(load_db(default_jobs_file)?, parent, commit)?;
+ }
}
Ok(())
diff --git a/src/ci/citool/src/merge_report.rs b/src/ci/citool/src/merge_report.rs
new file mode 100644
index 00000000000..5dd662280f0
--- /dev/null
+++ b/src/ci/citool/src/merge_report.rs
@@ -0,0 +1,257 @@
+use std::cmp::Reverse;
+use std::collections::HashMap;
+
+use anyhow::Context;
+use build_helper::metrics::{JsonRoot, TestOutcome};
+
+use crate::JobDatabase;
+use crate::metrics::get_test_suites;
+
+type Sha = String;
+type JobName = String;
+
+/// Computes a post merge CI analysis report between the `parent` and `current` commits.
+pub fn post_merge_report(job_db: JobDatabase, parent: Sha, current: Sha) -> anyhow::Result<()> {
+ let jobs = download_all_metrics(&job_db, &parent, ¤t)?;
+ let diffs = aggregate_test_diffs(&jobs)?;
+ report_test_changes(diffs);
+
+ Ok(())
+}
+
+struct JobMetrics {
+ parent: Option<JsonRoot>,
+ current: JsonRoot,
+}
+
+/// Download before/after metrics for all auto jobs in the job database.
+fn download_all_metrics(
+ job_db: &JobDatabase,
+ parent: &str,
+ current: &str,
+) -> anyhow::Result<HashMap<JobName, JobMetrics>> {
+ let mut jobs = HashMap::default();
+
+ for job in &job_db.auto_jobs {
+ eprintln!("Downloading metrics of job {}", job.name);
+ let metrics_parent = match download_job_metrics(&job.name, parent) {
+ Ok(metrics) => Some(metrics),
+ Err(error) => {
+ eprintln!(
+ r#"Did not find metrics for job `{}` at `{}`: {error:?}.
+Maybe it was newly added?"#,
+ job.name, parent
+ );
+ None
+ }
+ };
+ let metrics_current = download_job_metrics(&job.name, current)?;
+ jobs.insert(
+ job.name.clone(),
+ JobMetrics { parent: metrics_parent, current: metrics_current },
+ );
+ }
+ Ok(jobs)
+}
+
+fn download_job_metrics(job_name: &str, sha: &str) -> anyhow::Result<JsonRoot> {
+ let url = get_metrics_url(job_name, sha);
+ let mut response = ureq::get(&url).call()?;
+ if !response.status().is_success() {
+ return Err(anyhow::anyhow!(
+ "Cannot fetch metrics from {url}: {}\n{}",
+ response.status(),
+ response.body_mut().read_to_string()?
+ ));
+ }
+ let data: JsonRoot = response
+ .body_mut()
+ .read_json()
+ .with_context(|| anyhow::anyhow!("cannot deserialize metrics from {url}"))?;
+ Ok(data)
+}
+
+fn get_metrics_url(job_name: &str, sha: &str) -> String {
+ let suffix = if job_name.ends_with("-alt") { "-alt" } else { "" };
+ format!("https://ci-artifacts.rust-lang.org/rustc-builds{suffix}/{sha}/metrics-{job_name}.json")
+}
+
+fn aggregate_test_diffs(
+ jobs: &HashMap<JobName, JobMetrics>,
+) -> anyhow::Result<Vec<AggregatedTestDiffs>> {
+ let mut job_diffs = vec![];
+
+ // Aggregate test suites
+ for (name, metrics) in jobs {
+ if let Some(parent) = &metrics.parent {
+ let tests_parent = aggregate_tests(parent);
+ let tests_current = aggregate_tests(&metrics.current);
+ let test_diffs = calculate_test_diffs(tests_parent, tests_current);
+ if !test_diffs.is_empty() {
+ job_diffs.push((name.clone(), test_diffs));
+ }
+ }
+ }
+
+ // Aggregate jobs with the same diff, as often the same diff will appear in many jobs
+ let job_diffs: HashMap<Vec<(Test, TestOutcomeDiff)>, Vec<String>> =
+ job_diffs.into_iter().fold(HashMap::new(), |mut acc, (job, diffs)| {
+ acc.entry(diffs).or_default().push(job);
+ acc
+ });
+
+ Ok(job_diffs
+ .into_iter()
+ .map(|(test_diffs, jobs)| AggregatedTestDiffs { jobs, test_diffs })
+ .collect())
+}
+
+fn calculate_test_diffs(
+ reference: TestSuiteData,
+ current: TestSuiteData,
+) -> Vec<(Test, TestOutcomeDiff)> {
+ let mut diffs = vec![];
+ for (test, outcome) in ¤t.tests {
+ match reference.tests.get(test) {
+ Some(before) => {
+ if before != outcome {
+ diffs.push((
+ test.clone(),
+ TestOutcomeDiff::ChangeOutcome {
+ before: before.clone(),
+ after: outcome.clone(),
+ },
+ ));
+ }
+ }
+ None => diffs.push((test.clone(), TestOutcomeDiff::Added(outcome.clone()))),
+ }
+ }
+ for (test, outcome) in &reference.tests {
+ if !current.tests.contains_key(test) {
+ diffs.push((test.clone(), TestOutcomeDiff::Missing { before: outcome.clone() }));
+ }
+ }
+
+ diffs
+}
+
+/// Represents a difference in the outcome of tests between a base and a current commit.
+#[derive(Debug)]
+struct AggregatedTestDiffs {
+ /// All jobs that had the exact same test diffs.
+ jobs: Vec<String>,
+ test_diffs: Vec<(Test, TestOutcomeDiff)>,
+}
+
+#[derive(Eq, PartialEq, Hash, Debug)]
+enum TestOutcomeDiff {
+ ChangeOutcome { before: TestOutcome, after: TestOutcome },
+ Missing { before: TestOutcome },
+ Added(TestOutcome),
+}
+
+/// Aggregates test suite executions from all bootstrap invocations in a given CI job.
+#[derive(Default)]
+struct TestSuiteData {
+ tests: HashMap<Test, TestOutcome>,
+}
+
+#[derive(Hash, PartialEq, Eq, Debug, Clone)]
+struct Test {
+ name: String,
+}
+
+/// Extracts all tests from the passed metrics and map them to their outcomes.
+fn aggregate_tests(metrics: &JsonRoot) -> TestSuiteData {
+ let mut tests = HashMap::new();
+ let test_suites = get_test_suites(&metrics);
+ for suite in test_suites {
+ for test in &suite.tests {
+ let test_entry = Test { name: normalize_test_name(&test.name) };
+ tests.insert(test_entry, test.outcome.clone());
+ }
+ }
+ TestSuiteData { tests }
+}
+
+/// Normalizes Windows-style path delimiters to Unix-style paths.
+fn normalize_test_name(name: &str) -> String {
+ name.replace('\\', "/")
+}
+
+/// Prints test changes in Markdown format to stdout.
+fn report_test_changes(mut diffs: Vec<AggregatedTestDiffs>) {
+ println!("## Test differences");
+ if diffs.is_empty() {
+ println!("No test diffs found");
+ return;
+ }
+
+ // Sort diffs in decreasing order by diff count
+ diffs.sort_by_key(|entry| Reverse(entry.test_diffs.len()));
+
+ fn format_outcome(outcome: &TestOutcome) -> String {
+ match outcome {
+ TestOutcome::Passed => "pass".to_string(),
+ TestOutcome::Failed => "fail".to_string(),
+ TestOutcome::Ignored { ignore_reason } => {
+ let reason = match ignore_reason {
+ Some(reason) => format!(" ({reason})"),
+ None => String::new(),
+ };
+ format!("ignore{reason}")
+ }
+ }
+ }
+
+ fn format_diff(diff: &TestOutcomeDiff) -> String {
+ match diff {
+ TestOutcomeDiff::ChangeOutcome { before, after } => {
+ format!("{} -> {}", format_outcome(before), format_outcome(after))
+ }
+ TestOutcomeDiff::Missing { before } => {
+ format!("{} -> [missing]", format_outcome(before))
+ }
+ TestOutcomeDiff::Added(outcome) => {
+ format!("[missing] -> {}", format_outcome(outcome))
+ }
+ }
+ }
+
+ let max_diff_count = 10;
+ let max_job_count = 5;
+ let max_test_count = 10;
+
+ for diff in diffs.iter().take(max_diff_count) {
+ let mut jobs = diff.jobs.clone();
+ jobs.sort();
+
+ let jobs = jobs.iter().take(max_job_count).map(|j| format!("`{j}`")).collect::<Vec<_>>();
+
+ let extra_jobs = diff.jobs.len().saturating_sub(max_job_count);
+ let suffix = if extra_jobs > 0 {
+ format!(" (and {extra_jobs} {})", pluralize("other", extra_jobs))
+ } else {
+ String::new()
+ };
+ println!("- {}{suffix}", jobs.join(","));
+
+ let extra_tests = diff.test_diffs.len().saturating_sub(max_test_count);
+ for (test, outcome_diff) in diff.test_diffs.iter().take(max_test_count) {
+ println!(" - {}: {}", test.name, format_diff(&outcome_diff));
+ }
+ if extra_tests > 0 {
+ println!(" - (and {extra_tests} additional {})", pluralize("tests", extra_tests));
+ }
+ }
+
+ let extra_diffs = diffs.len().saturating_sub(max_diff_count);
+ if extra_diffs > 0 {
+ println!("\n(and {extra_diffs} additional {})", pluralize("diff", extra_diffs));
+ }
+}
+
+fn pluralize(text: &str, count: usize) -> String {
+ if count == 1 { text.to_string() } else { format!("{text}s") }
+}
diff --git a/src/ci/citool/src/metrics.rs b/src/ci/citool/src/metrics.rs
index 2386413ed6b..8548602b31c 100644
--- a/src/ci/citool/src/metrics.rs
+++ b/src/ci/citool/src/metrics.rs
@@ -105,17 +105,21 @@ struct TestSuiteRecord {
failed: u64,
}
+fn test_metadata_name(metadata: &TestSuiteMetadata) -> String {
+ match metadata {
+ TestSuiteMetadata::CargoPackage { crates, stage, .. } => {
+ format!("{} (stage {stage})", crates.join(", "))
+ }
+ TestSuiteMetadata::Compiletest { suite, stage, .. } => {
+ format!("{suite} (stage {stage})")
+ }
+ }
+}
+
fn aggregate_test_suites(suites: &[&TestSuite]) -> BTreeMap<String, TestSuiteRecord> {
let mut records: BTreeMap<String, TestSuiteRecord> = BTreeMap::new();
for suite in suites {
- let name = match &suite.metadata {
- TestSuiteMetadata::CargoPackage { crates, stage, .. } => {
- format!("{} (stage {stage})", crates.join(", "))
- }
- TestSuiteMetadata::Compiletest { suite, stage, .. } => {
- format!("{suite} (stage {stage})")
- }
- };
+ let name = test_metadata_name(&suite.metadata);
let record = records.entry(name).or_default();
for test in &suite.tests {
match test.outcome {
@@ -134,7 +138,7 @@ fn aggregate_test_suites(suites: &[&TestSuite]) -> BTreeMap<String, TestSuiteRec
records
}
-fn get_test_suites(metrics: &JsonRoot) -> Vec<&TestSuite> {
+pub fn get_test_suites(metrics: &JsonRoot) -> Vec<&TestSuite> {
fn visit_test_suites<'a>(nodes: &'a [JsonNode], suites: &mut Vec<&'a TestSuite>) {
for node in nodes {
match node {