From a3c7ab2d40547cf87b32f1f34e33d99b044a64c0 Mon Sep 17 00:00:00 2001 From: Chun-Heng Tai Date: Thu, 20 Nov 2025 16:37:25 -0800 Subject: [PATCH 1/2] [ci] Adds repo checks for batch release --- .../src/branch_for_batch_release_command.dart | 160 ++---------- .../lib/src/common/package_state_utils.dart | 7 + .../lib/src/common/repository_package.dart | 194 ++++++++++++++ .../src/repo_package_info_check_command.dart | 106 ++------ .../tool/lib/src/version_check_command.dart | 147 ++++++++--- .../test/common/package_state_utils_test.dart | 20 ++ .../test/common/repository_package_test.dart | 165 ++++++++++++ .../repo_package_info_check_command_test.dart | 59 ++++- .../tool/test/version_check_command_test.dart | 236 ++++++++++++++++++ 9 files changed, 827 insertions(+), 267 deletions(-) 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 dad963f1111..5ed0ad0dab6 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,10 +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; } @@ -85,7 +85,7 @@ class BranchForBatchReleaseCommand extends PackageCommand { throw ToolExit(_kExitPackageMalformed); } final _ReleaseInfo releaseInfo = _getReleaseInfo( - pendingChangelogs.entries, + pendingChangelogs, pubspec.version!, ); @@ -101,77 +101,36 @@ 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, + List pendingChangelogEntries, Version oldVersion, ) { - final changelogs = []; - int versionIndex = _VersionChange.skip.index; - for (final entry in pendingChangelogEntries) { + final List changelogs = []; + 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 => Version( + 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, @@ -305,18 +264,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. @@ -328,64 +275,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 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 8a728ad42d1..7be11774604 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 0edbb03ca5f..0df9acae102 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; @@ -116,6 +125,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 flutterDependency = 'flutter'; @@ -220,4 +238,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 81876092e5a..cd76defcc35 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 { @@ -149,10 +142,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. @@ -258,85 +263,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 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 e91f94c6723..6d49c8f85bf 100644 --- a/script/tool/lib/src/version_check_command.dart +++ b/script/tool/lib/src/version_check_command.dart @@ -167,6 +167,13 @@ 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'; @@ -212,32 +219,85 @@ class VersionCheckCommand extends PackageLoopingCommand { final 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 @@ -575,13 +635,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, @@ -624,16 +678,31 @@ ${indentation}The first version listed in CHANGELOG.md is $fromChangeLog. ); } 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 6bf145bc670..20b3c234e53 100644 --- a/script/tool/test/common/package_state_utils_test.dart +++ b/script/tool/test/common/package_state_utils_test.dart @@ -545,6 +545,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 be927d3f5b9..e92effa1030 100644 --- a/script/tool/test/common/repository_package_test.dart +++ b/script/tool/test/common/repository_package_test.dart @@ -279,4 +279,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 c7993b5e597..6056ed1ea85 100644 --- a/script/tool/test/repo_package_info_check_command_test.dart +++ b/script/tool/test/repo_package_info_check_command_test.dart @@ -575,7 +575,6 @@ release: expect( output, containsAll([ - contains(' Checking ci_config.yaml...'), contains('No issues found!'), ]), ); @@ -674,5 +673,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 6f2e6470b6e..b5d040e6c5f 100644 --- a/script/tool/test/version_check_command_test.dart +++ b/script/tool/test/version_check_command_test.dart @@ -1855,6 +1855,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', () { From fb0209c086f5d3a95e3cd4a19853afadcae5f2a7 Mon Sep 17 00:00:00 2001 From: Chun-Heng Tai Date: Fri, 5 Dec 2025 10:57:30 -0800 Subject: [PATCH 2/2] everything before non batch check --- .../pending_changelogs/template.yaml | 6 - .../pending_changelogs/test_only_1.yaml | 6 - .../pending_changelogs/test_only_2.yaml | 5 - .../src/branch_for_batch_release_command.dart | 4 +- script/tool/lib/src/common/ci_config.dart | 79 +++++ .../lib/src/common/package_state_utils.dart | 2 +- .../src/common/pending_changelog_entry.dart | 117 +++++++ .../lib/src/common/repository_package.dart | 175 ++-------- .../src/repo_package_info_check_command.dart | 15 +- .../tool/lib/src/version_check_command.dart | 111 ++++--- script/tool/test/common/ci_config_test.dart | 122 +++++++ .../test/common/package_state_utils_test.dart | 16 +- .../common/pending_changelog_entry_test.dart | 84 +++++ .../test/common/repository_package_test.dart | 163 +++------- .../repo_package_info_check_command_test.dart | 114 ++++--- .../tool/test/version_check_command_test.dart | 302 ++++++++++-------- 16 files changed, 817 insertions(+), 504 deletions(-) delete mode 100644 packages/go_router/pending_changelogs/template.yaml delete mode 100644 packages/go_router/pending_changelogs/test_only_1.yaml delete mode 100644 packages/go_router/pending_changelogs/test_only_2.yaml create mode 100644 script/tool/lib/src/common/ci_config.dart create mode 100644 script/tool/lib/src/common/pending_changelog_entry.dart create mode 100644 script/tool/test/common/ci_config_test.dart create mode 100644 script/tool/test/common/pending_changelog_entry_test.dart diff --git a/packages/go_router/pending_changelogs/template.yaml b/packages/go_router/pending_changelogs/template.yaml deleted file mode 100644 index 97107d891a9..00000000000 --- a/packages/go_router/pending_changelogs/template.yaml +++ /dev/null @@ -1,6 +0,0 @@ -# Use this file as template to draft a unreleased changelog file. -# Make a copy of this file in the same directory, rename it, and fill in the details. -changelog: | - - Can include a list of changes. - - with markdown supported. -version: diff --git a/packages/go_router/pending_changelogs/test_only_1.yaml b/packages/go_router/pending_changelogs/test_only_1.yaml deleted file mode 100644 index fcaa8a27736..00000000000 --- a/packages/go_router/pending_changelogs/test_only_1.yaml +++ /dev/null @@ -1,6 +0,0 @@ -# This file is for test purposes only. -# TODO(chuntai): remove this file before publishing. -changelog: | - - Adds 'batch' option to CI config for go_router package. - - Updates GitHub Actions workflow for batch releases of go_router. -version: major diff --git a/packages/go_router/pending_changelogs/test_only_2.yaml b/packages/go_router/pending_changelogs/test_only_2.yaml deleted file mode 100644 index dbecb2c8e07..00000000000 --- a/packages/go_router/pending_changelogs/test_only_2.yaml +++ /dev/null @@ -1,5 +0,0 @@ -# This file is for test purposes only. -# TODO(chuntai): remove this file before publishing. -changelog: | - - Adds some other features -version: minor 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 5ed0ad0dab6..a6ebb5d9617 100644 --- a/script/tool/lib/src/branch_for_batch_release_command.dart +++ b/script/tool/lib/src/branch_for_batch_release_command.dart @@ -117,9 +117,9 @@ class BranchForBatchReleaseCommand extends PackageCommand { List pendingChangelogEntries, Version oldVersion, ) { - final List changelogs = []; + final changelogs = []; int versionIndex = VersionChange.skip.index; - for (final PendingChangelogEntry entry in pendingChangelogEntries) { + for (final entry in pendingChangelogEntries) { changelogs.add(entry.changelog); versionIndex = math.min(versionIndex, entry.version.index); } diff --git a/script/tool/lib/src/common/ci_config.dart b/script/tool/lib/src/common/ci_config.dart new file mode 100644 index 00000000000..0f35fe42d15 --- /dev/null +++ b/script/tool/lib/src/common/ci_config.dart @@ -0,0 +1,79 @@ +// Copyright 2013 The Flutter Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:yaml/yaml.dart'; + +/// 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); + + var 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 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`'; +} diff --git a/script/tool/lib/src/common/package_state_utils.dart b/script/tool/lib/src/common/package_state_utils.dart index 7be11774604..74fb089e816 100644 --- a/script/tool/lib/src/common/package_state_utils.dart +++ b/script/tool/lib/src/common/package_state_utils.dart @@ -80,7 +80,7 @@ Future checkPackageChangeState( continue; } - if (package.parseCiConfig()?.isBatchRelease ?? false) { + if (package.parseCIConfig()?.isBatchRelease ?? false) { if (components.first == 'pending_changelogs') { hasChangelogChange = true; continue; diff --git a/script/tool/lib/src/common/pending_changelog_entry.dart b/script/tool/lib/src/common/pending_changelog_entry.dart new file mode 100644 index 00000000000..6e8589f86e7 --- /dev/null +++ b/script/tool/lib/src/common/pending_changelog_entry.dart @@ -0,0 +1,117 @@ +// Copyright 2013 The Flutter Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:file/file.dart'; +import 'package:path/path.dart' as p; +import 'package:yaml/yaml.dart'; + +/// The system for managing pending changelog entries. +/// +/// When package opts into batch release (through ci_config.yaml), it uses a "pending +/// changelog" system. When a PR makes a change that requires a changelog entry, +/// the entry is written to a new YAML file in the `pending_changelogs` directory +/// of the package, rather than editing `CHANGELOG.md` directly. +/// +/// ## Directory Structure +/// For batch release packages, it has a `pending_changelogs` directory containing: +/// - A `template.yaml` file (which is ignored by the release tooling). +/// - One or more YAML files for pending changes (e.g., `fix_issue_123.yaml`). +/// +/// ## File Format +/// The YAML file must contain the following keys: +/// - `changelog`: The text of the changelog entry. +/// - `version`: The type of version bump (`major`, `minor`, `patch`, or `skip`). +/// +/// Example: +/// ```yaml +/// changelog: Fixes a bug in the foo widget. +/// version: patch +/// ``` +/// +/// During a release (specifically the `update-changelogs` command), all +/// pending entries are aggregated, the package version is updated based on the +/// highest priority change, and `CHANGELOG.md` is updated. + +/// 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 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 template file name used to draft a pending changelog file. + /// This file will not be picked up by the batch release process. + static const String _batchReleaseChangelogTemplateFileName = 'template.yaml'; + + /// Returns true if the file is a template file. + static bool isTemplate(File file) { + return p.basename(file.path) == _batchReleaseChangelogTemplateFileName; + } + + /// 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/common/repository_package.dart b/script/tool/lib/src/common/repository_package.dart index 0df9acae102..35db44e4619 100644 --- a/script/tool/lib/src/common/repository_package.dart +++ b/script/tool/lib/src/common/repository_package.dart @@ -5,16 +5,15 @@ 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 'ci_config.dart'; import 'core.dart'; +import 'pending_changelog_entry.dart'; export 'package:pubspec_parse/pubspec_parse.dart' show Pubspec; +export 'ci_config.dart'; 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'; +export 'pending_changelog_entry.dart'; /// A package in the repository. // @@ -125,14 +124,14 @@ class RepositoryPackage { /// Caches for future use. Pubspec parsePubspec() => _parsedPubspec; - late final CiConfig? _parsedCiConfig = ciConfigFile.existsSync() - ? CiConfig._parse(ciConfigFile.readAsStringSync()) + 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; + CIConfig? parseCIConfig() => _parsedCIConfig; /// Returns true if the package depends on Flutter. bool requiresFlutter() { @@ -247,171 +246,43 @@ class RepositoryPackage { /// Throws if the folder does not exist, or if any of the files are not /// valid changelog entries. List getPendingChangelogs() { - final List entries = []; + final entries = []; final Directory pendingChangelogsDir = pendingChangelogsDirectory; if (!pendingChangelogsDir.existsSync()) { throw FormatException( - 'No pending_changelogs folder found for $displayName.'); + 'No pending_changelogs folder found for $displayName.', + ); } - final List allFiles = - pendingChangelogsDir.listSync().whereType().toList(); + final List allFiles = pendingChangelogsDir + .listSync() + .whereType() + .toList(); - final List pendingChangelogFiles = []; - for (final File file in allFiles) { + final pendingChangelogFiles = []; + for (final file in allFiles) { final String basename = p.basename(file.path); if (basename.endsWith('.yaml')) { - if (basename != _kTemplateFileName) { + if (!PendingChangelogEntry.isTemplate(file)) { pendingChangelogFiles.add(file); } } else { throw FormatException( - 'Found non-YAML file in pending_changelogs: ${file.path}'); + 'Found non-YAML file in pending_changelogs: ${file.path}', + ); } } - for (final File file in pendingChangelogFiles) { + for (final file in pendingChangelogFiles) { try { - entries - .add(PendingChangelogEntry._parse(file.readAsStringSync(), file)); + entries.add(PendingChangelogEntry.parse(file.readAsStringSync(), file)); } on FormatException catch (e) { throw FormatException( - 'Malformed pending changelog file: ${file.path}\n$e'); + '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 cd76defcc35..ca8aa44311e 100644 --- a/script/tool/lib/src/repo_package_info_check_command.dart +++ b/script/tool/lib/src/repo_package_info_check_command.dart @@ -142,22 +142,31 @@ class RepoPackageInfoCheckCommand extends PackageLoopingCommand { } // The content of ci_config.yaml must be valid if there is one. - CiConfig? ciConfig; + CIConfig? config; try { - ciConfig = package.parseCiConfig(); + config = 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) { + if (config?.isBatchRelease ?? false) { try { package.getPendingChangelogs(); } on FormatException catch (e) { printError('$indentation${e.message}'); errors.add(e.message); } + } else { + if (package.pendingChangelogsDirectory.existsSync()) { + printError( + '${indentation}Package does not use batch release but has pending changelogs.', + ); + errors.add( + 'Package does not use batch release but has pending changelogs.', + ); + } } // All published packages should have a README.md entry. diff --git a/script/tool/lib/src/version_check_command.dart b/script/tool/lib/src/version_check_command.dart index 6d49c8f85bf..72d25211801 100644 --- a/script/tool/lib/src/version_check_command.dart +++ b/script/tool/lib/src/version_check_command.dart @@ -219,61 +219,28 @@ class VersionCheckCommand extends PackageLoopingCommand { final errors = []; - final CiConfig? ciConfig = package.parseCiConfig(); + final CIConfig? ciConfig = package.parseCIConfig(); final _CurrentVersionState versionState = await _getVersionState( package, pubspec: pubspec, ); 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. + // change back to main branch. Proceed with ragular version check. final bool hasPostReleaseLabel = _prLabels.contains( 'post-release-${pubspec.name}', ); - bool versionChanged = false; + bool versionChanged; 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, + versionChanged = await _validateBatchRelease( + package: package, + changedFiles: changedFiles, + errors: errors, + versionState: versionState, ); - - // 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'); - } + if (errors.isNotEmpty) { + return PackageResult.fail(errors); } } else { switch (versionState) { @@ -678,9 +645,8 @@ ${indentation}The first version listed in CHANGELOG.md is $fromChangeLog. ); } else { missingChangelogChange = true; - final bool isBatchRelease = - package.parseCiConfig()?.isBatchRelease ?? false; - if (isBatchRelease) { + final CIConfig? config = package.parseCIConfig(); + if (config?.isBatchRelease ?? false) { printError( 'No new changelog files found in the pending_changelogs folder.\n' 'If this PR needs an exemption from the standard policy of listing ' @@ -688,7 +654,7 @@ ${indentation}The first version listed in CHANGELOG.md is $fromChangeLog. '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 ' + 'Otherwise, please add a changelog entry with version:skip in the pending_changelogs folder as described in ' 'the contributing guide.\n', ); } else { @@ -724,4 +690,55 @@ ${indentation}The first version listed in CHANGELOG.md is $fromChangeLog. return null; } + + Future _validateBatchRelease({ + required RepositoryPackage package, + required List changedFiles, + required List errors, + required _CurrentVersionState versionState, + }) async { + final String relativePackagePath = await _getRelativePackagePath(package); + final List changedFilesInPackage = changedFiles + .where((String path) => path.startsWith(relativePackagePath)) + .toList(); + + // For batch release, only check pending changelog files. + final List allChangelogs; + try { + allChangelogs = package.getPendingChangelogs(); + } on FormatException catch (e) { + errors.add(e.message); + return false; + } + + final List newEntries = allChangelogs + .where( + (PendingChangelogEntry entry) => changedFilesInPackage.any( + (String path) => path.endsWith(entry.file.path.split('/').last), + ), + ) + .toList(); + final bool 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'); + } + } + return versionChanged; + } } diff --git a/script/tool/test/common/ci_config_test.dart b/script/tool/test/common/ci_config_test.dart new file mode 100644 index 00000000000..9e5736495ac --- /dev/null +++ b/script/tool/test/common/ci_config_test.dart @@ -0,0 +1,122 @@ +// Copyright 2013 The Flutter Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:file/file.dart'; +import 'package:test/test.dart'; + +import '../util.dart'; + +void main() { + late Directory packagesDir; + + setUp(() { + (:packagesDir, processRunner: _, gitProcessRunner: _, gitDir: _) = + configureBaseCommandMocks(); + }); + + 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`'), + ), + ), + ); + }); + }); +} diff --git a/script/tool/test/common/package_state_utils_test.dart b/script/tool/test/common/package_state_utils_test.dart index 20b3c234e53..3a0e53ff8e3 100644 --- a/script/tool/test/common/package_state_utils_test.dart +++ b/script/tool/test/common/package_state_utils_test.dart @@ -547,20 +547,24 @@ void main() { }); test('detects pending changelog changes for batch release', () async { - final RepositoryPackage package = - createFakePackage('a_package', packagesDir); + final RepositoryPackage package = createFakePackage( + 'a_package', + packagesDir, + ); package.ciConfigFile.writeAsStringSync(''' release: batch: true '''); - const List changedFiles = [ + const changedFiles = [ 'packages/a_package/pending_changelogs/some_change.yaml', ]; - final PackageChangeState state = await checkPackageChangeState(package, - changedPaths: changedFiles, - relativePackagePath: 'packages/a_package/'); + 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/pending_changelog_entry_test.dart b/script/tool/test/common/pending_changelog_entry_test.dart new file mode 100644 index 00000000000..4263ca624e6 --- /dev/null +++ b/script/tool/test/common/pending_changelog_entry_test.dart @@ -0,0 +1,84 @@ +// Copyright 2013 The Flutter Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:file/file.dart'; +import 'package:file/memory.dart'; +import 'package:flutter_plugin_tools/src/common/pending_changelog_entry.dart'; +import 'package:test/test.dart'; + +void main() { + group('PendingChangelogEntry', () { + test('parses valid entry', () { + final File file = MemoryFileSystem().file('a.yaml'); + const yaml = ''' +changelog: A +version: patch +'''; + final entry = PendingChangelogEntry.parse(yaml, file); + + expect(entry.changelog, 'A'); + expect(entry.version, VersionChange.patch); + expect(entry.file, file); + }); + + test('throws for non-map YAML', () { + final File file = MemoryFileSystem().file('a.yaml'); + expect( + () => PendingChangelogEntry.parse('not a map', file), + throwsA( + isA().having( + (FormatException e) => e.message, + 'message', + contains('Expected a YAML map'), + ), + ), + ); + }); + + test('throws for missing changelog', () { + final File file = MemoryFileSystem().file('a.yaml'); + expect( + () => PendingChangelogEntry.parse('version: patch', file), + throwsA( + isA().having( + (FormatException e) => e.message, + 'message', + contains('Expected "changelog" to be a string'), + ), + ), + ); + }); + + test('throws for missing version', () { + final File file = MemoryFileSystem().file('a.yaml'); + expect( + () => PendingChangelogEntry.parse('changelog: A', file), + throwsA( + isA().having( + (FormatException e) => e.message, + 'message', + contains('Missing "version" key'), + ), + ), + ); + }); + + test('throws for invalid version', () { + final File file = MemoryFileSystem().file('a.yaml'); + expect( + () => PendingChangelogEntry.parse(''' +changelog: A +version: invalid +''', file), + throwsA( + isA().having( + (FormatException e) => e.message, + 'message', + contains('Invalid version type'), + ), + ), + ); + }); + }); +} diff --git a/script/tool/test/common/repository_package_test.dart b/script/tool/test/common/repository_package_test.dart index e92effa1030..ca2ed54becf 100644 --- a/script/tool/test/common/repository_package_test.dart +++ b/script/tool/test/common/repository_package_test.dart @@ -279,123 +279,51 @@ 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); + 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); + final RepositoryPackage package = createFakePackage( + 'a_package', + packagesDir, + ); package.pendingChangelogsDirectory.createSync(); - final List changelogs = - package.getPendingChangelogs(); + final List changelogs = package + .getPendingChangelogs(); expect(changelogs, isEmpty); }); test('returns entries for valid files', () { - final RepositoryPackage package = - createFakePackage('a_package', packagesDir); + final RepositoryPackage package = createFakePackage( + 'a_package', + packagesDir, + ); package.pendingChangelogsDirectory.createSync(); - package.pendingChangelogsDirectory - .childFile('a.yaml') - .writeAsStringSync(''' + package.pendingChangelogsDirectory.childFile('a.yaml').writeAsStringSync( + ''' changelog: A version: patch -'''); - package.pendingChangelogsDirectory - .childFile('b.yaml') - .writeAsStringSync(''' +''', + ); + package.pendingChangelogsDirectory.childFile('b.yaml').writeAsStringSync( + ''' changelog: B version: minor -'''); +''', + ); - final List changelogs = - package.getPendingChangelogs(); + final List changelogs = package + .getPendingChangelogs(); expect(changelogs, hasLength(2)); expect(changelogs[0].changelog, 'A'); @@ -405,31 +333,40 @@ version: minor }); test('returns an error for a malformed file', () { - final RepositoryPackage package = - createFakePackage('a_package', packagesDir); + final RepositoryPackage package = createFakePackage( + 'a_package', + packagesDir, + ); package.pendingChangelogsDirectory.createSync(); - final File changelogFile = - package.pendingChangelogsDirectory.childFile('a.yaml'); + 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')))); + () => 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); + final RepositoryPackage package = createFakePackage( + 'a_package', + packagesDir, + ); package.pendingChangelogsDirectory.createSync(); - package.pendingChangelogsDirectory - .childFile('a.yaml') - .writeAsStringSync(''' + package.pendingChangelogsDirectory.childFile('a.yaml').writeAsStringSync( + ''' changelog: A version: patch -'''); +''', + ); package.pendingChangelogsDirectory .childFile('template.yaml') .writeAsStringSync(''' @@ -437,8 +374,8 @@ changelog: TEMPLATE version: skip '''); - final List changelogs = - package.getPendingChangelogs(); + 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 6056ed1ea85..efceb1a8223 100644 --- a/script/tool/test/repo_package_info_check_command_test.dart +++ b/script/tool/test/repo_package_info_check_command_test.dart @@ -572,12 +572,7 @@ release: 'repo-package-info-check', ]); - expect( - output, - containsAll([ - contains('No issues found!'), - ]), - ); + expect(output, containsAll([contains('No issues found!')])); }); test('missing ci_config file is ok', () async { @@ -674,40 +669,49 @@ release: ); }); - test('fails for batch release package missing pending_changelogs', - () async { - final RepositoryPackage package = - createFakePackage('a_package', packagesDir); + test( + 'fails for batch release package missing pending_changelogs', + () async { + final RepositoryPackage package = createFakePackage( + 'a_package', + packagesDir, + ); - root.childFile('README.md').writeAsStringSync(''' + root.childFile('README.md').writeAsStringSync(''' ${readmeTableHeader()} ${readmeTableEntry('a_package')} '''); - writeAutoLabelerYaml([package]); - writeCodeOwners([package]); - package.ciConfigFile.writeAsStringSync(''' + 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.'), - ]), - ); - }); + 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); + final RepositoryPackage package = createFakePackage( + 'a_package', + packagesDir, + ); root.childFile('README.md').writeAsStringSync(''' ${readmeTableHeader()} @@ -721,15 +725,49 @@ release: '''); package.pendingChangelogsDirectory.createSync(); - final List output = - await runCapturingPrint(runner, ['repo-package-info-check']); + final List output = await runCapturingPrint(runner, [ + 'repo-package-info-check', + ]); - expect( - output, - containsAll([ - contains('No issues found!'), - ]), - ); + expect(output, containsAll([contains('No issues found!')])); }); + + test( + 'fails for non-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]); + // No ci_config.yaml means batch release is false by default. + package.pendingChangelogsDirectory.createSync(); + + Error? commandError; + final List output = await runCapturingPrint( + runner, + ['repo-package-info-check'], + errorHandler: (Error e) { + commandError = e; + }, + ); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains( + 'Package does not use batch release but has pending changelogs.', + ), + ]), + ); + }, + ); }); } diff --git a/script/tool/test/version_check_command_test.dart b/script/tool/test/version_check_command_test.dart index b5d040e6c5f..05bbc28ea55 100644 --- a/script/tool/test/version_check_command_test.dart +++ b/script/tool/test/version_check_command_test.dart @@ -1857,157 +1857,196 @@ ${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(''' + '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(); + // Create the pending_changelogs directory so the test doesn't fail on that check. + package.directory.childDirectory('pending_changelogs').createSync(); - gitProcessRunner.mockProcessesForExecutable['git-diff'] = - [ - FakeProcessInfo(MockProcess(stdout: ''' + 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')), - ]; +''', + ), + ), + ]; + 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', - ]); + 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!'), - ]), - ); - }); + 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'); + 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. + // Create the pending_changelogs directory so the test doesn't fail on that check. package.directory.childDirectory('pending_changelogs').createSync(); gitProcessRunner.mockProcessesForExecutable['git-diff'] = [ - FakeProcessInfo(MockProcess(stdout: ''' + FakeProcessInfo( + MockProcess( + stdout: ''' packages/package/CHANGELOG.md -''')), - ]; +''', + ), + ), + ]; gitProcessRunner.mockProcessesForExecutable['git-show'] = [ - FakeProcessInfo(MockProcess(stdout: 'version: 1.0.0')), - ]; + 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; - }); + 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.'), - ])); + 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'); + 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. + // Create the pending_changelogs directory so the test doesn't fail on that check. package.directory.childDirectory('pending_changelogs').createSync(); gitProcessRunner.mockProcessesForExecutable['git-diff'] = [ - FakeProcessInfo(MockProcess(stdout: ''' + FakeProcessInfo( + MockProcess( + stdout: ''' packages/package/pubspec.yaml -''')), - ]; +''', + ), + ), + ]; gitProcessRunner.mockProcessesForExecutable['git-show'] = [ - FakeProcessInfo(MockProcess(stdout: 'version: 1.0.0')), - ]; + 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; - }); + 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.'), - ])); + 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(''' + 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(); + // Create the pending_changelogs directory so the test doesn't fail on that check. + package.directory.childDirectory('pending_changelogs').createSync(); - gitProcessRunner.mockProcessesForExecutable['git-diff'] = - [ - FakeProcessInfo(MockProcess(stdout: ''' + gitProcessRunner.mockProcessesForExecutable['git-diff'] = + [ + FakeProcessInfo( + MockProcess( + stdout: ''' packages/package/pubspec.yaml -''')), - ]; - gitProcessRunner.mockProcessesForExecutable['git-show'] = - [ - FakeProcessInfo(MockProcess(stdout: 'version: 1.0.0')), - ]; +''', + ), + ), + ]; + gitProcessRunner.mockProcessesForExecutable['git-show'] = + [ + FakeProcessInfo(MockProcess(stdout: 'version: 1.0.0')), + ]; - final List output = await runCapturingPrint(runner, [ - 'version-check', - '--base-sha=main', - ]); + final List output = await runCapturingPrint(runner, [ + 'version-check', + '--base-sha=main', + ]); - expect( + expect( output, - containsAllInOrder([ - contains('No issues found!'), - ])); - }); + 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'); + final RepositoryPackage package = createFakePackage( + 'package', + packagesDir, + version: '1.0.0', + ); package.ciConfigFile.writeAsStringSync(''' release: batch: true @@ -2016,39 +2055,48 @@ release: package.libDirectory .childFile('foo.dart') .writeAsStringSync('void foo() {}'); - // Create the pending_changelogs directory so we don't fail on that check. + // Create the pending_changelogs directory so the test doesn't fail on that check. package.directory.childDirectory('pending_changelogs').createSync(); - gitProcessRunner.mockProcessesForExecutable['git-diff'] = - [ + gitProcessRunner + .mockProcessesForExecutable['git-diff'] = [ FakeProcessInfo(MockProcess(stdout: 'packages/package/lib/foo.dart')), ]; gitProcessRunner.mockProcessesForExecutable['git-show'] = [ - FakeProcessInfo(MockProcess(stdout: 'version: 1.0.0')), - ]; + 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; - }); + 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.'), - ])); + 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'); + final RepositoryPackage package = createFakePackage( + 'package', + packagesDir, + version: '1.0.0', + ); package.ciConfigFile.writeAsStringSync(''' release: batch: true @@ -2057,9 +2105,10 @@ release: 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'); + // Create the pending_changelogs directory so the test doesn't fail on that check. + final Directory pendingChangelogs = package.directory.childDirectory( + 'pending_changelogs', + ); pendingChangelogs.createSync(); pendingChangelogs.childFile('some_change.yaml').writeAsStringSync(''' changelog: "Some change" @@ -2068,15 +2117,19 @@ version: patch gitProcessRunner.mockProcessesForExecutable['git-diff'] = [ - FakeProcessInfo(MockProcess(stdout: ''' + 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')), - ]; + FakeProcessInfo(MockProcess(stdout: 'version: 1.0.0')), + ]; final List output = await runCapturingPrint(runner, [ 'version-check', @@ -2085,10 +2138,9 @@ packages/package/pending_changelogs/some_change.yaml ]); expect( - output, - containsAllInOrder([ - contains('No issues found!'), - ])); + output, + containsAllInOrder([contains('No issues found!')]), + ); }); }); });