Skip to content

Commit 249ffc0

Browse files
authored
Build engine binaries in post-submit on experimental branches. (#4710)
Closes flutter/flutter#169088.
1 parent e579ae8 commit 249ffc0

4 files changed

Lines changed: 48 additions & 27 deletions

File tree

app_dart/lib/src/model/ci_yaml/ci_yaml.dart

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import 'dart:convert';
66

7+
import 'package:cocoon_common/is_release_branch.dart';
78
import 'package:collection/collection.dart';
89
import 'package:github/github.dart';
910

@@ -210,29 +211,26 @@ class CiYaml {
210211
/// Pick targets out of the given [targets] list that should be run for the
211212
/// branch that's being tested.
212213
List<Target> _selectTargetsForBranch(List<Target> targets) {
213-
final isReleaseBranch = branch.contains(RegExp('^flutter-'));
214-
215-
if (isReleaseBranch) {
216-
// For release branches we don't want to run release targets or bringup
217-
// targets because they are built outside of Cocoon. This applies in both
218-
// fusion and non-fusion repos.
214+
if (!isFusion) {
215+
// Non-flutter/flutter repos run all targets in post-submit.
216+
return targets;
217+
} else if (isReleaseCandidateBranch(branchName: branch)) {
218+
// For release branches:
219+
// bringup: true --> Always omitted.
220+
// release_build: "true" --> Built by Linux flutter_release_builder.
219221
return [
220222
...targets.where(
221223
(target) => !target.isReleaseBuild && !target.isBringup,
222224
),
223225
];
226+
} else if (branch.contains('experimental')) {
227+
// For experimental branches:
228+
// Run all targets in post-submit.
229+
// TODO(matanlurey): Codify this elsewhere once its proven.
230+
return targets;
224231
} else {
225-
// For non-release branches we also want to include bringup targets.
226-
// However, there's a difference between fusion and non-fusion repos.
227-
if (isFusion) {
228-
// Fusion repos run release targets in the MQ, so we only need to
229-
// schedule non-release targets in post-submit.
230-
return [...targets.where((target) => !target.isReleaseBuild)];
231-
} else {
232-
// Non-fusion repos run all targets in post-submit. There's no MQ to run
233-
// release builds prior to post-submit.
234-
return targets;
235-
}
232+
// For flutter/flutter branches include "master".
233+
return [...targets.where((target) => !target.isReleaseBuild)];
236234
}
237235
}
238236

app_dart/lib/src/service/scheduler.dart

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
import 'dart:convert';
66
import 'dart:math';
77

8-
import 'package:cocoon_common/is_release_branch.dart';
98
import 'package:cocoon_common/task_status.dart';
109
import 'package:cocoon_server/logging.dart';
1110
import 'package:collection/collection.dart';
@@ -152,10 +151,10 @@ class Scheduler {
152151
}
153152

154153
final _TaskCommitScheduling scheduling;
155-
if (isReleaseCandidateBranch(branchName: commit.branch)) {
156-
scheduling = _TaskCommitScheduling.releaseCandidateSkipTasksByDefault;
157-
} else {
154+
if (Config.defaultBranch(commit.slug) == commit.branch) {
158155
scheduling = _TaskCommitScheduling.defaultUseTargetSchedulingPolicy;
156+
} else {
157+
scheduling = _TaskCommitScheduling.nonDefaultBranchSkipTestsByDefault;
159158
}
160159

161160
final ciYaml = await _ciYamlFetcher.getCiYamlByCommit(
@@ -178,7 +177,13 @@ class Scheduler {
178177
final toBeScheduled = <PendingTask>[];
179178
for (var target in targets) {
180179
final task = tasks.singleWhere((task) => task.taskName == target.name);
181-
if (scheduling.skipPostsubmitTasks) {
180+
// For flutter/flutter builds (non-master branch), we mark all builds that
181+
// do not build the engine as "skipped" for later manual scheduling. Most
182+
// branches won't have any release builds at this stage, but experimental
183+
// branches *will*, which is why "!target.isReleaseBuild" is used.
184+
//
185+
// See https://github.com/flutter/flutter/issues/169088.
186+
if (scheduling.skipPostsubmitTasks && !target.isReleaseBuild) {
182187
task.setStatus(TaskStatus.skipped);
183188
continue;
184189
}
@@ -1649,11 +1654,11 @@ enum _TaskCommitScheduling {
16491654
/// Schedule according to how [Target.schedulerPolicy] is computed.
16501655
defaultUseTargetSchedulingPolicy,
16511656

1652-
/// Release candidate where tasks are skipped to be manually run either.
1653-
releaseCandidateSkipTasksByDefault;
1657+
/// Non-default branches skip tests by default for later (manual) scheduling.
1658+
nonDefaultBranchSkipTestsByDefault;
16541659

16551660
/// Whether postsubmit tasks should be initially skipped.
16561661
bool get skipPostsubmitTasks {
1657-
return this == releaseCandidateSkipTasksByDefault;
1662+
return this == nonDefaultBranchSkipTestsByDefault;
16581663
}
16591664
}

app_dart/test/service/scheduler/ci_yaml_strings.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ enabled_branches:
6262
- main
6363
- codefu
6464
- flutter-\d+\.\d+-candidate\.\d+
65+
- ios-experimental
6566
targets:
6667
- name: Linux Z
6768
properties:

app_dart/test/service/scheduler_test.dart

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -409,7 +409,7 @@ void main() {
409409
});
410410

411411
// Regression test for https://github.com/flutter/flutter/issues/168738.
412-
test('marks all tasks as new for non-release/non-master', () async {
412+
test('experimental branches build the engine, skip tests', () async {
413413
ciYamlFetcher.setCiYamlFrom(otherBranchCiYaml, engine: fusionCiYaml);
414414

415415
final mergedPr = generatePullRequest(
@@ -436,7 +436,24 @@ void main() {
436436
firestore,
437437
existsInStorage(
438438
fs.Task.metadata,
439-
everyElement(isTask.hasStatus(TaskStatus.waitingForBackfill)),
439+
unorderedEquals([
440+
// release_build: "true"
441+
isTask
442+
.hasTaskName('Linux engine_build')
443+
.hasStatus(TaskStatus.waitingForBackfill),
444+
445+
// engine tests based on engine build
446+
isTask
447+
.hasTaskName('Linux engine_presubmit')
448+
.hasStatus(TaskStatus.skipped),
449+
isTask
450+
.hasTaskName('Linux runIf engine')
451+
.hasStatus(TaskStatus.skipped),
452+
isTask.hasTaskName('Linux Z').hasStatus(TaskStatus.skipped),
453+
454+
// framework tests based on engine build
455+
isTask.hasTaskName('Linux A').hasStatus(TaskStatus.skipped),
456+
]),
440457
),
441458
);
442459
});

0 commit comments

Comments
 (0)