Skip to content

Commit 1eb681f

Browse files
committed
Add try build on check run start and complete
1 parent 4ca55db commit 1eb681f

File tree

10 files changed

+532
-11
lines changed

10 files changed

+532
-11
lines changed

.sqlx/query-33cf1b803a0b658f197ab26d87e2c6c4fe42128cb481a234372626f35392e809.json

Lines changed: 15 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/bors/handlers/trybuild.rs

Lines changed: 108 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
use std::sync::Arc;
22

33
use anyhow::{Context, anyhow};
4+
use octocrab::params::checks::CheckRunOutput;
5+
use octocrab::params::checks::CheckRunStatus;
46

57
use crate::PgDbClient;
68
use crate::bors::RepositoryState;
@@ -33,6 +35,9 @@ pub(super) const TRY_MERGE_BRANCH_NAME: &str = "automation/bors/try-merge";
3335
// This branch should run CI checks.
3436
pub(super) const TRY_BRANCH_NAME: &str = "automation/bors/try";
3537

38+
// The name of the check run seen in the GitHub UI.
39+
const TRY_BUILD_CHECK_RUN_NAME: &str = "Bors try build";
40+
3641
/// Performs a so-called try build - merges the PR branch into a special branch designed
3742
/// for running CI checks.
3843
///
@@ -87,10 +92,34 @@ pub(super) async fn command_try_build(
8792
{
8893
MergeResult::Success(merge_sha) => {
8994
// If the merge was succesful, run CI with merged commit
90-
run_try_build(&repo.client, &db, &pr.db, merge_sha.clone(), base_sha).await?;
95+
let build_id =
96+
run_try_build(&repo.client, &db, &pr.db, merge_sha.clone(), base_sha).await?;
9197

9298
handle_label_trigger(repo, pr.number(), LabelTrigger::TryBuildStarted).await?;
9399

100+
// Create a check run to track the try build status in GitHub's UI.
101+
// This gets added to the PR's head SHA so GitHub shows UI in the checks tab and
102+
// the bottom of the PR.
103+
let check_run = repo
104+
.client
105+
.create_check_run(
106+
TRY_BUILD_CHECK_RUN_NAME,
107+
&pr.github.head.sha,
108+
CheckRunStatus::InProgress,
109+
CheckRunOutput {
110+
title: "Bors try build".to_string(),
111+
summary: "".to_string(),
112+
text: None,
113+
annotations: vec![],
114+
images: vec![],
115+
},
116+
&build_id.to_string(),
117+
)
118+
.await?;
119+
120+
db.update_build_check_run_id(build_id, check_run.id.into_inner() as i64)
121+
.await?;
122+
94123
repo.client
95124
.post_comment(
96125
pr.number(),
@@ -176,17 +205,18 @@ async fn run_try_build(
176205
pr: &PullRequestModel,
177206
commit_sha: CommitSha,
178207
parent_sha: CommitSha,
179-
) -> anyhow::Result<()> {
208+
) -> anyhow::Result<i32> {
180209
client
181210
.set_branch_to_sha(TRY_BRANCH_NAME, &commit_sha, ForcePush::Yes)
182211
.await
183212
.map_err(|error| anyhow!("Cannot set try branch to main branch: {error:?}"))?;
184213

185-
db.attach_try_build(pr, TRY_BRANCH_NAME.to_string(), commit_sha, parent_sha)
214+
let build_id = db
215+
.attach_try_build(pr, TRY_BRANCH_NAME.to_string(), commit_sha, parent_sha)
186216
.await?;
187217

188218
tracing::info!("Try build started");
189-
Ok(())
219+
Ok(build_id)
190220
}
191221

192222
enum MergeResult {
@@ -307,13 +337,16 @@ fn auto_merge_commit_message(
307337

308338
#[cfg(test)]
309339
mod tests {
310-
use crate::bors::handlers::trybuild::{TRY_BRANCH_NAME, TRY_MERGE_BRANCH_NAME};
340+
use crate::bors::handlers::trybuild::{
341+
TRY_BRANCH_NAME, TRY_BUILD_CHECK_RUN_NAME, TRY_MERGE_BRANCH_NAME,
342+
};
311343
use crate::database::operations::get_all_workflows;
312344
use crate::github::CommitSha;
313345
use crate::tests::mocks::{
314346
BorsBuilder, Comment, GitHubState, User, Workflow, WorkflowEvent, default_pr_number,
315347
default_repo_name, run_test,
316348
};
349+
use octocrab::params::checks::{CheckRunConclusion, CheckRunStatus};
317350

318351
#[sqlx::test]
319352
async fn try_success(pool: sqlx::PgPool) {
@@ -861,4 +894,74 @@ try_failed = ["+foo", "+bar", "-baz"]
861894
})
862895
.await;
863896
}
897+
898+
#[sqlx::test]
899+
async fn try_build_creates_check_run(pool: sqlx::PgPool) {
900+
run_test(pool.clone(), |mut tester| async {
901+
tester.create_branch(TRY_BRANCH_NAME).expect_suites(1);
902+
tester.post_comment("@bors try").await?;
903+
tester.expect_comments(1).await;
904+
905+
tester.expect_check_run(
906+
&tester.default_pr().await.get_gh_pr().head_sha,
907+
TRY_BUILD_CHECK_RUN_NAME,
908+
"Bors try build",
909+
CheckRunStatus::InProgress,
910+
None,
911+
);
912+
913+
let check_run = tester.get_check_run().await?;
914+
insta::assert_snapshot!(check_run.summary, @"");
915+
insta::assert_snapshot!(check_run.text, @"");
916+
917+
Ok(tester)
918+
})
919+
.await;
920+
}
921+
922+
#[sqlx::test]
923+
async fn try_build_updates_check_run_on_success(pool: sqlx::PgPool) {
924+
run_test(pool.clone(), |mut tester| async {
925+
tester.create_branch(TRY_BRANCH_NAME).expect_suites(1);
926+
tester.post_comment("@bors try").await?;
927+
tester.expect_comments(1).await;
928+
929+
tester.workflow_success(tester.try_branch()).await?;
930+
tester.expect_comments(1).await;
931+
932+
tester.expect_check_run(
933+
&tester.default_pr().await.get_gh_pr().head_sha,
934+
TRY_BUILD_CHECK_RUN_NAME,
935+
"Bors try build",
936+
CheckRunStatus::Completed,
937+
Some(CheckRunConclusion::Success),
938+
);
939+
940+
Ok(tester)
941+
})
942+
.await;
943+
}
944+
945+
#[sqlx::test]
946+
async fn try_build_updates_check_run_on_failure(pool: sqlx::PgPool) {
947+
run_test(pool.clone(), |mut tester| async {
948+
tester.create_branch(TRY_BRANCH_NAME).expect_suites(1);
949+
tester.post_comment("@bors try").await?;
950+
tester.expect_comments(1).await;
951+
952+
tester.workflow_failure(tester.try_branch()).await?;
953+
tester.expect_comments(1).await;
954+
955+
tester.expect_check_run(
956+
&tester.default_pr().await.get_gh_pr().head_sha,
957+
TRY_BUILD_CHECK_RUN_NAME,
958+
"Bors try build",
959+
CheckRunStatus::Completed,
960+
Some(CheckRunConclusion::Failure),
961+
);
962+
963+
Ok(tester)
964+
})
965+
.await;
966+
}
864967
}

src/bors/handlers/workflow.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
use std::sync::Arc;
22
use std::time::Duration;
33

4+
use octocrab::params::checks::CheckRunConclusion;
5+
use octocrab::params::checks::CheckRunStatus;
6+
47
use crate::PgDbClient;
58
use crate::bors::CheckSuiteStatus;
69
use crate::bors::RepositoryState;
@@ -193,6 +196,18 @@ async fn try_complete_build(
193196

194197
handle_label_trigger(repo, pr.number, trigger).await?;
195198

199+
if let Some(check_run_id) = build.check_run_id {
200+
let (status, conclusion) = if has_failure {
201+
(CheckRunStatus::Completed, Some(CheckRunConclusion::Failure))
202+
} else {
203+
(CheckRunStatus::Completed, Some(CheckRunConclusion::Success))
204+
};
205+
206+
repo.client
207+
.update_check_run(check_run_id as u64, status, conclusion, None)
208+
.await?;
209+
}
210+
196211
let message = if !has_failure {
197212
tracing::info!("Workflow succeeded");
198213
try_build_succeeded_comment(&workflows, payload.commit_sha, &build)

src/database/client.rs

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,9 @@ use super::operations::{
1515
get_prs_with_unknown_mergeable_state, get_pull_request, get_repository, get_repository_by_name,
1616
get_running_builds, get_workflow_urls_for_build, get_workflows_for_build,
1717
insert_repo_if_not_exists, set_pr_assignees, set_pr_priority, set_pr_rollup, set_pr_status,
18-
unapprove_pull_request, undelegate_pull_request, update_build_status,
19-
update_mergeable_states_by_base_branch, update_pr_mergeable_state, update_pr_try_build_id,
20-
update_workflow_status, upsert_pull_request, upsert_repository,
18+
unapprove_pull_request, undelegate_pull_request, update_build_check_run_id,
19+
update_build_status, update_mergeable_states_by_base_branch, update_pr_mergeable_state,
20+
update_pr_try_build_id, update_workflow_status, upsert_pull_request, upsert_repository,
2121
};
2222
use super::{ApprovalInfo, DelegatedPermission, MergeableState, RunId, UpsertPullRequestParams};
2323

@@ -181,13 +181,13 @@ impl PgDbClient {
181181
branch: String,
182182
commit_sha: CommitSha,
183183
parent: CommitSha,
184-
) -> anyhow::Result<()> {
184+
) -> anyhow::Result<i32> {
185185
let mut tx = self.pool.begin().await?;
186186
let build_id =
187187
create_build(&mut *tx, &pr.repository, &branch, &commit_sha, &parent).await?;
188188
update_pr_try_build_id(&mut *tx, pr.id, build_id).await?;
189189
tx.commit().await?;
190-
Ok(())
190+
Ok(build_id)
191191
}
192192

193193
pub async fn find_build(
@@ -214,6 +214,14 @@ impl PgDbClient {
214214
update_build_status(&self.pool, build.id, status).await
215215
}
216216

217+
pub async fn update_build_check_run_id(
218+
&self,
219+
build_id: i32,
220+
check_run_id: i64,
221+
) -> anyhow::Result<()> {
222+
update_build_check_run_id(&self.pool, build_id, check_run_id).await
223+
}
224+
217225
pub async fn create_workflow(
218226
&self,
219227
build: &BuildModel,

src/database/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,7 @@ pub struct BuildModel {
315315
/// The base commit SHA that this build is merged with (e.g., main branch HEAD).
316316
pub parent: String,
317317
pub created_at: DateTime<Utc>,
318+
/// The ID of the check run associated with the build.
318319
pub check_run_id: Option<i64>,
319320
}
320321

src/database/operations.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -946,6 +946,24 @@ pub(crate) async fn upsert_repository(
946946
.await
947947
}
948948

949+
pub(crate) async fn update_build_check_run_id(
950+
executor: impl PgExecutor<'_>,
951+
build_id: i32,
952+
check_run_id: i64,
953+
) -> anyhow::Result<()> {
954+
measure_db_query("update_build_check_run_id", || async {
955+
sqlx::query!(
956+
"UPDATE build SET check_run_id = $1 WHERE id = $2",
957+
check_run_id,
958+
build_id
959+
)
960+
.execute(executor)
961+
.await?;
962+
Ok(())
963+
})
964+
.await
965+
}
966+
949967
/// Removes the auto build associated with a PR and deletes the build record (if one exists).
950968
pub(crate) async fn delete_auto_build(
951969
executor: impl PgExecutor<'_>,

src/github/api/client.rs

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,18 @@
11
use anyhow::Context;
22
use octocrab::Octocrab;
3+
use octocrab::models::checks::CheckRun;
34
use octocrab::models::{App, Repository};
5+
use octocrab::params::checks::{CheckRunConclusion, CheckRunOutput, CheckRunStatus};
46
use tracing::log;
57

68
use crate::bors::event::PullRequestComment;
79
use crate::bors::{CheckSuite, CheckSuiteStatus, Comment};
810
use crate::config::{CONFIG_FILE_PATH, RepositoryConfig};
911
use crate::database::RunId;
1012
use crate::github::api::base_github_html_url;
11-
use crate::github::api::operations::{ForcePush, MergeError, merge_branches, set_branch_to_commit};
13+
use crate::github::api::operations::{
14+
ForcePush, MergeError, create_check_run, merge_branches, set_branch_to_commit, update_check_run,
15+
};
1216
use crate::github::{CommitSha, GithubRepoName, PullRequest, PullRequestNumber};
1317
use crate::utils::timing::{measure_network_request, perform_network_request_with_retry};
1418
use futures::TryStreamExt;
@@ -165,6 +169,39 @@ impl GithubRepositoryClient {
165169
.map_err(|_| MergeError::Timeout)?
166170
}
167171

172+
/// Create a check run for the given commit.
173+
pub async fn create_check_run(
174+
&self,
175+
name: &str,
176+
head_sha: &CommitSha,
177+
status: CheckRunStatus,
178+
output: CheckRunOutput,
179+
external_id: &str,
180+
) -> anyhow::Result<CheckRun> {
181+
measure_network_request("create_check_run", || async {
182+
create_check_run(self, name, head_sha, status, output, external_id)
183+
.await
184+
.context("Cannot create check run")
185+
})
186+
.await
187+
}
188+
189+
/// Update a check run with the given check run ID.
190+
pub async fn update_check_run(
191+
&self,
192+
check_run_id: u64,
193+
status: CheckRunStatus,
194+
conclusion: Option<CheckRunConclusion>,
195+
output: Option<CheckRunOutput>,
196+
) -> anyhow::Result<CheckRun> {
197+
measure_network_request("update_check_run", || async {
198+
update_check_run(self, check_run_id, status, conclusion, output)
199+
.await
200+
.context("Cannot update check run")
201+
})
202+
.await
203+
}
204+
168205
/// Find all check suites attached to the given commit and branch.
169206
pub async fn get_check_suites_for_commit(
170207
&self,

0 commit comments

Comments
 (0)