Skip to content

Commit

Permalink
Adding approval of the revert request (flutter#2089)
Browse files Browse the repository at this point in the history
  • Loading branch information
ricardoamador authored Aug 26, 2022
1 parent c4ce9f0 commit a95207a
Show file tree
Hide file tree
Showing 6 changed files with 75 additions and 9 deletions.
2 changes: 1 addition & 1 deletion auto_submit/lib/requests/check_pull_request.dart
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ class CheckPullRequest extends AuthenticatedRequestHandler {
await pubsub.acknowledge('auto-submit-queue-sub', message.ackId!);
continue;
} else {
await approver.approve(pullRequest);
await approver.autoApproval(pullRequest);
log.info('Approved pull request: $rawBody');
processingLog.add(pullRequest.number!);
}
Expand Down
24 changes: 22 additions & 2 deletions auto_submit/lib/service/approver_service.dart
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,34 @@ class ApproverService {
return ApproverService(config);
}

Future<void> approve(PullRequest pullRequest) async {
Future<void> autoApproval(PullRequest pullRequest) async {
final String? author = pullRequest.user!.login;

if (!config.rollerAccounts.contains(author)) {
log.info('Auto-review ignored for $author');
return;
} else {
await _approve(pullRequest, author);
}
}

/// Auto approves a pull request when the revert label is present.
Future<void> revertApproval(PullRequest pullRequest) async {
final Set<String> approvedAuthorAssociations = <String>{'MEMBER', 'OWNER'};

final String? author = pullRequest.user!.login;
final String? authorAssociation = pullRequest.authorAssociation;
final List<String> labelNames =
(pullRequest.labels as List<IssueLabel>).map<String>((IssueLabel labelMap) => labelMap.name).toList();

if (labelNames.contains(Config.kRevertLabel) &&
(config.rollerAccounts.contains(author) || approvedAuthorAssociations.contains(authorAssociation))) {
await _approve(pullRequest, author);
} else {
log.info('Auto-review ignored for $author');
}
}

Future<void> _approve(PullRequest pullRequest, String? author) async {
final RepositorySlug slug = pullRequest.base!.repo!.slug();
final GitHub botClient = await config.createFlutterGitHubBotClient(slug);

Expand Down
5 changes: 5 additions & 0 deletions auto_submit/lib/service/validation_service.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// found in the LICENSE file.

import 'package:auto_submit/requests/check_pull_request_queries.dart';
import 'package:auto_submit/service/approver_service.dart';
import 'package:auto_submit/service/config.dart';
import 'package:auto_submit/service/github_service.dart';
import 'package:auto_submit/service/graphql_service.dart';
Expand Down Expand Up @@ -196,6 +197,10 @@ class ValidationService {
final GithubService githubService = await config.createGithubService(slug);

if (revertValidationResult.result) {
// Approve the pull request automatically as it has been validated.
ApproverService approverService = ApproverService(config);
approverService.revertApproval(messagePullRequest);

bool processed = await processMerge(config, result, messagePullRequest);
if (processed) {
await pubsub.acknowledge('auto-submit-queue-sub', ackId);
Expand Down
2 changes: 1 addition & 1 deletion auto_submit/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ dependencies:
graphql: ^5.1.1
gql: ^0.13.1
http: ^0.13.5
json_annotation: ^4.5.0
json_annotation: ^4.6.0
meta: ^1.8.0
neat_cache: ^2.0.2
shelf: ^1.3.2
Expand Down
42 changes: 39 additions & 3 deletions auto_submit/test/service/approver_service_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,14 @@ void main() {

test('Verify approval ignored', () async {
PullRequest pr = generatePullRequest(author: 'not_a_user');
await service.approve(pr);
await service.autoApproval(pr);
verifyNever(pullRequests.createReview(any, captureAny));
});

test('Verify approve', () async {
when(pullRequests.listReviews(any, any)).thenAnswer((_) => const Stream<PullRequestReview>.empty());
PullRequest pr = generatePullRequest(author: 'dependabot[bot]');
await service.approve(pr);
await service.autoApproval(pr);
final List<dynamic> reviews = verify(pullRequests.createReview(any, captureAny)).captured;
expect(reviews.length, 1);
final CreatePullRequestReview review = reviews.single as CreatePullRequestReview;
Expand All @@ -46,7 +46,43 @@ void main() {
PullRequestReview review = PullRequestReview(id: 123, user: User(login: 'fluttergithubbot'), state: 'APPROVED');
when(pullRequests.listReviews(any, any)).thenAnswer((_) => Stream<PullRequestReview>.value(review));
PullRequest pr = generatePullRequest(author: 'dependabot[bot]');
await service.approve(pr);
await service.autoApproval(pr);
verifyNever(pullRequests.createReview(any, captureAny));
});

test('AutoApproval does not approve revert pull request.', () async {
PullRequest pr = generatePullRequest(author: 'not_a_user');
List<IssueLabel> issueLabels = pr.labels ?? [];
IssueLabel issueLabel = IssueLabel(name: 'revert');
issueLabels.add(issueLabel);
await service.autoApproval(pr);
verifyNever(pullRequests.createReview(any, captureAny));
});

test('Revert request is auto approved.', () async {
when(pullRequests.listReviews(any, any)).thenAnswer((_) => const Stream<PullRequestReview>.empty());
PullRequest pr = generatePullRequest(author: 'dependabot[bot]');

List<IssueLabel> issueLabels = pr.labels ?? [];
IssueLabel issueLabel = IssueLabel(name: 'revert');
issueLabels.add(issueLabel);

await service.revertApproval(pr);
final List<dynamic> reviews = verify(pullRequests.createReview(any, captureAny)).captured;
expect(reviews.length, 1);
final CreatePullRequestReview review = reviews.single as CreatePullRequestReview;
expect(review.event, 'APPROVE');
});

test('Revert request is not auto approved when the revert label is not present.', () async {
PullRequest pr = generatePullRequest(author: 'not_a_user');
await service.revertApproval(pr);
verifyNever(pullRequests.createReview(any, captureAny));
});

test('Revert request is not auto approved on bad author association.', () async {
PullRequest pr = generatePullRequest(author: 'not_a_user', authorAssociation: 'CONTRIBUTOR');
await service.revertApproval(pr);
verifyNever(pullRequests.createReview(any, captureAny));
});
}
9 changes: 7 additions & 2 deletions auto_submit/test/utilities/mocks.mocks.dart
Original file line number Diff line number Diff line change
Expand Up @@ -200,8 +200,13 @@ class MockApproverService extends _i1.Mock implements _i5.ApproverService {
(super.noSuchMethod(Invocation.getter(#config), returnValue: _FakeConfig_0(this, Invocation.getter(#config)))
as _i2.Config);
@override
_i6.Future<void> approve(_i4.PullRequest? pullRequest) =>
(super.noSuchMethod(Invocation.method(#approve, [pullRequest]),
_i6.Future<void> autoApproval(_i4.PullRequest? pullRequest) =>
(super.noSuchMethod(Invocation.method(#autoApproval, [pullRequest]),
returnValue: _i6.Future<void>.value(),
returnValueForMissingStub: _i6.Future<void>.value()) as _i6.Future<void>);
@override
_i6.Future<void> revertApproval(_i4.PullRequest? pullRequest) =>
(super.noSuchMethod(Invocation.method(#revertApproval, [pullRequest]),
returnValue: _i6.Future<void>.value(),
returnValueForMissingStub: _i6.Future<void>.value()) as _i6.Future<void>);
}
Expand Down

0 comments on commit a95207a

Please sign in to comment.