Skip to content

Commit b0c49b5

Browse files
committed
feat: add option to wait for ci on approval
1 parent 417c8e7 commit b0c49b5

File tree

4 files changed

+89
-5
lines changed

4 files changed

+89
-5
lines changed

src/bors/comment.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,10 @@ pub fn unclean_try_build_cancelled_comment() -> Comment {
7070
)
7171
}
7272

73+
pub fn unapproved_pr_comment() -> Comment {
74+
Comment::new(":x: CI checks failed, removing approval.".to_string())
75+
}
76+
7377
pub fn try_build_cancelled_comment(workflow_urls: impl Iterator<Item = String>) -> Comment {
7478
let mut try_build_cancelled_comment = r#"Try build cancelled.
7579
Cancelled workflows:"#

src/bors/handlers/review.rs

Lines changed: 60 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use crate::bors::command::Approver;
44
use crate::bors::event::PullRequestEdited;
55
use crate::bors::event::PullRequestPushed;
66
use crate::bors::handlers::labels::handle_label_trigger;
7+
use crate::bors::CheckSuiteStatus;
78
use crate::bors::Comment;
89
use crate::bors::RepositoryState;
910
use crate::github::CommitSha;
@@ -24,14 +25,43 @@ pub(super) async fn command_approve(
2425
approver: &Approver,
2526
) -> anyhow::Result<()> {
2627
tracing::info!("Approving PR {}", pr.number);
28+
2729
if !sufficient_approve_permission(repo_state.clone(), author) {
2830
deny_approve_request(&repo_state, pr, author).await?;
2931
return Ok(());
3032
};
33+
3134
let approver = match approver {
3235
Approver::Myself => author.username.clone(),
3336
Approver::Specified(approver) => approver.clone(),
3437
};
38+
39+
if repo_state.config.load().wait_on_ci_approval {
40+
// Get all the check suites for the PR.
41+
let checks = repo_state
42+
.client
43+
.get_check_suites_for_commit(&pr.head.name, &pr.head.sha)
44+
.await?;
45+
46+
// Check suite failed, don't approve.
47+
if checks
48+
.iter()
49+
.any(|check| matches!(check.status, CheckSuiteStatus::Failure))
50+
{
51+
notify_of_rejected_approval(&repo_state, pr.number).await?;
52+
return Ok(());
53+
}
54+
55+
// Check suite still running, notify user.
56+
if checks
57+
.iter()
58+
.any(|check| matches!(check.status, CheckSuiteStatus::Pending))
59+
{
60+
notify_of_pending_approval(&repo_state, pr.number).await?;
61+
return Ok(());
62+
}
63+
}
64+
3565
db.approve(repo_state.repository(), pr.number, approver.as_str())
3666
.await?;
3767
handle_label_trigger(&repo_state, pr.number, LabelTrigger::Approved).await?;
@@ -191,7 +221,7 @@ async fn notify_of_edited_pr(
191221
.post_comment(
192222
pr_number,
193223
Comment::new(format!(
194-
r#":warning: The base branch changed to `{base_name}`, and the
224+
r#":warning: The base branch changed to `{base_name}`, and the
195225
PR will need to be re-approved."#,
196226
)),
197227
)
@@ -207,14 +237,40 @@ async fn notify_of_pushed_pr(
207237
.post_comment(
208238
pr_number,
209239
Comment::new(format!(
210-
r#":warning: A new commit `{}` was pushed to the branch, the
240+
r#":warning: A new commit `{}` was pushed to the branch, the
211241
PR will need to be re-approved."#,
212242
head_sha
213243
)),
214244
)
215245
.await
216246
}
217247

248+
async fn notify_of_rejected_approval(
249+
repo: &RepositoryState,
250+
pr_number: PullRequestNumber,
251+
) -> anyhow::Result<()> {
252+
repo.client
253+
.post_comment(
254+
pr_number,
255+
Comment::new(format!(":x: Approval rejected, CI checks have failed.")),
256+
)
257+
.await
258+
}
259+
260+
async fn notify_of_pending_approval(
261+
repo: &RepositoryState,
262+
pr_number: PullRequestNumber,
263+
) -> anyhow::Result<()> {
264+
repo.client
265+
.post_comment(
266+
pr_number,
267+
Comment::new(format!(
268+
":hourglass_flowing_sand: Waiting for CI checks to complete before approving..."
269+
)),
270+
)
271+
.await
272+
}
273+
218274
#[cfg(test)]
219275
mod tests {
220276
use crate::{
@@ -348,7 +404,7 @@ mod tests {
348404

349405
assert_eq!(
350406
tester.get_comment().await?,
351-
r#":warning: The base branch changed to `main`, and the
407+
r#":warning: The base branch changed to `main`, and the
352408
PR will need to be re-approved."#,
353409
);
354410
check_pr_unapproved(&tester, default_pr_number().into()).await;
@@ -428,7 +484,7 @@ PR will need to be re-approved."#,
428484
assert_eq!(
429485
tester.get_comment().await?,
430486
format!(
431-
r#":warning: A new commit `pr-{}-sha` was pushed to the branch, the
487+
r#":warning: A new commit `pr-{}-sha` was pushed to the branch, the
432488
PR will need to be re-approved."#,
433489
default_pr_number()
434490
)

src/bors/handlers/workflow.rs

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
use std::sync::Arc;
22
use std::time::Duration;
33

4-
use crate::bors::comment::{try_build_succeeded_comment, workflow_failed_comment};
4+
use crate::bors::comment::{
5+
try_build_succeeded_comment, unapproved_pr_comment, workflow_failed_comment,
6+
};
57
use crate::bors::event::{CheckSuiteCompleted, WorkflowCompleted, WorkflowStarted};
68
use crate::bors::handlers::is_bors_observed_branch;
79
use crate::bors::handlers::labels::handle_label_trigger;
@@ -169,6 +171,22 @@ async fn try_complete_build(
169171
.iter()
170172
.any(|check| matches!(check.status, CheckSuiteStatus::Failure));
171173

174+
// Check suite failed, unapprove PR.
175+
if repo.config.load().wait_on_ci_approval && pr.approved_by.is_some() && has_failure {
176+
tracing::info!(
177+
"Unapproving PR {} due to CI failure on commit {}",
178+
pr.number,
179+
payload.commit_sha
180+
);
181+
182+
db.unapprove(repo.repository(), pr.number).await?;
183+
handle_label_trigger(repo, pr.number, LabelTrigger::Unapproved).await?;
184+
185+
repo.client
186+
.post_comment(pr.number, unapproved_pr_comment())
187+
.await?;
188+
}
189+
172190
let mut workflows = db.get_workflows_for_build(&build).await?;
173191
workflows.sort_by(|a, b| a.name.cmp(&b.name));
174192

src/config.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,18 @@ pub struct RepositoryConfig {
2121
pub labels: HashMap<LabelTrigger, Vec<LabelModification>>,
2222
#[serde(default, deserialize_with = "deserialize_duration_from_secs_opt")]
2323
pub min_ci_time: Option<Duration>,
24+
#[serde(default = "default_wait_on_ci_approval")]
25+
pub wait_on_ci_approval: bool,
2426
}
2527

2628
fn default_timeout() -> Duration {
2729
Duration::from_secs(3600)
2830
}
2931

32+
fn default_wait_on_ci_approval() -> bool {
33+
true
34+
}
35+
3036
fn deserialize_duration_from_secs_opt<'de, D>(deserializer: D) -> Result<Option<Duration>, D::Error>
3137
where
3238
D: Deserializer<'de>,

0 commit comments

Comments
 (0)