From f13381787c078ff3237b53968f486e6434e46529 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 17 Jul 2025 17:50:16 +0200 Subject: [PATCH] Add more logging around check suites --- src/bors/handlers/workflow.rs | 1 + src/bors/mod.rs | 5 ++-- src/github/api/client.rs | 49 ++++++++++++++++++++--------------- src/tests/mocks/repository.rs | 6 ++++- 4 files changed, 37 insertions(+), 24 deletions(-) diff --git a/src/bors/handlers/workflow.rs b/src/bors/handlers/workflow.rs index f3679894..d8ad4d6d 100644 --- a/src/bors/handlers/workflow.rs +++ b/src/bors/handlers/workflow.rs @@ -170,6 +170,7 @@ async fn try_complete_build( .iter() .any(|check| matches!(check.status, CheckSuiteStatus::Pending)) { + tracing::debug!("Some check suites are still pending: {checks:?}"); return Ok(()); } diff --git a/src/bors/mod.rs b/src/bors/mod.rs index 39c3413c..c612e841 100644 --- a/src/bors/mod.rs +++ b/src/bors/mod.rs @@ -2,12 +2,12 @@ use std::fmt; use std::str::FromStr; use arc_swap::ArcSwap; - pub use command::CommandParser; pub use command::RollupMode; pub use comment::Comment; pub use context::BorsContext; pub use handlers::{handle_bors_global_event, handle_bors_repository_event}; +use octocrab::models::CheckSuiteId; use serde::Serialize; use crate::config::RepositoryConfig; @@ -48,7 +48,8 @@ pub enum CheckSuiteStatus { /// Corresponds to a single GitHub actions workflow run, or to a single external CI check run. #[derive(Clone, Debug)] pub struct CheckSuite { - pub(crate) status: CheckSuiteStatus, + pub id: CheckSuiteId, + pub status: CheckSuiteStatus, } /// An access point to a single repository. diff --git a/src/github/api/client.rs b/src/github/api/client.rs index 89be0165..11fa3461 100644 --- a/src/github/api/client.rs +++ b/src/github/api/client.rs @@ -1,8 +1,9 @@ use anyhow::Context; use octocrab::Octocrab; use octocrab::models::checks::CheckRun; -use octocrab::models::{App, Repository}; +use octocrab::models::{App, CheckSuiteId, Repository}; use octocrab::params::checks::{CheckRunConclusion, CheckRunOutput, CheckRunStatus}; +use std::fmt::Debug; use tracing::log; use crate::bors::event::PullRequestComment; @@ -16,6 +17,7 @@ use crate::github::api::operations::{ use crate::github::{CommitSha, GithubRepoName, PullRequest, PullRequestNumber}; use crate::utils::timing::{measure_network_request, perform_network_request_with_retry}; use futures::TryStreamExt; +use serde::de::DeserializeOwned; /// Provides access to a single app installation (repository) using the GitHub API. pub struct GithubRepositoryClient { @@ -96,11 +98,7 @@ impl GithubRepositoryClient { perform_network_request_with_retry("get_branch_sha", || async { // https://docs.github.com/en/rest/branches/branches?apiVersion=2022-11-28#get-a-branch let branch: octocrab::models::repos::Branch = self - .client - .get( - format!("/repos/{}/branches/{name}", self.repository()).as_str(), - None::<&()>, - ) + .get_request(&format!("branches/{name}")) .await .context("Cannot deserialize branch")?; Ok(CommitSha(branch.commit.sha)) @@ -210,6 +208,7 @@ impl GithubRepositoryClient { ) -> anyhow::Result> { #[derive(serde::Deserialize, Debug)] struct CheckSuitePayload { + id: CheckSuiteId, conclusion: Option, head_branch: String, } @@ -221,17 +220,7 @@ impl GithubRepositoryClient { perform_network_request_with_retry("get_check_suites_for_commit", || async { let response: CheckSuiteResponse = self - .client - .get( - format!( - "/repos/{}/{}/commits/{}/check-suites", - self.repo_name.owner(), - self.repo_name.name(), - sha.0 - ) - .as_str(), - None::<&()>, - ) + .get_request(&format!("commits/{}/check-suites", sha.0)) .await .context("Cannot fetch CheckSuiteResponse")?; @@ -240,6 +229,7 @@ impl GithubRepositoryClient { .into_iter() .filter(|suite| suite.head_branch == branch) .map(|suite| CheckSuite { + id: suite.id, status: match suite.conclusion { Some(status) => match status.as_str() { "success" => CheckSuiteStatus::Success, @@ -249,14 +239,19 @@ impl GithubRepositoryClient { } _ => { tracing::warn!( - "Received unknown check suite status for {}/{}: {status}", - self.repo_name, - sha + "Received unknown check suite conclusion for {}@{sha}: {status}", + self.repo_name ); CheckSuiteStatus::Pending } }, - None => CheckSuiteStatus::Pending, + None => { + tracing::warn!( + "Received empty check suite conclusion for {}@{sha}", + self.repo_name, + ); + CheckSuiteStatus::Pending + } }, }) .collect(); @@ -384,6 +379,18 @@ impl GithubRepositoryClient { Ok(prs) } + async fn get_request(&self, path: &str) -> anyhow::Result { + let url = format!( + "/repos/{}/{}/{path}", + self.repo_name.owner(), + self.repo_name.name(), + ); + tracing::debug!("Sending request to {url}"); + let response: T = self.client.get(url.as_str(), None::<&()>).await?; + tracing::debug!("Received response: {response:?}"); + Ok(response) + } + fn format_pr(&self, pr: PullRequestNumber) -> String { format!("{}/{}", self.repository(), pr) } diff --git a/src/tests/mocks/repository.rs b/src/tests/mocks/repository.rs index 03bd0de7..2d764fc0 100644 --- a/src/tests/mocks/repository.rs +++ b/src/tests/mocks/repository.rs @@ -4,6 +4,7 @@ use std::{collections::HashMap, time::SystemTime}; use crate::bors::{CheckSuiteStatus, PullRequestStatus}; use base64::Engine; use chrono::{DateTime, Utc}; +use octocrab::models::CheckSuiteId; use octocrab::models::pulls::MergeableState; use octocrab::models::repos::Object; use octocrab::models::repos::Object::Commit; @@ -574,6 +575,7 @@ async fn mock_merge_branch(repo: Arc>, mock_server: &MockServer) { async fn mock_check_suites(repo: Arc>, mock_server: &MockServer) { #[derive(serde::Serialize)] struct CheckSuitePayload { + id: CheckSuiteId, conclusion: Option, head_branch: String, } @@ -594,7 +596,9 @@ async fn mock_check_suites(repo: Arc>, mock_server: &MockServer) { check_suites: branch .get_suites() .iter() - .map(|suite| CheckSuitePayload { + .enumerate() + .map(|(index, suite)| CheckSuitePayload { + id: CheckSuiteId(index as u64), conclusion: match suite { CheckSuiteStatus::Pending => None, CheckSuiteStatus::Success => Some("success".to_string()),