mirror of
https://github.com/rust-lang/rust.git
synced 2025-04-28 02:57:37 +00:00
Rollup merge of #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-report3cb02729ab
fd17deacce
``` 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`
This commit is contained in:
commit
f1051ed6f1
36
.github/workflows/post-merge.yml
vendored
Normal file
36
.github/workflows/post-merge.yml
vendored
Normal file
@ -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
|
@ -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,
|
||||
|
@ -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(())
|
||||
|
257
src/ci/citool/src/merge_report.rs
Normal file
257
src/ci/citool/src/merge_report.rs
Normal file
@ -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") }
|
||||
}
|
@ -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 {
|
||||
|
Loading…
Reference in New Issue
Block a user