Remove autosubmit on fail (#2091)
diff --git a/auto_submit/lib/service/approver_service.dart b/auto_submit/lib/service/approver_service.dart
index 4699a2f..49a14ff 100644
--- a/auto_submit/lib/service/approver_service.dart
+++ b/auto_submit/lib/service/approver_service.dart
@@ -50,7 +50,7 @@
if (labelNames.contains(Config.kRevertLabel) &&
(config.rollerAccounts.contains(author) || approvedAuthorAssociations.contains(authorAssociation))) {
log.info(
- 'Revert label and author has been validated. Attempting to approve the pull request. ${pullRequest.repo.toString()} by $author');
+ 'Revert label and author has been validated. Attempting to approve the pull request. ${pullRequest.repo} by $author');
await _approve(pullRequest, author);
} else {
log.info('Auto-review ignored for $author');
@@ -71,7 +71,7 @@
final gh.CreatePullRequestReview review =
gh.CreatePullRequestReview(slug.owner, slug.name, pullRequest.number!, 'APPROVE');
- botClient.pullRequests.createReview(slug, review);
+ await botClient.pullRequests.createReview(slug, review);
log.info('Review for $author complete');
}
}
diff --git a/auto_submit/lib/service/github_service.dart b/auto_submit/lib/service/github_service.dart
index c1649a8..3eda222 100644
--- a/auto_submit/lib/service/github_service.dart
+++ b/auto_submit/lib/service/github_service.dart
@@ -47,16 +47,24 @@
}
/// Create a new issue in github.
- Future<Issue> createIssue(
- RepositorySlug repositorySlug,
- String title,
- String body,
- ) async {
+ Future<Issue> createIssue({
+ required RepositorySlug slug,
+ required String title,
+ required String body,
+ List<String>? labels,
+ String? assignee,
+ List<String>? assignees,
+ String? state,
+ }) async {
IssueRequest issueRequest = IssueRequest(
title: title,
body: body,
+ labels: labels,
+ assignee: assignee,
+ assignees: assignees,
+ state: state,
);
- return await github.issues.create(repositorySlug, issueRequest);
+ return await github.issues.create(slug, issueRequest);
}
/// Fetches the specified pull request.
diff --git a/auto_submit/lib/service/revert_review_template.dart b/auto_submit/lib/service/revert_review_template.dart
new file mode 100644
index 0000000..73ab094
--- /dev/null
+++ b/auto_submit/lib/service/revert_review_template.dart
@@ -0,0 +1,54 @@
+// Copyright 2022 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+/// A template for creating follow up review issues for created revert requests.
+class RevertReviewTemplate {
+ RevertReviewTemplate({
+ required this.repositorySlug,
+ required this.revertPrNumber,
+ required this.revertPrAuthor,
+ required this.originalPrLink,
+ });
+
+ final String repositorySlug;
+ final int revertPrNumber;
+ final String revertPrAuthor;
+ final String originalPrLink;
+
+ /// Constructs the issues title.
+ String _constructTitle() {
+ return '''
+Review request for Revert PR $repositorySlug#$revertPrNumber
+''';
+ }
+
+ /// Constructs the issues body.
+ String _constructBody() {
+ return '''
+Pull request $repositorySlug#$revertPrNumber was submitted and merged by
+@$revertPrAuthor in order to revert changes made in this pull request $originalPrLink.
+
+Please assign this issue to the person that will make the formal review on the
+revert request listed above.
+
+Please do the following so that we may track this issue to completion:
+1. Add the reviewer of revert pull request as the assignee of this issue.
+2. Add the label 'revert_review' to this issue.
+3. Close only when the review has been completed.
+
+
+DO NOT EDIT, REVERT METADATA
+<!--
+{
+ 'originalPrLink': '$originalPrLink',
+ 'revertPrLink': '$repositorySlug#$revertPrNumber',
+ 'revertPrAuthor': '$revertPrAuthor'
+}
+-->
+''';
+ }
+
+ String? get title => _constructTitle();
+ String? get body => _constructBody();
+}
diff --git a/auto_submit/lib/service/validation_service.dart b/auto_submit/lib/service/validation_service.dart
index ad7968d..486d5eb 100644
--- a/auto_submit/lib/service/validation_service.dart
+++ b/auto_submit/lib/service/validation_service.dart
@@ -9,6 +9,7 @@
import 'package:auto_submit/service/graphql_service.dart';
import 'package:auto_submit/service/log.dart';
import 'package:auto_submit/service/process_method.dart';
+import 'package:auto_submit/service/revert_review_template.dart';
import 'package:auto_submit/validations/ci_successful.dart';
import 'package:auto_submit/validations/revert.dart';
import 'package:auto_submit/validations/unknown_mergeable.dart';
@@ -30,6 +31,7 @@
ValidationService(this.config) {
/// Validates a PR marked with the reverts label.
revertValidation = Revert(config: config);
+ approverService = ApproverService(config);
validations.addAll({
/// Validates the PR has been approved following the codereview guidelines.
@@ -53,21 +55,30 @@
}
Revert? revertValidation;
+ ApproverService? approverService;
final Config config;
final Set<Validation> validations = <Validation>{};
/// Processes a pub/sub message associated with PullRequest event.
Future<void> processMessage(github.PullRequest messagePullRequest, String ackId, PubSub pubsub) async {
- ProcessMethod processMethod = await processPullRequestMethod(messagePullRequest);
+ final ProcessMethod processMethod = await processPullRequestMethod(messagePullRequest);
switch (processMethod) {
case ProcessMethod.processAutosubmit:
await processPullRequest(
- config, await getNewestPullRequestInfo(config, messagePullRequest), messagePullRequest, ackId, pubsub);
+ config: config,
+ result: await getNewestPullRequestInfo(config, messagePullRequest),
+ messagePullRequest: messagePullRequest,
+ ackId: ackId,
+ pubsub: pubsub);
break;
case ProcessMethod.processRevert:
await processRevertRequest(
- config, await getNewestPullRequestInfo(config, messagePullRequest), messagePullRequest, ackId, pubsub);
+ config: config,
+ result: await getNewestPullRequestInfo(config, messagePullRequest),
+ messagePullRequest: messagePullRequest,
+ ackId: ackId,
+ pubsub: pubsub);
break;
case ProcessMethod.doNotProcess:
log.info('Should not process ${messagePullRequest.toJson()}, and ack the message.');
@@ -81,7 +92,7 @@
final github.RepositorySlug slug = pullRequest.base!.repo!.slug();
final graphql.GraphQLClient graphQLClient = await config.createGitHubGraphQLClient(slug);
final int? prNumber = pullRequest.number;
- GraphQlService graphQlService = GraphQlService();
+ final GraphQlService graphQlService = GraphQlService();
final Map<String, dynamic> data = await graphQlService.queryGraphQL(
slug,
prNumber!,
@@ -110,16 +121,21 @@
/// Processes a PullRequest running several validations to decide whether to
/// land the commit or remove the autosubmit label.
- Future<void> processPullRequest(
- Config config, QueryResult result, github.PullRequest messagePullRequest, String ackId, PubSub pubsub) async {
+ Future<void> processPullRequest({
+ required Config config,
+ required QueryResult result,
+ required github.PullRequest messagePullRequest,
+ required String ackId,
+ required PubSub pubsub,
+ }) async {
List<ValidationResult> results = <ValidationResult>[];
/// Runs all the validation defined in the service.
for (Validation validation in validations) {
- ValidationResult validationResult = await validation.validate(result, messagePullRequest);
+ final ValidationResult validationResult = await validation.validate(result, messagePullRequest);
results.add(validationResult);
}
- github.RepositorySlug slug = messagePullRequest.base!.repo!.slug();
+ final github.RepositorySlug slug = messagePullRequest.base!.repo!.slug();
final GithubService gitHubService = await config.createGithubService(slug);
/// If there is at least one action that requires to remove label do so and add comments for all the failures.
@@ -128,40 +144,161 @@
for (ValidationResult result in results) {
if (!result.result && result.action == Action.REMOVE_LABEL) {
final String commmentMessage = result.message.isEmpty ? 'Validations Fail.' : result.message;
- await gitHubService.createComment(slug, prNumber, commmentMessage);
- await gitHubService.removeLabel(slug, prNumber, Config.kAutosubmitLabel);
- log.info('auto label is removed for ${slug.fullName}, pr: $prNumber, due to $commmentMessage');
+
+ final String message = 'auto label is removed for ${slug.fullName}, pr: $prNumber, due to $commmentMessage';
+
+ await removeLabelAndComment(
+ githubService: gitHubService,
+ repositorySlug: slug,
+ prNumber: prNumber,
+ prLabel: Config.kAutosubmitLabel,
+ message: message);
+
+ log.info(message);
+
shouldReturn = true;
}
}
+
if (shouldReturn) {
log.info('The pr ${slug.fullName}/$prNumber with message: $ackId should be acknowledged.');
await pubsub.acknowledge('auto-submit-queue-sub', ackId);
log.info('The pr ${slug.fullName}/$prNumber is not feasible for merge and message: $ackId is acknowledged.');
return;
}
+
// If PR has some failures to ignore temporarily do nothing and continue.
for (ValidationResult result in results) {
if (!result.result && result.action == Action.IGNORE_TEMPORARILY) {
return;
}
}
+
// If we got to this point it means we are ready to submit the PR.
- bool processed = await processMerge(config, result, messagePullRequest);
- if (processed) await pubsub.acknowledge('auto-submit-queue-sub', ackId);
+ final ProcessMergeResult processed =
+ await processMerge(config: config, queryResult: result, messagePullRequest: messagePullRequest);
+
+ if (!processed.result) {
+ final String message = 'auto label is removed for ${slug.fullName}, pr: $prNumber, ${processed.message}.';
+
+ await removeLabelAndComment(
+ githubService: gitHubService,
+ repositorySlug: slug,
+ prNumber: prNumber,
+ prLabel: Config.kAutosubmitLabel,
+ message: message);
+
+ log.info(message);
+ }
+
log.info('Ack the processed message : $ackId.');
+ await pubsub.acknowledge('auto-submit-queue-sub', ackId);
+ }
+
+ /// The logic for processing a revert request and opening the follow up
+ /// review issue in github.
+ Future<void> processRevertRequest({
+ required Config config,
+ required QueryResult result,
+ required github.PullRequest messagePullRequest,
+ required String ackId,
+ required PubSub pubsub,
+ }) async {
+ final ValidationResult revertValidationResult = await revertValidation!.validate(result, messagePullRequest);
+
+ final github.RepositorySlug slug = messagePullRequest.base!.repo!.slug();
+ final int prNumber = messagePullRequest.number!;
+ final GithubService gitHubService = await config.createGithubService(slug);
+
+ if (revertValidationResult.result) {
+ // Approve the pull request automatically as it has been validated.
+ await approverService!.revertApproval(result, messagePullRequest);
+
+ final ProcessMergeResult processed = await processMerge(
+ config: config,
+ queryResult: result,
+ messagePullRequest: messagePullRequest,
+ );
+
+ if (processed.result) {
+ try {
+ final RevertReviewTemplate revertReviewTemplate = RevertReviewTemplate(
+ repositorySlug: slug.fullName,
+ revertPrNumber: prNumber,
+ revertPrAuthor: result.repository!.pullRequest!.author!.login!,
+ originalPrLink: revertValidation!.extractLinkFromText(messagePullRequest.body)!);
+
+ final github.Issue issue = await gitHubService.createIssue(
+ // Created issues are created and tracked within flutter/flutter.
+ slug: github.RepositorySlug('flutter', 'flutter'),
+ title: revertReviewTemplate.title!,
+ body: revertReviewTemplate.body!,
+ labels: <String>['P1'],
+ );
+ log.info('Issue #${issue.id} was created to track the review for $prNumber in ${slug.fullName}');
+ } on github.GitHubError catch (exception) {
+ // We have merged but failed to create follow up issue.
+ final String errorMessage = '''
+An exception has occurred while attempting to create the follow up review issue for $prNumber.
+Please create a follow up issue to track a review for this pull request.
+Exception: ${exception.message}
+''';
+ log.warning(errorMessage);
+ await gitHubService.createComment(slug, prNumber, errorMessage);
+ }
+ } else {
+ final String message = 'revert label is removed for ${slug.fullName}, pr: $prNumber, ${processed.message}.';
+
+ await removeLabelAndComment(
+ githubService: gitHubService,
+ repositorySlug: slug,
+ prNumber: prNumber,
+ prLabel: Config.kRevertLabel,
+ message: message,
+ );
+
+ log.info(message);
+ }
+ } else {
+ // since we do not temporarily ignore anything with a revert request we
+ // know we will report the error and remove the label.
+ final String commentMessage =
+ revertValidationResult.message.isEmpty ? 'Validations Fail.' : revertValidationResult.message;
+
+ await removeLabelAndComment(
+ githubService: gitHubService,
+ repositorySlug: slug,
+ prNumber: prNumber,
+ prLabel: Config.kRevertLabel,
+ message: commentMessage,
+ );
+
+ log.info('revert label is removed for ${slug.fullName}, pr: $prNumber, due to $commentMessage');
+ log.info('The pr ${slug.fullName}/$prNumber is not feasible for merge and message: $ackId is acknowledged.');
+ }
+
+ log.info('Ack the processed message : $ackId.');
+ await pubsub.acknowledge('auto-submit-queue-sub', ackId);
}
/// Merges the commit if the PullRequest passes all the validations.
- Future<bool> processMerge(Config config, QueryResult queryResult, github.PullRequest messagePullRequest) async {
- String id = queryResult.repository!.pullRequest!.id!;
- github.RepositorySlug slug = messagePullRequest.base!.repo!.slug();
+ Future<ProcessMergeResult> processMerge({
+ required Config config,
+ required QueryResult queryResult,
+ required github.PullRequest messagePullRequest,
+ }) async {
+ final String id = queryResult.repository!.pullRequest!.id!;
+ final github.RepositorySlug slug = messagePullRequest.base!.repo!.slug();
final PullRequest pullRequest = queryResult.repository!.pullRequest!;
- Commit commit = pullRequest.commits!.nodes!.single.commit!;
+ final Commit commit = pullRequest.commits!.nodes!.single.commit!;
final String? sha = commit.oid;
- int number = messagePullRequest.number!;
- final graphql.GraphQLClient client = await config.createGitHubGraphQLClient(slug);
+ final int number = messagePullRequest.number!;
+
try {
+ // The createGitHubGraphQLClient can throw Exception on github permissions
+ // errors.
+ final graphql.GraphQLClient client = await config.createGitHubGraphQLClient(slug);
+
final graphql.QueryResult result = await client.mutate(graphql.MutationOptions(
document: mergePullRequestMutation,
variables: <String, dynamic>{
@@ -171,60 +308,36 @@
},
));
if (result.hasException) {
- log.severe('Failed to merge pr#: $number with ${result.exception.toString()}');
- return false;
+ final String message = 'Failed to merge pr#: $number with ${result.exception}';
+ log.severe(message);
+ return ProcessMergeResult(false, message);
}
} catch (e) {
- log.severe('_processMerge error in $slug: $e');
- return false;
+ final String message = 'Failed to merge pr#: $number with $e';
+ log.severe(message);
+ return ProcessMergeResult(false, message);
}
- return true;
+ return ProcessMergeResult.noMessage(true);
}
- /// The logic for processing a revert request and opening the follow up
- /// review issue in github.
- Future<void> processRevertRequest(
- Config config,
- QueryResult result,
- github.PullRequest messagePullRequest,
- String ackId,
- PubSub pubsub,
- ) async {
- ValidationResult revertValidationResult = await revertValidation!.validate(result, messagePullRequest);
-
- github.RepositorySlug slug = messagePullRequest.base!.repo!.slug();
- final int prNumber = messagePullRequest.number!;
- final GithubService githubService = await config.createGithubService(slug);
-
- if (revertValidationResult.result) {
- // Approve the pull request automatically as it has been validated.
- ApproverService approverService = ApproverService(config);
- await approverService.revertApproval(result, messagePullRequest);
-
- bool processed = await processMerge(config, result, messagePullRequest);
- if (processed) {
- await pubsub.acknowledge('auto-submit-queue-sub', ackId);
- log.info('Ack the processed message : $ackId.');
- github.Issue issue = await githubService.createIssue(
- github.RepositorySlug('flutter', 'flutter'),
- 'Follow up review for revert pull request $prNumber',
- 'Revert request by author ${result.repository!.pullRequest!.author}',
- );
- log.info('Issue #${issue.id} was created to track the review for $prNumber in ${slug.fullName}');
- } else {
- log.warning('Could not process pull merge request.');
- }
- } else {
- // since we do not temporarily ignore anything with a revert request we
- // know we will report the error and remove the label.
- final String commmentMessage =
- revertValidationResult.message.isEmpty ? 'Validations Fail.' : revertValidationResult.message;
- await githubService.createComment(slug, prNumber, commmentMessage);
- await githubService.removeLabel(slug, prNumber, Config.kRevertLabel);
- log.info('revert label is removed for ${slug.fullName}, pr: $prNumber, due to $commmentMessage');
- log.info('The pr ${slug.fullName}/$prNumber with message: $ackId should be acknowledged.');
- await pubsub.acknowledge('auto-submit-queue-sub', ackId);
- log.info('The pr ${slug.fullName}/$prNumber is not feasible for merge and message: $ackId is acknowledged.');
- }
+ /// Remove a pull request label and add a comment to the pull request.
+ Future<void> removeLabelAndComment(
+ {required GithubService githubService,
+ required github.RepositorySlug repositorySlug,
+ required int prNumber,
+ required String prLabel,
+ required String message}) async {
+ await githubService.removeLabel(repositorySlug, prNumber, prLabel);
+ await githubService.createComment(repositorySlug, prNumber, message);
}
}
+
+/// Small wrapper class to allow us to capture and create a comment in the PR with
+/// the issue that caused the merge failure.
+class ProcessMergeResult {
+ ProcessMergeResult.noMessage(this.result);
+ ProcessMergeResult(this.result, this.message);
+
+ bool result = false;
+ String? message;
+}
diff --git a/auto_submit/test/requests/check_pull_request_test.dart b/auto_submit/test/requests/check_pull_request_test.dart
index 1ff9815..863ecb9 100644
--- a/auto_submit/test/requests/check_pull_request_test.dart
+++ b/auto_submit/test/requests/check_pull_request_test.dart
@@ -167,11 +167,13 @@
};
final List<LogRecord> records = <LogRecord>[];
log.onRecord.listen((LogRecord record) => records.add(record));
+ // this is the test.
await checkPullRequest.get();
- expect(pubsub.messagesQueue.length, 1);
+ // every failure is now acknowledged from the queue.
+ expect(pubsub.messagesQueue.length, 0);
final List<LogRecord> errorLogs = records.where((LogRecord record) => record.level == Level.SEVERE).toList();
expect(errorLogs.length, 1);
- expect(errorLogs[0].message.contains('_processMerge'), true);
+ expect(errorLogs[0].message.contains('Failed to merge'), true);
pubsub.messagesQueue.clear();
});
diff --git a/auto_submit/test/requests/github_webhook_test_data.dart b/auto_submit/test/requests/github_webhook_test_data.dart
index 54fea84..062575d 100644
--- a/auto_submit/test/requests/github_webhook_test_data.dart
+++ b/auto_submit/test/requests/github_webhook_test_data.dart
@@ -70,15 +70,17 @@
}''';
}
-PullRequest generatePullRequest(
- {String? labelName,
- String? autosubmitLabel = Config.kAutosubmitLabel,
- String? repoName,
- String? login,
- String? authorAssociation,
- String? author,
- int? prNumber,
- String? state}) {
+PullRequest generatePullRequest({
+ String? labelName,
+ String? autosubmitLabel = Config.kAutosubmitLabel,
+ String? repoName,
+ String? login,
+ String? authorAssociation,
+ String? author,
+ int? prNumber,
+ String? state,
+ String? body,
+}) {
return PullRequest.fromJson(json.decode('''{
"id": 1,
"number": ${prNumber ?? 1347},
@@ -88,7 +90,7 @@
"login": "${author ?? "octocat"}",
"id": 1
},
- "body": "Please pull these awesome changes in!",
+ "body": "${body ?? "Please pull these awesome changes in!"}",
"labels": [
{
"id": 487496476,
diff --git a/auto_submit/test/service/approver_service_test.dart b/auto_submit/test/service/approver_service_test.dart
index b6c16bf..f44a6f7 100644
--- a/auto_submit/test/service/approver_service_test.dart
+++ b/auto_submit/test/service/approver_service_test.dart
@@ -29,14 +29,14 @@
});
test('Verify approval ignored', () async {
- gh.PullRequest pr = generatePullRequest(author: 'not_a_user');
+ final gh.PullRequest pr = generatePullRequest(author: 'not_a_user');
await service.autoApproval(pr);
verifyNever(pullRequests.createReview(any, captureAny));
});
test('Verify approve', () async {
when(pullRequests.listReviews(any, any)).thenAnswer((_) => const Stream<gh.PullRequestReview>.empty());
- gh.PullRequest pr = generatePullRequest(author: 'dependabot[bot]');
+ final gh.PullRequest pr = generatePullRequest(author: 'dependabot[bot]');
await service.autoApproval(pr);
final List<dynamic> reviews = verify(pullRequests.createReview(any, captureAny)).captured;
expect(reviews.length, 1);
@@ -45,18 +45,18 @@
});
test('Already approved', () async {
- gh.PullRequestReview review =
+ final gh.PullRequestReview review =
gh.PullRequestReview(id: 123, user: gh.User(login: 'fluttergithubbot'), state: 'APPROVED');
when(pullRequests.listReviews(any, any)).thenAnswer((_) => Stream<gh.PullRequestReview>.value(review));
- gh.PullRequest pr = generatePullRequest(author: 'dependabot[bot]');
+ final gh.PullRequest pr = generatePullRequest(author: 'dependabot[bot]');
await service.autoApproval(pr);
verifyNever(pullRequests.createReview(any, captureAny));
});
test('AutoApproval does not approve revert pull request.', () async {
- gh.PullRequest pr = generatePullRequest(author: 'not_a_user');
- List<gh.IssueLabel> issueLabels = pr.labels ?? [];
- gh.IssueLabel issueLabel = gh.IssueLabel(name: 'revert');
+ final gh.PullRequest pr = generatePullRequest(author: 'not_a_user');
+ final List<gh.IssueLabel> issueLabels = pr.labels ?? [];
+ final gh.IssueLabel issueLabel = gh.IssueLabel(name: 'revert');
issueLabels.add(issueLabel);
await service.autoApproval(pr);
verifyNever(pullRequests.createReview(any, captureAny));
@@ -64,18 +64,18 @@
test('Revert request is auto approved.', () async {
when(pullRequests.listReviews(any, any)).thenAnswer((_) => const Stream<gh.PullRequestReview>.empty());
- gh.PullRequest pr = generatePullRequest(author: 'dependabot[bot]');
+ final gh.PullRequest pr = generatePullRequest(author: 'dependabot[bot]');
- List<gh.IssueLabel> issueLabels = pr.labels ?? [];
- gh.IssueLabel issueLabel = gh.IssueLabel(name: 'revert');
+ final List<gh.IssueLabel> issueLabels = pr.labels ?? [];
+ final gh.IssueLabel issueLabel = gh.IssueLabel(name: 'revert');
issueLabels.add(issueLabel);
- PullRequestHelper flutterRequest = PullRequestHelper(
+ final PullRequestHelper flutterRequest = PullRequestHelper(
prNumber: 0,
lastCommitHash: oid,
reviews: <PullRequestReviewHelper>[],
);
- QueryResult queryResult = createQueryResult(flutterRequest);
+ final QueryResult queryResult = createQueryResult(flutterRequest);
await service.revertApproval(queryResult, pr);
final List<dynamic> reviews = verify(pullRequests.createReview(any, captureAny)).captured;
@@ -85,28 +85,28 @@
});
test('Revert request is not auto approved when the revert label is not present.', () async {
- gh.PullRequest pr = generatePullRequest(author: 'not_a_user');
+ final gh.PullRequest pr = generatePullRequest(author: 'not_a_user');
- PullRequestHelper flutterRequest = PullRequestHelper(
+ final PullRequestHelper flutterRequest = PullRequestHelper(
prNumber: 0,
lastCommitHash: oid,
reviews: <PullRequestReviewHelper>[],
);
- QueryResult queryResult = createQueryResult(flutterRequest);
+ final QueryResult queryResult = createQueryResult(flutterRequest);
await service.revertApproval(queryResult, pr);
verifyNever(pullRequests.createReview(any, captureAny));
});
test('Revert request is not auto approved on bad author association.', () async {
- gh.PullRequest pr = generatePullRequest(author: 'not_a_user', authorAssociation: 'CONTRIBUTOR');
+ final gh.PullRequest pr = generatePullRequest(author: 'not_a_user', authorAssociation: 'CONTRIBUTOR');
- PullRequestHelper flutterRequest = PullRequestHelper(
+ final PullRequestHelper flutterRequest = PullRequestHelper(
prNumber: 0,
lastCommitHash: oid,
reviews: <PullRequestReviewHelper>[],
);
- QueryResult queryResult = createQueryResult(flutterRequest);
+ final QueryResult queryResult = createQueryResult(flutterRequest);
await service.revertApproval(queryResult, pr);
verifyNever(pullRequests.createReview(any, captureAny));
diff --git a/auto_submit/test/service/validation_service_test.dart b/auto_submit/test/service/validation_service_test.dart
index 9c04cf7..3bee455 100644
--- a/auto_submit/test/service/validation_service_test.dart
+++ b/auto_submit/test/service/validation_service_test.dart
@@ -2,17 +2,21 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-import 'package:auto_submit/model/auto_submit_query_result.dart' hide PullRequest;
+import 'package:auto_submit/model/auto_submit_query_result.dart' as auto hide PullRequest;
import 'package:auto_submit/service/process_method.dart';
import 'package:auto_submit/service/validation_service.dart';
+import 'package:auto_submit/validations/validation.dart';
import 'package:github/github.dart';
+import 'package:graphql/client.dart';
import 'package:test/test.dart';
import '../requests/github_webhook_test_data.dart';
import '../src/request_handling/fake_pubsub.dart';
+import '../src/service/fake_approver_service.dart';
import '../src/service/fake_config.dart';
import '../src/service/fake_graphql_client.dart';
import '../src/service/fake_github_service.dart';
+import '../src/validations/fake_revert.dart';
import '../utilities/utils.dart';
import '../utilities/mocks.dart';
@@ -31,8 +35,8 @@
slug = RepositorySlug('flutter', 'cocoon');
});
- test('removes label and post comment when no approval', () async {
- PullRequestHelper flutterRequest = PullRequestHelper(
+ test('Removes label and post comment when no approval', () async {
+ final PullRequestHelper flutterRequest = PullRequestHelper(
prNumber: 0,
lastCommitHash: oid,
reviews: <PullRequestReviewHelper>[],
@@ -42,17 +46,194 @@
final FakePubSub pubsub = FakePubSub();
final PullRequest pullRequest = generatePullRequest(prNumber: 0, repoName: slug.name);
pubsub.publish('auto-submit-queue-sub', pullRequest);
- QueryResult queryResult = createQueryResult(flutterRequest);
+ final auto.QueryResult queryResult = createQueryResult(flutterRequest);
- await validationService.processPullRequest(config, queryResult, pullRequest, 'test', pubsub);
+ await validationService.processPullRequest(
+ config: config, result: queryResult, messagePullRequest: pullRequest, ackId: 'test', pubsub: pubsub);
expect(githubService.issueComment, isNotNull);
expect(githubService.labelRemoved, true);
assert(pubsub.messagesQueue.isEmpty);
});
- group('shouldProcess pull request', () {
- test('should process message when autosubmit label exists and pr is open', () async {
+ test('Remove label and post comment when no revert label.', () async {
+ final PullRequestHelper flutterRequest = PullRequestHelper(
+ prNumber: 0,
+ lastCommitHash: oid,
+ reviews: <PullRequestReviewHelper>[],
+ );
+ githubService.checkRunsData = checkRunsMock;
+ githubService.createCommentData = createCommentMock;
+ final FakePubSub pubsub = FakePubSub();
+ final PullRequest pullRequest = generatePullRequest(
+ prNumber: 0,
+ repoName: slug.name,
+ );
+ pubsub.publish('auto-submit-queue-sub', pullRequest);
+ final auto.QueryResult queryResult = createQueryResult(flutterRequest);
+
+ await validationService.processRevertRequest(
+ config: config, result: queryResult, messagePullRequest: pullRequest, ackId: 'test', pubsub: pubsub);
+
+ expect(githubService.issueComment, isNotNull);
+ expect(githubService.labelRemoved, true);
+ assert(pubsub.messagesQueue.isEmpty);
+ });
+
+ group('Processing revert reqeuests.', () {
+ test('Merge valid revert request, issue created and message is acknowledged.', () async {
+ githubGraphQLClient.mutateResultForOptions = (MutationOptions options) => createFakeQueryResult();
+ final PullRequestHelper flutterRequest = PullRequestHelper(
+ prNumber: 0,
+ lastCommitHash: oid,
+ reviews: <PullRequestReviewHelper>[],
+ );
+
+ githubService.checkRunsData = checkRunsMock;
+ githubService.createCommentData = createCommentMock;
+ final FakePubSub pubsub = FakePubSub();
+ final PullRequest pullRequest = generatePullRequest(
+ prNumber: 0,
+ repoName: slug.name,
+ authorAssociation: 'OWNER',
+ labelName: 'revert',
+ body: 'Reverts flutter/flutter#1234');
+
+ final FakeRevert fakeRevert = FakeRevert(config: config);
+ fakeRevert.validationResult = ValidationResult(true, Action.REMOVE_LABEL, '');
+ validationService.revertValidation = fakeRevert;
+ final FakeApproverService fakeApproverService = FakeApproverService(config);
+ validationService.approverService = fakeApproverService;
+
+ final Issue issue = Issue(
+ id: 1234,
+ );
+ githubService.githubIssueMock = issue;
+
+ pubsub.publish('auto-submit-queue-sub', pullRequest);
+ final auto.QueryResult queryResult = createQueryResult(flutterRequest);
+
+ await validationService.processRevertRequest(
+ config: config, result: queryResult, messagePullRequest: pullRequest, ackId: 'test', pubsub: pubsub);
+
+ // if the merge is successful we do not remove the label and we do not add a comment to the issue.
+ expect(githubService.issueComment, isNull);
+ expect(githubService.labelRemoved, false);
+ // We acknowledge the issue.
+ assert(pubsub.messagesQueue.isEmpty);
+ });
+
+ test('Fail to merge non valid revert, issue not created, comment is added and message is acknowledged.', () async {
+ githubGraphQLClient.mutateResultForOptions = (MutationOptions options) => createFakeQueryResult();
+ final PullRequestHelper flutterRequest = PullRequestHelper(
+ prNumber: 0,
+ lastCommitHash: oid,
+ reviews: <PullRequestReviewHelper>[],
+ );
+
+ githubService.checkRunsData = checkRunsMock;
+ githubService.createCommentData = createCommentMock;
+ final FakePubSub pubsub = FakePubSub();
+ final PullRequest pullRequest = generatePullRequest(
+ prNumber: 0,
+ repoName: slug.name,
+ authorAssociation: 'OWNER',
+ labelName: 'revert',
+ body: 'Reverts flutter/flutter#1234',
+ );
+
+ final FakeRevert fakeRevert = FakeRevert(config: config);
+ fakeRevert.validationResult = ValidationResult(false, Action.REMOVE_LABEL, '');
+ validationService.revertValidation = fakeRevert;
+ final FakeApproverService fakeApproverService = FakeApproverService(config);
+ validationService.approverService = fakeApproverService;
+
+ pubsub.publish('auto-submit-queue-sub', pullRequest);
+ final auto.QueryResult queryResult = createQueryResult(flutterRequest);
+
+ await validationService.processRevertRequest(
+ config: config, result: queryResult, messagePullRequest: pullRequest, ackId: 'test', pubsub: pubsub);
+
+ // if the merge is successful we do not remove the label and we do not add a comment to the issue.
+ expect(githubService.issueComment, isNotNull);
+ expect(githubService.labelRemoved, true);
+ // We acknowledge the issue.
+ assert(pubsub.messagesQueue.isEmpty);
+ });
+
+ test('Remove label and post comment when unable to process merge.', () async {
+ final PullRequestHelper flutterRequest = PullRequestHelper(
+ prNumber: 0,
+ lastCommitHash: oid,
+ reviews: <PullRequestReviewHelper>[],
+ );
+ githubService.checkRunsData = checkRunsMock;
+ githubService.createCommentData = createCommentMock;
+ final FakePubSub pubsub = FakePubSub();
+ final PullRequest pullRequest =
+ generatePullRequest(prNumber: 0, repoName: slug.name, authorAssociation: 'OWNER', labelName: 'revert');
+
+ final FakeRevert fakeRevert = FakeRevert(config: config);
+ fakeRevert.validationResult = ValidationResult(true, Action.REMOVE_LABEL, '');
+ validationService.revertValidation = fakeRevert;
+ final FakeApproverService fakeApproverService = FakeApproverService(config);
+ validationService.approverService = fakeApproverService;
+
+ pubsub.publish('auto-submit-queue-sub', pullRequest);
+ final auto.QueryResult queryResult = createQueryResult(flutterRequest);
+
+ await validationService.processRevertRequest(
+ config: config, result: queryResult, messagePullRequest: pullRequest, ackId: 'test', pubsub: pubsub);
+
+ expect(githubService.issueComment, isNotNull);
+ expect(githubService.labelRemoved, true);
+ assert(pubsub.messagesQueue.isEmpty);
+ });
+
+ test('Fail to create follow up review issue, comment is added and message is acknowledged.', () async {
+ githubGraphQLClient.mutateResultForOptions = (MutationOptions options) => createFakeQueryResult();
+ final PullRequestHelper flutterRequest = PullRequestHelper(
+ prNumber: 0,
+ lastCommitHash: oid,
+ reviews: <PullRequestReviewHelper>[],
+ );
+
+ githubService.checkRunsData = checkRunsMock;
+ githubService.createCommentData = createCommentMock;
+ githubService.throwOnCreateIssue = true;
+ githubService.useRealComment = true;
+ final FakePubSub pubsub = FakePubSub();
+ final PullRequest pullRequest = generatePullRequest(
+ prNumber: 0,
+ repoName: slug.name,
+ authorAssociation: 'OWNER',
+ labelName: 'revert',
+ body: 'Reverts flutter/flutter#1234');
+
+ final FakeRevert fakeRevert = FakeRevert(config: config);
+ fakeRevert.validationResult = ValidationResult(true, Action.REMOVE_LABEL, '');
+ validationService.revertValidation = fakeRevert;
+ final FakeApproverService fakeApproverService = FakeApproverService(config);
+ validationService.approverService = fakeApproverService;
+
+ pubsub.publish('auto-submit-queue-sub', pullRequest);
+ final auto.QueryResult queryResult = createQueryResult(flutterRequest);
+
+ await validationService.processRevertRequest(
+ config: config, result: queryResult, messagePullRequest: pullRequest, ackId: 'test', pubsub: pubsub);
+
+ // if the merge is successful we do not remove the label and we do not add a comment to the issue.
+ expect(githubService.issueComment, isNotNull);
+ final IssueComment issueComment = githubService.issueComment!;
+ assert(issueComment.body!.contains('create the follow up review issue'));
+ expect(githubService.labelRemoved, false);
+ // We acknowledge the issue.
+ assert(pubsub.messagesQueue.isEmpty);
+ });
+ });
+
+ group('Process pull request', () {
+ test('Should process message when autosubmit label exists and pr is open', () async {
final PullRequest pullRequest = generatePullRequest(prNumber: 0, repoName: slug.name);
githubService.pullRequestData = pullRequest;
final ProcessMethod processMethod = await validationService.processPullRequestMethod(pullRequest);
@@ -60,7 +241,7 @@
expect(processMethod, ProcessMethod.processAutosubmit);
});
- test('skip processing message when autosubmit label does not exist anymore', () async {
+ test('Skip processing message when autosubmit label does not exist anymore', () async {
final PullRequest pullRequest = generatePullRequest(prNumber: 0, repoName: slug.name);
pullRequest.labels = <IssueLabel>[];
githubService.pullRequestData = pullRequest;
@@ -69,7 +250,7 @@
expect(processMethod, ProcessMethod.doNotProcess);
});
- test('skip processing message when the pull request is closed', () async {
+ test('Skip processing message when the pull request is closed', () async {
final PullRequest pullRequest = generatePullRequest(prNumber: 0, repoName: slug.name);
pullRequest.state = 'closed';
githubService.pullRequestData = pullRequest;
@@ -78,9 +259,9 @@
expect(processMethod, ProcessMethod.doNotProcess);
});
- test('should process message when revert label exists and pr is open', () async {
+ test('Should process message when revert label exists and pr is open', () async {
final PullRequest pullRequest = generatePullRequest(prNumber: 0, repoName: slug.name);
- IssueLabel issueLabel = IssueLabel(name: 'revert');
+ final IssueLabel issueLabel = IssueLabel(name: 'revert');
pullRequest.labels = <IssueLabel>[issueLabel];
githubService.pullRequestData = pullRequest;
final ProcessMethod processMethod = await validationService.processPullRequestMethod(pullRequest);
@@ -88,9 +269,9 @@
expect(processMethod, ProcessMethod.processRevert);
});
- test('should process message as revert when revert and autosubmit labels are present and pr is open', () async {
+ test('Should process message as revert when revert and autosubmit labels are present and pr is open', () async {
final PullRequest pullRequest = generatePullRequest(prNumber: 0, repoName: slug.name);
- IssueLabel issueLabel = IssueLabel(name: 'revert');
+ final IssueLabel issueLabel = IssueLabel(name: 'revert');
pullRequest.labels!.add(issueLabel);
githubService.pullRequestData = pullRequest;
final ProcessMethod processMethod = await validationService.processPullRequestMethod(pullRequest);
@@ -98,10 +279,10 @@
expect(processMethod, ProcessMethod.processRevert);
});
- test('skip processing message when revert label exists and pr is closed', () async {
+ test('Skip processing message when revert label exists and pr is closed', () async {
final PullRequest pullRequest = generatePullRequest(prNumber: 0, repoName: slug.name);
pullRequest.state = 'closed';
- IssueLabel issueLabel = IssueLabel(name: 'revert');
+ final IssueLabel issueLabel = IssueLabel(name: 'revert');
pullRequest.labels = <IssueLabel>[issueLabel];
githubService.pullRequestData = pullRequest;
final ProcessMethod processMethod = await validationService.processPullRequestMethod(pullRequest);
diff --git a/auto_submit/test/src/service/fake_approver_service.dart b/auto_submit/test/src/service/fake_approver_service.dart
new file mode 100644
index 0000000..fb25486
--- /dev/null
+++ b/auto_submit/test/src/service/fake_approver_service.dart
@@ -0,0 +1,21 @@
+// Copyright 2022 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'package:auto_submit/model/auto_submit_query_result.dart';
+import 'package:auto_submit/service/approver_service.dart';
+import 'package:github/github.dart' as gh;
+
+class FakeApproverService extends ApproverService {
+ FakeApproverService(super.config);
+
+ @override
+ Future<void> autoApproval(gh.PullRequest pullRequest) async {
+ // no op
+ }
+
+ @override
+ Future<void> revertApproval(QueryResult queryResult, gh.PullRequest pullRequest) async {
+ // no op
+ }
+}
diff --git a/auto_submit/test/src/service/fake_github_service.dart b/auto_submit/test/src/service/fake_github_service.dart
index 4dafb32..05ba02d 100644
--- a/auto_submit/test/src/service/fake_github_service.dart
+++ b/auto_submit/test/src/service/fake_github_service.dart
@@ -36,6 +36,8 @@
String? pullRequestFilesJsonMock;
Issue? githubIssueMock;
+ bool throwOnCreateIssue = false;
+
/// Setting either of these flags to true will pop the front element from the
/// list. Setting either to false will just return the non list version from
/// the appropriate method.
@@ -46,6 +48,7 @@
List<PullRequest?> pullRequestMockList = [];
IssueComment? issueComment;
+ bool useRealComment = false;
bool labelRemoved = false;
bool compareReturnValue = false;
@@ -134,7 +137,11 @@
@override
Future<IssueComment> createComment(RepositorySlug slug, int number, String commentBody) async {
- issueComment = IssueComment.fromJson(jsonDecode(createCommentMock!) as Map<String, dynamic>);
+ if (useRealComment) {
+ issueComment = IssueComment(id: number, body: commentBody);
+ } else {
+ issueComment = IssueComment.fromJson(jsonDecode(createCommentMock!) as Map<String, dynamic>);
+ }
return issueComment!;
}
@@ -174,7 +181,18 @@
}
@override
- Future<Issue> createIssue(RepositorySlug repositorySlug, String title, String body) async {
+ Future<Issue> createIssue({
+ required RepositorySlug slug,
+ required String title,
+ required String body,
+ List<String>? labels,
+ String? assignee,
+ List<String>? assignees,
+ String? state,
+ }) async {
+ if (throwOnCreateIssue) {
+ throw GitHubError(github, 'Exception on github create issue.');
+ }
return githubIssueMock!;
}
diff --git a/auto_submit/test/src/validations/fake_revert.dart b/auto_submit/test/src/validations/fake_revert.dart
new file mode 100644
index 0000000..63c84e3
--- /dev/null
+++ b/auto_submit/test/src/validations/fake_revert.dart
@@ -0,0 +1,20 @@
+// Copyright 2022 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'package:auto_submit/model/auto_submit_query_result.dart' hide PullRequest;
+import 'package:auto_submit/validations/revert.dart';
+import 'package:auto_submit/validations/validation.dart';
+
+import 'package:github/github.dart';
+
+class FakeRevert extends Revert {
+ FakeRevert({required super.config});
+
+ ValidationResult? validationResult;
+
+ @override
+ Future<ValidationResult> validate(QueryResult result, PullRequest messagePullRequest) async {
+ return validationResult ?? ValidationResult(false, Action.IGNORE_TEMPORARILY, '');
+ }
+}
