Skip to content

Add a feature that allows treating presubmit: false as true, conditionally. #3256

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Nov 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 32 additions & 4 deletions app_dart/lib/src/service/scheduler.dart
Original file line number Diff line number Diff line change
Expand Up @@ -436,10 +436,25 @@ class Scheduler {
log.info('Collecting presubmit targets for ${pullRequest.number}');

// Filter out schedulers targets with schedulers different than luci or cocoon.
final Iterable<Target> presubmitTargets = ciYaml.presubmitTargets.where(
(Target target) =>
target.value.scheduler == pb.SchedulerSystem.luci || target.value.scheduler == pb.SchedulerSystem.cocoon,
);
final List<Target> presubmitTargets = ciYaml.presubmitTargets
.where(
(Target target) =>
target.value.scheduler == pb.SchedulerSystem.luci || target.value.scheduler == pb.SchedulerSystem.cocoon,
)
.toList();

// See https://github.com/flutter/flutter/issues/138430.
if (_includePostsubmitAsPresubmit(ciYaml, pullRequest)) {
log.info('Including postsubmit targets as presubmit for ${pullRequest.number}');

for (Target target in ciYaml.postsubmitTargets) {
// We don't want to include a presubmit twice
// We don't want to run the builder_cache target as a presubmit
if (!target.value.presubmit && !target.value.properties.containsKey('cache_name')) {
presubmitTargets.add(target);
}
}
}

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

/// Returns `true` if [ciYaml.postsubmitTargets] should be ran during presubmit.
static bool _includePostsubmitAsPresubmit(CiYaml ciYaml, PullRequest pullRequest) {
Copy link
Contributor

@ricardoamador ricardoamador Nov 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (ciYaml.slug != Config.engineSlug) {
    return false;
}

For now until the pool usage can be evaluated and we can see the effect, if any on our SLO.

// Only allow this for flutter/engine.
// See https://github.com/flutter/cocoon/pull/3256#issuecomment-1811624351.
if (ciYaml.slug != Config.engineSlug) {
return false;
}
if (pullRequest.labels?.any((label) => label.name.contains('test: all')) ?? false) {
return true;
}
return false;
}

/// Reschedules a failed build using a [CheckRunEvent]. The CheckRunEvent is
/// generated when someone clicks the re-run button from a failed build from
/// the Github UI.
Expand Down
66 changes: 66 additions & 0 deletions app_dart/test/service/scheduler_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -690,6 +690,72 @@ targets:
);
});

group('treats postsubmit as presubmit if a label is present', () {
final IssueLabel runAllTests = IssueLabel(name: 'test: all');
setUp(() async {
httpClient = MockClient((http.Request request) async {
if (request.url.path.contains('.ci.yaml')) {
return http.Response(
'''
enabled_branches:
- master
targets:
- name: Linux Presubmit
presubmit: true
scheduler: luci
- name: Linux Postsubmit
presubmit: false
scheduler: luci
- name: Linux Cache
presubmit: false
scheduler: luci
properties:
cache_name: "builder"
''',
200,
);
}
throw Exception('Failed to find ${request.url.path}');
});
});

test('with a specific label in the flutter/engine repo', () async {
final enginePr = generatePullRequest(
labels: <IssueLabel>[runAllTests],
repo: Config.engineSlug.name,
);
final List<Target> presubmitTargets = await scheduler.getPresubmitTargets(enginePr);
expect(
presubmitTargets.map((Target target) => target.value.name).toList(),
<String>['Linux Presubmit', 'Linux Postsubmit'],
);
});

test('with a specific label in the flutter/flutter repo', () async {
final enginePr = generatePullRequest(
labels: <IssueLabel>[runAllTests],
repo: Config.flutterSlug.name,
);
final List<Target> presubmitTargets = await scheduler.getPresubmitTargets(enginePr);
expect(
presubmitTargets.map((Target target) => target.value.name).toList(),
<String>['Linux Presubmit'],
);
});

test('without a specific label', () async {
final enginePr = generatePullRequest(
labels: <IssueLabel>[],
repo: Config.engineSlug.name,
);
final List<Target> presubmitTargets = await scheduler.getPresubmitTargets(enginePr);
expect(
presubmitTargets.map((Target target) => target.value.name).toList(),
(<String>['Linux Presubmit']),
);
});
});

test('checks for release branches', () async {
const String branch = 'flutter-1.24-candidate.1';
httpClient = MockClient((http.Request request) async {
Expand Down