Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion auto_submit/lib/configuration/repository_configuration.dart
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ class RepositoryConfiguration {
static const String supportNoReviewRevertKey = 'support_no_review_revert';
static const String requiredCheckRunsOnRevertKey =
'required_checkruns_on_revert';
static const String baseCommitAllowedDaysKey = 'base_commit_allowed_days';

static const String defaultBranchStr = 'default';

Expand All @@ -33,14 +34,16 @@ class RepositoryConfiguration {
bool? runCi,
bool? supportNoReviewReverts,
Set<String>? requiredCheckRunsOnRevert,
int? baseCommitAllowedDays,
}) : allowConfigOverride = allowConfigOverride ?? false,
defaultBranch = defaultBranch ?? defaultBranchStr,
autoApprovalAccounts = autoApprovalAccounts ?? <String>{},
approvingReviews = approvingReviews ?? 2,
approvalGroup = approvalGroup ?? 'flutter-hackers',
runCi = runCi ?? true,
supportNoReviewReverts = supportNoReviewReverts ?? true,
requiredCheckRunsOnRevert = requiredCheckRunsOnRevert ?? <String>{};
requiredCheckRunsOnRevert = requiredCheckRunsOnRevert ?? <String>{},
baseCommitAllowedDays = baseCommitAllowedDays ?? 0;

/// This flag allows the repository to override the org level configuration.
bool allowConfigOverride;
Expand Down Expand Up @@ -71,6 +74,10 @@ class RepositoryConfiguration {
/// merged.
Set<String> requiredCheckRunsOnRevert;

/// The number of days that the base commit of the pull request can not be
/// older than. If less than 1 then it will not be checked.
int baseCommitAllowedDays;

@override
String toString() {
final stringBuffer = StringBuffer();
Expand All @@ -88,6 +95,7 @@ class RepositoryConfiguration {
for (var checkrun in requiredCheckRunsOnRevert) {
stringBuffer.writeln(' - $checkrun');
}
stringBuffer.writeln('$baseCommitAllowedDaysKey: $baseCommitAllowedDays');
return stringBuffer.toString();
}

Expand Down Expand Up @@ -125,6 +133,7 @@ class RepositoryConfiguration {
runCi: yamlDoc[runCiKey] as bool?,
supportNoReviewReverts: yamlDoc[supportNoReviewRevertKey] as bool?,
requiredCheckRunsOnRevert: requiredCheckRunsOnRevert,
baseCommitAllowedDays: yamlDoc[baseCommitAllowedDaysKey] as int?,
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ class RepositoryConfigurationManager {
runCi: globalConfiguration.runCi,
supportNoReviewReverts: globalConfiguration.supportNoReviewReverts,
requiredCheckRunsOnRevert: globalConfiguration.requiredCheckRunsOnRevert,
baseCommitAllowedDays: globalConfiguration.baseCommitAllowedDays,
);

// auto approval accounts, they should be empty if nothing was defined
Expand Down Expand Up @@ -186,6 +187,15 @@ class RepositoryConfigurationManager {
);
}

// if the local configuration is not the default value then use it.
// Can be setted to negative value in local configuration to turn-off
// base commit allowed days validation.
final localBaseCommitAllowedDays = localConfiguration.baseCommitAllowedDays;
if (localBaseCommitAllowedDays != 0) {
mergedRepositoryConfiguration.baseCommitAllowedDays =
localConfiguration.baseCommitAllowedDays;
}

return mergedRepositoryConfiguration;
}
}
84 changes: 84 additions & 0 deletions auto_submit/lib/validations/base_commit_date_allowed.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
// Copyright 2025 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:cocoon_server/logging.dart';
import 'package:github/github.dart' as github;

import '../model/auto_submit_query_result.dart';
import 'validation.dart';

/// Validates that the base of the PR commit is not older than a specified in
/// configuration number of days.
class BaseCommitDateAllowed extends Validation {
BaseCommitDateAllowed({required super.config});

@override
String get name => 'BaseCommitDateAllowed';

@override
/// Implements the validation.
Future<ValidationResult> validate(
QueryResult result,
github.PullRequest messagePullRequest,
) async {
final slug = messagePullRequest.base!.repo!.slug();
final gitHubService = await config.createGithubService(slug);
final sha = messagePullRequest.head!.sha!;
final repositoryConfiguration = await config.getRepositoryConfiguration(
slug,
);

// If the base commit allowed days is less than or equal to 0 then the
// validation is turned off and the base commit creation date is ignored.
if (repositoryConfiguration.baseCommitAllowedDays <= 0) {
log.info(
'PR base commit creation date validation turned off by setting '
'base_commit_allowed_days to ${repositoryConfiguration.baseCommitAllowedDays}.',
);
return ValidationResult(
true,
Action.IGNORE_FAILURE,
'PR base commit creation date validation turned off by setting '
'base_commit_allowed_days to ${repositoryConfiguration.baseCommitAllowedDays}.',
);
}

// If the base commit creation date is null then the validation fails but
// should not block the PR merging.
final commit = await gitHubService.getCommit(slug, sha);
if (commit.commit?.author?.date == null) {
log.info(
'PR ${slug.fullName}/${messagePullRequest.number} base commit creation '
'date is null.',
);
return ValidationResult(
false,
Action.IGNORE_FAILURE,
'Could not find the base commit creation date of the PR '
'${slug.fullName}/${messagePullRequest.number}.',
);
}

log.info(
'PR ${slug.fullName}/${messagePullRequest.number} base creation date is: '
'${commit.commit?.author?.date}. Allowed age is '
'${repositoryConfiguration.baseCommitAllowedDays} days.',
);

final isBaseRecent = commit.commit!.author!.date!.isAfter(
DateTime.now().subtract(
Duration(days: repositoryConfiguration.baseCommitAllowedDays),
),
);

final message = isBaseRecent
? 'The base commit of the PR is recent enough for merging.'
: 'The base commit of the PR is older than '
'${repositoryConfiguration.baseCommitAllowedDays} days and can not '
'be merged. Please merge the latest changes from the main into '
'this branch and resubmit the PR.';

return ValidationResult(isBaseRecent, Action.REMOVE_LABEL, message);
}
}
2 changes: 2 additions & 0 deletions auto_submit/lib/validations/validation_filter.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import '../configuration/repository_configuration.dart';
import '../service/config.dart';
import '../service/process_method.dart';
import 'approval.dart';
import 'base_commit_date_allowed.dart';
import 'ci_successful.dart';
import 'empty_checks.dart';
import 'mergeable.dart';
Expand Down Expand Up @@ -47,6 +48,7 @@ class PullRequestValidationFilter implements ValidationFilter {
Set<Validation> getValidations() {
final validationsToRun = <Validation>{};

validationsToRun.add(BaseCommitDateAllowed(config: config));
validationsToRun.add(Approval(config: config));
// If we are running ci then we need to check the checkRuns and make sure
// there are check runs created.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,5 @@ const String sampleConfigWithOverride = '''
support_no_review_revert: true
required_checkruns_on_revert:
- ci.yaml validation
base_commit_allowed_days: 7
''';
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ void main() {
- Google-testing
- test (ubuntu-latest, 2.18.0)
- cla/google
base_commit_allowed_days: 10
''';

githubService.fileContentsMockList.add(sampleConfig);
Expand Down Expand Up @@ -92,6 +93,7 @@ void main() {
repositoryConfiguration.requiredCheckRunsOnRevert.contains('cla/google'),
isTrue,
);
expect(repositoryConfiguration.baseCommitAllowedDays, 10);
});

test(
Expand Down Expand Up @@ -189,6 +191,7 @@ void main() {
),
isTrue,
);
expect(repositoryConfiguration.baseCommitAllowedDays, 0);
});

test('Default branch collected if omitted main', () async {
Expand All @@ -204,6 +207,7 @@ void main() {
required_checkruns_on_revert:
- ci.yaml validation
- Google-testing
base_commit_allowed_days: 7
''';

githubService.fileContentsMockList.add(sampleConfig);
Expand Down Expand Up @@ -236,6 +240,7 @@ void main() {
),
isTrue,
);
expect(repositoryConfiguration.baseCommitAllowedDays, 7);
});

group('Merging configurations tests', () {
Expand All @@ -253,6 +258,7 @@ void main() {
support_no_review_revert: true
required_checkruns_on_revert:
- ci.yaml validation
base_commit_allowed_days: 7
*/

test('Global config merged with default local config', () {
Expand Down Expand Up @@ -296,6 +302,7 @@ void main() {
),
isTrue,
);
expect(mergedRepositoryConfiguration.baseCommitAllowedDays, 7);
});

test('Auto approval accounts is additive, they cannot be removed', () {
Expand Down Expand Up @@ -398,29 +405,59 @@ void main() {
);
});

test('Approving reviews is not overridden if less than global config', () {
const expectedApprovingReviews = 2;
final localRepositoryConfiguration = RepositoryConfiguration(
approvingReviews: 1,
);
final globalRepositoryConfiguration = RepositoryConfiguration.fromYaml(
sampleConfigWithOverride,
);
final mergedRepositoryConfiguration = repositoryConfigurationManager
.mergeConfigurations(
globalRepositoryConfiguration,
localRepositoryConfiguration,
);
expect(
globalRepositoryConfiguration.approvingReviews ==
mergedRepositoryConfiguration.approvingReviews,
isTrue,
);
expect(
mergedRepositoryConfiguration.approvingReviews,
expectedApprovingReviews,
);
});
test(
'Base commit recent days overridden by local config if value is not 0',
() {
const expectedBaseCommitAllowedDays = -1;
final localRepositoryConfiguration = RepositoryConfiguration(
baseCommitAllowedDays: expectedBaseCommitAllowedDays,
);
final globalRepositoryConfiguration = RepositoryConfiguration.fromYaml(
sampleConfigWithOverride,
);
final mergedRepositoryConfiguration = repositoryConfigurationManager
.mergeConfigurations(
globalRepositoryConfiguration,
localRepositoryConfiguration,
);
expect(
globalRepositoryConfiguration.baseCommitAllowedDays !=
mergedRepositoryConfiguration.baseCommitAllowedDays,
isTrue,
);
expect(
mergedRepositoryConfiguration.baseCommitAllowedDays,
expectedBaseCommitAllowedDays,
);
},
);

test(
'Base commit recent days is not overridden by local config if value is 0',
() {
const expectedBaseCommitAllowedDays = 7;
final localRepositoryConfiguration = RepositoryConfiguration(
baseCommitAllowedDays: 0,
);
final globalRepositoryConfiguration = RepositoryConfiguration.fromYaml(
sampleConfigWithOverride,
);
final mergedRepositoryConfiguration = repositoryConfigurationManager
.mergeConfigurations(
globalRepositoryConfiguration,
localRepositoryConfiguration,
);
expect(
globalRepositoryConfiguration.baseCommitAllowedDays ==
mergedRepositoryConfiguration.baseCommitAllowedDays,
isTrue,
);
expect(
mergedRepositoryConfiguration.baseCommitAllowedDays,
expectedBaseCommitAllowedDays,
);
},
);

test('Approval group is overridden if defined', () {
const expectedApprovalGroup = 'flutter-devs';
Expand Down
Loading