Merge pull request #349 from Sakib25800/update-check-run-build-cancelled

Update check run on build cancelled

Author: Kobzol

src/bors/handlers/pr_events.rs

@@ -10,6 +10,7 @@ use crate::bors::mergeable_queue::MergeableQueueSender;
 use crate::bors::{Comment, PullRequestStatus, RepositoryState};
 use crate::database::{BuildStatus, MergeableState};
 use crate::github::{CommitSha, LabelTrigger, PullRequestNumber};
+use octocrab::params::checks::CheckRunConclusion;
 use std::sync::Arc;
 
 pub(super) async fn handle_pull_request_edited(
@@ -58,7 +59,14 @@ pub(super) async fn handle_push_to_pull_request(
         if auto_build.status == BuildStatus::Pending {
             tracing::info!("Cancelling auto build for PR {pr_number} due to push");
 
-            match cancel_build_workflows(&repo_state.client, db.as_ref(), auto_build).await {
+            match cancel_build_workflows(
+                &repo_state.client,
+                db.as_ref(),
+                auto_build,
+                CheckRunConclusion::Cancelled,
+            )
+            .await
+            {
                 Err(error) => {
                     tracing::error!(
                         "Could not cancel workflows for SHA {}: {error:?}",

src/bors/handlers/refresh.rs

@@ -3,15 +3,14 @@ use std::time::Duration;
 
 use anyhow::Context;
 use chrono::{DateTime, Utc};
-use octocrab::params::checks::{CheckRunConclusion, CheckRunStatus};
+use octocrab::params::checks::CheckRunConclusion;
 use std::collections::BTreeMap;
 
 use crate::bors::Comment;
 use crate::bors::PullRequestStatus;
 use crate::bors::RepositoryState;
 use crate::bors::handlers::trybuild::cancel_build_workflows;
 use crate::bors::mergeable_queue::MergeableQueueSender;
-use crate::database::BuildStatus;
 use crate::{PgDbClient, TeamApiClient};
 
 /// Cancel CI builds that have been running for too long
@@ -27,26 +26,11 @@ pub async fn cancel_timed_out_builds(
         if elapsed_time(build.created_at) >= timeout {
             tracing::info!("Cancelling build {}", build.commit_sha);
 
-            db.update_build_status(&build, BuildStatus::Cancelled)
-                .await?;
-
-            if let Some(check_run_id) = build.check_run_id {
-                if let Err(error) = repo
-                    .client
-                    .update_check_run(
-                        check_run_id as u64,
-                        CheckRunStatus::Completed,
-                        Some(CheckRunConclusion::TimedOut),
-                        None,
-                    )
-                    .await
-                {
-                    tracing::error!("Could not update check run {check_run_id}: {error:?}");
-                }
-            }
-
             if let Some(pr) = db.find_pr_by_build(&build).await? {
-                if let Err(error) = cancel_build_workflows(&repo.client, db, &build).await {
+                if let Err(error) =
+                    cancel_build_workflows(&repo.client, db, &build, CheckRunConclusion::TimedOut)
+                        .await
+                {
                     tracing::error!(
                         "Could not cancel workflows for SHA {}: {error:?}",
                         build.commit_sha

src/bors/handlers/trybuild.rs

@@ -19,6 +19,7 @@ use crate::github::{CommitSha, GithubUser, LabelTrigger, MergeError, PullRequest
 use crate::permissions::PermissionType;
 use crate::utils::text::suppress_github_mentions;
 use anyhow::{Context, anyhow};
+use octocrab::params::checks::CheckRunConclusion;
 use octocrab::params::checks::CheckRunOutput;
 use octocrab::params::checks::CheckRunStatus;
 use tracing::log;
@@ -155,7 +156,7 @@ async fn cancel_previous_try_build(
 ) -> anyhow::Result<Vec<String>> {
     assert_eq!(build.status, BuildStatus::Pending);
 
-    match cancel_build_workflows(&repo.client, db, build).await {
+    match cancel_build_workflows(&repo.client, db, build, CheckRunConclusion::Cancelled).await {
         Ok(workflow_ids) => {
             tracing::info!("Try build cancelled");
             Ok(repo
@@ -267,7 +268,14 @@ pub(super) async fn command_try_cancel(
         return Ok(());
     };
 
-    match cancel_build_workflows(&repo.client, db.as_ref(), build).await {
+    match cancel_build_workflows(
+        &repo.client,
+        db.as_ref(),
+        build,
+        CheckRunConclusion::Cancelled,
+    )
+    .await
+    {
         Err(error) => {
             tracing::error!(
                 "Could not cancel workflows for SHA {}: {error:?}",
@@ -298,6 +306,7 @@ pub async fn cancel_build_workflows(
     client: &GithubRepositoryClient,
     db: &PgDbClient,
     build: &BuildModel,
+    check_run_conclusion: CheckRunConclusion,
 ) -> anyhow::Result<Vec<RunId>> {
     let pending_workflows = db.get_pending_workflows_for_build(build).await?;
 
@@ -307,6 +316,20 @@ pub async fn cancel_build_workflows(
     db.update_build_status(build, BuildStatus::Cancelled)
         .await?;
 
+    if let Some(check_run_id) = build.check_run_id {
+        if let Err(error) = client
+            .update_check_run(
+                check_run_id as u64,
+                CheckRunStatus::Completed,
+                Some(check_run_conclusion),
+                None,
+            )
+            .await
+        {
+            tracing::error!("Could not update check run {check_run_id}: {error:?}");
+        }
+    }
+
     Ok(pending_workflows)
 }
 
@@ -967,4 +990,60 @@ try_failed = ["+foo", "+bar", "-baz"]
         })
         .await;
     }
+
+    #[sqlx::test]
+    async fn try_cancel_updates_check_run_to_cancelled(pool: sqlx::PgPool) {
+        run_test(pool.clone(), |mut tester| async {
+            tester.create_branch(TRY_BRANCH_NAME).expect_suites(1);
+            tester.post_comment("@bors try").await?;
+            tester.expect_comments(1).await;
+
+            tester.post_comment("@bors try cancel").await?;
+            tester.expect_comments(1).await;
+
+            tester.expect_check_run(
+                &tester.default_pr().await.get_gh_pr().head_sha,
+                TRY_BUILD_CHECK_RUN_NAME,
+                "Bors try build",
+                CheckRunStatus::Completed,
+                Some(CheckRunConclusion::Cancelled),
+            );
+
+            Ok(tester)
+        })
+        .await;
+    }
+
+    #[sqlx::test]
+    async fn new_try_build_cancels_previous_and_updates_check_run(pool: sqlx::PgPool) {
+        run_test(pool.clone(), |mut tester| async {
+            tester.create_branch(TRY_BRANCH_NAME).expect_suites(1);
+            tester.post_comment("@bors try").await?;
+            tester.expect_comments(1).await;
+
+            let prev_sha = tester.default_pr().await.get_gh_pr().head_sha;
+            tester
+                .push_to_pr(default_repo_name(), default_pr_number())
+                .await?;
+            tester.post_comment("@bors try").await?;
+            tester.expect_comments(1).await;
+
+            tester.expect_check_run(
+                &prev_sha,
+                TRY_BUILD_CHECK_RUN_NAME,
+                "Bors try build",
+                CheckRunStatus::Completed,
+                Some(CheckRunConclusion::Cancelled),
+            );
+            tester.expect_check_run(
+                &tester.default_pr().await.get_gh_pr().head_sha,
+                TRY_BUILD_CHECK_RUN_NAME,
+                "Bors try build",
+                CheckRunStatus::InProgress,
+                None,
+            );
+            Ok(tester)
+        })
+        .await;
+    }
 }

src/bors/handlers/workflow.rs

@@ -203,9 +203,13 @@ async fn try_complete_build(
             (CheckRunStatus::Completed, Some(CheckRunConclusion::Success))
         };
 
-        repo.client
+        if let Err(error) = repo
+            .client
             .update_check_run(check_run_id as u64, status, conclusion, None)
-            .await?;
+            .await
+        {
+            tracing::error!("Could not update check run {check_run_id}: {error:?}");
+        }
     }
 
     let message = if !has_failure {