From 9193008b1df86c32562049c448289c6e8d17019b Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Thu, 15 May 2025 16:01:42 -0700 Subject: [PATCH] When using revert-bot, it applies the right base-ref/branch-name. --- .../lib/action/git_cli_revert_method.dart | 13 +- .../action/git_cli_revert_method_test.dart | 163 ++++++++++++++++++ .../requests/github_webhook_test_data.dart | 2 + 3 files changed, 173 insertions(+), 5 deletions(-) create mode 100644 auto_submit/test/action/git_cli_revert_method_test.dart diff --git a/auto_submit/lib/action/git_cli_revert_method.dart b/auto_submit/lib/action/git_cli_revert_method.dart index 9895360bc3..d76f58b058 100644 --- a/auto_submit/lib/action/git_cli_revert_method.dart +++ b/auto_submit/lib/action/git_cli_revert_method.dart @@ -6,6 +6,7 @@ import 'dart:io'; import 'package:cocoon_server/logging.dart'; import 'package:github/github.dart' as github; +import 'package:meta/meta.dart'; import 'package:retry/retry.dart'; import '../git/cli_command.dart'; @@ -19,6 +20,11 @@ import '../service/github_service.dart'; import 'revert_method.dart'; class GitCliRevertMethod implements RevertMethod { + GitCliRevertMethod({ + @visibleForTesting GitCli? gitCli, // + }) : _gitCli = gitCli ?? GitCli(GitAccessMethod.HTTP, CliCommand()); + final GitCli _gitCli; + @override Future createRevert( Config config, @@ -30,17 +36,14 @@ class GitCliRevertMethod implements RevertMethod { final commitSha = pullRequestToRevert.mergeCommitSha!; // we will need to collect the pr number after the revert request is generated. - final repositoryConfiguration = await config.getRepositoryConfiguration( - slug, - ); - final baseBranch = repositoryConfiguration.defaultBranch; + final baseBranch = pullRequestToRevert.base!.ref!; final cloneToDirectory = '${slug.name}_$commitSha'; final gitRepositoryManager = GitRepositoryManager( slug: slug, workingDirectory: Directory.current.path, cloneToDirectory: cloneToDirectory, - gitCli: GitCli(GitAccessMethod.HTTP, CliCommand()), + gitCli: _gitCli, ); // The exception is caught by the thrower. diff --git a/auto_submit/test/action/git_cli_revert_method_test.dart b/auto_submit/test/action/git_cli_revert_method_test.dart new file mode 100644 index 0000000000..30b007ea83 --- /dev/null +++ b/auto_submit/test/action/git_cli_revert_method_test.dart @@ -0,0 +1,163 @@ +// Copyright 2023 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 'dart:io'; + +import 'package:auto_submit/action/git_cli_revert_method.dart'; +import 'package:auto_submit/git/git_cli.dart'; +import 'package:auto_submit/service/config.dart'; +import 'package:auto_submit/service/github_service.dart'; +import 'package:cocoon_server_test/test_logging.dart'; +import 'package:github/src/common/model/pulls.dart'; +import 'package:github/src/common/model/repos.dart'; +import 'package:test/fake.dart'; +import 'package:test/test.dart'; + +import '../requests/github_webhook_test_data.dart'; + +void main() { + useTestLoggerPerTest(); + + late _FakeGitCli gitCli; + + setUp(() { + gitCli = _FakeGitCli(); + }); + + test('createRevert uses the branch from the originating PR', () async { + final config = _FakeConfig(); + final method = GitCliRevertMethod(gitCli: gitCli); + final revert = await method.createRevert( + config, + 'matanlurey', + 'I said so', + generatePullRequest( + baseRef: 'flutter-3.32-candidate.0', + mergeCommitSha: 'abc123', + ), + ); + expect(gitCli.createBranch$newBranchName, 'revert_abc123'); + expect(gitCli.setUpstream$branchName, 'revert_abc123'); + expect(gitCli.pushBranch$branchName, 'revert_abc123'); + expect(revert?.base?.ref, 'flutter-3.32-candidate.0'); + }); +} + +final class _FakeConfig extends Fake implements Config { + @override + Future generateGithubToken(RepositorySlug slug) async { + return 'a_github_token'; + } + + @override + Future createGithubService(RepositorySlug slug) async { + return _FakeGithubService(); + } +} + +final class _FakeGithubService extends Fake implements GithubService { + @override + Future getBranch(RepositorySlug slug, String branchName) async { + return Branch(branchName, null); + } + + @override + Future> getPullRequestReviews( + RepositorySlug slug, + int pullRequestNumber, + ) async { + return []; + } + + @override + Future createPullRequest({ + required RepositorySlug slug, + String? title, + String? head, + required String base, + bool draft = false, + String? body, + }) async { + return PullRequest(base: PullRequestHead(ref: base)); + } +} + +final class _FakeGitCli extends Fake implements GitCli { + @override + Future cloneRepository({ + required RepositorySlug slug, + required String workingDirectory, + required String targetDirectory, + List? options, + bool throwOnError = true, + }) async { + return ProcessResult(0, 0, '', ''); + } + + @override + Future setupUserConfig({ + required RepositorySlug slug, + required String workingDirectory, + bool throwOnError = true, + }) async { + return ProcessResult(0, 0, '', ''); + } + + @override + Future setupUserEmailConfig({ + required RepositorySlug slug, + required String workingDirectory, + bool throwOnError = true, + }) async { + return ProcessResult(0, 0, '', ''); + } + + late String createBranch$newBranchName; + + @override + Future createBranch({ + required String newBranchName, + required String workingDirectory, + bool useCheckout = false, + bool throwOnError = true, + }) async { + createBranch$newBranchName = newBranchName; + return ProcessResult(0, 0, '', ''); + } + + late String setUpstream$branchName; + + @override + Future setUpstream({ + required RepositorySlug slug, + required String workingDirectory, + required String branchName, + required String token, + bool throwOnError = true, + }) async { + setUpstream$branchName = branchName; + return ProcessResult(0, 0, '', ''); + } + + @override + Future revertChange({ + required String commitSha, + required String workingDirectory, + bool throwOnError = true, + }) async { + return ProcessResult(0, 0, '', ''); + } + + late String pushBranch$branchName; + + @override + Future pushBranch({ + required String branchName, + required String workingDirectory, + bool throwOnError = true, + }) async { + pushBranch$branchName = branchName; + return ProcessResult(0, 0, '', ''); + } +} diff --git a/auto_submit/test/requests/github_webhook_test_data.dart b/auto_submit/test/requests/github_webhook_test_data.dart index c1d458955c..109ac915cf 100644 --- a/auto_submit/test/requests/github_webhook_test_data.dart +++ b/auto_submit/test/requests/github_webhook_test_data.dart @@ -94,6 +94,7 @@ PullRequest generatePullRequest({ bool? mergeable = true, String baseRef = 'main', DateTime? mergedAt, + String mergeCommitSha = 'abc123', }) { return PullRequest.fromJson( json.decode('''{ @@ -119,6 +120,7 @@ PullRequest generatePullRequest({ ], "created_at": "2011-01-26T19:01:12Z", "merged_at": "${mergedAt ?? DateTime.now().subtract(const Duration(hours: 12))}", + "merge_commit_sha": "$mergeCommitSha", "closed_at": "2011-01-26T19:10:12Z", "head": { "label": "octocat:new-topic",