Skip to content

Commit 4071b91

Browse files
committed
Deal with auto build success
- Post comment - Update check run - Modify labels
1 parent 76c1d7a commit 4071b91

File tree

6 files changed

+230
-24
lines changed

6 files changed

+230
-24
lines changed

rust-bors.example.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,3 +19,5 @@ approve = ["+approved"]
1919
try = ["+foo", "-bar"]
2020
try_succeed = ["+foobar", "+foo", "+baz"]
2121
try_failed = []
22+
succeeded = ["+foo", "+bar"]
23+
failed = ["+foo", "+bar"]

src/bors/comment.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,3 +192,20 @@ pub fn auto_build_started_comment(head_sha: &CommitSha, merge_sha: &CommitSha) -
192192
head_sha, merge_sha
193193
))
194194
}
195+
196+
pub fn auto_build_succeeded_comment(
197+
workflows: &[WorkflowModel],
198+
approved_by: &str,
199+
merge_sha: &CommitSha,
200+
base_ref: &str,
201+
) -> Comment {
202+
let urls = workflows
203+
.iter()
204+
.map(|w| format!("[{}]({})", w.name, w.url))
205+
.collect::<Vec<_>>()
206+
.join(", ");
207+
208+
Comment::new(format!(
209+
":sunny: Test successful - {urls}\nApproved by: `{approved_by}`\nPushing {merge_sha} to `{base_ref}`...",
210+
))
211+
}

src/bors/handlers/workflow.rs

Lines changed: 152 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use crate::bors::CheckSuiteStatus;
99
use crate::bors::RepositoryState;
1010
use crate::bors::comment::{try_build_succeeded_comment, workflow_failed_comment};
1111
use crate::bors::event::{CheckSuiteCompleted, WorkflowCompleted, WorkflowStarted};
12+
use crate::bors::handlers::TRY_BRANCH_NAME;
1213
use crate::bors::handlers::is_bors_observed_branch;
1314
use crate::bors::handlers::labels::handle_label_trigger;
1415
use crate::bors::merge_queue::AUTO_BRANCH_NAME;
@@ -176,6 +177,7 @@ async fn try_complete_build(
176177
let has_failure = checks
177178
.iter()
178179
.any(|check| matches!(check.status, CheckSuiteStatus::Failure));
180+
let build_succeeded = !has_failure;
179181

180182
let mut workflows = db.get_workflows_for_build(&build).await?;
181183
workflows.sort_by(|a, b| a.name.cmp(&b.name));
@@ -192,42 +194,78 @@ async fn try_complete_build(
192194
return Ok(());
193195
}
194196

195-
let (status, trigger) = if has_failure {
196-
(BuildStatus::Failure, LabelTrigger::TryBuildFailed)
197+
let pr_num = pr.number;
198+
199+
if build_succeeded {
200+
tracing::info!("Build succeeded for PR {pr_num}");
197201
} else {
198-
(BuildStatus::Success, LabelTrigger::TryBuildSucceeded)
202+
tracing::info!("Build failed for PR {pr_num}");
203+
}
204+
205+
let branch = payload.branch.as_str();
206+
let (status, trigger, comment_opt) = match (branch, build_succeeded) {
207+
(TRY_BRANCH_NAME, true) => (
208+
BuildStatus::Success,
209+
LabelTrigger::TryBuildSucceeded,
210+
Some(try_build_succeeded_comment(
211+
&workflows,
212+
payload.commit_sha.clone(),
213+
&build,
214+
)),
215+
),
216+
(TRY_BRANCH_NAME, false) => (
217+
BuildStatus::Failure,
218+
LabelTrigger::TryBuildFailed,
219+
Some(workflow_failed_comment(&workflows)),
220+
),
221+
(AUTO_BRANCH_NAME, true) => (BuildStatus::Success, LabelTrigger::Succeeded, None),
222+
(AUTO_BRANCH_NAME, false) => (
223+
BuildStatus::Failure,
224+
LabelTrigger::Failed,
225+
Some(workflow_failed_comment(&workflows)),
226+
),
227+
_ => unreachable!("Branch should be bors observed branch"),
199228
};
229+
200230
db.update_build_status(&build, status).await?;
231+
handle_label_trigger(repo, pr_num, trigger).await?;
201232

202-
handle_label_trigger(repo, pr.number, trigger).await?;
233+
if let Some(comment) = comment_opt {
234+
repo.client.post_comment(pr_num, comment).await?;
235+
}
203236

204237
if let Some(check_run_id) = build.check_run_id {
205-
let (status, conclusion) = if has_failure {
206-
(CheckRunStatus::Completed, Some(CheckRunConclusion::Failure))
238+
tracing::info!(
239+
"Updating check run {check_run_id} for build {} on {branch}",
240+
build.commit_sha,
241+
);
242+
let conclusion = if build_succeeded {
243+
Some(CheckRunConclusion::Success)
207244
} else {
208-
(CheckRunStatus::Completed, Some(CheckRunConclusion::Success))
245+
Some(CheckRunConclusion::Failure)
209246
};
210247

211248
if let Err(error) = repo
212249
.client
213-
.update_check_run(check_run_id as u64, status, conclusion, None)
250+
.update_check_run(
251+
check_run_id as u64,
252+
CheckRunStatus::Completed,
253+
conclusion,
254+
None,
255+
)
214256
.await
215257
{
216258
tracing::error!("Could not update check run {check_run_id}: {error:?}");
217259
}
218-
}
219-
220-
let message = if !has_failure {
221-
tracing::info!("Workflow succeeded");
222-
try_build_succeeded_comment(&workflows, payload.commit_sha, &build)
223260
} else {
224-
tracing::info!("Workflow failed");
225-
workflow_failed_comment(&workflows)
226-
};
227-
repo.client.post_comment(pr.number, message).await?;
261+
tracing::warn!(
262+
"No check_run_id found for build {} on {branch}",
263+
build.commit_sha,
264+
);
265+
}
228266

229267
// Trigger merge queue when an auto build completes
230-
if payload.branch == AUTO_BRANCH_NAME {
268+
if branch == AUTO_BRANCH_NAME {
231269
merge_queue_tx.trigger().await?;
232270
}
233271

@@ -236,8 +274,10 @@ async fn try_complete_build(
236274

237275
#[cfg(test)]
238276
mod tests {
277+
use octocrab::params::checks::{CheckRunConclusion, CheckRunStatus};
278+
239279
use crate::bors::handlers::trybuild::TRY_BRANCH_NAME;
240-
use crate::bors::merge_queue::{AUTO_BUILD_CHECK_RUN_NAME, AUTO_BRANCH_NAME};
280+
use crate::bors::merge_queue::{AUTO_BRANCH_NAME, AUTO_BUILD_CHECK_RUN_NAME};
241281
use crate::database::WorkflowStatus;
242282
use crate::database::operations::get_all_workflows;
243283
use crate::tests::mocks::{
@@ -424,4 +464,97 @@ mod tests {
424464
})
425465
.await;
426466
}
467+
468+
#[sqlx::test]
469+
async fn auto_build_success_comment(pool: sqlx::PgPool) {
470+
run_test(pool, |mut tester| async {
471+
tester.create_branch(AUTO_BRANCH_NAME).expect_suites(1);
472+
473+
tester.post_comment("@bors r+").await?;
474+
tester.expect_comments(1).await;
475+
476+
tester.process_merge_queue().await;
477+
tester.expect_comments(1).await;
478+
479+
tester.workflow_success(tester.auto_branch()).await?;
480+
tester.process_merge_queue().await;
481+
insta::assert_snapshot!(
482+
tester.get_comment().await?,
483+
@r"
484+
:sunny: Test successful - [Workflow1](https://github.com/workflows/Workflow1/1)
485+
Approved by: `default-user`
486+
Pushing merge-main-sha1-pr-1-sha-0 to `main`...
487+
"
488+
);
489+
490+
Ok(tester)
491+
})
492+
.await;
493+
}
494+
495+
#[sqlx::test]
496+
async fn auto_build_check_run_success(pool: sqlx::PgPool) {
497+
run_test(pool, |mut tester| async {
498+
tester.create_branch(AUTO_BRANCH_NAME).expect_suites(1);
499+
500+
tester.post_comment("@bors r+").await?;
501+
tester.expect_comments(1).await;
502+
503+
tester.process_merge_queue().await;
504+
tester.expect_comments(1).await;
505+
506+
tester.workflow_success(tester.auto_branch()).await?;
507+
tester.process_merge_queue().await;
508+
tester.expect_comments(1).await;
509+
510+
tester.expect_check_run(
511+
&tester.default_pr().await.get_gh_pr().head_sha,
512+
AUTO_BUILD_CHECK_RUN_NAME,
513+
AUTO_BUILD_CHECK_RUN_NAME,
514+
CheckRunStatus::Completed,
515+
Some(CheckRunConclusion::Success),
516+
);
517+
518+
Ok(tester)
519+
})
520+
.await;
521+
}
522+
523+
#[sqlx::test]
524+
async fn auto_build_success_labels(pool: sqlx::PgPool) {
525+
let github = GitHubState::default().with_default_config(
526+
r#"
527+
merge_queue_enabled = true
528+
529+
[labels]
530+
succeeded = ["+foo", "+bar", "-baz"]
531+
"#,
532+
);
533+
534+
BorsBuilder::new(pool)
535+
.github(github)
536+
.run_test(|mut tester| async {
537+
tester.create_branch(AUTO_BRANCH_NAME).expect_suites(1);
538+
tester.post_comment("@bors r+").await?;
539+
tester.expect_comments(1).await;
540+
541+
tester.process_merge_queue().await;
542+
tester.expect_comments(1).await;
543+
544+
let repo = tester.default_repo();
545+
repo.lock()
546+
.get_pr(default_pr_number())
547+
.check_added_labels(&[]);
548+
tester.workflow_success(tester.auto_branch()).await?;
549+
tester.process_merge_queue().await;
550+
tester.expect_comments(1).await;
551+
552+
let pr = repo.lock().get_pr(default_pr_number()).clone();
553+
pr.check_added_labels(&["foo", "bar"]);
554+
pr.check_removed_labels(&["baz"]);
555+
556+
Ok(tester)
557+
})
558+
.await;
559+
}
427560
}

src/bors/merge_queue.rs

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ use tokio::sync::mpsc;
66
use tracing::Instrument;
77

88
use crate::BorsContext;
9-
use crate::bors::RepositoryState;
10-
use crate::bors::comment::auto_build_started_comment;
9+
use crate::bors::comment::{auto_build_started_comment, auto_build_succeeded_comment};
10+
use crate::bors::{PullRequestStatus, RepositoryState};
1111
use crate::database::{BuildStatus, PullRequestModel};
1212
use crate::github::api::client::GithubRepositoryClient;
1313
use crate::github::api::operations::ForcePush;
@@ -84,9 +84,39 @@ pub async fn merge_queue_tick(ctx: Arc<BorsContext>) -> anyhow::Result<()> {
8484
let pr_num = pr.number;
8585

8686
if let Some(auto_build) = &pr.auto_build {
87+
let commit_sha = CommitSha(auto_build.commit_sha.clone());
88+
8789
match auto_build.status {
8890
// Build successful - point the base branch to the merged commit.
8991
BuildStatus::Success => {
92+
let workflows = ctx.db.get_workflows_for_build(auto_build).await?;
93+
let comment = auto_build_succeeded_comment(
94+
&workflows,
95+
pr.approver().unwrap_or("<unknown>"),
96+
&commit_sha,
97+
&pr.base_branch,
98+
);
99+
repo.client.post_comment(pr.number, comment).await?;
100+
101+
match repo
102+
.client
103+
.set_branch_to_sha(&pr.base_branch, &commit_sha, ForcePush::No)
104+
.await
105+
{
106+
Ok(()) => {
107+
tracing::info!("Auto build succeeded and merged for PR {pr_num}");
108+
109+
ctx.db
110+
.set_pr_status(
111+
&pr.repository,
112+
pr.number,
113+
PullRequestStatus::Merged,
114+
)
115+
.await?;
116+
}
117+
Err(_) => {}
118+
};
119+
90120
// Break to give GitHub time to update the base branch.
91121
break;
92122
}
@@ -337,7 +367,7 @@ mod tests {
337367
}
338368

339369
#[sqlx::test]
340-
async fn auto_workflow_started_comment(pool: sqlx::PgPool) {
370+
async fn auto_build_started_comment(pool: sqlx::PgPool) {
341371
run_test(pool, |mut tester| async {
342372
tester.create_branch(AUTO_BRANCH_NAME).expect_suites(1);
343373

src/config.rs

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,8 @@ where
6969
Try,
7070
TrySucceed,
7171
TryFailed,
72+
Succeeded,
73+
Failed,
7274
}
7375

7476
impl From<Trigger> for LabelTrigger {
@@ -79,6 +81,8 @@ where
7981
Trigger::Try => LabelTrigger::TryBuildStarted,
8082
Trigger::TrySucceed => LabelTrigger::TryBuildSucceeded,
8183
Trigger::TryFailed => LabelTrigger::TryBuildFailed,
84+
Trigger::Succeeded => LabelTrigger::Succeeded,
85+
Trigger::Failed => LabelTrigger::Failed,
8286
}
8387
}
8488
}
@@ -206,9 +210,11 @@ approve = ["+approved"]
206210
try = ["+foo", "-bar"]
207211
try_succeed = ["+foobar", "+foo", "+baz"]
208212
try_failed = []
213+
auto_succeed = ["+foobar", "-foo"]
214+
auto_failed = ["+bar", "+baz"]
209215
"#;
210216
let config = load_config(content);
211-
insta::assert_debug_snapshot!(config.labels.into_iter().collect::<BTreeMap<_, _>>(), @r###"
217+
insta::assert_debug_snapshot!(config.labels.into_iter().collect::<BTreeMap<_, _>>(), @r#"
212218
{
213219
Approved: [
214220
Add(
@@ -240,8 +246,24 @@ try_failed = []
240246
),
241247
],
242248
TryBuildFailed: [],
249+
AutoBuildSucceeded: [
250+
Add(
251+
"foobar",
252+
),
253+
Remove(
254+
"foo",
255+
),
256+
],
257+
AutoBuildFailed: [
258+
Add(
259+
"bar",
260+
),
261+
Add(
262+
"baz",
263+
),
264+
],
243265
}
244-
"###);
266+
"#);
245267
}
246268

247269
#[test]

src/github/labels.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ pub enum LabelTrigger {
66
TryBuildStarted,
77
TryBuildSucceeded,
88
TryBuildFailed,
9+
Succeeded,
10+
Failed,
911
}
1012

1113
#[derive(Debug, Eq, PartialEq)]

0 commit comments

Comments
 (0)