Merge pull request #346 from Kobzol/check-run-disable

Temporarily partially revert 338

Author: Kobzol

.sqlx/query-33cf1b803a0b658f197ab26d87e2c6c4fe42128cb481a234372626f35392e809.json

@@ -53,7 +53,7 @@ One-time setup:
 - Create your own GitHub app.
   - Configure its webhook secret.
   - Configure its private key.
-  - Give it permissions for `Actions` (r/w), `Checks` (r), `Commit statuses` (r), `Contents` (r/w), `Issues` (r/w) and
+  - Give it permissions for `Actions` (r/w), `Checks` (r/w), `Commit statuses` (r), `Contents` (r/w), `Issues` (r/w) and
   `Pull requests` (r/w).
   - Subscribe it to webhook events `Check suite`, `Check run`, `Issue comment`, `Issues`, `Pull request`,
     `Pull request review`, `Pull request review comment` and `Workflow run`.

docs/development.md

@@ -3,7 +3,6 @@ use std::time::Duration;
 
 use anyhow::Context;
 use chrono::{DateTime, Utc};
-use octocrab::params::checks::{CheckRunConclusion, CheckRunStatus};
 use std::collections::BTreeMap;
 
 use crate::bors::Comment;
@@ -29,22 +28,6 @@ pub async fn cancel_timed_out_builds(
 
             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 {
                     tracing::error!(
@@ -198,13 +181,11 @@ fn elapsed_time(date: DateTime<Utc>) -> Duration {
 mod tests {
     use crate::bors::PullRequestStatus;
     use crate::bors::handlers::refresh::MOCK_TIME;
-    use crate::bors::handlers::trybuild::TRY_BUILD_CHECK_RUN_NAME;
     use crate::database::{MergeableState, OctocrabMergeableState};
     use crate::tests::mocks::{
         BorsBuilder, GitHubState, default_pr_number, default_repo_name, run_test,
     };
     use chrono::Utc;
-    use octocrab::params::checks::{CheckRunConclusion, CheckRunStatus};
     use std::future::Future;
     use std::time::Duration;
     use tokio::runtime::RuntimeFlavor;
@@ -286,33 +267,6 @@ timeout = 3600
             .await;
     }
 
-    #[sqlx::test]
-    async fn refresh_cancel_build_updates_check_run(pool: sqlx::PgPool) {
-        BorsBuilder::new(pool)
-            .github(gh_state_with_long_timeout())
-            .run_test(|mut tester| async move {
-                tester.post_comment("@bors try").await?;
-                tester.expect_comments(1).await;
-
-                with_mocked_time(Duration::from_secs(4000), async {
-                    tester.cancel_timed_out_builds().await;
-                })
-                .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::TimedOut),
-                );
-
-                Ok(tester)
-            })
-            .await;
-    }
-
     #[sqlx::test]
     async fn refresh_cancel_workflow_after_timeout(pool: sqlx::PgPool) {
         let gh = BorsBuilder::new(pool)

src/bors/handlers/refresh.rs

@@ -1,8 +1,6 @@
 use std::sync::Arc;
 
 use anyhow::{Context, anyhow};
-use octocrab::params::checks::CheckRunOutput;
-use octocrab::params::checks::CheckRunStatus;
 
 use crate::PgDbClient;
 use crate::bors::RepositoryState;
@@ -35,9 +33,6 @@ pub(super) const TRY_MERGE_BRANCH_NAME: &str = "automation/bors/try-merge";
 // This branch should run CI checks.
 pub(super) const TRY_BRANCH_NAME: &str = "automation/bors/try";
 
-// The name of the check run seen in the GitHub UI.
-pub(super) const TRY_BUILD_CHECK_RUN_NAME: &str = "Bors try build";
-
 /// Performs a so-called try build - merges the PR branch into a special branch designed
 /// for running CI checks.
 ///
@@ -92,34 +87,10 @@ pub(super) async fn command_try_build(
     {
         MergeResult::Success(merge_sha) => {
             // If the merge was succesful, run CI with merged commit
-            let build_id =
-                run_try_build(&repo.client, &db, &pr.db, merge_sha.clone(), base_sha).await?;
+            run_try_build(&repo.client, &db, &pr.db, merge_sha.clone(), base_sha).await?;
 
             handle_label_trigger(repo, pr.number(), LabelTrigger::TryBuildStarted).await?;
 
-            // Create a check run to track the try build status in GitHub's UI.
-            // This gets added to the PR's head SHA so GitHub shows UI in the checks tab and
-            // the bottom of the PR.
-            let check_run = repo
-                .client
-                .create_check_run(
-                    TRY_BUILD_CHECK_RUN_NAME,
-                    &pr.github.head.sha,
-                    CheckRunStatus::InProgress,
-                    CheckRunOutput {
-                        title: "Bors try build".to_string(),
-                        summary: "".to_string(),
-                        text: None,
-                        annotations: vec![],
-                        images: vec![],
-                    },
-                    &build_id.to_string(),
-                )
-                .await?;
-
-            db.update_build_check_run_id(build_id, check_run.id.into_inner() as i64)
-                .await?;
-
             repo.client
                 .post_comment(
                     pr.number(),
@@ -205,18 +176,17 @@ async fn run_try_build(
     pr: &PullRequestModel,
     commit_sha: CommitSha,
     parent_sha: CommitSha,
-) -> anyhow::Result<i32> {
+) -> anyhow::Result<()> {
     client
         .set_branch_to_sha(TRY_BRANCH_NAME, &commit_sha, ForcePush::Yes)
         .await
         .map_err(|error| anyhow!("Cannot set try branch to main branch: {error:?}"))?;
 
-    let build_id = db
-        .attach_try_build(pr, TRY_BRANCH_NAME.to_string(), commit_sha, parent_sha)
+    db.attach_try_build(pr, TRY_BRANCH_NAME.to_string(), commit_sha, parent_sha)
         .await?;
 
     tracing::info!("Try build started");
-    Ok(build_id)
+    Ok(())
 }
 
 enum MergeResult {
@@ -337,16 +307,13 @@ fn auto_merge_commit_message(
 
 #[cfg(test)]
 mod tests {
-    use crate::bors::handlers::trybuild::{
-        TRY_BRANCH_NAME, TRY_BUILD_CHECK_RUN_NAME, TRY_MERGE_BRANCH_NAME,
-    };
+    use crate::bors::handlers::trybuild::{TRY_BRANCH_NAME, TRY_MERGE_BRANCH_NAME};
     use crate::database::operations::get_all_workflows;
     use crate::github::CommitSha;
     use crate::tests::mocks::{
         BorsBuilder, Comment, GitHubState, User, Workflow, WorkflowEvent, default_pr_number,
         default_repo_name, run_test,
     };
-    use octocrab::params::checks::{CheckRunConclusion, CheckRunStatus};
 
     #[sqlx::test]
     async fn try_success(pool: sqlx::PgPool) {
@@ -894,70 +861,4 @@ try_failed = ["+foo", "+bar", "-baz"]
             })
             .await;
     }
-
-    #[sqlx::test]
-    async fn try_build_creates_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;
-
-            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;
-    }
-
-    #[sqlx::test]
-    async fn try_build_updates_check_run_on_success(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.workflow_success(tester.try_branch()).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::Success),
-            );
-
-            Ok(tester)
-        })
-        .await;
-    }
-
-    #[sqlx::test]
-    async fn try_build_updates_check_run_on_failure(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.workflow_failure(tester.try_branch()).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::Failure),
-            );
-
-            Ok(tester)
-        })
-        .await;
-    }
 }

src/bors/handlers/trybuild.rs

@@ -1,9 +1,6 @@
 use std::sync::Arc;
 use std::time::Duration;
 
-use octocrab::params::checks::CheckRunConclusion;
-use octocrab::params::checks::CheckRunStatus;
-
 use crate::PgDbClient;
 use crate::bors::CheckSuiteStatus;
 use crate::bors::RepositoryState;
@@ -196,18 +193,6 @@ async fn try_complete_build(
 
     handle_label_trigger(repo, pr.number, trigger).await?;
 
-    if let Some(check_run_id) = build.check_run_id {
-        let (status, conclusion) = if has_failure {
-            (CheckRunStatus::Completed, Some(CheckRunConclusion::Failure))
-        } else {
-            (CheckRunStatus::Completed, Some(CheckRunConclusion::Success))
-        };
-
-        repo.client
-            .update_check_run(check_run_id as u64, status, conclusion, None)
-            .await?;
-    }
-
     let message = if !has_failure {
         tracing::info!("Workflow succeeded");
         try_build_succeeded_comment(&workflows, payload.commit_sha, &build)

src/bors/handlers/workflow.rs

@@ -15,9 +15,9 @@ use super::operations::{
     get_prs_with_unknown_mergeable_state, get_pull_request, get_repository, get_repository_by_name,
     get_running_builds, get_workflow_urls_for_build, get_workflows_for_build,
     insert_repo_if_not_exists, set_pr_assignees, set_pr_priority, set_pr_rollup, set_pr_status,
-    unapprove_pull_request, undelegate_pull_request, update_build_check_run_id,
-    update_build_status, update_mergeable_states_by_base_branch, update_pr_mergeable_state,
-    update_pr_try_build_id, update_workflow_status, upsert_pull_request, upsert_repository,
+    unapprove_pull_request, undelegate_pull_request, update_build_status,
+    update_mergeable_states_by_base_branch, update_pr_mergeable_state, update_pr_try_build_id,
+    update_workflow_status, upsert_pull_request, upsert_repository,
 };
 use super::{ApprovalInfo, DelegatedPermission, MergeableState, RunId, UpsertPullRequestParams};
 
@@ -181,13 +181,13 @@ impl PgDbClient {
         branch: String,
         commit_sha: CommitSha,
         parent: CommitSha,
-    ) -> anyhow::Result<i32> {
+    ) -> anyhow::Result<()> {
         let mut tx = self.pool.begin().await?;
         let build_id =
             create_build(&mut *tx, &pr.repository, &branch, &commit_sha, &parent).await?;
         update_pr_try_build_id(&mut *tx, pr.id, build_id).await?;
         tx.commit().await?;
-        Ok(build_id)
+        Ok(())
     }
 
     pub async fn find_build(
@@ -214,14 +214,6 @@ impl PgDbClient {
         update_build_status(&self.pool, build.id, status).await
     }
 
-    pub async fn update_build_check_run_id(
-        &self,
-        build_id: i32,
-        check_run_id: i64,
-    ) -> anyhow::Result<()> {
-        update_build_check_run_id(&self.pool, build_id, check_run_id).await
-    }
-
     pub async fn create_workflow(
         &self,
         build: &BuildModel,

src/database/client.rs

@@ -315,7 +315,6 @@ pub struct BuildModel {
     /// The base commit SHA that this build is merged with (e.g., main branch HEAD).
     pub parent: String,
     pub created_at: DateTime<Utc>,
-    /// The ID of the check run associated with the build.
     pub check_run_id: Option<i64>,
 }
 

src/database/mod.rs

@@ -946,24 +946,6 @@ pub(crate) async fn upsert_repository(
     .await
 }
 
-pub(crate) async fn update_build_check_run_id(
-    executor: impl PgExecutor<'_>,
-    build_id: i32,
-    check_run_id: i64,
-) -> anyhow::Result<()> {
-    measure_db_query("update_build_check_run_id", || async {
-        sqlx::query!(
-            "UPDATE build SET check_run_id = $1 WHERE id = $2",
-            check_run_id,
-            build_id
-        )
-        .execute(executor)
-        .await?;
-        Ok(())
-    })
-    .await
-}
-
 /// Removes the auto build associated with a PR and deletes the build record (if one exists).
 pub(crate) async fn delete_auto_build(
     executor: impl PgExecutor<'_>,