Skip to content

Commit

Permalink
Revert not correctly collecting default branch (#3110)
Browse files Browse the repository at this point in the history
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: flutter/flutter#135692

*List which issues are fixed by this PR. You must list at least one issue.*
Fixes flutter/flutter#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].*
  • Loading branch information
ricardoamador authored Sep 28, 2023
1 parent 3ac2559 commit 959d73e
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 13 deletions.
12 changes: 2 additions & 10 deletions auto_submit/lib/configuration/repository_configuration.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> autoApprovalAccountsDefault = <String>{};
static const int approvingReviewsDefault = 2;
static const String approvalGroupDefault = 'flutter-hackers';
static const bool runCiDefault = true;
static const bool supportNoReviewRevertsDefault = true;
static const Set<String> requiredCheckRunsOnRevertDefault = <String>{};
static const String defaultBranchStr = 'default';

RepositoryConfiguration({
allowConfigOverride,
Expand All @@ -40,7 +32,7 @@ class RepositoryConfiguration {
supportNoReviewReverts,
requiredCheckRunsOnRevert,
}) : allowConfigOverride = allowConfigOverride ?? false,
defaultBranch = defaultBranch ?? 'main',
defaultBranch = defaultBranch ?? defaultBranchStr,
autoApprovalAccounts = autoApprovalAccounts ?? <String>{},
approvingReviews = approvingReviews ?? 2,
approvalGroup = approvalGroup ?? 'flutter-hackers',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}.');
Expand Down
1 change: 1 addition & 0 deletions auto_submit/lib/git/git_repository_manager.dart
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ class GitRepositoryManager {
Future<void> 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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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);
Expand Down

0 comments on commit 959d73e

Please sign in to comment.