Skip to content

Commit

Permalink
When packaging builds are run on beta/stable, display on the dashboard (
Browse files Browse the repository at this point in the history
#3060)

* flutter/flutter#133690
* Under the dart-internal build results, add the packaging build to the list of valid builds
* Add handling of `PushEvent` so that when a push to beta/stable is made, the associated commit is added to the datastore
* Manual upgrade of `github` package to utilize `PushEvent`
  • Loading branch information
drewroengoogle authored Sep 13, 2023
1 parent b714fbe commit b1935c3
Show file tree
Hide file tree
Showing 8 changed files with 205 additions and 11 deletions.
1 change: 1 addition & 0 deletions app_dart/bin/server.dart
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ Future<void> main() async {
gerritService: gerritService,
githubChecksService: githubChecksService,
scheduler: scheduler,
commitService: commitService,
),
'/api/github/webhook-branch-subscription': GithubBranchWebhookSubscription(
config: config,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,10 @@ class DartInternalSubscription extends SubscriptionHandler {
}

// Only publish the parent release_builder builds to the datastore.
// TODO(drewroengoogle): Determine which builds we want to save to the datastore and check for them.
final regex = RegExp(r'(Linux|Mac|Windows)\s+(engine_release_builder|packaging_release_builder)');
// TODO(drewroengoogle): Remove this regex in favor of supporting *all* dart-internal build results.
// Issue: https://github.com/flutter/flutter/issues/134674
final regex =
RegExp(r'(Linux|Mac|Windows)\s+(engine_release_builder|packaging_release_builder|flutter_release_builder)');
if (!regex.hasMatch(builder)) {
log.info("Ignoring builder that is not a release builder");
return Body.empty;
Expand Down
15 changes: 15 additions & 0 deletions app_dart/lib/src/request_handlers/github/webhook_subscription.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import 'dart:convert';

import 'package:cocoon_service/src/service/commit_service.dart';
import 'package:github/github.dart';
import 'package:github/hooks.dart';
import 'package:meta/meta.dart';
Expand Down Expand Up @@ -70,6 +71,7 @@ class GithubWebhookSubscription extends SubscriptionHandler {
required super.config,
required this.scheduler,
required this.gerritService,
required this.commitService,
this.githubChecksService,
this.datastoreProvider = DatastoreService.defaultProvider,
super.authProvider,
Expand All @@ -81,6 +83,9 @@ class GithubWebhookSubscription extends SubscriptionHandler {
/// To verify whether a commit was mirrored to GoB.
final GerritService gerritService;

/// Used to handle push events and create commits based on those events.
final CommitService commitService;

/// To provide build status updates to GitHub pull requests.
final GithubChecksService? githubChecksService;

Expand Down Expand Up @@ -108,6 +113,16 @@ class GithubWebhookSubscription extends SubscriptionHandler {
throw InternalServerError('Failed to process check_run event. checkRunEvent: $checkRunEvent');
}
break;
case 'push':
final PushEvent pushEvent = PushEvent.fromJson(json.decode(webhook.payload) as Map<String, dynamic>);
final String branch = pushEvent.ref!.split('/')[2]; // Eg: refs/heads/beta would return beta.
final String repository = pushEvent.repository!.name;
// If the branch is beta/stable, then a commit wasn't created through a PR,
// meaning the commit needs to be added to the datastore here instead.
if (repository == 'flutter' && (branch == 'stable' || branch == 'beta')) {
await commitService.handlePushGithubRequest(pushEvent);
}
break;
}

return Body.empty;
Expand Down
41 changes: 34 additions & 7 deletions app_dart/lib/src/service/commit_service.dart
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,30 @@ class CommitService {
final String branch = createEvent.ref!;
log.info('Creating commit object for branch $branch in repository ${slug.fullName}');
final Commit commit = await _createCommitFromBranchEvent(datastore, slug, branch);
await _insertCommitIntoDatastore(datastore, commit);
}

try {
log.info('Checking for existing commit in the datastore');
await datastore.lookupByValue<Commit>(commit.key);
} on KeyNotFoundException {
log.info('Commit does not exist in datastore, inserting into datastore');
await datastore.insert(<Commit>[commit]);
}
/// Add a commit based on a [PushEvent] to the Datastore.
Future<void> handlePushGithubRequest(PushEvent pushEvent) async {
final DatastoreService datastore = datastoreProvider(config.db);
final RepositorySlug slug = RepositorySlug.full(pushEvent.repository!.fullName);
final String sha = pushEvent.headCommit!.id!;
final String branch = pushEvent.ref!.split('/')[2];
final String id = '${slug.fullName}/$branch/$sha';
final Key<String> key = datastore.db.emptyKey.append<String>(Commit, id: id);
final Commit commit = Commit(
key: key,
timestamp: pushEvent.headCommit!.timestamp!.millisecondsSinceEpoch,
repository: slug.fullName,
sha: pushEvent.headCommit?.id,
author: pushEvent.sender?.login,
authorAvatarUrl: pushEvent.sender?.avatarUrl,
// The field has a size of 1500 we need to ensure the commit message
// is at most 1500 chars long.
message: truncate(pushEvent.headCommit!.message!, 1490, omission: '...'),
branch: branch,
);
await _insertCommitIntoDatastore(datastore, commit);
}

Future<Commit> _createCommitFromBranchEvent(DatastoreService datastore, RepositorySlug slug, String branch) async {
Expand All @@ -64,4 +80,15 @@ class CommitService {
branch: branch,
);
}

Future<void> _insertCommitIntoDatastore(DatastoreService datastore, Commit commit) async {
final DatastoreService datastore = datastoreProvider(config.db);
try {
log.info('Checking for existing commit in the datastore');
await datastore.lookupByValue<Commit>(commit.key);
} on KeyNotFoundException {
log.info('Commit does not exist in datastore, inserting into datastore');
await datastore.insert(<Commit>[commit]);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ void main() {
late FakeHttpRequest request;
late FakeScheduler scheduler;
late FakeGerritService gerritService;
late MockCommitService commitService;
late MockGitHub gitHubClient;
late MockGithubChecksUtil mockGithubChecksUtil;
late MockGithubChecksService mockGithubChecksService;
Expand All @@ -53,6 +54,7 @@ void main() {
db = FakeDatastoreDB();
gitHubClient = MockGitHub();
githubService = FakeGithubService();
commitService = MockCommitService();
final MockTabledataResource tabledataResource = MockTabledataResource();
when(tabledataResource.insertAll(any, any, any, any)).thenAnswer((_) async => TableDataInsertAllResponse());
config = FakeConfig(
Expand Down Expand Up @@ -110,6 +112,7 @@ void main() {
gerritService: gerritService,
githubChecksService: mockGithubChecksService,
scheduler: scheduler,
commitService: commitService,
);
});

Expand Down Expand Up @@ -2256,4 +2259,38 @@ void foo() {
await tester.post(webhook);
});
});

group('github webhook push event', () {
test('handles push events for flutter/flutter beta branch', () async {
tester.message = generatePushMessage('beta', 'flutter', 'flutter');

await tester.post(webhook);

verify(commitService.handlePushGithubRequest(any)).called(1);
});

test('handles push events for flutter/flutter stable branch', () async {
tester.message = generatePushMessage('stable', 'flutter', 'flutter');

await tester.post(webhook);

verify(commitService.handlePushGithubRequest(any)).called(1);
});

test('does not handle push events for branches that are not beta|stable', () async {
tester.message = generatePushMessage('main', 'flutter', 'flutter');

await tester.post(webhook);

verifyNever(commitService.handlePushGithubRequest(any)).called(0);
});

test('does not handle push events for repositories that are not flutter/flutter', () async {
tester.message = generatePushMessage('beta', 'flutter', 'engine');

await tester.post(webhook);

verifyNever(commitService.handlePushGithubRequest(any)).called(0);
});
});
}
60 changes: 58 additions & 2 deletions app_dart/test/service/commit_service_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ void main() {
late MockRepositoriesService repositories;
late MockGitHub github;
const String owner = "flutter";
const String repository = "engine";
const String repository = "flutter";
const String branch = "coolest-branch";
const String sha = "1234";
const String message = "Adding null safety";
Expand All @@ -45,7 +45,7 @@ void main() {
when(config.db).thenReturn(db);
});

group('handleCreateRequest', () {
group('handleCreateGithubRequest', () {
test('adds commit to db if it does not exist in the datastore', () async {
expect(db.values.values.whereType<Commit>().length, 0);

Expand Down Expand Up @@ -125,4 +125,60 @@ void main() {
expect(commit, existingCommit);
});
});

group('handlePushGithubRequest', () {
test('adds commit to db if it does not exist in the datastore', () async {
expect(db.values.values.whereType<Commit>().length, 0);

final PushEvent pushEvent = generatePushEvent(
branch,
owner,
repository,
message: message,
sha: sha,
avatarUrl: avatarUrl,
username: username,
);
await commitService.handlePushGithubRequest(pushEvent);

expect(db.values.values.whereType<Commit>().length, 1);
final Commit commit = db.values.values.whereType<Commit>().single;
expect(commit.repository, "$owner/$repository");
expect(commit.message, message);
expect(commit.key.id, "$owner/$repository/$branch/$sha");
expect(commit.sha, sha);
expect(commit.author, username);
expect(commit.authorAvatarUrl, avatarUrl);
expect(commit.branch, branch);
});

test('does not add commit to db if it exists in the datastore', () async {
final Commit existingCommit = generateCommit(
1,
sha: sha,
branch: branch,
owner: owner,
repo: repository,
timestamp: 0,
);
final List<Commit> datastoreCommit = <Commit>[existingCommit];
await config.db.commit(inserts: datastoreCommit);
expect(db.values.values.whereType<Commit>().length, 1);

final PushEvent pushEvent = generatePushEvent(
branch,
owner,
repository,
message: message,
sha: sha,
avatarUrl: avatarUrl,
username: username,
);
await commitService.handlePushGithubRequest(pushEvent);

expect(db.values.values.whereType<Commit>().length, 1);
final Commit commit = db.values.values.whereType<Commit>().single;
expect(commit, existingCommit);
});
});
}
9 changes: 9 additions & 0 deletions app_dart/test/src/utilities/mocks.mocks.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1522,6 +1522,15 @@ class MockCommitService extends _i1.Mock implements _i28.CommitService {
returnValue: _i20.Future<void>.value(),
returnValueForMissingStub: _i20.Future<void>.value(),
) as _i20.Future<void>);
@override
_i20.Future<void> handlePushGithubRequest(_i27.PushEvent? pushEvent) => (super.noSuchMethod(
Invocation.method(
#handlePushGithubRequest,
[pushEvent],
),
returnValue: _i20.Future<void>.value(),
returnValueForMissingStub: _i20.Future<void>.value(),
) as _i20.Future<void>);
}

/// A class which mocks [Config].
Expand Down
47 changes: 47 additions & 0 deletions app_dart/test/src/utilities/webhook_generators.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1025,3 +1025,50 @@ CreateEvent generateCreateBranchEvent(String branchName, String repository, {boo
}
}''') as Map<String, dynamic>,
);

PushMessage generatePushMessage(String branch, String organization, String repository) {
final PushEvent event = generatePushEvent(branch, organization, repository);
final pb.GithubWebhookMessage message = pb.GithubWebhookMessage(event: 'push', payload: jsonEncode(event));
return PushMessage(data: message.writeToJson(), messageId: 'abc123');
}

PushEvent generatePushEvent(
String branch,
String organization,
String repository, {
String sha = "def456def456def456",
String message = "Commit-message",
String avatarUrl = "https://fakegithubcontent.com/google_profile",
String username = "googledotcom",
}) =>
PushEvent.fromJson(
jsonDecode('''
{
"ref": "refs/heads/$branch",
"before": "abc123abc123abc123",
"after": "$sha",
"sender": {
"login": "$username",
"avatar_url": "$avatarUrl"
},
"commits": [
{
"id": "ba2f6608108d174c4a6e6e093a4ddcf313656748",
"message": "Adding null safety",
"timestamp": "2023-09-05T15:01:04-05:00",
"url": "https://github.com/org/repo/commit/abc123abc123abc123"
}
],
"head_commit": {
"id": "$sha",
"message": "$message",
"timestamp": "2023-09-05T15:01:04-05:00",
"url": "https://github.com/org/repo/commit/abc123abc123abc123"
},
"repository": {
"name": "$repository",
"full_name": "$organization/$repository"
}
}
'''),
);

0 comments on commit b1935c3

Please sign in to comment.