diff --git a/.sqlx/query-b1fe02180d44f3cdc04ca0be76163606a84a5ed390079e4786e471b2e8850e49.json b/.sqlx/query-92d160378de8ed75192f25e393d1eceee06cd95b22cc6efe813e5d1c9812ee14.json similarity index 90% rename from .sqlx/query-b1fe02180d44f3cdc04ca0be76163606a84a5ed390079e4786e471b2e8850e49.json rename to .sqlx/query-92d160378de8ed75192f25e393d1eceee06cd95b22cc6efe813e5d1c9812ee14.json index 90148840..92c75d52 100644 --- a/.sqlx/query-b1fe02180d44f3cdc04ca0be76163606a84a5ed390079e4786e471b2e8850e49.json +++ b/.sqlx/query-92d160378de8ed75192f25e393d1eceee06cd95b22cc6efe813e5d1c9812ee14.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "\n SELECT\n pr.id,\n pr.repository as \"repository: GithubRepoName\",\n pr.number as \"number!: i64\",\n pr.title,\n pr.author,\n pr.assignees as \"assignees: Assignees\",\n (\n pr.approved_by,\n pr.approved_sha\n ) AS \"approval_status!: ApprovalStatus\",\n pr.status as \"pr_status: PullRequestStatus\",\n pr.priority,\n pr.rollup as \"rollup: RollupMode\",\n pr.delegated_permission as \"delegated_permission: DelegatedPermission\",\n pr.base_branch,\n pr.mergeable_state as \"mergeable_state: MergeableState\",\n pr.created_at as \"created_at: DateTime\",\n try_build AS \"try_build: BuildModel\",\n auto_build AS \"auto_build: BuildModel\"\n FROM pull_request as pr\n LEFT JOIN build AS try_build ON pr.try_build_id = try_build.id\n LEFT JOIN build AS auto_build ON pr.auto_build_id = auto_build.id\n WHERE pr.repository = $1\n AND pr.status = 'open'\n AND pr.approved_by IS NOT NULL\n AND pr.mergeable_state = 'mergeable'\n -- Include only PRs with pending or successful auto builds\n AND (auto_build.status IS NULL OR auto_build.status IN ('pending', 'success'))\n -- Tree closure check (if tree_priority is set)\n AND ($2::int IS NULL OR pr.priority >= $2)\n ", + "query": "\n SELECT\n pr.id,\n pr.repository as \"repository: GithubRepoName\",\n pr.number as \"number!: i64\",\n pr.title,\n pr.author,\n pr.assignees as \"assignees: Assignees\",\n (\n pr.approved_by,\n pr.approved_sha\n ) AS \"approval_status!: ApprovalStatus\",\n pr.status as \"pr_status: PullRequestStatus\",\n pr.priority,\n pr.rollup as \"rollup: RollupMode\",\n pr.delegated_permission as \"delegated_permission: DelegatedPermission\",\n pr.base_branch,\n pr.mergeable_state as \"mergeable_state: MergeableState\",\n pr.created_at as \"created_at: DateTime\",\n try_build AS \"try_build: BuildModel\",\n auto_build AS \"auto_build: BuildModel\"\n FROM pull_request as pr\n LEFT JOIN build AS try_build ON pr.try_build_id = try_build.id\n LEFT JOIN build AS auto_build ON pr.auto_build_id = auto_build.id\n WHERE pr.repository = $1\n AND pr.status = 'open'\n AND pr.approved_by IS NOT NULL\n AND pr.mergeable_state = 'mergeable'\n AND\n -- We ALWAYS need to return pending and successful PRs, regardless of tree state\n auto_build.status IN ('pending', 'success') OR (\n -- For PRs without a build status, we check if they pass the tree state\n -- priority check, if the tree is closed\n auto_build.status IS NULL AND ($2::int IS NULL OR pr.priority >= $2)\n )\n ", "describe": { "columns": [ { @@ -185,9 +185,9 @@ false, false, false, - null, - null + true, + true ] }, - "hash": "b1fe02180d44f3cdc04ca0be76163606a84a5ed390079e4786e471b2e8850e49" + "hash": "92d160378de8ed75192f25e393d1eceee06cd95b22cc6efe813e5d1c9812ee14" } diff --git a/docs/development.md b/docs/development.md index b4a7b00a..aa02a07f 100644 --- a/docs/development.md +++ b/docs/development.md @@ -137,13 +137,17 @@ Make sure to also run `cargo sqlx migrate run` to apply the migrations to the da The goal is to check that we have a test database with production-like data, so that we can test that applying migrations will not produce errors on a non-empty database. - If it doesn't make sense to add any data to the migration (e.g. if the migration only adds an index), put `-- Empty to satisfy migration tests` into the file. -### Generate `.sqlx` directory +### Regenerate `.sqlx` directory ```console +$ rm -rf .sqlx $ cargo sqlx prepare -- --all-targets +$ git add .sqlx ``` After that, you should commit the changes to the `.sqlx` directory. +Make sure to remove the `.sqlx` directory before running the `prepare` command, to ensure that leftover queries are not do not remain committed to the repository. + ## Updating commands When modifying commands, make sure to update both: diff --git a/src/bors/merge_queue.rs b/src/bors/merge_queue.rs index b61c2ec7..161ed3bf 100644 --- a/src/bors/merge_queue.rs +++ b/src/bors/merge_queue.rs @@ -1045,4 +1045,59 @@ auto_build_failed = ["+foo", "+bar", "-baz"] }) .await; } + + #[sqlx::test] + async fn finish_auto_build_while_tree_is_closed_1(pool: sqlx::PgPool) { + run_test(pool, async |tester: &mut BorsTester| { + // Start an auto build with the default priority (0) + tester.approve(()).await?; + tester.start_auto_build(()).await?; + + // Now close the tree for priority below 100 + tester.post_comment("@bors treeclosed=100").await?; + tester.expect_comments((), 1).await; + + // Then finish the auto build AFTER the tree has been closed and then + // run the merge queue + tester.finish_auto_build(()).await?; + + // And ensure that the PR was indeed merged + tester + .get_pr_copy(()) + .await + .expect_status(PullRequestStatus::Merged) + .expect_auto_build(|b| b.status == BuildStatus::Success); + Ok(()) + }) + .await; + } + + #[sqlx::test] + async fn finish_auto_build_while_tree_is_closed_2(pool: sqlx::PgPool) { + run_test(pool, async |tester: &mut BorsTester| { + // Start an auto build with the default priority (0) + tester.approve(()).await?; + tester.start_auto_build(()).await?; + + // Finish the auto build BEFORE the tree has been closed, then close the tree, + // and only then run the merge queue + tester + .workflow_full_success(tester.auto_branch().await) + .await?; + + tester.post_comment("@bors treeclosed=100").await?; + tester.expect_comments((), 1).await; + tester.process_merge_queue().await; + tester.expect_comments((), 1).await; + + // And ensure that the PR was indeed merged + tester + .get_pr_copy(()) + .await + .expect_status(PullRequestStatus::Merged) + .expect_auto_build(|b| b.status == BuildStatus::Success); + Ok(()) + }) + .await; + } } diff --git a/src/database/operations.rs b/src/database/operations.rs index a1b60014..ff394ec0 100644 --- a/src/database/operations.rs +++ b/src/database/operations.rs @@ -970,7 +970,7 @@ pub(crate) async fn update_build_check_run_id( /// Fetches pull requests eligible for merge: /// - Only approved PRs that are open and mergeable /// - Includes only PRs with pending or successful auto builds -/// - Excludes PRs that do not meet the tree closure priority threshold (if tree closed) +/// - Excludes non-pending PRs that do not meet the tree closure priority threshold (if tree closed) pub(crate) async fn get_merge_queue_prs( executor: impl PgExecutor<'_>, repo: &GithubRepoName, @@ -1007,10 +1007,13 @@ pub(crate) async fn get_merge_queue_prs( AND pr.status = 'open' AND pr.approved_by IS NOT NULL AND pr.mergeable_state = 'mergeable' - -- Include only PRs with pending or successful auto builds - AND (auto_build.status IS NULL OR auto_build.status IN ('pending', 'success')) - -- Tree closure check (if tree_priority is set) - AND ($2::int IS NULL OR pr.priority >= $2) + AND + -- We ALWAYS need to return pending and successful PRs, regardless of tree state + auto_build.status IN ('pending', 'success') OR ( + -- For PRs without a build status, we check if they pass the tree state + -- priority check, if the tree is closed + auto_build.status IS NULL AND ($2::int IS NULL OR pr.priority >= $2) + ) "#, repo as &GithubRepoName, tree_priority