From 20901285d498c839416ea74c24e965cb18c8e3d2 Mon Sep 17 00:00:00 2001 From: "auto-submit[bot]" Date: Fri, 16 May 2025 16:05:53 +0000 Subject: [PATCH] Revert "Conditionally lock MQG on `flutter/flutter`+`master` only. (#4685)" This reverts commit dd59cbb2d6ec8f0335376eb0bd226beeb17b0ab9. --- app_dart/lib/src/service/scheduler.dart | 44 +++++++++-------------- app_dart/test/service/scheduler_test.dart | 22 +++++++++++- 2 files changed, 37 insertions(+), 29 deletions(-) diff --git a/app_dart/lib/src/service/scheduler.dart b/app_dart/lib/src/service/scheduler.dart index 16938aaff3..2ac3487e13 100644 --- a/app_dart/lib/src/service/scheduler.dart +++ b/app_dart/lib/src/service/scheduler.dart @@ -325,35 +325,27 @@ class Scheduler { await cancelPreSubmitTargets(pullRequest: pullRequest, reason: reason); final slug = pullRequest.base!.repo!.slug(); - final isFusion = slug == Config.flutterSlug; // The MQ only waits for "required status checks" before deciding whether to // merge the PR into the target branch. This required check added to both // the PR and to the merge group, and so it must be completed in both cases. - final CheckRun? lock; - { - final prBranch = pullRequest.head!.ref; - if (isFusion && Config.defaultBranch(slug) == prBranch) { - lock = await _lockMergeGroupChecks(slug, pullRequest.head!.sha!); - } else { - lock = null; - log.debug('Skipping merge queue guard: $slug/$prBranch'); - } - } + final lock = await lockMergeGroupChecks(slug, pullRequest.head!.sha!); - // Track if we should unlock the merge group lock in case of rever bots. - // This is different than simply not having a lock - because flutter/flutter - // master requires to see the lock existing and have been passed (versus - // non-existence). + // Track if we should unlock the merge group lock in case of non-fusion or + // revert bots. var unlockMergeGroup = false; final ciValidationCheckRun = await _createCiYamlCheckRun(pullRequest, slug); log.info('Creating presubmit targets for ${pullRequest.number}'); Object? exception; + final isFusion = slug == Config.flutterSlug; do { try { final sha = pullRequest.head!.sha!; + if (!isFusion) { + unlockMergeGroup = true; + } // Both the author and label should be checked to make sure that no one is // attempting to get a pull request without check through. @@ -389,7 +381,7 @@ class Scheduler { await _runCiTestingStage( pullRequest: pullRequest, - checkRunGuard: lock?.toString() ?? '', + checkRunGuard: '$lock', logCrumb: logCrumb, // The if-branch already skips the engine build phase. @@ -478,8 +470,8 @@ class Scheduler { // Normally the lock stays pending until the PR is ready to be enqueued, but // there are situations (see code above) when it needs to be unlocked // immediately. - if (lock != null && unlockMergeGroup) { - await _unlockMergeQueueGuard(slug, pullRequest.head!.sha!, lock); + if (unlockMergeGroup) { + await unlockMergeQueueGuard(slug, pullRequest.head!.sha!, lock); } log.info( 'Finished triggering builds for: pr ${pullRequest.number}, commit ${pullRequest.base!.sha}, branch ${pullRequest.head!.ref} and slug $slug}', @@ -596,12 +588,12 @@ class Scheduler { 'triggerTargetsForMergeGroup($slug, $headSha, ${isFusion ? 'real' : 'simulated'})'; log.info('$logCrumb: scheduling merge group checks'); - final lock = await _lockMergeGroupChecks(slug, headSha); + final lock = await lockMergeGroupChecks(slug, headSha); // If the repo is not fusion, it doesn't run anything in the MQ, so just // close the merge group guard. if (!isFusion) { - await _unlockMergeQueueGuard(slug, headSha, lock); + await unlockMergeQueueGuard(slug, headSha, lock); return; } @@ -794,7 +786,7 @@ $s /// While this check is still in progress, the merge queue will not merge the /// respective PR onto the target branch (e.g. main or master), because this /// check is "required". - Future _lockMergeGroupChecks( + Future lockMergeGroupChecks( RepositorySlug slug, String headSha, ) async { @@ -818,7 +810,7 @@ $s /// /// If the guard is guarding a pull request, this immediately makes the pull /// request eligible for enqueuing into the merge queue. - Future _unlockMergeQueueGuard( + Future unlockMergeQueueGuard( RepositorySlug slug, String headSha, CheckRun lock, @@ -1177,11 +1169,7 @@ $s required String logCrumb, }) async { log.info('$logCrumb: Stage completed: ${CiStage.fusionTests}'); - await _unlockMergeQueueGuard( - slug, - sha, - checkRunFromString(mergeQueueGuard), - ); + await unlockMergeQueueGuard(slug, sha, checkRunFromString(mergeQueueGuard)); } /// Returns the presubmit targets for the fusion repo [pullRequest] that should run for the given [stage]. @@ -1214,7 +1202,7 @@ $s // Unlock the guarding check_run. final checkRunGuard = checkRunFromString(mergeQueueGuard); - await _unlockMergeQueueGuard(slug, sha, checkRunGuard); + await unlockMergeQueueGuard(slug, sha, checkRunGuard); } /// Schedules post-engine build tests (i.e. engine tests, and framework tests). diff --git a/app_dart/test/service/scheduler_test.dart b/app_dart/test/service/scheduler_test.dart index f2c21cd46b..ebe117d578 100644 --- a/app_dart/test/service/scheduler_test.dart +++ b/app_dart/test/service/scheduler_test.dart @@ -2285,6 +2285,11 @@ targets: ), ).captured, [ + Config.kMergeQueueLockName, + const CheckRunOutput( + title: Config.kMergeQueueLockName, + summary: Scheduler.kMergeQueueLockDescription, + ), Config.kCiYamlCheckName, const CheckRunOutput( title: Config.kCiYamlCheckName, @@ -2466,6 +2471,11 @@ targets: ), ).captured, [ + Config.kMergeQueueLockName, + const CheckRunOutput( + title: Config.kMergeQueueLockName, + summary: Scheduler.kMergeQueueLockDescription, + ), Config.kCiYamlCheckName, const CheckRunOutput( title: Config.kCiYamlCheckName, @@ -2502,6 +2512,11 @@ targets: ), ).captured, [ + Config.kMergeQueueLockName, + const CheckRunOutput( + title: Config.kMergeQueueLockName, + summary: Scheduler.kMergeQueueLockDescription, + ), Config.kCiYamlCheckName, const CheckRunOutput( title: Config.kCiYamlCheckName, @@ -2532,7 +2547,12 @@ targets: output: anyNamed('output'), ), ).captured, - [CheckRunStatus.completed, CheckRunConclusion.success], + [ + CheckRunStatus.completed, + CheckRunConclusion.success, + CheckRunStatus.completed, + CheckRunConclusion.success, + ], ); });