Skip to content

Commit

Permalink
Push build status based on Firestore (#3582)
Browse files Browse the repository at this point in the history 
Part of flutter/flutter#142951.

The github build status API has been querying and writing updates to Datastore.

This PR:

- starts querying from Firestore
- supports both datastore/Firestore queries and writes
- updates all unit tests to cover Firestore logics
  • Loading branch information
@keyonghan
keyonghan committed Mar 19, 2024
1 parent 02192ce commit 95d6986
Show file tree
Hide file tree
Showing 6 changed files with 706 additions and 620 deletions.
15 changes: 15 additions & 0 deletions app_dart/lib/src/model/firestore/github_build_status.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,21 @@ class GithubBuildStatus extends Document {

int? get updates => int.parse(fields![kGithubBuildStatusUpdatesField]!.integerValue!);

String setStatus(String status) {
fields![kGithubBuildStatusStatusField] = Value(stringValue: status);
return status;
}

int setUpdates(int updates) {
fields![kGithubBuildStatusUpdatesField] = Value(integerValue: updates.toString());
return updates;
}

int setUpdateTimeMillis(int updateTimeMillis) {
fields![kGithubBuildStatusUpdateTimeMillisField] = Value(integerValue: updateTimeMillis.toString());
return updateTimeMillis;
}

@override
String toString() {
final StringBuffer buf = StringBuffer()
Expand Down
42 changes: 24 additions & 18 deletions app_dart/lib/src/request_handlers/push_build_status_to_github.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,58 +46,64 @@ class PushBuildStatusToGithub extends ApiRequestHandler<Body> {

final BuildStatus status = (await buildStatusService.calculateCumulativeStatus(slug))!;
await _insertBigquery(slug, status.githubStatus, Config.defaultBranch(slug), config);
await _updatePRs(slug, status.githubStatus, datastore);
await _updatePRs(slug, status.githubStatus, datastore, firestoreService);
log.fine('All the PRs for $repository have been updated with $status');

return Body.empty;
}

Future<void> _updatePRs(RepositorySlug slug, String status, DatastoreService datastore) async {
Future<void> _updatePRs(
RepositorySlug slug,
String status,
DatastoreService datastore,
FirestoreService firestoreService,
) async {
final GitHub github = await config.createGitHubClient(slug: slug);
final List<GithubBuildStatusUpdate> updates = <GithubBuildStatusUpdate>[];
final List<GithubBuildStatus> githubBuildStatuses = <GithubBuildStatus>[];
await for (PullRequest pr in github.pullRequests.list(slug)) {
// Tree status is only put on PRs merging into ToT.
if (pr.base!.ref != Config.defaultBranch(slug)) {
log.fine('This PR is not staged to land on ${Config.defaultBranch(slug)}, skipping.');
continue;
}
final GithubBuildStatusUpdate update = await datastore.queryLastStatusUpdate(slug, pr);
if (update.status != status) {
log.fine('Updating status of ${slug.fullName}#${pr.number} from ${update.status} to $status');
final GithubBuildStatus githubBuildStatus =
await firestoreService.queryLastBuildStatus(slug, pr.number!, pr.head!.sha!);
if (githubBuildStatus.status != status) {
log.fine('Updating status of ${slug.fullName}#${pr.number} from ${githubBuildStatus.status} to $status');
final CreateStatus request = CreateStatus(status);
request.targetUrl = 'https://flutter-dashboard.appspot.com/#/build?repo=${slug.name}';
request.context = 'tree-status';
if (status != GithubBuildStatusUpdate.statusSuccess) {
if (status != GithubBuildStatus.statusSuccess) {
request.description = config.flutterBuildDescription;
}
try {
await github.repositories.createStatus(slug, pr.head!.sha!, request);
final int currentTimeMillisecondsSinceEpoch = DateTime.now().millisecondsSinceEpoch;
update.status = status;
update.updates = (update.updates ?? 0) + 1;
update.updateTimeMillis = DateTime.now().millisecondsSinceEpoch;
update.updateTimeMillis = currentTimeMillisecondsSinceEpoch;
updates.add(update);

githubBuildStatus.setStatus(status);
githubBuildStatus.setUpdates((githubBuildStatus.updates ?? 0) + 1);
githubBuildStatus.setUpdateTimeMillis(currentTimeMillisecondsSinceEpoch);
githubBuildStatuses.add(githubBuildStatus);
} catch (error) {
log.severe('Failed to post status update to ${slug.fullName}#${pr.number}: $error');
}
}
}
await datastore.insert(updates);
// TODO(keyonghan): remove try block after fully migrated to firestore
// https://github.com/flutter/flutter/issues/142951
try {
await updateGithubBuildStatusDocuments(updates);
} catch (error) {
log.warning('Failed to update github build status in Firestore: $error');
}
await updateGithubBuildStatusDocuments(githubBuildStatuses);
}

Future<void> updateGithubBuildStatusDocuments(List<GithubBuildStatusUpdate> updates) async {
if (updates.isEmpty) {
Future<void> updateGithubBuildStatusDocuments(List<GithubBuildStatus> githubBuildStatuses) async {
if (githubBuildStatuses.isEmpty) {
return;
}
final List<GithubBuildStatus> githubBuildStatusDocuments =
updates.map((e) => githubBuildStatusToDocument(e)).toList();
final List<Write> writes = documentsToWrites(githubBuildStatusDocuments);
final List<Write> writes = documentsToWrites(githubBuildStatuses);
final FirestoreService firestoreService = await config.createFirestoreService();
await firestoreService.batchWriteDocuments(BatchWriteRequest(writes: writes), kDatabase);
}
Expand Down
37 changes: 37 additions & 0 deletions app_dart/lib/src/service/firestore.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import 'package:googleapis/firestore/v1.dart';
import 'package:http/http.dart';

import '../model/firestore/commit.dart';
import '../model/firestore/github_build_status.dart';
import '../model/firestore/github_gold_status.dart';
import '../model/firestore/task.dart';
import 'access_client_provider.dart';
Expand Down Expand Up @@ -154,6 +155,42 @@ class FirestoreService {
}
}

/// Queries the last updated build status for the [slug], [prNumber], and [head].
///
/// If not existing, returns a fresh new Build status.
Future<GithubBuildStatus> queryLastBuildStatus(RepositorySlug slug, int prNumber, String head) async {
final Map<String, Object> filterMap = <String, Object>{
'$kGithubBuildStatusPrNumberField =': prNumber,
'$kGithubBuildStatusRepositoryField =': slug.fullName,
'$kGithubBuildStatusHeadField =': head,
};
final List<Document> documents = await query(kGithubBuildStatusCollectionId, filterMap);
final List<GithubBuildStatus> githubBuildStatuses =
documents.map((Document document) => GithubBuildStatus.fromDocument(githubBuildStatus: document)).toList();
if (githubBuildStatuses.isEmpty) {
return GithubBuildStatus.fromDocument(
githubBuildStatus: Document(
name: '$kDatabase/documents/$kGithubBuildStatusCollectionId/${head}_$prNumber',
fields: <String, Value>{
kGithubBuildStatusPrNumberField: Value(integerValue: prNumber.toString()),
kGithubBuildStatusHeadField: Value(stringValue: head),
kGithubBuildStatusStatusField: Value(stringValue: ''),
kGithubBuildStatusUpdatesField: Value(integerValue: '0'),
kGithubBuildStatusUpdateTimeMillisField:
Value(integerValue: DateTime.now().millisecondsSinceEpoch.toString()),
kGithubBuildStatusRepositoryField: Value(stringValue: slug.fullName),
},
),
);
} else {
if (githubBuildStatuses.length > 1) {
throw StateError('GithubBuildStatus should have no more than one entry on '
'repository ${slug.fullName}, pr $prNumber, and head $head.');
}
return githubBuildStatuses.single;
}
}

/// Returns Firestore [Value] based on corresponding object type.
Value getValueFromFilter(Object comparisonOject) {
if (comparisonOject is int) {
Expand Down
65 changes: 26 additions & 39 deletions app_dart/test/request_handlers/push_build_status_to_github_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,10 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'package:cocoon_service/src/model/appengine/github_build_status_update.dart';
import 'package:cocoon_service/src/model/firestore/github_build_status.dart';
import 'package:cocoon_service/src/request_handlers/push_build_status_to_github.dart';
import 'package:cocoon_service/src/request_handling/body.dart';
import 'package:cocoon_service/src/service/build_status_provider.dart';
import 'package:cocoon_service/src/service/config.dart';
import 'package:cocoon_service/src/service/datastore.dart';
import 'package:gcloud/db.dart';
import 'package:github/github.dart';
Expand Down Expand Up @@ -42,17 +40,8 @@ void main() {
late MockIssuesService issuesService;
late MockRepositoriesService repositoriesService;
late FakeGithubService githubService;

GithubBuildStatusUpdate newStatusUpdate(PullRequest pr, BuildStatus status) {
return GithubBuildStatusUpdate(
key: db.emptyKey.append(GithubBuildStatusUpdate, id: pr.number),
repository: Config.flutterSlug.fullName,
status: status.githubStatus,
pr: pr.number!,
head: pr.head!.sha,
updates: 0,
);
}
RepositorySlug? slug;
GithubBuildStatus? githubBuildStatus;

setUp(() async {
mockFirestoreService = MockFirestoreService();
Expand Down Expand Up @@ -82,6 +71,17 @@ void main() {
datastoreProvider: (DatastoreDB db) => DatastoreService(config.db, 5),
);

slug = RepositorySlug('flutter', 'flutter');
githubBuildStatus = null;

when(
mockFirestoreService.queryLastBuildStatus(slug, 123, 'sha1'),
).thenAnswer((Invocation invocation) {
return Future<GithubBuildStatus>.value(
githubBuildStatus,
);
});

when(github.pullRequests).thenReturn(pullRequestsService);
when(github.issues).thenReturn(issuesService);
when(github.repositories).thenReturn(repositoriesService);
Expand All @@ -93,20 +93,11 @@ void main() {
test('development environment does nothing', () async {
clientContext.isDevelopmentEnvironment = true;
config.githubClient = ThrowingGitHub();
db.onCommit = (List<Model<dynamic>> insert, List<Key<dynamic>> deletes) => throw AssertionError();
db.addOnQuery<GithubBuildStatusUpdate>((Iterable<GithubBuildStatusUpdate> results) {
throw AssertionError();
});
final Body body = await tester.get<Body>(handler);
expect(body, same(Body.empty));
});

group('does not update anything', () {
setUp(() {
db.onCommit = (List<Model<dynamic>> insert, List<Key<dynamic>> deletes) => throw AssertionError();
when(repositoriesService.createStatus(any, any, any)).thenThrow(AssertionError());
});

test('if there are no PRs', () async {
when(pullRequestsService.list(any, base: anyNamed('base')))
.thenAnswer((_) => const Stream<PullRequest>.empty());
Expand Down Expand Up @@ -134,27 +125,24 @@ void main() {
});

test('if status has not changed since last update', () async {
final PullRequest pr = generatePullRequest(id: 1, sha: '1');
final PullRequest pr = generatePullRequest(id: 1, sha: 'sha1');
when(pullRequestsService.list(any, base: anyNamed('base'))).thenAnswer((_) => Stream<PullRequest>.value(pr));
buildStatusService.cumulativeStatus = BuildStatus.success();
final GithubBuildStatusUpdate status = newStatusUpdate(pr, BuildStatus.success());
db.values[status.key] = status;
githubBuildStatus = generateFirestoreGithubBuildStatus(1);
final Body body = await tester.get<Body>(handler);
expect(body, same(Body.empty));
expect(status.updates, 0);
expect(githubBuildStatus!.updates, 0);
});

test('if there is no pr found for a targeted branch', () async {
final PullRequest pr = generatePullRequest(id: 1, sha: '1');
final PullRequest pr = generatePullRequest(id: 1, sha: 'sha1', branch: 'test_branch');
when(pullRequestsService.list(any, base: anyNamed('base'))).thenAnswer((_) => Stream<PullRequest>.value(pr));
buildStatusService.cumulativeStatus = BuildStatus.success();
final GithubBuildStatusUpdate status =
newStatusUpdate(pr, BuildStatus.failure(const <String>['failed_task_1']));
db.values[status.key] = status;
githubBuildStatus = generateFirestoreGithubBuildStatus(1, status: GithubBuildStatus.statusFailure);
final Body body = await tester.get<Body>(handler);
expect(body, same(Body.empty));
expect(status.updates, 0);
expect(status.status, BuildStatus.failure().githubStatus);
expect(githubBuildStatus!.updates, 0);
expect(githubBuildStatus!.status, GithubBuildStatus.statusFailure);
});
});

Expand All @@ -167,24 +155,23 @@ void main() {
).thenAnswer((Invocation invocation) {
return Future<BatchWriteResponse>.value(BatchWriteResponse());
});
final PullRequest pr = generatePullRequest(id: 1, sha: '1');
final PullRequest pr = generatePullRequest(id: 1, sha: 'sha1');
when(pullRequestsService.list(any, base: anyNamed('base'))).thenAnswer((_) => Stream<PullRequest>.value(pr));
buildStatusService.cumulativeStatus = BuildStatus.success();
final GithubBuildStatusUpdate status = newStatusUpdate(pr, BuildStatus.failure(const <String>['failed_test_1']));
db.values[status.key] = status;
githubBuildStatus = generateFirestoreGithubBuildStatus(1, status: GithubBuildStatus.statusFailure);
final Body body = await tester.get<Body>(handler);
expect(body, same(Body.empty));
expect(status.updates, 1);
expect(status.updateTimeMillis, isNotNull);
expect(status.status, BuildStatus.success().githubStatus);
expect(githubBuildStatus!.updates, 1);
expect(githubBuildStatus!.updateTimeMillis, isNotNull);
expect(githubBuildStatus!.status, BuildStatus.success().githubStatus);

final List<dynamic> captured = verify(mockFirestoreService.batchWriteDocuments(captureAny, captureAny)).captured;
expect(captured.length, 2);
final BatchWriteRequest batchWriteRequest = captured[0] as BatchWriteRequest;
expect(batchWriteRequest.writes!.length, 1);
final GithubBuildStatus updatedDocument =
GithubBuildStatus.fromDocument(githubBuildStatus: batchWriteRequest.writes![0].update!);
expect(updatedDocument.updates, status.updates);
expect(updatedDocument.updates, githubBuildStatus!.updates);
});
});
}
4 changes: 3 additions & 1 deletion app_dart/test/src/utilities/entity_generators.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,8 @@ GithubBuildStatus generateFirestoreGithubBuildStatus(
int? pr,
String owner = 'flutter',
String repo = 'flutter',
int? updates,
int updates = 0,
String status = GithubBuildStatus.statusSuccess,
}) {
pr ??= i;
head ??= 'sha$i';
Expand All @@ -193,6 +194,7 @@ GithubBuildStatus generateFirestoreGithubBuildStatus(
kGithubBuildStatusPrNumberField: Value(integerValue: pr.toString()),
kGithubBuildStatusRepositoryField: Value(stringValue: '$owner/$repo'),
kGithubBuildStatusUpdatesField: Value(integerValue: updates.toString()),
kGithubBuildStatusStatusField: Value(stringValue: status),
};
return githubBuildStatus;
}
Expand Down
Loading

0 comments on commit 95d6986

Please sign in to comment.