Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/bors/comment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,12 @@ Hint: Remove **{keyword}** from this PR's title when it is ready for review.
))
}

pub fn approve_non_clean_pr() -> Comment {
Comment::new(
r":clipboard: Looks like this PR is not ready to be merged, ignoring approval.".to_string(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
r":clipboard: Looks like this PR is not ready to be merged, ignoring approval.".to_string(),
r":clipboard: This pull request has merge conflicts. These have to be resolved before the PR can be approved.".to_string(),

)
}

pub fn approve_blocking_labels_present(blocking_labels: &[&str]) -> Comment {
let labels = blocking_labels
.iter()
Expand Down
24 changes: 22 additions & 2 deletions src/bors/handlers/review.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@ use crate::bors::RepositoryState;
use crate::bors::command::RollupMode;
use crate::bors::command::{Approver, CommandPrefix};
use crate::bors::comment::{
approve_blocking_labels_present, approve_non_open_pr_comment, approve_wip_title,
approved_comment, delegate_comment, delegate_try_builds_comment, unapprove_non_open_pr_comment,
approve_blocking_labels_present, approve_non_clean_pr, approve_non_open_pr_comment,
approve_wip_title, approved_comment, delegate_comment, delegate_try_builds_comment,
unapprove_non_open_pr_comment,
};
use crate::bors::handlers::labels::handle_label_trigger;
use crate::bors::handlers::workflow::{AutoBuildCancelReason, maybe_cancel_auto_build};
Expand All @@ -20,6 +21,7 @@ use crate::github::LabelTrigger;
use crate::github::{GithubUser, PullRequestNumber};
use crate::permissions::PermissionType;
use crate::{BorsContext, PgDbClient};
use octocrab::models::pulls::MergeableState;

/// Approve a pull request.
/// A pull request can only be approved by a user of sufficient authority.
Expand Down Expand Up @@ -81,6 +83,10 @@ async fn check_pr_approval_validity(
return Ok(Some(approve_non_open_pr_comment()));
}

if matches!(pr.github.mergeable_state, MergeableState::Dirty) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do indeed treat both Dirty and Blocked as having conflicts. Could you please construct database::MergeableState from the GH state using its From impl, and then check if the resulting state is mergeable? To use the same logic everywhere. Thanks!

return Ok(Some(approve_non_clean_pr()));
}

// Check WIP title
let title = pr.github.title.to_lowercase();
if let Some(wip_kw) = WIP_KEYWORDS.iter().find(|kw| title.contains(*kw)) {
Expand Down Expand Up @@ -1214,6 +1220,20 @@ mod tests {
.await;
}

#[sqlx::test]
async fn approve_dirty_pr(pool: sqlx::PgPool) {
run_test(pool, async |tester: &mut BorsTester| {
tester.edit_pr((), |pr| pr.dirty_pull_request()).await?;
tester.post_comment("@bors r+").await?;
insta::assert_snapshot!(tester.get_next_comment_text(()).await?, @r"
:clipboard: Looks like this PR is not ready to be merged, ignoring approval.
");
tester.get_pr_copy(()).await.expect_unapproved();
Ok(())
})
.await;
}

#[sqlx::test]
async fn approve_closed_pr(pool: sqlx::PgPool) {
run_test(pool, async |tester: &mut BorsTester| {
Expand Down
4 changes: 4 additions & 0 deletions src/tests/mocks/pull_request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,10 @@ impl PullRequest {
self.status = PullRequestStatus::Draft;
}

pub fn dirty_pull_request(&mut self) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pub fn dirty_pull_request(&mut self) {
pub fn make_merge_conflict(&mut self) {

self.mergeable_state = MergeableState::Dirty;
}

pub fn add_comment_to_history(&mut self, comment: Comment) {
self.comment_history.push(comment);
}
Expand Down