Skip to content

Commit

Permalink
Add post-merge analysis CI workflow
Browse files Browse the repository at this point in the history
  • Loading branch information
Kobzol committed Mar 7, 2025
1 parent 91a0e16 commit f04a2cc
Show file tree
Hide file tree
Showing 5 changed files with 319 additions and 10 deletions.
36 changes: 36 additions & 0 deletions .github/workflows/post-merge.yml
Original file line number Diff line number Diff line change
@@ -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 <[email protected]>' -n1 --first-parent HEAD^1`
# Find PRs for current and parent 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
2 changes: 1 addition & 1 deletion src/build_helper/src/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
12 changes: 12 additions & 0 deletions src/ci/citool/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
mod cpu_usage;
mod datadog;
mod merge_report;
mod metrics;
mod utils;

Expand All @@ -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;

Expand Down Expand Up @@ -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)]
Expand Down Expand Up @@ -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(())
Expand Down
257 changes: 257 additions & 0 deletions src/ci/citool/src/merge_report.rs
Original file line number Diff line number Diff line change
@@ -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, &current)?;
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 &current.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") }
}
22 changes: 13 additions & 9 deletions src/ci/citool/src/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down

0 comments on commit f04a2cc

Please sign in to comment.