Skip to content

Commit 679f51f

Browse files
authored
Add a feature that allows treating presubmit: false as true, conditionally. (#3256)
Closes flutter/flutter#138430 and I believe flutter/flutter#111418. As @CaseyHillers mentions in flutter/flutter#138430, this would _not_ be sufficient to understand the addition of a label (only if the PR is _first created_ with a label, or a commit is pushed after the label is added), but it might be worth merging as-is (assuming I got all of the assumptions correct).
1 parent 818801e commit 679f51f

File tree

2 files changed

+98
-4
lines changed

2 files changed

+98
-4
lines changed

app_dart/lib/src/service/scheduler.dart

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -436,10 +436,25 @@ class Scheduler {
436436
log.info('Collecting presubmit targets for ${pullRequest.number}');
437437

438438
// Filter out schedulers targets with schedulers different than luci or cocoon.
439-
final Iterable<Target> presubmitTargets = ciYaml.presubmitTargets.where(
440-
(Target target) =>
441-
target.value.scheduler == pb.SchedulerSystem.luci || target.value.scheduler == pb.SchedulerSystem.cocoon,
442-
);
439+
final List<Target> presubmitTargets = ciYaml.presubmitTargets
440+
.where(
441+
(Target target) =>
442+
target.value.scheduler == pb.SchedulerSystem.luci || target.value.scheduler == pb.SchedulerSystem.cocoon,
443+
)
444+
.toList();
445+
446+
// See https://github.com/flutter/flutter/issues/138430.
447+
if (_includePostsubmitAsPresubmit(ciYaml, pullRequest)) {
448+
log.info('Including postsubmit targets as presubmit for ${pullRequest.number}');
449+
450+
for (Target target in ciYaml.postsubmitTargets) {
451+
// We don't want to include a presubmit twice
452+
// We don't want to run the builder_cache target as a presubmit
453+
if (!target.value.presubmit && !target.value.properties.containsKey('cache_name')) {
454+
presubmitTargets.add(target);
455+
}
456+
}
457+
}
443458

444459
log.info('Collected ${presubmitTargets.length} presubmit targets.');
445460
// Release branches should run every test.
@@ -462,6 +477,19 @@ class Scheduler {
462477
return getTargetsToRun(presubmitTargets, files);
463478
}
464479

480+
/// Returns `true` if [ciYaml.postsubmitTargets] should be ran during presubmit.
481+
static bool _includePostsubmitAsPresubmit(CiYaml ciYaml, PullRequest pullRequest) {
482+
// Only allow this for flutter/engine.
483+
// See https://github.com/flutter/cocoon/pull/3256#issuecomment-1811624351.
484+
if (ciYaml.slug != Config.engineSlug) {
485+
return false;
486+
}
487+
if (pullRequest.labels?.any((label) => label.name.contains('test: all')) ?? false) {
488+
return true;
489+
}
490+
return false;
491+
}
492+
465493
/// Reschedules a failed build using a [CheckRunEvent]. The CheckRunEvent is
466494
/// generated when someone clicks the re-run button from a failed build from
467495
/// the Github UI.

app_dart/test/service/scheduler_test.dart

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -690,6 +690,72 @@ targets:
690690
);
691691
});
692692

693+
group('treats postsubmit as presubmit if a label is present', () {
694+
final IssueLabel runAllTests = IssueLabel(name: 'test: all');
695+
setUp(() async {
696+
httpClient = MockClient((http.Request request) async {
697+
if (request.url.path.contains('.ci.yaml')) {
698+
return http.Response(
699+
'''
700+
enabled_branches:
701+
- master
702+
targets:
703+
- name: Linux Presubmit
704+
presubmit: true
705+
scheduler: luci
706+
- name: Linux Postsubmit
707+
presubmit: false
708+
scheduler: luci
709+
- name: Linux Cache
710+
presubmit: false
711+
scheduler: luci
712+
properties:
713+
cache_name: "builder"
714+
''',
715+
200,
716+
);
717+
}
718+
throw Exception('Failed to find ${request.url.path}');
719+
});
720+
});
721+
722+
test('with a specific label in the flutter/engine repo', () async {
723+
final enginePr = generatePullRequest(
724+
labels: <IssueLabel>[runAllTests],
725+
repo: Config.engineSlug.name,
726+
);
727+
final List<Target> presubmitTargets = await scheduler.getPresubmitTargets(enginePr);
728+
expect(
729+
presubmitTargets.map((Target target) => target.value.name).toList(),
730+
<String>['Linux Presubmit', 'Linux Postsubmit'],
731+
);
732+
});
733+
734+
test('with a specific label in the flutter/flutter repo', () async {
735+
final enginePr = generatePullRequest(
736+
labels: <IssueLabel>[runAllTests],
737+
repo: Config.flutterSlug.name,
738+
);
739+
final List<Target> presubmitTargets = await scheduler.getPresubmitTargets(enginePr);
740+
expect(
741+
presubmitTargets.map((Target target) => target.value.name).toList(),
742+
<String>['Linux Presubmit'],
743+
);
744+
});
745+
746+
test('without a specific label', () async {
747+
final enginePr = generatePullRequest(
748+
labels: <IssueLabel>[],
749+
repo: Config.engineSlug.name,
750+
);
751+
final List<Target> presubmitTargets = await scheduler.getPresubmitTargets(enginePr);
752+
expect(
753+
presubmitTargets.map((Target target) => target.value.name).toList(),
754+
(<String>['Linux Presubmit']),
755+
);
756+
});
757+
});
758+
693759
test('checks for release branches', () async {
694760
const String branch = 'flutter-1.24-candidate.1';
695761
httpClient = MockClient((http.Request request) async {

0 commit comments

Comments
 (0)