Merge pull request #432 from Sakib25800/failed-count

Add failed count to queue stats

Author: Kobzol

src/bors/handlers/retry.rs

@@ -4,6 +4,7 @@ use crate::PgDbClient;
 use crate::bors::handlers::{PullRequestData, deny_request, has_permission};
 use crate::bors::merge_queue::MergeQueueSender;
 use crate::bors::{Comment, RepositoryState};
+use crate::database::QueueStatus;
 use crate::github::{GithubUser, PullRequestNumber};
 use crate::permissions::PermissionType;
 
@@ -21,7 +22,7 @@ pub(super) async fn command_retry(
 
     let pr_model = pr.db;
 
-    if pr_model.is_stalled() {
+    if matches!(pr_model.queue_status(), QueueStatus::Stalled(_, _)) {
         db.clear_auto_build(pr_model).await?;
         merge_queue_tx.notify().await?;
     } else {

src/bors/handlers/workflow.rs

@@ -6,7 +6,9 @@ use crate::bors::handlers::{BuildType, is_bors_observed_branch};
 use crate::bors::handlers::{get_build_type, hide_try_build_started_comments};
 use crate::bors::merge_queue::MergeQueueSender;
 use crate::bors::{FailedWorkflowRun, RepositoryState, WorkflowRun};
-use crate::database::{BuildModel, BuildStatus, PullRequestModel, WorkflowModel, WorkflowStatus};
+use crate::database::{
+    BuildModel, BuildStatus, PullRequestModel, QueueStatus, WorkflowModel, WorkflowStatus,
+};
 use crate::github::api::client::GithubRepositoryClient;
 use crate::github::{CommitSha, LabelTrigger};
 use octocrab::models::CheckRunId;
@@ -407,16 +409,14 @@ pub async fn maybe_cancel_auto_build(
     pr: &PullRequestModel,
     reason: AutoBuildCancelReason,
 ) -> anyhow::Result<Option<String>> {
-    let Some(auto_build) = &pr.auto_build else {
-        return Ok(None);
+    let auto_build = match pr.queue_status() {
+        QueueStatus::Pending(_, build) => build,
+        _ => return Ok(None),
     };
-    if auto_build.status != BuildStatus::Pending {
-        return Ok(None);
-    }
 
     tracing::info!("Cancelling auto build {auto_build:?}");
 
-    match cancel_build(client, db, auto_build, CheckRunConclusion::Cancelled).await {
+    match cancel_build(client, db, &auto_build, CheckRunConclusion::Cancelled).await {
         Ok(workflows) => {
             tracing::info!("Auto build cancelled");
             let workflow_urls = workflows.into_iter().map(|w| w.url).collect();

src/bors/merge_queue.rs

@@ -12,10 +12,13 @@ use crate::bors::comment::{
     merge_conflict_comment,
 };
 use crate::bors::{PullRequestStatus, RepositoryState};
-use crate::database::{BuildStatus, MergeableState, OctocrabMergeableState, PullRequestModel};
+use crate::database::{
+    ApprovalInfo, BuildModel, BuildStatus, MergeableState, OctocrabMergeableState,
+    PullRequestModel, QueueStatus,
+};
 use crate::github::api::client::CheckRunOutput;
 use crate::github::api::operations::{BranchUpdateError, ForcePush};
-use crate::github::{CommitSha, PullRequest};
+use crate::github::{CommitSha, PullRequest, PullRequestNumber};
 use crate::github::{MergeResult, attempt_merge};
 use crate::utils::sort_queue::sort_queue_prs;
 
@@ -130,114 +133,126 @@ async fn process_repository(repo: &RepositoryState, ctx: &BorsContext) -> anyhow
     for pr in prs {
         let pr_num = pr.number;
 
-        let Some(auto_build) = &pr.auto_build else {
-            // No build exists for this PR - start a new auto build.
-            match start_auto_build(repo, ctx, &pr).await {
-                Ok(()) => {
-                    tracing::info!("Starting auto build for PR {pr_num}");
-                    break;
-                }
-                Err(StartAutoBuildError::MergeConflict) => {
-                    let gh_pr = repo.client.get_pull_request(pr.number).await?;
-
-                    tracing::debug!(
-                        "Failed to start auto build for PR {pr_num} due to merge conflict"
-                    );
-
-                    ctx.db
-                        .update_pr_mergeable_state(&pr, MergeableState::Unknown)
-                        .await?;
-                    repo.client
-                        .post_comment(pr.number, merge_conflict_comment(&gh_pr.head.name))
-                        .await?;
-                    continue;
-                }
-                Err(StartAutoBuildError::SanityCheckFailed(error)) => {
-                    tracing::info!("Sanity check failed for PR {pr_num}: {error:?}");
-                    break;
-                }
-                Err(StartAutoBuildError::GitHubError(error)) => {
-                    tracing::debug!(
-                        "Failed to start auto build for PR {pr_num} due to a GitHub error: {error:?}"
-                    );
-                    break;
-                }
-                Err(StartAutoBuildError::DatabaseError(error)) => {
-                    tracing::debug!(
-                        "Failed to start auto build for PR {pr_num} due to database error: {error:?}"
-                    );
-                    break;
-                }
-            }
-        };
-
-        let commit_sha = CommitSha(auto_build.commit_sha.clone());
-
-        match auto_build.status {
-            // Build successful - point the base branch to the merged commit.
-            BuildStatus::Success => {
-                let workflows = ctx.db.get_workflows_for_build(auto_build).await?;
-                let comment = auto_build_succeeded_comment(
-                    &workflows,
-                    pr.approver().unwrap_or("<unknown>"),
-                    &commit_sha,
-                    &pr.base_branch,
-                );
-
-                if let Err(error) = repo
-                    .client
-                    .set_branch_to_sha(&pr.base_branch, &commit_sha, ForcePush::No)
-                    .await
-                {
-                    tracing::error!(
-                        "Failed to fast-forward base branch for PR {pr_num}: {error:?}"
-                    );
-
-                    let comment = match &error {
-                        BranchUpdateError::Conflict(branch_name) => auto_build_push_failed_comment(
-                            &format!("this PR has conflicts with the `{branch_name}` branch"),
-                        ),
-                        BranchUpdateError::ValidationFailed(branch_name) => {
-                            auto_build_push_failed_comment(&format!(
-                                "the tested commit was behind the `{branch_name}` branch"
-                            ))
-                        }
-                        error => auto_build_push_failed_comment(&error.to_string()),
-                    };
-
-                    ctx.db
-                        .update_build_status(auto_build, BuildStatus::Failure)
-                        .await?;
-
-                    repo.client.post_comment(pr_num, comment).await?;
-                } else {
-                    tracing::info!("Auto build succeeded and merged for PR {pr_num}");
-
-                    ctx.db
-                        .set_pr_status(&pr.repository, pr.number, PullRequestStatus::Merged)
-                        .await?;
-
-                    repo.client.post_comment(pr.number, comment).await?;
-                }
-
-                // Break to give GitHub time to update the base branch.
+        match pr.queue_status() {
+            QueueStatus::NotApproved => unreachable!(),
+            QueueStatus::Stalled(..) => unreachable!(),
+            QueueStatus::Pending(..) => {
+                // Build in progress - stop queue. We can only have one PR being built
+                // at a time.
+                tracing::info!("PR {pr_num} has a pending build - blocking queue");
                 break;
             }
-            // Build in progress - stop queue. We can only have one PR being built
-            // at a time.
-            BuildStatus::Pending => {
-                tracing::info!("PR {pr_num} has a pending build - blocking queue");
+            QueueStatus::ReadyForMerge(approval_info, auto_build) => {
+                handle_successful_build(repo, ctx, &pr, &auto_build, &approval_info, pr_num)
+                    .await?;
                 break;
             }
-            BuildStatus::Failure | BuildStatus::Cancelled | BuildStatus::Timeouted => {
-                unreachable!("Failed auto builds should be filtered out by SQL query");
+            QueueStatus::Approved(..) => {
+                if handle_start_auto_build(repo, ctx, &pr, pr_num).await? {
+                    break;
+                }
             }
         }
     }
 
     Ok(())
 }
 
+/// Handle a successful auto build by pointing the base branch to the merged commit.
+async fn handle_successful_build(
+    repo: &RepositoryState,
+    ctx: &BorsContext,
+    pr: &PullRequestModel,
+    auto_build: &BuildModel,
+    approval_info: &ApprovalInfo,
+    pr_num: PullRequestNumber,
+) -> anyhow::Result<()> {
+    let commit_sha = CommitSha(auto_build.commit_sha.clone());
+    let workflows = ctx.db.get_workflows_for_build(auto_build).await?;
+    let comment = auto_build_succeeded_comment(
+        &workflows,
+        &approval_info.approver,
+        &commit_sha,
+        &pr.base_branch,
+    );
+
+    if let Err(error) = repo
+        .client
+        .set_branch_to_sha(&pr.base_branch, &commit_sha, ForcePush::No)
+        .await
+    {
+        tracing::error!("Failed to fast-forward base branch for PR {pr_num}: {error:?}");
+
+        let error_comment = match &error {
+            BranchUpdateError::Conflict(branch_name) => auto_build_push_failed_comment(&format!(
+                "this PR has conflicts with the `{branch_name}` branch"
+            )),
+            BranchUpdateError::ValidationFailed(branch_name) => auto_build_push_failed_comment(
+                &format!("the tested commit was behind the `{branch_name}` branch"),
+            ),
+            error => auto_build_push_failed_comment(&error.to_string()),
+        };
+
+        ctx.db
+            .update_build_status(auto_build, BuildStatus::Failure)
+            .await?;
+        repo.client.post_comment(pr_num, error_comment).await?;
+    } else {
+        tracing::info!("Auto build succeeded and merged for PR {pr_num}");
+        ctx.db
+            .set_pr_status(&pr.repository, pr.number, PullRequestStatus::Merged)
+            .await?;
+        repo.client.post_comment(pr.number, comment).await?;
+    }
+
+    Ok(())
+}
+
+/// Handle starting a new auto build for an approved PR.
+/// Returns true if the queue should break, false to continue.
+async fn handle_start_auto_build(
+    repo: &RepositoryState,
+    ctx: &BorsContext,
+    pr: &PullRequestModel,
+    pr_num: PullRequestNumber,
+) -> anyhow::Result<bool> {
+    let Err(error) = start_auto_build(repo, ctx, pr).await else {
+        tracing::info!("Starting auto build for PR {pr_num}");
+        return Ok(true);
+    };
+
+    match error {
+        StartAutoBuildError::MergeConflict => {
+            let gh_pr = repo.client.get_pull_request(pr.number).await?;
+            tracing::debug!("Failed to start auto build for PR {pr_num} due to merge conflict");
+
+            ctx.db
+                .update_pr_mergeable_state(pr, MergeableState::Unknown)
+                .await?;
+            repo.client
+                .post_comment(pr.number, merge_conflict_comment(&gh_pr.head.name))
+                .await?;
+            Ok(false)
+        }
+        StartAutoBuildError::SanityCheckFailed(error) => {
+            tracing::info!("Sanity check failed for PR {pr_num}: {error:?}");
+            Ok(true)
+        }
+        StartAutoBuildError::GitHubError(error) => {
+            tracing::debug!(
+                "Failed to start auto build for PR {pr_num} due to a GitHub error: {error:?}"
+            );
+            Ok(true)
+        }
+        StartAutoBuildError::DatabaseError(error) => {
+            tracing::debug!(
+                "Failed to start auto build for PR {pr_num} due to database error: {error:?}"
+            );
+            Ok(true)
+        }
+    }
+}
+
 #[must_use]
 pub enum StartAutoBuildError {
     /// Merge conflict between PR head and base branch.

src/database/mod.rs

@@ -181,6 +181,20 @@ pub struct ApprovalInfo {
     pub sha: String,
 }
 
+#[derive(Debug, Clone, PartialEq)]
+pub enum QueueStatus {
+    /// Approved with running auto build.
+    Pending(ApprovalInfo, BuildModel),
+    /// Approved with failed auto build.
+    Stalled(ApprovalInfo, BuildModel),
+    /// Approved with no auto build started yet or a failed auto build was reset
+    /// with `@bors retry`.
+    Approved(ApprovalInfo),
+    /// Approved with passing CI.
+    ReadyForMerge(ApprovalInfo, BuildModel),
+    NotApproved,
+}
+
 /// Represents the approval status of a pull request.
 #[derive(Debug, Clone, PartialEq)]
 pub enum ApprovalStatus {
@@ -271,7 +285,7 @@ impl FromStr for DelegatedPermission {
 }
 
 /// Status of a GitHub build.
-#[derive(Debug, PartialEq, sqlx::Type)]
+#[derive(Debug, Clone, PartialEq, sqlx::Type)]
 #[sqlx(type_name = "TEXT")]
 #[sqlx(rename_all = "lowercase")]
 pub enum BuildStatus {
@@ -309,7 +323,7 @@ impl Display for BuildStatus {
 }
 
 /// Represents a single (merged) commit.
-#[derive(Debug, sqlx::Type)]
+#[derive(Debug, Clone, PartialEq, sqlx::Type)]
 #[sqlx(type_name = "build")]
 pub struct BuildModel {
     pub id: PrimaryKey,
@@ -382,13 +396,25 @@ impl PullRequestModel {
         }
     }
 
-    /// Approved but with a failed auto build.
-    pub fn is_stalled(&self) -> bool {
-        self.is_approved()
-            && self
-                .auto_build
-                .as_ref()
-                .is_some_and(|build| build.status.is_failure())
+    /// Get the merge queue status of this pull request.
+    pub fn queue_status(&self) -> QueueStatus {
+        match &self.approval_status {
+            ApprovalStatus::NotApproved => QueueStatus::NotApproved,
+            ApprovalStatus::Approved(approval_info) => match &self.auto_build {
+                Some(build) => match build.status {
+                    BuildStatus::Pending => {
+                        QueueStatus::Pending(approval_info.clone(), build.clone())
+                    }
+                    BuildStatus::Success => {
+                        QueueStatus::ReadyForMerge(approval_info.clone(), build.clone())
+                    }
+                    BuildStatus::Failure | BuildStatus::Cancelled | BuildStatus::Timeouted => {
+                        QueueStatus::Stalled(approval_info.clone(), build.clone())
+                    }
+                },
+                None => QueueStatus::Approved(approval_info.clone()),
+            },
+        }
     }
 }