Merge pull request #359 from Kobzol/logging

Add more logging around check suites

Author: Kobzol

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(());
     }
 

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.

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<Vec<CheckSuite>> {
         #[derive(serde::Deserialize, Debug)]
         struct CheckSuitePayload {
+            id: CheckSuiteId,
             conclusion: Option<String>,
             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<T: DeserializeOwned + Debug>(&self, path: &str) -> anyhow::Result<T> {
+        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)
     }

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<Mutex<Repo>>, mock_server: &MockServer) {
 async fn mock_check_suites(repo: Arc<Mutex<Repo>>, mock_server: &MockServer) {
     #[derive(serde::Serialize)]
     struct CheckSuitePayload {
+        id: CheckSuiteId,
         conclusion: Option<String>,
         head_branch: String,
     }
@@ -594,7 +596,9 @@ async fn mock_check_suites(repo: Arc<Mutex<Repo>>, 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()),