From 959d73ec2503026f5c49717d4fa215ad04fc55b7 Mon Sep 17 00:00:00 2001 From: Ricardo Amador <32242716+ricardoamador@users.noreply.github.com> Date: Thu, 28 Sep 2023 14:23:00 -0700 Subject: [PATCH] Revert not correctly collecting default branch (#3110) This is a fix to the way the default branch is collected. The code was wrongly checking for if the default branch was empty after the config was collected. It was never empty because a default value of main was assigned so that was used each time. This changes the default branch to have a default value of 'default' and then we check against that value to get true default branch from the repo. Example pull request of the fix: https://github.com/flutter/flutter/pull/135692 *List which issues are fixed by this PR. You must list at least one issue.* Fixes https://github.com/flutter/flutter/issues/135670 *If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].* --- .../repository_configuration.dart | 12 +----- .../repository_configuration_manager.dart | 2 +- .../lib/git/git_repository_manager.dart | 1 + ...repository_configuration_manager_test.dart | 40 ++++++++++++++++++- 4 files changed, 42 insertions(+), 13 deletions(-) diff --git a/auto_submit/lib/configuration/repository_configuration.dart b/auto_submit/lib/configuration/repository_configuration.dart index 787ae582b..7eea7d15a 100644 --- a/auto_submit/lib/configuration/repository_configuration.dart +++ b/auto_submit/lib/configuration/repository_configuration.dart @@ -20,15 +20,7 @@ class RepositoryConfiguration { static const String supportNoReviewRevertKey = 'support_no_review_revert'; static const String requiredCheckRunsOnRevertKey = 'required_checkruns_on_revert'; - // Default values - static const bool allowConfigOverrideDefault = false; - static const String defaultBranchDefault = 'main'; - static const Set autoApprovalAccountsDefault = {}; - static const int approvingReviewsDefault = 2; - static const String approvalGroupDefault = 'flutter-hackers'; - static const bool runCiDefault = true; - static const bool supportNoReviewRevertsDefault = true; - static const Set requiredCheckRunsOnRevertDefault = {}; + static const String defaultBranchStr = 'default'; RepositoryConfiguration({ allowConfigOverride, @@ -40,7 +32,7 @@ class RepositoryConfiguration { supportNoReviewReverts, requiredCheckRunsOnRevert, }) : allowConfigOverride = allowConfigOverride ?? false, - defaultBranch = defaultBranch ?? 'main', + defaultBranch = defaultBranch ?? defaultBranchStr, autoApprovalAccounts = autoApprovalAccounts ?? {}, approvingReviews = approvingReviews ?? 2, approvalGroup = approvalGroup ?? 'flutter-hackers', diff --git a/auto_submit/lib/configuration/repository_configuration_manager.dart b/auto_submit/lib/configuration/repository_configuration_manager.dart index 324ebdd27..56f59e00a 100644 --- a/auto_submit/lib/configuration/repository_configuration_manager.dart +++ b/auto_submit/lib/configuration/repository_configuration_manager.dart @@ -69,7 +69,7 @@ class RepositoryConfigurationManager { final RepositoryConfiguration globalRepositoryConfiguration = RepositoryConfiguration.fromYaml(orgLevelConfig); // Collect the default branch if it was not supplied. - if (globalRepositoryConfiguration.defaultBranch.isEmpty) { + if (globalRepositoryConfiguration.defaultBranch == RepositoryConfiguration.defaultBranchStr) { globalRepositoryConfiguration.defaultBranch = await githubService.getDefaultBranch(slug); } log.info('Default branch was found to be ${globalRepositoryConfiguration.defaultBranch} for ${slug.fullName}.'); diff --git a/auto_submit/lib/git/git_repository_manager.dart b/auto_submit/lib/git/git_repository_manager.dart index eed98cff3..b5402a70f 100644 --- a/auto_submit/lib/git/git_repository_manager.dart +++ b/auto_submit/lib/git/git_repository_manager.dart @@ -66,6 +66,7 @@ class GitRepositoryManager { Future revertCommit(String baseBranchName, String commitSha, RepositorySlug slug, String token) async { final GitRevertBranchName revertBranchName = GitRevertBranchName(commitSha); // Working directory for these must be repo checkout directory. + // Check out the baseBranchName before doing anything. await gitCli.createBranch( newBranchName: revertBranchName.branch, workingDirectory: targetCloneDirectory, diff --git a/auto_submit/test/configuration/repository_configuration_manager_test.dart b/auto_submit/test/configuration/repository_configuration_manager_test.dart index 01045484d..37b4ce824 100644 --- a/auto_submit/test/configuration/repository_configuration_manager_test.dart +++ b/auto_submit/test/configuration/repository_configuration_manager_test.dart @@ -105,7 +105,43 @@ void main() { expect(repositoryConfiguration.requiredCheckRunsOnRevert.contains('Google-testing'), isTrue); }); - test('Default branch collected if omitted', () async { + test('Default branch collected if omitted master', () async { + const String sampleConfig = ''' + auto_approval_accounts: + - dependabot[bot] + - dependabot + - DartDevtoolWorkflowBot + approving_reviews: 2 + approval_group: flutter-hackers + run_ci: true + support_no_review_revert: true + required_checkruns_on_revert: + - ci.yaml validation + - Google-testing + '''; + + githubService.fileContentsMockList.add(sampleConfig); + githubService.defaultBranch = 'master'; + final RepositoryConfiguration repositoryConfiguration = + await repositoryConfigurationManager.readRepositoryConfiguration( + RepositorySlug('flutter', 'flutter'), + ); + + expect(repositoryConfiguration.allowConfigOverride, isFalse); + expect(repositoryConfiguration.defaultBranch, 'master'); + expect(repositoryConfiguration.autoApprovalAccounts.isNotEmpty, isTrue); + expect(repositoryConfiguration.autoApprovalAccounts.length, 3); + expect(repositoryConfiguration.approvingReviews, 2); + expect(repositoryConfiguration.approvalGroup, 'flutter-hackers'); + expect(repositoryConfiguration.runCi, isTrue); + expect(repositoryConfiguration.supportNoReviewReverts, isTrue); + expect(repositoryConfiguration.requiredCheckRunsOnRevert.isNotEmpty, isTrue); + expect(repositoryConfiguration.requiredCheckRunsOnRevert.length, 2); + expect(repositoryConfiguration.requiredCheckRunsOnRevert.contains('ci.yaml validation'), isTrue); + expect(repositoryConfiguration.requiredCheckRunsOnRevert.contains('Google-testing'), isTrue); + }); + + test('Default branch collected if omitted main', () async { const String sampleConfig = ''' auto_approval_accounts: - dependabot[bot] @@ -124,7 +160,7 @@ void main() { githubService.defaultBranch = 'main'; final RepositoryConfiguration repositoryConfiguration = await repositoryConfigurationManager.readRepositoryConfiguration( - RepositorySlug('flutter', 'cocoon'), + RepositorySlug('flutter', 'flutter'), ); expect(repositoryConfiguration.allowConfigOverride, isFalse);