Skip to content

Commit 5475492

Browse files
Create a separate "Flutter Presubmits" checkrun for the unified checkrun flow. (#5022)
This allows us to add "Flutter Presubmits" to the required checks for release candidate branches as well, and have a less confusing name (since release candidate branches don't use the merge queue). Merge Queue Guard is still created but immediately completed when unified checkruns are created, because this is a required check. Similarly, "Flutter Presubmits" is always created, and if the unified checkruns are disabled, we immediately complete it as well. This allows us to make this a required check for the `master` branch and the release candidate branches as well.
1 parent 0ed7595 commit 5475492

7 files changed

Lines changed: 248 additions & 25 deletions

File tree

app_dart/lib/src/model/common/presubmit_completed_check.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ class PresubmitCompletedJob {
118118
cocoon_checks.CheckRun get checkRun {
119119
return cocoon_checks.CheckRun(
120120
id: checkRunId,
121-
name: isUnifiedCheckRun ? Config.kMergeQueueLockName : name,
121+
name: isUnifiedCheckRun ? Config.kFlutterPresubmitsName : name,
122122
headSha: sha,
123123
conclusion: status.toConclusion(),
124124
checkSuite: CheckSuite(

app_dart/lib/src/service/config.dart

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,10 @@ interface class Config extends DynamicallyUpdatedConfig {
8181
/// workflow.
8282
static const String kMergeQueueLockName = 'Merge Queue Guard';
8383

84+
/// A required check that stays in pending state until all presubmit checks pass
85+
/// for users opted into the unified checkrun flow.
86+
static const String kFlutterPresubmitsName = 'Flutter Presubmits';
87+
8488
final CacheService _cache;
8589
final SecretManager _secrets;
8690
final http.Client _httpClient;

app_dart/lib/src/service/scheduler.dart

Lines changed: 56 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,10 @@ class Scheduler {
117117
'If you need to merge your PR without tests (a rare situation, typically '
118118
'an emergency), then you can use the `emergency` label.';
119119

120+
/// Briefly describes what the "Flutter Presubmits" check is for.
121+
static const String kFlutterPresubmitsDescription =
122+
'Flutter Presubmits unified check run.';
123+
120124
/// Ensure [commits] exist in Cocoon.
121125
///
122126
/// If the commit already exists, it is ignored.
@@ -332,6 +336,10 @@ class Scheduler {
332336

333337
final slug = pullRequest.base!.repo!.slug();
334338

339+
final isUnifiedCheckRun = _config.flags.isUnifiedCheckRunFlowEnabledForUser(
340+
pullRequest.user!.login!,
341+
);
342+
335343
// The MQ only waits for "required status checks" before deciding whether to
336344
// merge the PR into the target branch. This required check added to both
337345
// the PR and to the merge group, and so it must be completed in both cases.
@@ -340,12 +348,10 @@ class Scheduler {
340348
pullRequest.head!.sha!,
341349
// Override details url of merge queue guard check for users with unified
342350
// check run flow enabled
343-
detailsUrl:
344-
_config.flags.isUnifiedCheckRunFlowEnabledForUser(
345-
pullRequest.user!.login!,
346-
)
351+
detailsUrl: isUnifiedCheckRun
347352
? 'https://flutter-dashboard.appspot.com/#/presubmit?repo=${slug.name}&sha=${pullRequest.head!.sha}'
348353
: null,
354+
isUnifiedCheckRun: isUnifiedCheckRun,
349355
);
350356

351357
// Track if we should unlock the merge group lock in case of non-fusion or
@@ -611,7 +617,11 @@ class Scheduler {
611617
'${contentHash != null ? ', contentHash: $contentHash' : ''})';
612618
log.info('$logCrumb: scheduling merge group checks');
613619

614-
final lock = await lockMergeGroupChecks(slug, headSha);
620+
final lock = await lockMergeGroupChecks(
621+
slug,
622+
headSha,
623+
isUnifiedCheckRun: false,
624+
);
615625

616626
// If the repo is not fusion, it doesn't run anything in the MQ, so just
617627
// close the merge group guard.
@@ -827,8 +837,9 @@ $s
827837
RepositorySlug slug,
828838
String headSha, {
829839
String? detailsUrl,
840+
required bool isUnifiedCheckRun,
830841
}) async {
831-
return _githubChecksService.githubChecksUtil.createCheckRun(
842+
final mqGuard = await _githubChecksService.githubChecksUtil.createCheckRun(
832843
_config,
833844
slug,
834845
headSha,
@@ -837,8 +848,43 @@ $s
837848
title: Config.kMergeQueueLockName,
838849
summary: kMergeQueueLockDescription,
839850
),
840-
detailsUrl: detailsUrl,
851+
detailsUrl: isUnifiedCheckRun ? null : detailsUrl,
841852
);
853+
854+
final flutterPresubmits = await _githubChecksService.githubChecksUtil
855+
.createCheckRun(
856+
_config,
857+
slug,
858+
headSha,
859+
Config.kFlutterPresubmitsName,
860+
output: const CheckRunOutput(
861+
title: Config.kFlutterPresubmitsName,
862+
summary: kFlutterPresubmitsDescription,
863+
),
864+
detailsUrl: isUnifiedCheckRun ? detailsUrl : null,
865+
);
866+
867+
if (isUnifiedCheckRun) {
868+
// Skip MQ Guard
869+
await _githubChecksService.githubChecksUtil.updateCheckRun(
870+
_config,
871+
slug,
872+
mqGuard,
873+
status: CheckRunStatus.completed,
874+
conclusion: CheckRunConclusion.success,
875+
);
876+
return flutterPresubmits;
877+
} else {
878+
// Skip Flutter Presubmits
879+
await _githubChecksService.githubChecksUtil.updateCheckRun(
880+
_config,
881+
slug,
882+
flutterPresubmits,
883+
status: CheckRunStatus.completed,
884+
conclusion: CheckRunConclusion.success,
885+
);
886+
return mqGuard;
887+
}
842888
}
843889

844890
/// Creates a pending check run for "Awaiting CICD label" if it doesn't exist.
@@ -1592,11 +1638,12 @@ $stacktrace
15921638
);
15931639
final name = checkRunEvent.checkRun!.name;
15941640
var success = false;
1595-
if (name == Config.kMergeQueueLockName) {
1641+
if (name == Config.kMergeQueueLockName ||
1642+
name == Config.kFlutterPresubmitsName) {
15961643
final slug = checkRunEvent.repository!.slug();
15971644
final checkSuiteId = checkRunEvent.checkRun!.checkSuite!.id!;
15981645
log.debug(
1599-
'$logCrumb: Requested re-run of "${Config.kMergeQueueLockName}" for '
1646+
'$logCrumb: Requested re-run of "$name" for '
16001647
'$slug / $checkSuiteId - ignoring',
16011648
);
16021649
success = true;

app_dart/test/model/common/presubmit_completed_check_test.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ void main() {
7272
expect(check.checkSuiteId, 456);
7373
expect(check.headBranch, 'gh-readonly-queue/master/pr-123-abc');
7474
expect(check.isUnifiedCheckRun, true);
75-
expect(check.checkRun.name, Config.kMergeQueueLockName);
75+
expect(check.checkRun.name, Config.kFlutterPresubmitsName);
7676
expect(check.buildNumber, 0);
7777
});
7878

app_dart/test/request_handlers/github/webhook_subscription_test.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3351,7 +3351,7 @@ void foo() {
33513351
status: CheckRunStatus.completed,
33523352
conclusion: CheckRunConclusion.success,
33533353
),
3354-
).called(1);
3354+
).called(2);
33553355

33563356
expect(
33573357
log,

0 commit comments

Comments
 (0)