Skip to content
Merged
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

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

28 changes: 28 additions & 0 deletions src/bors/merge_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1900,4 +1900,32 @@ also include this pls"
})
.await;
}

#[sqlx::test]
async fn github_reopens_merged_pr(pool: sqlx::PgPool) {
// Sometimes, GitHub does not notice that a pull request was actually merged, and it still
// claims that it is open.
// We should not ever reopen a PR that was already merged in our DB though, as that can
// break merge queue invariants.
// We thus simulate what happens when we run the PR refresh job while GitHub changes its
// mind.
// See https://github.com/rust-lang/rust/pull/154327 and
// https://rust-lang.zulipchat.com/#narrow/channel/242791-t-infra/topic/Bors.20posting.20success.20message.20a.20dozen.20times/with/593477072
run_test(pool, async |ctx: &mut BorsTester| {
ctx.approve(()).await?;
ctx.start_and_finish_auto_build(()).await?;

ctx.pr(()).await.expect_status(PullRequestStatus::Merged);

// Simulate GitHub thinking that the PR has been reopened
ctx.edit_pr((), |pr| pr.reopen()).await?;
// Refresh PRs, which must not reopen the PR in the DB
ctx.refresh_prs().await;

ctx.pr(()).await.expect_status(PullRequestStatus::Merged);

Ok(())
})
.await;
}
}
11 changes: 10 additions & 1 deletion src/database/operations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,16 @@ pub(crate) async fn upsert_pull_request(
WHEN $9 = true THEN true
ELSE pull_request.mergeable_state_is_stale
END,
status = $10
-- The merged state is final, there is no going back from it.
-- Sometimes, GitHub can return inconsistent data, and claim that a merged PR
-- is open again.
-- We do not ever want to revert a merged state in the DB, as that could
-- break some invariants in the merge queue.
status =
CASE
WHEN pull_request.status = 'merged' THEN pull_request.status
ELSE $10
END
RETURNING *
)
SELECT
Expand Down