diff --git a/script/tool/lib/src/branch_for_batch_release_command.dart b/script/tool/lib/src/branch_for_batch_release_command.dart index 68d7c5905c3..e852f462c48 100644 --- a/script/tool/lib/src/branch_for_batch_release_command.dart +++ b/script/tool/lib/src/branch_for_batch_release_command.dart @@ -8,7 +8,6 @@ import 'dart:math' as math; import 'package:file/file.dart'; import 'package:git/git.dart'; import 'package:pub_semver/pub_semver.dart'; -import 'package:yaml/yaml.dart'; import 'package:yaml_edit/yaml_edit.dart'; import 'common/core.dart'; @@ -19,10 +18,6 @@ import 'common/repository_package.dart'; const int _kExitPackageMalformed = 3; const int _kGitFailedToPush = 4; -// The template file name used to draft a pending changelog file. -// This file will not be picked up by the batch release process. -const String _kTemplateFileName = 'template.yaml'; - /// A command to create a remote branch with release changes for a single package. class BranchForBatchReleaseCommand extends PackageCommand { /// Creates a new `branch-for-batch-release` command. @@ -69,9 +64,15 @@ class BranchForBatchReleaseCommand extends PackageCommand { final GitDir repository = await gitDir; print('Parsing package "${package.displayName}"...'); - final _PendingChangelogs pendingChangelogs = - await _getPendingChangelogs(package); - if (pendingChangelogs.entries.isEmpty) { + final List pendingChangelogs; + try { + pendingChangelogs = package.getPendingChangelogs(); + } on FormatException catch (e) { + printError('Failed to parse pending changelogs: ${e.message}'); + throw ToolExit(_kExitPackageMalformed); + } + + if (pendingChangelogs.isEmpty) { print('No pending changelogs found for ${package.displayName}.'); return; } @@ -84,7 +85,7 @@ class BranchForBatchReleaseCommand extends PackageCommand { throw ToolExit(_kExitPackageMalformed); } final _ReleaseInfo releaseInfo = - _getReleaseInfo(pendingChangelogs.entries, pubspec.version!); + _getReleaseInfo(pendingChangelogs, pubspec.version!); if (releaseInfo.newVersion == null) { print('No version change specified in pending changelogs for ' @@ -96,67 +97,34 @@ class BranchForBatchReleaseCommand extends PackageCommand { git: repository, package: package, branchName: branchName, - pendingChangelogFiles: pendingChangelogs.files, + pendingChangelogFiles: pendingChangelogs + .map((PendingChangelogEntry e) => e.file) + .toList(), releaseInfo: releaseInfo, remoteName: remoteName, ); } - /// Returns the parsed changelog entries for the given package. - /// - /// This method read through the files in the pending_changelogs folder - /// and parsed each file as a changelog entry. - /// - /// Throws a [ToolExit] if the package does not have a pending_changelogs folder. - Future<_PendingChangelogs> _getPendingChangelogs( - RepositoryPackage package) async { - final Directory pendingChangelogsDir = - package.directory.childDirectory('pending_changelogs'); - if (!pendingChangelogsDir.existsSync()) { - printError( - 'No pending_changelogs folder found for ${package.displayName}.'); - throw ToolExit(_kExitPackageMalformed); - } - final List pendingChangelogFiles = pendingChangelogsDir - .listSync() - .whereType() - .where((File f) => - f.basename.endsWith('.yaml') && f.basename != _kTemplateFileName) - .toList(); - try { - final List<_PendingChangelogEntry> entries = pendingChangelogFiles - .map<_PendingChangelogEntry>( - (File f) => _PendingChangelogEntry.parse(f.readAsStringSync())) - .toList(); - return _PendingChangelogs(entries, pendingChangelogFiles); - } on FormatException catch (e) { - printError('Malformed pending changelog file: $e'); - throw ToolExit(_kExitPackageMalformed); - } - } - /// Returns the release info for the given package. /// /// This method read through the parsed changelog entries decide the new version /// by following the version change rules. See [_VersionChange] for more details. _ReleaseInfo _getReleaseInfo( - List<_PendingChangelogEntry> pendingChangelogEntries, - Version oldVersion) { + List pendingChangelogEntries, Version oldVersion) { final List changelogs = []; - int versionIndex = _VersionChange.skip.index; - for (final _PendingChangelogEntry entry in pendingChangelogEntries) { + int versionIndex = VersionChange.skip.index; + for (final PendingChangelogEntry entry in pendingChangelogEntries) { changelogs.add(entry.changelog); versionIndex = math.min(versionIndex, entry.version.index); } - final _VersionChange effectiveVersionChange = - _VersionChange.values[versionIndex]; + final VersionChange effectiveVersionChange = + VersionChange.values[versionIndex]; final Version? newVersion = switch (effectiveVersionChange) { - _VersionChange.skip => null, - _VersionChange.major => Version(oldVersion.major + 1, 0, 0), - _VersionChange.minor => - Version(oldVersion.major, oldVersion.minor + 1, 0), - _VersionChange.patch => + VersionChange.skip => null, + VersionChange.major => Version(oldVersion.major + 1, 0, 0), + VersionChange.minor => Version(oldVersion.major, oldVersion.minor + 1, 0), + VersionChange.patch => Version(oldVersion.major, oldVersion.minor, oldVersion.patch + 1), }; return _ReleaseInfo(newVersion, changelogs); @@ -266,18 +234,6 @@ class BranchForBatchReleaseCommand extends PackageCommand { } } -/// A data class for pending changelogs. -class _PendingChangelogs { - /// Creates a new instance. - _PendingChangelogs(this.entries, this.files); - - /// The parsed pending changelog entries. - final List<_PendingChangelogEntry> entries; - - /// The files that the pending changelog entries were parsed from. - final List files; -} - /// A data class for processed release information. class _ReleaseInfo { /// Creates a new instance. @@ -289,62 +245,3 @@ class _ReleaseInfo { /// The combined changelog entries. final List changelogs; } - -/// The type of version change for a release. -/// -/// The order of the enum values is important as it is used to determine which version -/// take priority when multiple version changes are specified. The top most value -/// (the samller the index) has the highest priority. -enum _VersionChange { - /// A major version change (e.g., 1.2.3 -> 2.0.0). - major, - - /// A minor version change (e.g., 1.2.3 -> 1.3.0). - minor, - - /// A patch version change (e.g., 1.2.3 -> 1.2.4). - patch, - - /// No version change. - skip, -} - -/// Represents a single entry in the pending changelog. -class _PendingChangelogEntry { - /// Creates a new pending changelog entry. - _PendingChangelogEntry({required this.changelog, required this.version}); - - /// Creates a PendingChangelogEntry from a YAML string. - factory _PendingChangelogEntry.parse(String yamlContent) { - final dynamic yaml = loadYaml(yamlContent); - if (yaml is! YamlMap) { - throw FormatException( - 'Expected a YAML map, but found ${yaml.runtimeType}.'); - } - - final dynamic changelogYaml = yaml['changelog']; - if (changelogYaml is! String) { - throw FormatException( - 'Expected "changelog" to be a string, but found ${changelogYaml.runtimeType}.'); - } - final String changelog = changelogYaml.trim(); - - final String? versionString = yaml['version'] as String?; - if (versionString == null) { - throw const FormatException('Missing "version" key.'); - } - final _VersionChange version = _VersionChange.values.firstWhere( - (_VersionChange e) => e.name == versionString, - orElse: () => - throw FormatException('Invalid version type: $versionString'), - ); - - return _PendingChangelogEntry(changelog: changelog, version: version); - } - - /// The changelog messages for this entry. - final String changelog; - - /// The type of version change for this entry. - final _VersionChange version; -} diff --git a/script/tool/lib/src/common/package_state_utils.dart b/script/tool/lib/src/common/package_state_utils.dart index a5ac2daeb8a..fd52bf3df3e 100644 --- a/script/tool/lib/src/common/package_state_utils.dart +++ b/script/tool/lib/src/common/package_state_utils.dart @@ -80,6 +80,13 @@ Future checkPackageChangeState( continue; } + if (package.parseCiConfig()?.isBatchRelease ?? false) { + if (components.first == 'pending_changelogs') { + hasChangelogChange = true; + continue; + } + } + if (!needsVersionChange) { // Developer-only changes don't need version changes or changelog changes. if (await _isDevChange(components, git: git, repoPath: path)) { diff --git a/script/tool/lib/src/common/repository_package.dart b/script/tool/lib/src/common/repository_package.dart index cbcd023743c..e0ac1680f65 100644 --- a/script/tool/lib/src/common/repository_package.dart +++ b/script/tool/lib/src/common/repository_package.dart @@ -5,12 +5,17 @@ import 'package:file/file.dart'; import 'package:path/path.dart' as p; import 'package:pubspec_parse/pubspec_parse.dart'; +import 'package:yaml/yaml.dart'; import 'core.dart'; export 'package:pubspec_parse/pubspec_parse.dart' show Pubspec; export 'core.dart' show FlutterPlatform; +// The template file name used to draft a pending changelog file. +// This file will not be picked up by the batch release process. +const String _kTemplateFileName = 'template.yaml'; + /// A package in the repository. // // TODO(stuartmorgan): Add more package-related info here, such as an on-demand @@ -77,6 +82,10 @@ class RepositoryPackage { File get prePublishScript => directory.childDirectory('tool').childFile('pre_publish.dart'); + /// The directory containing pending changelog entries. + Directory get pendingChangelogsDirectory => + directory.childDirectory('pending_changelogs'); + /// Returns the directory containing support for [platform]. Directory platformDirectory(FlutterPlatform platform) { late final String directoryName; @@ -115,6 +124,15 @@ class RepositoryPackage { /// Caches for future use. Pubspec parsePubspec() => _parsedPubspec; + late final CiConfig? _parsedCiConfig = ciConfigFile.existsSync() + ? CiConfig._parse(ciConfigFile.readAsStringSync()) + : null; + + /// Returns the parsed [ciConfigFile], or null if it does not exist. + /// + /// Throws if the file exists but is not a valid ci_config.yaml. + CiConfig? parseCiConfig() => _parsedCiConfig; + /// Returns true if the package depends on Flutter. bool requiresFlutter() { const String flutterDependency = 'flutter'; @@ -214,4 +232,180 @@ class RepositoryPackage { bool isPublishable() { return parsePubspec().publishTo != 'none'; } + + /// Returns the parsed changelog entries for the package. + /// + /// This method reads through the files in the pending_changelogs folder + /// and parses each file as a changelog entry. + /// + /// Throws if the folder does not exist, or if any of the files are not + /// valid changelog entries. + List getPendingChangelogs() { + final List entries = []; + + final Directory pendingChangelogsDir = pendingChangelogsDirectory; + if (!pendingChangelogsDir.existsSync()) { + throw FormatException( + 'No pending_changelogs folder found for $displayName.'); + } + + final List allFiles = + pendingChangelogsDir.listSync().whereType().toList(); + + final List pendingChangelogFiles = []; + for (final File file in allFiles) { + final String basename = p.basename(file.path); + if (basename.endsWith('.yaml')) { + if (basename != _kTemplateFileName) { + pendingChangelogFiles.add(file); + } + } else { + throw FormatException( + 'Found non-YAML file in pending_changelogs: ${file.path}'); + } + } + + for (final File file in pendingChangelogFiles) { + try { + entries + .add(PendingChangelogEntry._parse(file.readAsStringSync(), file)); + } on FormatException catch (e) { + throw FormatException( + 'Malformed pending changelog file: ${file.path}\n$e'); + } + } + return entries; + } +} + +/// A class representing the parsed content of a `ci_config.yaml` file. +class CiConfig { + /// Creates a [CiConfig] from a parsed YAML map. + CiConfig._(this.isBatchRelease); + + /// Parses a [CiConfig] from a YAML string. + /// + /// Throws if the YAML is not a valid ci_config.yaml. + factory CiConfig._parse(String yaml) { + final Object? loaded = loadYaml(yaml); + if (loaded is! YamlMap) { + throw const FormatException('Root of ci_config.yaml must be a map.'); + } + + _checkCiConfigEntries(loaded, syntax: _validCiConfigSyntax); + + bool isBatchRelease = false; + final Object? release = loaded['release']; + if (release is Map) { + isBatchRelease = release['batch'] == true; + } + + return CiConfig._(isBatchRelease); + } + + static const Map _validCiConfigSyntax = { + 'release': { + 'batch': {true, false} + }, + }; + + /// Returns true if the package is configured for batch release. + final bool isBatchRelease; + + static void _checkCiConfigEntries(YamlMap config, + {required Map syntax, String configPrefix = ''}) { + for (final MapEntry entry in config.entries) { + if (!syntax.containsKey(entry.key)) { + throw FormatException( + 'Unknown key `${entry.key}` in config${_formatConfigPrefix(configPrefix)}, the possible keys are ${syntax.keys.toList()}'); + } else { + final Object syntaxValue = syntax[entry.key]!; + final String newConfigPrefix = configPrefix.isEmpty + ? entry.key! as String + : '$configPrefix.${entry.key}'; + if (syntaxValue is Set) { + if (!syntaxValue.contains(entry.value)) { + throw FormatException( + 'Invalid value `${entry.value}` for key${_formatConfigPrefix(newConfigPrefix)}, the possible values are ${syntaxValue.toList()}'); + } + } else if (entry.value is! YamlMap) { + throw FormatException( + 'Invalid value `${entry.value}` for key${_formatConfigPrefix(newConfigPrefix)}, the value must be a map'); + } else { + _checkCiConfigEntries(entry.value! as YamlMap, + syntax: syntaxValue as Map, + configPrefix: newConfigPrefix); + } + } + } + } + + static String _formatConfigPrefix(String configPrefix) => + configPrefix.isEmpty ? '' : ' `$configPrefix`'; +} + +/// The type of version change described by a changelog entry. +/// +/// The order of the enum values is important as it is used to determine which version +/// take priority when multiple version changes are specified. The top most value +/// (the samller the index) has the highest priority. +enum VersionChange { + /// A major version change (e.g., 1.2.3 -> 2.0.0). + major, + + /// A minor version change (e.g., 1.2.3 -> 1.3.0). + minor, + + /// A patch version change (e.g., 1.2.3 -> 1.2.4). + patch, + + /// No version change. + skip, +} + +/// Represents a single entry in the pending changelog. +class PendingChangelogEntry { + /// Creates a new pending changelog entry. + PendingChangelogEntry( + {required this.changelog, required this.version, required this.file}); + + /// Creates a PendingChangelogEntry from a YAML string. + /// + /// Throws if the YAML is not a valid pending changelog entry. + factory PendingChangelogEntry._parse(String yamlContent, File file) { + final dynamic yaml = loadYaml(yamlContent); + if (yaml is! YamlMap) { + throw FormatException( + 'Expected a YAML map, but found ${yaml.runtimeType}.'); + } + + final dynamic changelogYaml = yaml['changelog']; + if (changelogYaml is! String) { + throw FormatException( + 'Expected "changelog" to be a string, but found ${changelogYaml.runtimeType}.'); + } + final String changelog = changelogYaml.trim(); + + final String? versionString = yaml['version'] as String?; + if (versionString == null) { + throw const FormatException('Missing "version" key.'); + } + final VersionChange version = VersionChange.values.firstWhere( + (VersionChange e) => e.name == versionString, + orElse: () => + throw FormatException('Invalid version type: $versionString'), + ); + + return PendingChangelogEntry( + changelog: changelog, version: version, file: file); + } + + /// The changelog messages for this entry. + final String changelog; + + /// The type of version change for this entry. + final VersionChange version; + + /// The file that this entry was parsed from. + final File file; } diff --git a/script/tool/lib/src/repo_package_info_check_command.dart b/script/tool/lib/src/repo_package_info_check_command.dart index c8d50eef9e1..07bbe684cf8 100644 --- a/script/tool/lib/src/repo_package_info_check_command.dart +++ b/script/tool/lib/src/repo_package_info_check_command.dart @@ -3,7 +3,6 @@ // found in the LICENSE file. import 'package:file/file.dart'; -import 'package:yaml/yaml.dart'; import 'common/core.dart'; import 'common/output_utils.dart'; @@ -13,12 +12,6 @@ import 'common/repository_package.dart'; const int _exitBadTableEntry = 3; const int _exitUnknownPackageEntry = 4; -const Map _validCiConfigSyntax = { - 'release': { - 'batch': {true, false} - }, -}; - /// A command to verify repository-level metadata about packages, such as /// repo README and CODEOWNERS entries. class RepoPackageInfoCheckCommand extends PackageLoopingCommand { @@ -144,9 +137,22 @@ class RepoPackageInfoCheckCommand extends PackageLoopingCommand { } // The content of ci_config.yaml must be valid if there is one. - if (package.ciConfigFile.existsSync()) { - errors.addAll( - _validateCiConfig(package.ciConfigFile, mainPackage: package)); + CiConfig? ciConfig; + try { + ciConfig = package.parseCiConfig(); + } on FormatException catch (e) { + printError('$indentation${e.message}'); + errors.add(e.message); + } + + // All packages with batch release enabled should have valid pending changelogs. + if (ciConfig?.isBatchRelease ?? false) { + try { + package.getPendingChangelogs(); + } on FormatException catch (e) { + printError('$indentation${e.message}'); + errors.add(e.message); + } } // All published packages should have a README.md entry. @@ -241,67 +247,6 @@ class RepoPackageInfoCheckCommand extends PackageLoopingCommand { return errors; } - List _validateCiConfig(File ciConfig, - {required RepositoryPackage mainPackage}) { - print('${indentation}Checking ' - '${getRelativePosixPath(ciConfig, from: mainPackage.directory)}...'); - final YamlMap config; - try { - final Object? yaml = loadYaml(ciConfig.readAsStringSync()); - if (yaml is! YamlMap) { - printError('${indentation}The ci_config.yaml file must be a map.'); - return ['Root of config is not a map.']; - } - config = yaml; - } on YamlException catch (e) { - printError( - '${indentation}Invalid YAML in ${getRelativePosixPath(ciConfig, from: mainPackage.directory)}:'); - printError(e.toString()); - return ['Invalid YAML']; - } - - return _checkCiConfigEntries(config, syntax: _validCiConfigSyntax); - } - - List _checkCiConfigEntries(YamlMap config, - {required Map syntax, String configPrefix = ''}) { - final List errors = []; - for (final MapEntry entry in config.entries) { - if (!syntax.containsKey(entry.key)) { - printError( - '${indentation}Unknown key `${entry.key}` in config${_formatConfigPrefix(configPrefix)}, the possible keys are ${syntax.keys.toList()}'); - errors.add( - 'Unknown key `${entry.key}` in config${_formatConfigPrefix(configPrefix)}'); - } else { - final Object syntaxValue = syntax[entry.key]!; - configPrefix = configPrefix.isEmpty - ? entry.key! as String - : '$configPrefix.${entry.key}'; - if (syntaxValue is Set) { - if (!syntaxValue.contains(entry.value)) { - printError( - '${indentation}Invalid value `${entry.value}` for key${_formatConfigPrefix(configPrefix)}, the possible values are ${syntaxValue.toList()}'); - errors.add( - 'Invalid value `${entry.value}` for key${_formatConfigPrefix(configPrefix)}'); - } - } else if (entry.value is! YamlMap) { - printError( - '${indentation}Invalid value `${entry.value}` for key${_formatConfigPrefix(configPrefix)}, the value must be a map'); - errors.add( - 'Invalid value `${entry.value}` for key${_formatConfigPrefix(configPrefix)}'); - } else { - errors.addAll(_checkCiConfigEntries(entry.value! as YamlMap, - syntax: syntaxValue as Map, - configPrefix: configPrefix)); - } - } - } - return errors; - } - - String _formatConfigPrefix(String configPrefix) => - configPrefix.isEmpty ? '' : ' `$configPrefix`'; - String _prTagForPackage(String packageName) => 'p: $packageName'; String _issueTagForPackage(String packageName) { diff --git a/script/tool/lib/src/version_check_command.dart b/script/tool/lib/src/version_check_command.dart index 4d147cb92ba..9fac6b1c1d3 100644 --- a/script/tool/lib/src/version_check_command.dart +++ b/script/tool/lib/src/version_check_command.dart @@ -157,6 +157,12 @@ class VersionCheckCommand extends PackageLoopingCommand { late final Set _prLabels = _getPRLabels(); + Future _getRelativePackagePath(RepositoryPackage package) async { + final Directory gitRoot = + packagesDir.fileSystem.directory((await gitDir).path); + return getRelativePosixPath(package.directory, from: gitRoot); + } + @override final String name = 'version-check'; @@ -200,27 +206,73 @@ class VersionCheckCommand extends PackageLoopingCommand { final List errors = []; - bool versionChanged; + final CiConfig? ciConfig = package.parseCiConfig(); final _CurrentVersionState versionState = await _getVersionState(package, pubspec: pubspec); - switch (versionState) { - case _CurrentVersionState.unchanged: - versionChanged = false; - case _CurrentVersionState.validIncrease: - case _CurrentVersionState.validRevert: - case _CurrentVersionState.newPackage: - versionChanged = true; - case _CurrentVersionState.invalidChange: - versionChanged = true; - errors.add('Disallowed version change.'); - case _CurrentVersionState.unknown: - versionChanged = false; - errors.add('Unable to determine previous version.'); - } - - if (!(await _validateChangelogVersion(package, - pubspec: pubspec, pubspecVersionState: versionState))) { - errors.add('CHANGELOG.md failed validation.'); + final bool usesBatchRelease = ciConfig?.isBatchRelease ?? false; + // PR with post release label is going to sync changelog.md and pubspec.yaml + // change back to main branch. We can proceed with ragular version check. + final bool hasPostReleaseLabel = + _prLabels.contains('post-release-${pubspec.name}'); + bool versionChanged = false; + + if (usesBatchRelease && !hasPostReleaseLabel) { + final String relativePackagePath = await _getRelativePackagePath(package); + final List changedFilesInPackage = changedFiles + .where((String path) => path.startsWith(relativePackagePath)) + .toList(); + + // For batch release, we only check pending changelog files. + final List allChangelogs; + try { + allChangelogs = package.getPendingChangelogs(); + } on FormatException catch (e) { + errors.add(e.message); + return PackageResult.fail(errors); + } + + final List newEntries = allChangelogs + .where((PendingChangelogEntry entry) => changedFilesInPackage.any( + (String path) => path.endsWith(entry.file.path.split('/').last))) + .toList(); + versionChanged = newEntries.any( + (PendingChangelogEntry entry) => entry.version != VersionChange.skip); + + // The changelog.md and pubspec.yaml's version should not be updated directly. + if (changedFilesInPackage.contains('$relativePackagePath/CHANGELOG.md')) { + printError( + 'This package uses batch release, so CHANGELOG.md should not be changed directly.\n' + 'Instead, create a pending changelog file in pending_changelogs folder.'); + errors.add('CHANGELOG.md changed'); + } + if (changedFilesInPackage.contains('$relativePackagePath/pubspec.yaml')) { + if (versionState != _CurrentVersionState.unchanged) { + printError( + 'This package uses batch release, so the version in pubspec.yaml should not be changed directly.\n' + 'Instead, create a pending changelog file in pending_changelogs folder.'); + errors.add('pubspec.yaml version changed'); + } + } + } else { + switch (versionState) { + case _CurrentVersionState.unchanged: + versionChanged = false; + case _CurrentVersionState.validIncrease: + case _CurrentVersionState.validRevert: + case _CurrentVersionState.newPackage: + versionChanged = true; + case _CurrentVersionState.invalidChange: + versionChanged = true; + errors.add('Disallowed version change.'); + case _CurrentVersionState.unknown: + versionChanged = false; + errors.add('Unable to determine previous version.'); + } + + if (!(await _validateChangelogVersion(package, + pubspec: pubspec, pubspecVersionState: versionState))) { + errors.add('CHANGELOG.md failed validation.'); + } } // If there are no other issues, make sure that there isn't a missing @@ -533,10 +585,7 @@ ${indentation}The first version listed in CHANGELOG.md is $fromChangeLog. // Find the relative path to the current package, as it would appear at the // beginning of a path reported by changedFiles (which always uses // Posix paths). - final Directory gitRoot = - packagesDir.fileSystem.directory((await gitDir).path); - final String relativePackagePath = - getRelativePosixPath(package.directory, from: gitRoot); + final String relativePackagePath = await _getRelativePackagePath(package); final PackageChangeState state = await checkPackageChangeState(package, changedPaths: changedFiles, @@ -572,14 +621,28 @@ ${indentation}The first version listed in CHANGELOG.md is $fromChangeLog. '"$_missingChangelogChangeOverrideLabel" label.'); } else { missingChangelogChange = true; - printError('No CHANGELOG change found.\n' - 'If this PR needs an exemption from the standard policy of listing ' - 'all changes in the CHANGELOG,\n' - 'comment in the PR to explain why the PR is exempt, and add (or ' - 'ask your reviewer to add) the\n' - '"$_missingChangelogChangeOverrideLabel" label.\n' - 'Otherwise, please add a NEXT entry in the CHANGELOG as described in ' - 'the contributing guide.\n'); + final bool isBatchRelease = + package.parseCiConfig()?.isBatchRelease ?? false; + if (isBatchRelease) { + printError( + 'No new changelog files found in the pending_changelogs folder.\n' + 'If this PR needs an exemption from the standard policy of listing ' + 'all changes in the CHANGELOG,\n' + 'comment in the PR to explain why the PR is exempt, and add (or ' + 'ask your reviewer to add) the\n' + '"$_missingChangelogChangeOverrideLabel" label.\n' + 'Otherwise, please add a NEXT entry in the CHANGELOG as described in ' + 'the contributing guide.\n'); + } else { + printError('No CHANGELOG change found.\n' + 'If this PR needs an exemption from the standard policy of listing ' + 'all changes in the CHANGELOG,\n' + 'comment in the PR to explain why the PR is exempt, and add (or ' + 'ask your reviewer to add) the\n' + '"$_missingChangelogChangeOverrideLabel" label.\n' + 'Otherwise, please add a NEXT entry in the CHANGELOG as described in ' + 'the contributing guide.\n'); + } } } diff --git a/script/tool/test/common/package_state_utils_test.dart b/script/tool/test/common/package_state_utils_test.dart index 7ec4089dbe0..29f1de33638 100644 --- a/script/tool/test/common/package_state_utils_test.dart +++ b/script/tool/test/common/package_state_utils_test.dart @@ -498,6 +498,26 @@ void main() { expect(state.needsVersionChange, true); expect(state.needsChangelogChange, true); }); + + test('detects pending changelog changes for batch release', () async { + final RepositoryPackage package = + createFakePackage('a_package', packagesDir); + package.ciConfigFile.writeAsStringSync(''' +release: + batch: true +'''); + + const List changedFiles = [ + 'packages/a_package/pending_changelogs/some_change.yaml', + ]; + + final PackageChangeState state = await checkPackageChangeState(package, + changedPaths: changedFiles, + relativePackagePath: 'packages/a_package/'); + + expect(state.hasChanges, true); + expect(state.hasChangelogChange, true); + }); }); } diff --git a/script/tool/test/common/repository_package_test.dart b/script/tool/test/common/repository_package_test.dart index b43fb80fca5..ccf7d3e1246 100644 --- a/script/tool/test/common/repository_package_test.dart +++ b/script/tool/test/common/repository_package_test.dart @@ -238,4 +238,169 @@ void main() { expect(package.requiresFlutter(), false); }); }); + group('ciConfig', () { + test('file', () async { + final RepositoryPackage plugin = + createFakePlugin('a_plugin', packagesDir); + + final File ciConfigFile = plugin.ciConfigFile; + + expect( + ciConfigFile.path, plugin.directory.childFile('ci_config.yaml').path); + }); + + test('parsing', () async { + final RepositoryPackage plugin = + createFakePlugin('a_plugin', packagesDir); + plugin.ciConfigFile.writeAsStringSync(''' +release: + batch: true +'''); + + final CiConfig? config = plugin.parseCiConfig(); + + expect(config, isNotNull); + expect(config!.isBatchRelease, isTrue); + }); + + test('parsing missing file returns null', () async { + final RepositoryPackage plugin = + createFakePlugin('a_plugin', packagesDir); + + final CiConfig? config = plugin.parseCiConfig(); + + expect(config, isNull); + }); + + test('parsing invalid file throws', () async { + final RepositoryPackage plugin = + createFakePlugin('a_plugin', packagesDir); + plugin.ciConfigFile.writeAsStringSync('not a map'); + + expect( + () => plugin.parseCiConfig(), + throwsA(isA().having( + (FormatException e) => e.message, + 'message', + contains('Root of ci_config.yaml must be a map')))); + }); + + test('reports unknown keys', () { + final RepositoryPackage plugin = + createFakePlugin('a_plugin', packagesDir); + plugin.ciConfigFile.writeAsStringSync(''' +foo: bar +'''); + + expect( + () => plugin.parseCiConfig(), + throwsA(isA().having( + (FormatException e) => e.message, + 'message', + contains('Unknown key `foo` in config')))); + }); + + test('reports invalid values', () { + final RepositoryPackage plugin = + createFakePlugin('a_plugin', packagesDir); + plugin.ciConfigFile.writeAsStringSync(''' +release: + batch: not-a-bool +'''); + + expect( + () => plugin.parseCiConfig(), + throwsA(isA().having( + (FormatException e) => e.message, + 'message', + contains('Invalid value `not-a-bool` for key `release.batch`')))); + }); + }); + + group('getPendingChangelogs', () { + test('returns an error if the directory is missing', () { + final RepositoryPackage package = + createFakePackage('a_package', packagesDir); + + expect(() => package.getPendingChangelogs(), throwsFormatException); + }); + + test('returns empty lists if the directory is empty', () { + final RepositoryPackage package = + createFakePackage('a_package', packagesDir); + package.pendingChangelogsDirectory.createSync(); + + final List changelogs = + package.getPendingChangelogs(); + + expect(changelogs, isEmpty); + }); + + test('returns entries for valid files', () { + final RepositoryPackage package = + createFakePackage('a_package', packagesDir); + package.pendingChangelogsDirectory.createSync(); + package.pendingChangelogsDirectory + .childFile('a.yaml') + .writeAsStringSync(''' +changelog: A +version: patch +'''); + package.pendingChangelogsDirectory + .childFile('b.yaml') + .writeAsStringSync(''' +changelog: B +version: minor +'''); + + final List changelogs = + package.getPendingChangelogs(); + + expect(changelogs, hasLength(2)); + expect(changelogs[0].changelog, 'A'); + expect(changelogs[0].version, VersionChange.patch); + expect(changelogs[1].changelog, 'B'); + expect(changelogs[1].version, VersionChange.minor); + }); + + test('returns an error for a malformed file', () { + final RepositoryPackage package = + createFakePackage('a_package', packagesDir); + package.pendingChangelogsDirectory.createSync(); + final File changelogFile = + package.pendingChangelogsDirectory.childFile('a.yaml'); + changelogFile.writeAsStringSync('not yaml'); + + expect( + () => package.getPendingChangelogs(), + throwsA(isA().having( + (FormatException e) => e.message, + 'message', + contains('Expected a YAML map, but found String')))); + }); + + test('ignores template.yaml', () { + final RepositoryPackage package = + createFakePackage('a_package', packagesDir); + package.pendingChangelogsDirectory.createSync(); + package.pendingChangelogsDirectory + .childFile('a.yaml') + .writeAsStringSync(''' +changelog: A +version: patch +'''); + package.pendingChangelogsDirectory + .childFile('template.yaml') + .writeAsStringSync(''' +changelog: TEMPLATE +version: skip +'''); + + final List changelogs = + package.getPendingChangelogs(); + + expect(changelogs, hasLength(1)); + expect(changelogs[0].changelog, 'A'); + }); + }); } diff --git a/script/tool/test/repo_package_info_check_command_test.dart b/script/tool/test/repo_package_info_check_command_test.dart index 01d3195bb62..696af070871 100644 --- a/script/tool/test/repo_package_info_check_command_test.dart +++ b/script/tool/test/repo_package_info_check_command_test.dart @@ -489,7 +489,6 @@ release: expect( output, containsAll([ - contains(' Checking ci_config.yaml...'), contains('No issues found!'), ]), ); @@ -577,5 +576,63 @@ release: ]), ); }); + + test('fails for batch release package missing pending_changelogs', + () async { + final RepositoryPackage package = + createFakePackage('a_package', packagesDir); + + root.childFile('README.md').writeAsStringSync(''' +${readmeTableHeader()} +${readmeTableEntry('a_package')} +'''); + writeAutoLabelerYaml([package]); + writeCodeOwners([package]); + package.ciConfigFile.writeAsStringSync(''' +release: + batch: true +'''); + + Error? commandError; + final List output = await runCapturingPrint( + runner, ['repo-package-info-check'], errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains('No pending_changelogs folder found for a_package.'), + ]), + ); + }); + + test('passes for batch release package with pending_changelogs', () async { + final RepositoryPackage package = + createFakePackage('a_package', packagesDir); + + root.childFile('README.md').writeAsStringSync(''' +${readmeTableHeader()} +${readmeTableEntry('a_package')} +'''); + writeAutoLabelerYaml([package]); + writeCodeOwners([package]); + package.ciConfigFile.writeAsStringSync(''' +release: + batch: true +'''); + package.pendingChangelogsDirectory.createSync(); + + final List output = + await runCapturingPrint(runner, ['repo-package-info-check']); + + expect( + output, + containsAll([ + contains('No issues found!'), + ]), + ); + }); }); } diff --git a/script/tool/test/version_check_command_test.dart b/script/tool/test/version_check_command_test.dart index d23b7535fda..dbb63350b66 100644 --- a/script/tool/test/version_check_command_test.dart +++ b/script/tool/test/version_check_command_test.dart @@ -1554,6 +1554,242 @@ ${indentation}HTTP response: null ])); }); }); + group('batch release', () { + test( + 'ignores changelog and pubspec yaml version modifications check with post-release label', + () async { + final RepositoryPackage package = + createFakePackage('package', packagesDir, version: '1.0.0'); + package.ciConfigFile.writeAsStringSync(''' +release: + batch: true +'''); + // Create the pending_changelogs directory so we don't fail on that check. + package.directory.childDirectory('pending_changelogs').createSync(); + + gitProcessRunner.mockProcessesForExecutable['git-diff'] = + [ + FakeProcessInfo(MockProcess(stdout: ''' +packages/package/CHANGELOG.md +packages/package/pubspec.yaml +''')), + ]; + gitProcessRunner.mockProcessesForExecutable['git-show'] = + [ + FakeProcessInfo(MockProcess(stdout: 'version: 1.0.0')), + ]; + + final List output = await runCapturingPrint(runner, [ + 'version-check', + '--base-sha=main', + '--pr-labels=post-release-package', + ]); + + expect( + output, + containsAllInOrder([ + contains('Running for package'), + contains('No issues found!'), + ]), + ); + }); + + test('fails when there is changelog modifications', () async { + final RepositoryPackage package = + createFakePackage('package', packagesDir, version: '1.0.0'); + package.ciConfigFile.writeAsStringSync(''' +release: + batch: true +'''); + // Create the pending_changelogs directory so we don't fail on that check. + package.directory.childDirectory('pending_changelogs').createSync(); + + gitProcessRunner.mockProcessesForExecutable['git-diff'] = + [ + FakeProcessInfo(MockProcess(stdout: ''' +packages/package/CHANGELOG.md +''')), + ]; + gitProcessRunner.mockProcessesForExecutable['git-show'] = + [ + FakeProcessInfo(MockProcess(stdout: 'version: 1.0.0')), + ]; + + Error? commandError; + final List output = await runCapturingPrint(runner, [ + 'version-check', + '--base-sha=main', + ], errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains( + 'This package uses batch release, so CHANGELOG.md should not be changed directly.'), + ])); + }); + + test('fails when there is pubspec version modifications', () async { + final RepositoryPackage package = + createFakePackage('package', packagesDir, version: '1.0.1'); + package.ciConfigFile.writeAsStringSync(''' +release: + batch: true +'''); + // Create the pending_changelogs directory so we don't fail on that check. + package.directory.childDirectory('pending_changelogs').createSync(); + + gitProcessRunner.mockProcessesForExecutable['git-diff'] = + [ + FakeProcessInfo(MockProcess(stdout: ''' +packages/package/pubspec.yaml +''')), + ]; + gitProcessRunner.mockProcessesForExecutable['git-show'] = + [ + FakeProcessInfo(MockProcess(stdout: 'version: 1.0.0')), + ]; + + Error? commandError; + final List output = await runCapturingPrint(runner, [ + 'version-check', + '--base-sha=main', + ], errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains( + 'This package uses batch release, so the version in pubspec.yaml should not be changed directly.'), + ])); + }); + + test('passes when there is pubspec modification but no version change', + () async { + final RepositoryPackage package = + createFakePackage('package', packagesDir, version: '1.0.0'); + package.ciConfigFile.writeAsStringSync(''' +release: + batch: true +'''); + // Create the pending_changelogs directory so we don't fail on that check. + package.directory.childDirectory('pending_changelogs').createSync(); + + gitProcessRunner.mockProcessesForExecutable['git-diff'] = + [ + FakeProcessInfo(MockProcess(stdout: ''' +packages/package/pubspec.yaml +''')), + ]; + gitProcessRunner.mockProcessesForExecutable['git-show'] = + [ + FakeProcessInfo(MockProcess(stdout: 'version: 1.0.0')), + ]; + + final List output = await runCapturingPrint(runner, [ + 'version-check', + '--base-sha=main', + ]); + + expect( + output, + containsAllInOrder([ + contains('No issues found!'), + ])); + }); + + test('fails for batch release package with no new changelog', () async { + final RepositoryPackage package = + createFakePackage('package', packagesDir, version: '1.0.0'); + package.ciConfigFile.writeAsStringSync(''' +release: + batch: true +'''); + // Simulate a code change + package.libDirectory + .childFile('foo.dart') + .writeAsStringSync('void foo() {}'); + // Create the pending_changelogs directory so we don't fail on that check. + package.directory.childDirectory('pending_changelogs').createSync(); + + gitProcessRunner.mockProcessesForExecutable['git-diff'] = + [ + FakeProcessInfo(MockProcess(stdout: 'packages/package/lib/foo.dart')), + ]; + gitProcessRunner.mockProcessesForExecutable['git-show'] = + [ + FakeProcessInfo(MockProcess(stdout: 'version: 1.0.0')), + ]; + + Error? commandError; + final List output = await runCapturingPrint(runner, [ + 'version-check', + '--base-sha=main', + '--check-for-missing-changes', + ], errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains( + 'No new changelog files found in the pending_changelogs folder.'), + ])); + }); + + test('passes for batch release package with new changelog', () async { + final RepositoryPackage package = + createFakePackage('package', packagesDir, version: '1.0.0'); + package.ciConfigFile.writeAsStringSync(''' +release: + batch: true +'''); + // Simulate a code change + package.libDirectory + .childFile('foo.dart') + .writeAsStringSync('void foo() {}'); + // Create the pending_changelogs directory so we don't fail on that check. + final Directory pendingChangelogs = + package.directory.childDirectory('pending_changelogs'); + pendingChangelogs.createSync(); + pendingChangelogs.childFile('some_change.yaml').writeAsStringSync(''' +changelog: "Some change" +version: patch +'''); + + gitProcessRunner.mockProcessesForExecutable['git-diff'] = + [ + FakeProcessInfo(MockProcess(stdout: ''' +packages/package/lib/foo.dart +packages/package/pending_changelogs/some_change.yaml +''')), + ]; + gitProcessRunner.mockProcessesForExecutable['git-show'] = + [ + FakeProcessInfo(MockProcess(stdout: 'version: 1.0.0')), + ]; + + final List output = await runCapturingPrint(runner, [ + 'version-check', + '--base-sha=main', + '--check-for-missing-changes', + ]); + + expect( + output, + containsAllInOrder([ + contains('No issues found!'), + ])); + }); + }); }); group('Pre 1.0', () {