From 418ec8625ff146678ef1b734a90be23157480826 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 2 Mar 2023 23:21:29 +0100 Subject: [PATCH] Do not set GitHub label for try runs on rolled up PRs --- site/src/github.rs | 6 +++- site/src/github/comparison_summary.rs | 52 ++++++++++++++++++++------- 2 files changed, 45 insertions(+), 13 deletions(-) diff --git a/site/src/github.rs b/site/src/github.rs index 1b4f5f422..ef95f66a9 100644 --- a/site/src/github.rs +++ b/site/src/github.rs @@ -16,6 +16,9 @@ pub const RUST_REPO_GITHUB_API_URL: &str = "https://api.github.com/repos/rust-la /// They are removed once a perf. run comparison summary is posted on a PR. pub const COMMENT_MARK_TEMPORARY: &str = ""; +/// Used for comment that contains unrolled commits for merged rolled-up PRs. +pub const COMMENT_MARK_ROLLUP: &str = ""; + pub use comparison_summary::post_finished; /// Enqueues try build artifacts and posts a message about them on the original rollup PR @@ -51,7 +54,8 @@ pub async fn unroll_rollup( format!("📌 Perf builds for each rolled up PR:\n\n\ |PR# | Perf Build Sha|\n|----|:-----:|\n\ {mapping}\n\n*previous master*: {previous_master}\n\nIn the case of a perf regression, \ - run the following command for each PR you suspect might be the cause: `@rust-timer build $SHA`"); + run the following command for each PR you suspect might be the cause: `@rust-timer build $SHA`\n\ + {COMMENT_MARK_ROLLUP}"); main_repo_client.post_comment(rollup_pr_number, msg).await; Ok(()) } diff --git a/site/src/github/comparison_summary.rs b/site/src/github/comparison_summary.rs index c411b34c3..9eacc6694 100644 --- a/site/src/github/comparison_summary.rs +++ b/site/src/github/comparison_summary.rs @@ -6,7 +6,7 @@ use crate::load::SiteCtxt; use database::{ArtifactId, QueuedCommit}; -use crate::github::{COMMENT_MARK_TEMPORARY, RUST_REPO_GITHUB_API_URL}; +use crate::github::{COMMENT_MARK_ROLLUP, COMMENT_MARK_TEMPORARY, RUST_REPO_GITHUB_API_URL}; use std::collections::HashSet; use std::fmt::Write; @@ -76,13 +76,11 @@ async fn post_comparison_comment( ) -> anyhow::Result<()> { let client = super::client::Client::from_ctxt(ctxt, RUST_REPO_GITHUB_API_URL.to_owned()); let pr = commit.pr; - let body = match summarize_run(ctxt, commit, is_master_commit).await { - Ok(message) => message, - Err(error) => error, - }; - client.post_comment(pr, body).await; + // Was this perf. run triggered from a PR that was already merged and is a rollup? + let mut is_rollup = false; + // Scan comments to hide outdated ones and gather context let graph_client = super::client::GraphQLClient::from_ctxt(ctxt); for comment in graph_client.get_comments(pr).await? { // If this bot is the author of the comment, the comment is not yet minimized and it is @@ -94,7 +92,27 @@ async fn post_comparison_comment( log::debug!("Hiding comment {}", comment.id); graph_client.hide_comment(&comment.id, "OUTDATED").await?; } + + if comment.viewer_did_author && comment.body.contains(COMMENT_MARK_ROLLUP) { + is_rollup = true; + } } + + let source = if is_master_commit { + PerfRunSource::MasterCommit + } else if is_rollup { + PerfRunSource::TryBuildRollup + } else { + PerfRunSource::TryBuild + }; + + let body = match summarize_run(ctxt, commit, source).await { + Ok(message) => message, + Err(error) => error, + }; + + client.post_comment(pr, body).await; + Ok(()) } @@ -125,10 +143,20 @@ async fn calculate_metric_comparison( } } +/// What caused this perf. run to be executed? +enum PerfRunSource { + // PR merged to master + MasterCommit, + // Manual try build on a PR + TryBuild, + // Manual try build on a merged rollup PR + TryBuildRollup, +} + async fn summarize_run( ctxt: &SiteCtxt, commit: QueuedCommit, - is_master_commit: bool, + source: PerfRunSource, ) -> Result { let benchmark_map = ctxt.get_benchmark_category_map().await; @@ -179,16 +207,16 @@ async fn summarize_run( ) .unwrap(); - let next_steps = if is_master_commit { - master_run_body(is_regression) - } else { - try_run_body(is_regression) + let next_steps = match source { + PerfRunSource::TryBuild => try_run_body(is_regression), + PerfRunSource::TryBuildRollup => "".to_string(), + PerfRunSource::MasterCommit => master_run_body(is_regression), }; writeln!(&mut message, "{next_steps}\n").unwrap(); if !errors.is_empty() { writeln!(&mut message, "\n{errors}").unwrap(); - if is_master_commit { + if matches!(source, PerfRunSource::MasterCommit) { writeln!(&mut message, "\ncc @rust-lang/wg-compiler-performance").unwrap(); } }