Skip to content

Commit

Permalink
Merge branch 'flutter:main' into main
Browse files Browse the repository at this point in the history
  • Loading branch information
ricardoamador authored Nov 30, 2023
2 parents 1a1a89a + 3ec1a41 commit 59decbb
Show file tree
Hide file tree
Showing 53 changed files with 3,521 additions and 272 deletions.
2 changes: 2 additions & 0 deletions .github/dependabot.yml
Original file line number Diff line number Diff line change
Expand Up @@ -87,3 +87,5 @@ updates:
directory: '/gh_actions/third_party/no-response'
schedule:
interval: 'daily'
labels:
- "autosubmit"
3 changes: 2 additions & 1 deletion CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,9 @@ app_dart/lib/src/request_handlers/update_existing_flaky_issues.dart @key
app_dart/lib/src/request_handlers/update_task_status.dart @keyonghan
app_dart/lib/src/request_handlers/vacuum_github_commits.dart @keyonghan

## auto_submit app
## apps
auto_submit @ricardoamador
triage_bot @Hixie

## cipd packages
cipd_packages/codesign/** @XilaiZhang
Expand Down
2 changes: 1 addition & 1 deletion analyze/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,5 @@ dependencies:
platform: 3.1.3

dev_dependencies:
mockito: 5.4.2
mockito: 5.4.3
test_api: 0.6.1
2 changes: 1 addition & 1 deletion app_dart/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@


# Dart Docker official images can be found here: https://hub.docker.com/_/dart
FROM dart:beta@sha256:88ced76ff4a63e565872df26fe2442f060e3ecf828a272090ad10c79e9d044af
FROM dart:beta@sha256:8ecef6dda0e56347ce8ec6c5b114e7a6c599d041651c39b57eae160e178f4c9a

WORKDIR /app

Expand Down
23 changes: 14 additions & 9 deletions app_dart/lib/src/request_handlers/github/webhook_subscription.dart
Original file line number Diff line number Diff line change
Expand Up @@ -183,9 +183,11 @@ class GithubWebhookSubscription extends SubscriptionHandler {
}
break;
case 'edited':
// Editing a PR should not trigger new jobs, but may update whether
// it has tests.
await _checkForTests(pullRequestEvent);
// In the event of the base ref changing we want to start new checks.
if (pullRequestEvent.changes != null && pullRequestEvent.changes!.base != null) {
await _scheduleIfMergeable(pullRequestEvent);
}
break;
case 'opened':
case 'reopened':
Expand Down Expand Up @@ -371,6 +373,7 @@ class GithubWebhookSubscription extends SubscriptionHandler {
filename.contains('analysis_options.yaml') ||
filename.contains('AUTHORS') ||
filename.contains('CODEOWNERS') ||
filename == 'DEPS' ||
filename.contains('TESTOWNERS') ||
filename.contains('pubspec.yaml') ||
// Exempt categories.
Expand Down Expand Up @@ -398,16 +401,18 @@ class GithubWebhookSubscription extends SubscriptionHandler {
bool needsTests = false;

await for (PullRequestFile file in files) {
final String filename = file.filename!.toLowerCase();
if (_fileContainsAddedCode(file) && filename.endsWith('.dart') ||
filename.endsWith('.mm') ||
filename.endsWith('.m') ||
filename.endsWith('.java') ||
filename.endsWith('.cc')) {
final String filename = file.filename!;
if (_fileContainsAddedCode(file) &&
!_isTestExempt(filename) &&
// License goldens are auto-generated.
!filename.startsWith('ci/licenses_golden/') &&
// Build files don't need unit tests.
!filename.endsWith('.gn') &&
!filename.endsWith('.gni')) {
needsTests = !_allChangesAreCodeComments(file);
}

if (kEngineTestRegExp.hasMatch(filename)) {
if (kEngineTestRegExp.hasMatch(filename.toLowerCase())) {
hasTests = true;
}
}
Expand Down
43 changes: 38 additions & 5 deletions app_dart/lib/src/service/scheduler.dart
Original file line number Diff line number Diff line change
Expand Up @@ -436,16 +436,36 @@ 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.
final includePostsubmitAsPresubmit = _includePostsubmitAsPresubmit(ciYaml, pullRequest);
if (includePostsubmitAsPresubmit) {
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.
if (pullRequest.base!.ref != Config.defaultBranch(pullRequest.base!.repo!.slug())) {
log.info('Release branch found, scheduling all targets for ${pullRequest.number}');
return presubmitTargets.toList();
return presubmitTargets;
}
if (includePostsubmitAsPresubmit) {
log.info('Postsubmit targets included as presubmit, scheduling all targets for ${pullRequest.number}');
return presubmitTargets;
}

// Filter builders based on the PR diff
Expand All @@ -462,6 +482,19 @@ class Scheduler {
return getTargetsToRun(presubmitTargets, files);
}

/// Returns `true` if [ciYaml.postsubmitTargets] should be ran during presubmit.
static bool _includePostsubmitAsPresubmit(CiYaml ciYaml, PullRequest pullRequest) {
// 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
10 changes: 5 additions & 5 deletions app_dart/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@ dependencies:
file: 7.0.0
fixnum: 1.1.0
gcloud: 0.8.11
github: 9.19.0
github: 9.20.0
googleapis: 11.4.0
googleapis_auth: 1.4.1
gql: 1.0.1-alpha+1696717343881
graphql: 5.2.0-beta.6
grpc: 3.2.4
http: 1.1.0
http: 1.1.2
json_annotation: 4.8.1
logging: 1.2.0
meta: 1.11.0
Expand All @@ -42,12 +42,12 @@ dependencies:
yaml: 3.1.2

dev_dependencies:
analyzer: 5.13.0
build_runner: 2.4.6
analyzer: 6.3.0
build_runner: 2.4.7
fake_async: 1.3.1
flutter_lints: 3.0.1
json_serializable: 6.7.1
mockito: 5.4.2
mockito: 5.4.3
platform: 3.1.3
test: 1.24.9

Expand Down
167 changes: 165 additions & 2 deletions app_dart/test/request_handlers/github/webhook_subscription_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import 'package:cocoon_service/src/model/appengine/commit.dart';
import 'package:cocoon_service/src/model/luci/buildbucket.dart';
import 'package:cocoon_service/src/model/luci/push_message.dart' as pm;
import 'package:cocoon_service/src/request_handlers/github/webhook_subscription.dart';
import 'package:cocoon_service/src/service/cache_service.dart';
import 'package:cocoon_service/src/service/config.dart';
Expand Down Expand Up @@ -272,13 +273,15 @@ void main() {
test('Acts on opened against master when default is main', () async {
const int issueNumber = 123;

tester.message = generateGithubWebhookMessage(
final pm.PushMessage pushMessage = generateGithubWebhookMessage(
action: 'opened',
number: issueNumber,
baseRef: 'master',
slug: Config.engineSlug,
);

tester.message = pushMessage;

when(pullRequestsService.listFiles(Config.engineSlug, issueNumber)).thenAnswer(
(_) => Stream<PullRequestFile>.value(
PullRequestFile()..filename = 'packages/flutter/blah.dart',
Expand Down Expand Up @@ -308,6 +311,56 @@ void main() {
argThat(contains('master -> main')),
),
).called(1);

expect(scheduler.triggerPresubmitTargetsCallCount, 1);
scheduler.resetTriggerPresubmitTargetsCallCount();
});

test('Acts on edited against master when default is main', () async {
const int issueNumber = 123;

final pm.PushMessage pushMessage = generateGithubWebhookMessage(
action: 'edited',
number: issueNumber,
baseRef: 'master',
slug: Config.engineSlug,
includeChanges: true,
);

tester.message = pushMessage;

when(pullRequestsService.listFiles(Config.engineSlug, issueNumber)).thenAnswer(
(_) => Stream<PullRequestFile>.value(
PullRequestFile()..filename = 'packages/flutter/blah.dart',
),
);

when(issuesService.listCommentsByIssue(Config.engineSlug, issueNumber)).thenAnswer(
(_) => Stream<IssueComment>.value(
IssueComment()..body = 'some other comment',
),
);

await tester.post(webhook);

verify(
pullRequestsService.edit(
Config.engineSlug,
issueNumber,
base: 'main',
),
).called(1);

verify(
issuesService.createComment(
Config.engineSlug,
issueNumber,
argThat(contains('master -> main')),
),
).called(1);

expect(scheduler.triggerPresubmitTargetsCallCount, 1);
scheduler.resetTriggerPresubmitTargetsCallCount();
});

// We already schedule checks when a draft is opened, don't need to re-test
Expand Down Expand Up @@ -1256,6 +1309,40 @@ void foo() {
).called(1);
});

test('Engine labels PRs, comment if no tests for unknown file', () async {
const int issueNumber = 123;

tester.message = generateGithubWebhookMessage(
action: 'opened',
number: issueNumber,
slug: Config.engineSlug,
baseRef: Config.defaultBranch(Config.engineSlug),
);
final RepositorySlug slug = RepositorySlug('flutter', 'engine');

when(pullRequestsService.listFiles(slug, issueNumber)).thenAnswer(
(_) => Stream<PullRequestFile>.value(
PullRequestFile()..filename = 'foo/bar/baz.madeupextension',
),
);

when(issuesService.listCommentsByIssue(slug, issueNumber)).thenAnswer(
(_) => Stream<IssueComment>.value(
IssueComment()..body = 'some other comment',
),
);

await tester.post(webhook);

verify(
issuesService.createComment(
slug,
issueNumber,
argThat(contains(config.missingTestsPullRequestMessageValue)),
),
).called(1);
});

group('Auto-roller accounts do not label Engine PR with test label or comment.', () {
final Set<String> inputs = {
'engine-flutter-autoroll',
Expand Down Expand Up @@ -1365,7 +1452,7 @@ void foo() {
);
});

test('Engine labels PRs, no code files', () async {
test('Engine labels PRs, no comment if DEPS-only', () async {
const int issueNumber = 123;

tester.message = generateGithubWebhookMessage(
Expand Down Expand Up @@ -1400,6 +1487,77 @@ void foo() {
);
});

test('Engine labels PRs, no comment if build-file-only', () async {
const int issueNumber = 123;

tester.message = generateGithubWebhookMessage(
action: 'opened',
number: issueNumber,
baseRef: 'main',
slug: Config.engineSlug,
);

when(pullRequestsService.listFiles(Config.engineSlug, issueNumber)).thenAnswer(
(_) => Stream<PullRequestFile>.fromIterable(<PullRequestFile>[
PullRequestFile()..filename = 'shell/config.gni',
PullRequestFile()..filename = 'shell/BUILD.gn',
]),
);

await tester.post(webhook);

verifyNever(
issuesService.addLabelsToIssue(
Config.engineSlug,
issueNumber,
any,
),
);

verifyNever(
issuesService.createComment(
Config.engineSlug,
issueNumber,
any,
),
);
});

test('Engine labels PRs, no comment for license goldens', () async {
const int issueNumber = 123;

tester.message = generateGithubWebhookMessage(
action: 'opened',
number: issueNumber,
baseRef: 'main',
slug: Config.engineSlug,
);

when(pullRequestsService.listFiles(Config.engineSlug, issueNumber)).thenAnswer(
(_) => Stream<PullRequestFile>.value(
PullRequestFile()..filename = 'ci/licenses_golden/licenses_dart',
),
);

await tester.post(webhook);

verifyNever(
issuesService.addLabelsToIssue(
Config.engineSlug,
issueNumber,
any,
),
);

verifyNever(
issuesService.createComment(
Config.engineSlug,
issueNumber,
any,
),
);
});

test('Engine labels PRs, no comment if Java tests', () async {
const int issueNumber = 123;

Expand Down Expand Up @@ -1692,6 +1850,11 @@ void foo() {
..deletionsCount = 20
..additionsCount = 0
..changesCount = 20,
PullRequestFile()
..filename = 'shell/platform/darwin/ios/platform_view_ios.mm'
..deletionsCount = 20
..additionsCount = 0
..changesCount = 20,
]),
);

Expand Down
Loading

0 comments on commit 59decbb

Please sign in to comment.