Skip to content

Commit d9c76f2

Browse files
jtmcdolejtmcdole
authored
Handle flagwaitOnContentHash (#4688)
This ONLY waits on the content hash before triggering all builds. - renames `triggerMergeGroupTargets` -> `handleMergeGroupEvent`. We want to be able to trigger targets in the merge group later from synthetic data. See `triggerTargetsForMergeGroup`. - Moves some test strings out to ci_yaml_test_strings - Moves some test check run methods to new file. - Tests really need to be broken out; follow up PR
1 parent e5a5c48 commit d9c76f2

8 files changed

Lines changed: 613 additions & 219 deletions

File tree

‎app_dart/lib/src/request_handlers/github/webhook_subscription.dart‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -323,9 +323,7 @@ final class GithubWebhookSubscription extends SubscriptionHandler {
323323
log.info(
324324
'$slug/$headSha was found on GoB mirror. Scheduling merge group tasks',
325325
);
326-
await scheduler.triggerMergeGroupTargets(
327-
mergeGroupEvent: mergeGroupEvent,
328-
);
326+
await scheduler.handleMergeGroupEvent(mergeGroupEvent: mergeGroupEvent);
329327

330328
// A merge group was deleted. This can happen when a PR is pulled from the
331329
// merge queue. All CI jobs pertaining to this merge group should be

‎app_dart/lib/src/service/scheduler.dart‎

Lines changed: 72 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -543,8 +543,45 @@ class Scheduler {
543543

544544
static Duration debugCheckPretendDelay = const Duration(minutes: 1);
545545

546-
Future<void> triggerMergeGroupTargets({
546+
Future<void> handleMergeGroupEvent({
547547
required cocoon_checks.MergeGroupEvent mergeGroupEvent,
548+
}) async {
549+
final MergeGroup(:headSha, :headRef, :baseRef) = mergeGroupEvent.mergeGroup;
550+
final slug = mergeGroupEvent.repository!.slug();
551+
final isFusion = slug == Config.flutterSlug;
552+
553+
final logCrumb =
554+
'triggerTargetsForMergeGroup($slug, $headSha, ${isFusion ? 'real' : 'simulated'})';
555+
556+
if (isFusion) {
557+
// Temporarily trigger content-aware-hash for merge groups.
558+
// We will not actually wait for the results yet.
559+
try {
560+
await _contentAwareHash.triggerWorkflow(headRef);
561+
} catch (e, s) {
562+
log.warn('contentAwareHash unexpectedly threw', e, s);
563+
}
564+
if (_config.flags.contentAwareHashing.waitOnContentHash) {
565+
log.info(
566+
'$logCrumb: content hashing requested; waiting on job to complete',
567+
);
568+
return;
569+
}
570+
}
571+
log.info('$logCrumb: scheduling merge group checks');
572+
return triggerTargetsForMergeGroup(
573+
baseRef: baseRef,
574+
headSha: headSha,
575+
headRef: headRef,
576+
slug: slug,
577+
);
578+
}
579+
580+
Future<void> triggerTargetsForMergeGroup({
581+
required String headSha,
582+
required String headRef,
583+
required String baseRef,
584+
required RepositorySlug slug,
548585
}) async {
549586
// Behave similar to addPullRequest, except we're not yet merged into master.
550587
// - We are mirrored in to GoB
@@ -553,20 +590,10 @@ class Scheduler {
553590
// - We want updates on check_runs to the presubmit pubsub.
554591
// We do not want "Task" objects because these are for flutter-dashboard tracking (post submit)
555592
// final mergeGroup = mergeGroupEvent.mergeGroup;
556-
final MergeGroup(:headSha, :headRef, :baseRef) = mergeGroupEvent.mergeGroup;
557-
final slug = mergeGroupEvent.repository!.slug();
558593
final isFusion = slug == Config.flutterSlug;
559594

560595
final logCrumb =
561-
'triggerMergeGroupTargets($slug, $headSha, ${isFusion ? 'real' : 'simulated'})';
562-
563-
// Temporarily trigger content-aware-hash for merge groups.
564-
// We will not actually wait for the results yet.
565-
try {
566-
await _contentAwareHash.triggerWorkflow(headRef);
567-
} catch (e, s) {
568-
log.warn('contentAwareHash unexpectedly threw', e, s);
569-
}
596+
'triggerTargetsForMergeGroup($slug, $headSha, ${isFusion ? 'real' : 'simulated'})';
570597
log.info('$logCrumb: scheduling merge group checks');
571598

572599
final lock = await _lockMergeGroupChecks(slug, headSha);
@@ -654,16 +681,45 @@ $s
654681
}
655682

656683
// Work in progress - Content Aware hash retrieval.
657-
Future<void> processWorkflowJob(WorkflowJobEvent job) async {
684+
Future<void> processWorkflowJob(WorkflowJobEvent event) async {
658685
try {
659-
final artifactStatus = await _contentAwareHash.processWorkflowJob(job);
686+
final artifactStatus = await _contentAwareHash.processWorkflowJob(event);
660687
log.info(
661688
'scheduler.processWorkflowJob(): artifacts status: $artifactStatus '
662-
'for ${job.workflowJob?.checkRunUrl}',
689+
'for ${event.workflowJob?.checkRunUrl}',
663690
);
691+
692+
// TODO: MergeQueueHashStatus
693+
// - .build: trigger targets!
694+
// - .wait: something is building.
695+
// - .*: do nothing
696+
// FOR NOW: trigger builds for build/wait
697+
switch (artifactStatus) {
698+
case MergeQueueHashStatus.build || MergeQueueHashStatus.wait
699+
when _config.flags.contentAwareHashing.waitOnContentHash:
700+
// Note from codefu: We do not have the merge queue lock yet.
701+
// It was short-circuited if the waitOnContentHash is set. The
702+
// CAH document _has_ been created. In the future, "wait" will need
703+
// to create the lock - and auto complete it - to let the merge
704+
// group complete.
705+
final job = event.workflowJob!;
706+
log.info(
707+
'triggering merge group targets for $artifactStatus / ${job.headSha}',
708+
);
709+
await triggerTargetsForMergeGroup(
710+
headSha: job.headSha!,
711+
headRef: 'refs/heads/${job.headBranch!}',
712+
baseRef:
713+
'refs/heads/${tryParseGitHubMergeQueueBranch(job.headBranch!).branch}',
714+
slug: event.repository!.slug(),
715+
);
716+
717+
default:
718+
break;
719+
}
664720
} catch (e, s) {
665721
log.debug(
666-
'scheduler.processWorkflowJob(${job.workflowJob?.checkRunUrl}) failed (no-op)',
722+
'scheduler.processWorkflowJob(${event.workflowJob?.checkRunUrl}) failed (no-op)',
667723
e,
668724
s,
669725
);

‎app_dart/test/request_handlers/github/webhook_subscription_test.dart‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3245,7 +3245,7 @@ void foo() {
32453245
),
32463246
logThat(
32473247
message: equals(
3248-
'triggerMergeGroupTargets(flutter/packages, c9affbbb12aa40cb3afbe94b9ea6b119a256bebf, simulated): scheduling merge group checks',
3248+
'triggerTargetsForMergeGroup(flutter/packages, c9affbbb12aa40cb3afbe94b9ea6b119a256bebf, simulated): scheduling merge group checks',
32493249
),
32503250
),
32513251
logThat(

‎app_dart/test/service/scheduler/ci_yaml_strings.dart‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
// Copyright 2025 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
const String otherBranchCiYaml = r'''
6+
enabled_branches:
7+
- ios-experimental
8+
targets:
9+
- name: Linux A
10+
''';
11+
12+
const String singleCiYaml = r'''
13+
enabled_branches:
14+
- master
15+
- main
16+
- flutter-\d+\.\d+-candidate\.\d+
17+
targets:
18+
- name: Linux A
19+
properties:
20+
custom: abc
21+
- name: Linux B
22+
enabled_branches:
23+
- stable
24+
scheduler: luci
25+
- name: Linux runIf
26+
runIf:
27+
- .ci.yaml
28+
- DEPS
29+
- dev/**
30+
- engine/**
31+
- name: Google Internal Roll
32+
postsubmit: true
33+
presubmit: false
34+
scheduler: google_internal
35+
''';
36+
37+
const String singleCiYamlWithLinuxAnalyze = r'''
38+
enabled_branches:
39+
- master
40+
- main
41+
- flutter-\d+\.\d+-candidate\.\d+
42+
targets:
43+
- name: Linux A
44+
properties:
45+
custom: abc
46+
- name: Linux B
47+
enabled_branches:
48+
- stable
49+
scheduler: luci
50+
- name: Linux runIf
51+
runIf:
52+
- .ci.yaml
53+
- DEPS
54+
- dev/**
55+
- engine/**
56+
- name: Linux analyze
57+
''';
58+
59+
const String fusionCiYaml = r'''
60+
enabled_branches:
61+
- master
62+
- main
63+
- codefu
64+
- flutter-\d+\.\d+-candidate\.\d+
65+
targets:
66+
- name: Linux Z
67+
properties:
68+
custom: abc
69+
- name: Linux Y
70+
enabled_branches:
71+
- stable
72+
scheduler: luci
73+
- name: Linux engine_presubmit
74+
- name: Linux engine_build
75+
scheduler: luci
76+
properties:
77+
release_build: "true"
78+
- name: Linux runIf engine
79+
runIf:
80+
- DEPS
81+
- engine/src/flutter/.ci.yaml
82+
- engine/src/flutter/dev/**
83+
''';
84+
85+
const String fusionDualCiYaml = r'''
86+
enabled_branches:
87+
- master
88+
- main
89+
- codefu
90+
- flutter-\d+\.\d+-candidate\.\d+
91+
targets:
92+
- name: Linux Z
93+
properties:
94+
custom: abc
95+
- name: Linux Y
96+
enabled_branches:
97+
- stable
98+
scheduler: luci
99+
- name: Linux engine_build
100+
scheduler: luci
101+
properties:
102+
release_build: "true"
103+
- name: Mac engine_build
104+
scheduler: luci
105+
properties:
106+
release_build: "true"
107+
- name: Linux runIf engine
108+
runIf:
109+
- DEPS
110+
- engine/src/flutter/.ci.yaml
111+
- engine/src/flutter/dev/**
112+
''';

‎app_dart/test/service/scheduler/create_check_run.dart‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
// Copyright 2021 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
import 'dart:convert';
6+
import 'package:github/github.dart';
7+
8+
CheckRun createCheckRun({
9+
int id = 1,
10+
String sha = '1234',
11+
String? name = 'Linux unit_test',
12+
String conclusion = 'success',
13+
String owner = 'flutter',
14+
String repo = 'flutter',
15+
String headBranch = 'master',
16+
CheckRunStatus status = CheckRunStatus.completed,
17+
int checkSuiteId = 668083231,
18+
}) {
19+
final checkRunJson = checkRunFor(
20+
id: id,
21+
sha: sha,
22+
name: name,
23+
conclusion: conclusion,
24+
owner: owner,
25+
repo: repo,
26+
headBranch: headBranch,
27+
status: status,
28+
checkSuiteId: checkSuiteId,
29+
);
30+
return CheckRun.fromJson(jsonDecode(checkRunJson) as Map<String, dynamic>);
31+
}
32+
33+
String checkRunFor({
34+
int id = 1,
35+
String sha = '1234',
36+
String? name = 'Linux unit_test',
37+
String conclusion = 'success',
38+
String owner = 'flutter',
39+
String repo = 'flutter',
40+
String headBranch = 'master',
41+
CheckRunStatus status = CheckRunStatus.completed,
42+
int checkSuiteId = 668083231,
43+
}) {
44+
final externalId = id * 2;
45+
return '''{
46+
"id": $id,
47+
"external_id": "{$externalId}",
48+
"head_sha": "$sha",
49+
"name": "$name",
50+
"conclusion": "$conclusion",
51+
"started_at": "2020-05-10T02:49:31Z",
52+
"completed_at": "2020-05-10T03:11:08Z",
53+
"status": "$status",
54+
"check_suite": {
55+
"id": $checkSuiteId,
56+
"pull_requests": [],
57+
"conclusion": "$conclusion",
58+
"head_branch": "$headBranch"
59+
}
60+
}''';
61+
}
62+
63+
String checkRunEventFor({
64+
String action = 'completed',
65+
String sha = '1234',
66+
String test = 'Linux unit_test',
67+
String conclusion = 'success',
68+
String owner = 'flutter',
69+
String repo = 'flutter',
70+
String headBranch = 'master',
71+
}) => '''{
72+
"action": "$action",
73+
"check_run": ${checkRunFor(name: test, sha: sha, conclusion: conclusion, owner: owner, repo: repo, headBranch: headBranch)},
74+
"repository": {
75+
"name": "$repo",
76+
"full_name": "$owner/$repo",
77+
"owner": {
78+
"avatar_url": "",
79+
"html_url": "",
80+
"login": "$owner",
81+
"id": 54371434
82+
}
83+
}
84+
}''';

0 commit comments

Comments
 (0)