diff --git a/script/tool/lib/src/common/core.dart b/script/tool/lib/src/common/core.dart index b2be8f56d172..3b07baf5dc1e 100644 --- a/script/tool/lib/src/common/core.dart +++ b/script/tool/lib/src/common/core.dart @@ -58,6 +58,16 @@ void printSuccess(String successMessage) { print(Colorize(successMessage)..green()); } +/// Prints `warningMessage` in yellow. +/// +/// Warnings are not surfaced in CI summaries, so this is only useful for +/// highlighting something when someone is already looking though the log +/// messages. DO NOT RELY on someone noticing a warning; instead, use it for +/// things that might be useful to someone debugging an unexpected result. +void printWarning(String warningMessage) { + print(Colorize(warningMessage)..yellow()); +} + /// Prints `errorMessage` in red. void printError(String errorMessage) { print(Colorize(errorMessage)..red()); diff --git a/script/tool/lib/src/common/package_looping_command.dart b/script/tool/lib/src/common/package_looping_command.dart index 1349a5ed5dcc..cfe99313068e 100644 --- a/script/tool/lib/src/common/package_looping_command.dart +++ b/script/tool/lib/src/common/package_looping_command.dart @@ -35,6 +35,10 @@ abstract class PackageLoopingCommand extends PluginCommand { /// in the final summary. An empty list indicates success. Future> runForPackage(Directory package); + /// Called during [run] after all calls to [runForPackage]. This provides an + /// opportunity to do any cleanup of run-level state. + Future completeRun() async {} + /// Whether or not the output (if any) of [runForPackage] is long, or short. /// /// This changes the logging that happens at the start of each package's @@ -99,6 +103,9 @@ abstract class PackageLoopingCommand extends PluginCommand { return packageName; } + /// The suggested indentation for printed output. + String get indentation => hasLongOutput ? '' : ' '; + // ---------------------------------------- @override @@ -115,6 +122,8 @@ abstract class PackageLoopingCommand extends PluginCommand { results[package] = await runForPackage(package); } + completeRun(); + // If there were any errors reported, summarize them and exit. if (results.values.any((List failures) => failures.isNotEmpty)) { const String indentation = ' '; diff --git a/script/tool/lib/src/common/plugin_command.dart b/script/tool/lib/src/common/plugin_command.dart index 4c095858e45a..2eef48c63878 100644 --- a/script/tool/lib/src/common/plugin_command.dart +++ b/script/tool/lib/src/common/plugin_command.dart @@ -20,8 +20,8 @@ abstract class PluginCommand extends Command { PluginCommand( this.packagesDir, { this.processRunner = const ProcessRunner(), - this.gitDir, - }) { + GitDir? gitDir, + }) : _gitDir = gitDir { argParser.addMultiOption( _pluginsArg, splitCommas: true, @@ -76,10 +76,11 @@ abstract class PluginCommand extends Command { /// This can be overridden for testing. final ProcessRunner processRunner; - /// The git directory to use. By default it uses the parent directory. + /// The git directory to use. If unset, [gitDir] populates it from the + /// packages directory's enclosing repository. /// /// This can be mocked for testing. - final GitDir? gitDir; + GitDir? _gitDir; int? _shardIndex; int? _shardCount; @@ -100,6 +101,26 @@ abstract class PluginCommand extends Command { return _shardCount!; } + /// Returns the [GitDir] containing [packagesDir]. + Future get gitDir async { + GitDir? gitDir = _gitDir; + if (gitDir != null) { + return gitDir; + } + + // Ensure there are no symlinks in the path, as it can break + // GitDir's allowSubdirectory:true. + final String packagesPath = packagesDir.resolveSymbolicLinksSync(); + if (!await GitDir.isGitDir(packagesPath)) { + printError('$packagesPath is not a valid Git repository.'); + throw ToolExit(2); + } + gitDir = + await GitDir.fromExisting(packagesDir.path, allowSubdirectory: true); + _gitDir = gitDir; + return gitDir; + } + /// Convenience accessor for boolean arguments. bool getBoolArg(String key) { return (argResults![key] as bool?) ?? false; @@ -285,22 +306,10 @@ abstract class PluginCommand extends Command { /// /// Throws tool exit if [gitDir] nor root directory is a git directory. Future retrieveVersionFinder() async { - final String rootDir = packagesDir.parent.absolute.path; final String baseSha = getStringArg(_kBaseSha); - GitDir? baseGitDir = gitDir; - if (baseGitDir == null) { - if (!await GitDir.isGitDir(rootDir)) { - printError( - '$rootDir is not a valid Git repository.', - ); - throw ToolExit(2); - } - baseGitDir = await GitDir.fromExisting(rootDir); - } - final GitVersionFinder gitVersionFinder = - GitVersionFinder(baseGitDir, baseSha); + GitVersionFinder(await gitDir, baseSha); return gitVersionFinder; } diff --git a/script/tool/lib/src/publish_plugin_command.dart b/script/tool/lib/src/publish_plugin_command.dart index 622a1a3cb133..045c2c2f1853 100644 --- a/script/tool/lib/src/publish_plugin_command.dart +++ b/script/tool/lib/src/publish_plugin_command.dart @@ -149,15 +149,7 @@ class PublishPluginCommand extends PluginCommand { } _print('Checking local repo...'); - // Ensure there are no symlinks in the path, as it can break - // GitDir's allowSubdirectory:true. - final String packagesPath = packagesDir.resolveSymbolicLinksSync(); - if (!await GitDir.isGitDir(packagesPath)) { - _print('$packagesPath is not a valid Git repository.'); - throw ToolExit(1); - } - final GitDir baseGitDir = gitDir ?? - await GitDir.fromExisting(packagesPath, allowSubdirectory: true); + final GitDir repository = await gitDir; final bool shouldPushTag = getBoolArg(_pushTagsOption); _RemoteInfo? remote; @@ -179,7 +171,7 @@ class PublishPluginCommand extends PluginCommand { bool successful; if (publishAllChanged) { successful = await _publishAllChangedPackages( - baseGitDir: baseGitDir, + baseGitDir: repository, remoteForTagPush: remote, ); } else { diff --git a/script/tool/lib/src/pubspec_check_command.dart b/script/tool/lib/src/pubspec_check_command.dart index 44b6b061542c..d257638971db 100644 --- a/script/tool/lib/src/pubspec_check_command.dart +++ b/script/tool/lib/src/pubspec_check_command.dart @@ -70,7 +70,6 @@ class PubspecCheckCommand extends PackageLoopingCommand { File pubspecFile, { required String packageName, }) async { - const String indentation = ' '; final String contents = pubspecFile.readAsStringSync(); final Pubspec? pubspec = _tryParsePubspec(contents); if (pubspec == null) { diff --git a/script/tool/lib/src/version_check_command.dart b/script/tool/lib/src/version_check_command.dart index 5e9f55333f8e..2584d70c5fc9 100644 --- a/script/tool/lib/src/version_check_command.dart +++ b/script/tool/lib/src/version_check_command.dart @@ -6,12 +6,13 @@ import 'package:file/file.dart'; import 'package:git/git.dart'; import 'package:http/http.dart' as http; import 'package:meta/meta.dart'; +import 'package:path/path.dart' as p; import 'package:pub_semver/pub_semver.dart'; import 'package:pubspec_parse/pubspec_parse.dart'; import 'common/core.dart'; import 'common/git_version_finder.dart'; -import 'common/plugin_command.dart'; +import 'common/package_looping_command.dart'; import 'common/process_runner.dart'; import 'common/pub_version_finder.dart'; @@ -31,46 +32,47 @@ enum NextVersionType { } /// Returns the set of allowed next versions, with their change type, for -/// [masterVersion]. +/// [version]. /// -/// [headVerison] is used to check whether this is a pre-1.0 version bump, as +/// [newVersion] is used to check whether this is a pre-1.0 version bump, as /// those have different semver rules. @visibleForTesting Map getAllowedNextVersions( - {required Version masterVersion, required Version headVersion}) { + Version version, { + required Version newVersion, +}) { final Map allowedNextVersions = { - masterVersion.nextMajor: NextVersionType.BREAKING_MAJOR, - masterVersion.nextMinor: NextVersionType.MINOR, - masterVersion.nextPatch: NextVersionType.PATCH, + version.nextMajor: NextVersionType.BREAKING_MAJOR, + version.nextMinor: NextVersionType.MINOR, + version.nextPatch: NextVersionType.PATCH, }; - if (masterVersion.major < 1 && headVersion.major < 1) { + if (version.major < 1 && newVersion.major < 1) { int nextBuildNumber = -1; - if (masterVersion.build.isEmpty) { + if (version.build.isEmpty) { nextBuildNumber = 1; } else { - final int currentBuildNumber = masterVersion.build.first as int; + final int currentBuildNumber = version.build.first as int; nextBuildNumber = currentBuildNumber + 1; } final Version preReleaseVersion = Version( - masterVersion.major, - masterVersion.minor, - masterVersion.patch, + version.major, + version.minor, + version.patch, build: nextBuildNumber.toString(), ); allowedNextVersions.clear(); - allowedNextVersions[masterVersion.nextMajor] = NextVersionType.RELEASE; - allowedNextVersions[masterVersion.nextMinor] = - NextVersionType.BREAKING_MAJOR; - allowedNextVersions[masterVersion.nextPatch] = NextVersionType.MINOR; + allowedNextVersions[version.nextMajor] = NextVersionType.RELEASE; + allowedNextVersions[version.nextMinor] = NextVersionType.BREAKING_MAJOR; + allowedNextVersions[version.nextPatch] = NextVersionType.MINOR; allowedNextVersions[preReleaseVersion] = NextVersionType.PATCH; } return allowedNextVersions; } /// A command to validate version changes to packages. -class VersionCheckCommand extends PluginCommand { +class VersionCheckCommand extends PackageLoopingCommand { /// Creates an instance of the version check command. VersionCheckCommand( Directory packagesDir, { @@ -91,6 +93,8 @@ class VersionCheckCommand extends PluginCommand { static const String _againstPubFlag = 'against-pub'; + final PubVersionFinder _pubVersionFinder; + @override final String name = 'version-check'; @@ -100,175 +104,173 @@ class VersionCheckCommand extends PluginCommand { 'Also checks if the latest version in CHANGELOG matches the version in pubspec.\n\n' 'This command requires "pub" and "flutter" to be in your path.'; - final PubVersionFinder _pubVersionFinder; + @override + bool get hasLongOutput => false; @override - Future run() async { - final GitVersionFinder gitVersionFinder = await retrieveVersionFinder(); + Future initializeRun() async {} - final List changedPubspecs = - await gitVersionFinder.getChangedPubSpecs(); + @override + Future> runForPackage(Directory package) async { + final List errors = []; - final List badVersionChangePubspecs = []; + final Pubspec? pubspec = _tryParsePubspec(package); + if (pubspec == null) { + errors.add('Invalid pubspec.yaml.'); + return errors; // No remaining checks make sense. + } - const String indentation = ' '; - for (final String pubspecPath in changedPubspecs) { - print('Checking versions for $pubspecPath...'); - final File pubspecFile = packagesDir.fileSystem.file(pubspecPath); - if (!pubspecFile.existsSync()) { - print('${indentation}Deleted; skipping.'); - continue; - } - final Pubspec pubspec = Pubspec.parse(pubspecFile.readAsStringSync()); - if (pubspec.publishTo == 'none') { - print('${indentation}Found "publish_to: none"; skipping.'); - continue; - } + if (pubspec.publishTo == 'none') { + printSkip('${indentation}Found "publish_to: none".'); + return PackageLoopingCommand.success; + } - final Version? headVersion = - await gitVersionFinder.getPackageVersion(pubspecPath, gitRef: 'HEAD'); - if (headVersion == null) { - printError('${indentation}No version found. A package that ' - 'intentionally has no version should be marked ' - '"publish_to: none".'); - badVersionChangePubspecs.add(pubspecPath); - continue; - } - Version? sourceVersion; - if (getBoolArg(_againstPubFlag)) { - final String packageName = pubspecFile.parent.basename; - final PubVersionFinderResponse pubVersionFinderResponse = - await _pubVersionFinder.getPackageVersion(package: packageName); - switch (pubVersionFinderResponse.result) { - case PubVersionFinderResult.success: - sourceVersion = pubVersionFinderResponse.versions.first; - print( - '$indentation$packageName: Current largest version on pub: $sourceVersion'); - break; - case PubVersionFinderResult.fail: - printError(''' + final Version? currentPubspecVersion = pubspec.version; + if (currentPubspecVersion == null) { + printError('${indentation}No version found in pubspec.yaml. A package ' + 'that intentionally has no version should be marked ' + '"publish_to: none".'); + errors.add('No pubspec.yaml version.'); + return errors; // No remaining checks make sense. + } + + if (!await _hasValidVersionChange(package, pubspec: pubspec)) { + errors.add('Disallowed version change.'); + } + + if (!(await _hasConsistentVersion(package, pubspec: pubspec))) { + errors.add('pubspec.yaml and CHANGELOG.md have different versions'); + } + + return errors; + } + + @override + Future completeRun() async { + _pubVersionFinder.httpClient.close(); + } + + /// Returns the previous published version of [package]. + /// + /// [packageName] must be the actual name of the package as published (i.e., + /// the name from pubspec.yaml, not the on disk name if different.) + Future _fetchPreviousVersionFromPub(String packageName) async { + final PubVersionFinderResponse pubVersionFinderResponse = + await _pubVersionFinder.getPackageVersion(package: packageName); + switch (pubVersionFinderResponse.result) { + case PubVersionFinderResult.success: + return pubVersionFinderResponse.versions.first; + case PubVersionFinderResult.fail: + printError(''' ${indentation}Error fetching version on pub for $packageName. ${indentation}HTTP Status ${pubVersionFinderResponse.httpResponse.statusCode} ${indentation}HTTP response: ${pubVersionFinderResponse.httpResponse.body} '''); - badVersionChangePubspecs.add(pubspecPath); - continue; - case PubVersionFinderResult.noPackageFound: - sourceVersion = null; - break; - } - } else { - sourceVersion = await gitVersionFinder.getPackageVersion(pubspecPath); - } - if (sourceVersion == null) { - String safeToIgnoreMessage; - if (getBoolArg(_againstPubFlag)) { - safeToIgnoreMessage = - '${indentation}Unable to find package on pub server.'; - } else { - safeToIgnoreMessage = - '${indentation}Unable to find pubspec in master.'; - } - print('$safeToIgnoreMessage Safe to ignore if the project is new.'); - continue; - } + return null; + case PubVersionFinderResult.noPackageFound: + return Version.none; + } + } - if (sourceVersion == headVersion) { - print('${indentation}No version change.'); - continue; - } + /// Returns the version of [package] from git at the base comparison hash. + Future _getPreviousVersionFromGit( + Directory package, { + required GitVersionFinder gitVersionFinder, + }) async { + final File pubspecFile = package.childFile('pubspec.yaml'); + return await gitVersionFinder.getPackageVersion( + p.relative(pubspecFile.absolute.path, from: (await gitDir).path)); + } - // Check for reverts when doing local validation. - if (!getBoolArg(_againstPubFlag) && headVersion < sourceVersion) { - final Map possibleVersionsFromNewVersion = - getAllowedNextVersions( - masterVersion: headVersion, headVersion: sourceVersion); - // Since this skips validation, try to ensure that it really is likely - // to be a revert rather than a typo by checking that the transition - // from the lower version to the new version would have been valid. - if (possibleVersionsFromNewVersion.containsKey(sourceVersion)) { - print('${indentation}New version is lower than previous version. ' - 'This is assumed to be a revert.'); - continue; - } + /// Returns true if the version of [package] is either unchanged relative to + /// the comparison base (git or pub, depending on flags), or is a valid + /// version transition. + Future _hasValidVersionChange( + Directory package, { + required Pubspec pubspec, + }) async { + // This method isn't called unless `version` is non-null. + final Version currentVersion = pubspec.version!; + Version? previousVersion; + if (getBoolArg(_againstPubFlag)) { + previousVersion = await _fetchPreviousVersionFromPub(pubspec.name); + if (previousVersion == null) { + return false; } - - final Map allowedNextVersions = - getAllowedNextVersions( - masterVersion: sourceVersion, headVersion: headVersion); - - if (!allowedNextVersions.containsKey(headVersion)) { - final String source = (getBoolArg(_againstPubFlag)) ? 'pub' : 'master'; - printError('${indentation}Incorrectly updated version.\n' - '${indentation}HEAD: $headVersion, $source: $sourceVersion.\n' - '${indentation}Allowed versions: $allowedNextVersions'); - badVersionChangePubspecs.add(pubspecPath); - continue; - } else { - print('$indentation$headVersion -> $sourceVersion'); + if (previousVersion != Version.none) { + print( + '$indentation${pubspec.name}: Current largest version on pub: $previousVersion'); } + } else { + final GitVersionFinder gitVersionFinder = await retrieveVersionFinder(); + previousVersion = await _getPreviousVersionFromGit(package, + gitVersionFinder: gitVersionFinder) ?? + Version.none; + } + if (previousVersion == Version.none) { + print('${indentation}Unable to find previous version ' + '${getBoolArg(_againstPubFlag) ? 'on pub server' : 'at git base'}.'); + printWarning( + '${indentation}If this plugin is not new, something has gone wrong.'); + return true; + } - final bool isPlatformInterface = - pubspec.name.endsWith('_platform_interface'); - if (isPlatformInterface && - allowedNextVersions[headVersion] == NextVersionType.BREAKING_MAJOR) { - printError('$pubspecPath breaking change detected.\n' - 'Breaking changes to platform interfaces are strongly discouraged.\n'); - badVersionChangePubspecs.add(pubspecPath); - continue; - } + if (previousVersion == currentVersion) { + print('${indentation}No version change.'); + return true; } - _pubVersionFinder.httpClient.close(); - // TODO(stuartmorgan): Unify the way iteration works for these checks; the - // two checks shouldn't be operating independently on different lists. - final List mismatchedVersionPlugins = []; - await for (final Directory plugin in getPlugins()) { - if (!(await _checkVersionsMatch(plugin))) { - mismatchedVersionPlugins.add(plugin.basename); + // Check for reverts when doing local validation. + if (!getBoolArg(_againstPubFlag) && currentVersion < previousVersion) { + final Map possibleVersionsFromNewVersion = + getAllowedNextVersions(currentVersion, newVersion: previousVersion); + // Since this skips validation, try to ensure that it really is likely + // to be a revert rather than a typo by checking that the transition + // from the lower version to the new version would have been valid. + if (possibleVersionsFromNewVersion.containsKey(previousVersion)) { + print('${indentation}New version is lower than previous version. ' + 'This is assumed to be a revert.'); + return true; } } - bool passed = true; - if (badVersionChangePubspecs.isNotEmpty) { - passed = false; - printError(''' -The following pubspecs failed validaton: -$indentation${badVersionChangePubspecs.join('\n$indentation')} -'''); - } - if (mismatchedVersionPlugins.isNotEmpty) { - passed = false; - printError(''' -The following pubspecs have different versions in pubspec.yaml and CHANGELOG.md: -$indentation${mismatchedVersionPlugins.join('\n$indentation')} -'''); - } - if (!passed) { - throw ToolExit(1); + final Map allowedNextVersions = + getAllowedNextVersions(previousVersion, newVersion: currentVersion); + + if (allowedNextVersions.containsKey(currentVersion)) { + print('$indentation$previousVersion -> $currentVersion'); + } else { + final String source = (getBoolArg(_againstPubFlag)) ? 'pub' : 'master'; + printError('${indentation}Incorrectly updated version.\n' + '${indentation}HEAD: $currentVersion, $source: $previousVersion.\n' + '${indentation}Allowed versions: $allowedNextVersions'); + return false; } - print('No version check errors found!'); + final bool isPlatformInterface = + pubspec.name.endsWith('_platform_interface'); + // TODO(stuartmorgan): Relax this check. See + // https://github.com/flutter/flutter/issues/85391 + if (isPlatformInterface && + allowedNextVersions[currentVersion] == NextVersionType.BREAKING_MAJOR) { + printError('${indentation}Breaking change detected.\n' + '${indentation}Breaking changes to platform interfaces are strongly discouraged.\n'); + return false; + } + return true; } /// Returns whether or not the pubspec version and CHANGELOG version for /// [plugin] match. - Future _checkVersionsMatch(Directory plugin) async { - // get version from pubspec - final String packageName = plugin.basename; - print('-----------------------------------------'); - print( - 'Checking the first version listed in CHANGELOG.md matches the version in pubspec.yaml for $packageName.'); - - final Pubspec? pubspec = _tryParsePubspec(plugin); - if (pubspec == null) { - printError('Cannot parse version from pubspec.yaml'); - return false; - } - final Version? fromPubspec = pubspec.version; + Future _hasConsistentVersion( + Directory package, { + required Pubspec pubspec, + }) async { + // This method isn't called unless `version` is non-null. + final Version fromPubspec = pubspec.version!; // get first version from CHANGELOG - final File changelog = plugin.childFile('CHANGELOG.md'); + final File changelog = package.childFile('CHANGELOG.md'); final List lines = changelog.readAsLinesSync(); String? firstLineWithText; final Iterator iterator = lines.iterator; @@ -285,7 +287,8 @@ $indentation${mismatchedVersionPlugins.join('\n$indentation')} // changes that don't warrant publishing on their own. final bool hasNextSection = versionString == 'NEXT'; if (hasNextSection) { - print('Found NEXT; validating next version in the CHANGELOG.'); + print( + '${indentation}Found NEXT; validating next version in the CHANGELOG.'); // Ensure that the version in pubspec hasn't changed without updating // CHANGELOG. That means the next version entry in the CHANGELOG pass the // normal validation. @@ -301,15 +304,15 @@ $indentation${mismatchedVersionPlugins.join('\n$indentation')} versionString == null ? null : Version.parse(versionString); if (fromChangeLog == null) { printError( - 'Cannot find version on the first line of ${plugin.path}/CHANGELOG.md'); + '${indentation}Cannot find version on the first line CHANGELOG.md'); return false; } if (fromPubspec != fromChangeLog) { printError(''' -versions for $packageName in CHANGELOG.md and pubspec.yaml do not match. -The version in pubspec.yaml is $fromPubspec. -The first version listed in CHANGELOG.md is $fromChangeLog. +${indentation}Versions in CHANGELOG.md and pubspec.yaml do not match. +${indentation}The version in pubspec.yaml is $fromPubspec. +${indentation}The first version listed in CHANGELOG.md is $fromChangeLog. '''); return false; } @@ -318,15 +321,13 @@ The first version listed in CHANGELOG.md is $fromChangeLog. if (!hasNextSection) { final RegExp nextRegex = RegExp(r'^#+\s*NEXT\s*$'); if (lines.any((String line) => nextRegex.hasMatch(line))) { - printError(''' -When bumping the version for release, the NEXT section should be incorporated -into the new version's release notes. -'''); + printError('${indentation}When bumping the version for release, the ' + 'NEXT section should be incorporated into the new version\'s ' + 'release notes.'); return false; } } - print('$packageName passed version check'); return true; } @@ -337,9 +338,8 @@ into the new version's release notes. final Pubspec pubspec = Pubspec.parse(pubspecFile.readAsStringSync()); return pubspec; } on Exception catch (exception) { - printError( - 'Failed to parse `pubspec.yaml` at ${pubspecFile.path}: $exception}'); + printError('${indentation}Failed to parse `pubspec.yaml`: $exception}'); + return null; } - return null; } } diff --git a/script/tool/test/version_check_test.dart b/script/tool/test/version_check_command_test.dart similarity index 70% rename from script/tool/test/version_check_test.dart rename to script/tool/test/version_check_command_test.dart index 6035360a221c..4d884692046d 100644 --- a/script/tool/test/version_check_test.dart +++ b/script/tool/test/version_check_command_test.dart @@ -29,7 +29,7 @@ void testAllowedVersion( final Version master = Version.parse(masterVersion); final Version head = Version.parse(headVersion); final Map allowedVersions = - getAllowedNextVersions(masterVersion: master, headVersion: head); + getAllowedNextVersions(master, newVersion: head); if (allowed) { expect(allowedVersions, contains(head)); if (nextVersionType != null) { @@ -42,14 +42,6 @@ void testAllowedVersion( class MockProcessResult extends Mock implements io.ProcessResult {} -const String _redColorMessagePrefix = '\x1B[31m'; -const String _redColorMessagePostfix = '\x1B[0m'; - -// Some error message was printed in a "Colorized" red message. So `\x1B[31m` and `\x1B[0m` needs to be included. -String _redColorString(String string) { - return '$_redColorMessagePrefix$string$_redColorMessagePostfix'; -} - void main() { const String indentation = ' '; group('$VersionCheckCommand', () { @@ -58,7 +50,6 @@ void main() { late CommandRunner runner; late RecordingProcessRunner processRunner; late List> gitDirCommands; - String gitDiffResponse; Map gitShowResponses; late MockGitDir gitDir; @@ -66,17 +57,14 @@ void main() { fileSystem = MemoryFileSystem(); packagesDir = createPackagesDirectory(fileSystem: fileSystem); gitDirCommands = >[]; - gitDiffResponse = ''; gitShowResponses = {}; gitDir = MockGitDir(); + when(gitDir.path).thenReturn(packagesDir.parent.path); when(gitDir.runCommand(any, throwOnError: anyNamed('throwOnError'))) .thenAnswer((Invocation invocation) { gitDirCommands.add(invocation.positionalArguments[0] as List); final MockProcessResult mockProcessResult = MockProcessResult(); - if (invocation.positionalArguments[0][0] == 'diff') { - when(mockProcessResult.stdout as String?) - .thenReturn(gitDiffResponse); - } else if (invocation.positionalArguments[0][0] == 'show') { + if (invocation.positionalArguments[0][0] == 'show') { final String? response = gitShowResponses[invocation.positionalArguments[0][1]]; if (response == null) { @@ -100,39 +88,32 @@ void main() { }); test('allows valid version', () async { - const String newVersion = '2.0.0'; - createFakePlugin('plugin', packagesDir, version: newVersion); - gitDiffResponse = 'packages/plugin/pubspec.yaml'; + createFakePlugin('plugin', packagesDir, version: '2.0.0'); gitShowResponses = { 'master:packages/plugin/pubspec.yaml': 'version: 1.0.0', - 'HEAD:packages/plugin/pubspec.yaml': 'version: $newVersion', }; final List output = await runCapturingPrint( runner, ['version-check', '--base-sha=master']); expect( output, - containsAllInOrder([ - 'No version check errors found!', + containsAllInOrder([ + contains('Running for plugin'), + contains('1.0.0 -> 2.0.0'), ]), ); - expect(gitDirCommands.length, equals(3)); + expect(gitDirCommands.length, equals(1)); expect( gitDirCommands, containsAll([ - equals(['diff', '--name-only', 'master', 'HEAD']), equals(['show', 'master:packages/plugin/pubspec.yaml']), - equals(['show', 'HEAD:packages/plugin/pubspec.yaml']), ])); }); test('denies invalid version', () async { - const String newVersion = '0.2.0'; - createFakePlugin('plugin', packagesDir, version: newVersion); - gitDiffResponse = 'packages/plugin/pubspec.yaml'; + createFakePlugin('plugin', packagesDir, version: '0.2.0'); gitShowResponses = { 'master:packages/plugin/pubspec.yaml': 'version: 0.0.1', - 'HEAD:packages/plugin/pubspec.yaml': 'version: $newVersion', }; final Future> result = runCapturingPrint( runner, ['version-check', '--base-sha=master']); @@ -141,80 +122,66 @@ void main() { result, throwsA(const TypeMatcher()), ); - expect(gitDirCommands.length, equals(3)); + expect(gitDirCommands.length, equals(1)); expect( gitDirCommands, containsAll([ - equals(['diff', '--name-only', 'master', 'HEAD']), equals(['show', 'master:packages/plugin/pubspec.yaml']), - equals(['show', 'HEAD:packages/plugin/pubspec.yaml']), ])); }); test('allows valid version without explicit base-sha', () async { - const String newVersion = '2.0.0'; - createFakePlugin('plugin', packagesDir, version: newVersion); - gitDiffResponse = 'packages/plugin/pubspec.yaml'; + createFakePlugin('plugin', packagesDir, version: '2.0.0'); gitShowResponses = { 'abc123:packages/plugin/pubspec.yaml': 'version: 1.0.0', - 'HEAD:packages/plugin/pubspec.yaml': 'version: $newVersion', }; final List output = await runCapturingPrint(runner, ['version-check']); expect( output, - containsAllInOrder([ - 'No version check errors found!', + containsAllInOrder([ + contains('Running for plugin'), + contains('1.0.0 -> 2.0.0'), ]), ); }); test('allows valid version for new package.', () async { - const String newVersion = '1.0.0'; - createFakePlugin('plugin', packagesDir, version: newVersion); - gitDiffResponse = 'packages/plugin/pubspec.yaml'; - gitShowResponses = { - 'HEAD:packages/plugin/pubspec.yaml': 'version: $newVersion', - }; + createFakePlugin('plugin', packagesDir, version: '1.0.0'); final List output = await runCapturingPrint(runner, ['version-check']); expect( output, - containsAllInOrder([ - '${indentation}Unable to find pubspec in master. Safe to ignore if the project is new.', - 'No version check errors found!', + containsAllInOrder([ + contains('Running for plugin'), + contains('Unable to find previous version at git base.'), ]), ); }); test('allows likely reverts.', () async { - const String newVersion = '0.6.1'; - createFakePlugin('plugin', packagesDir, version: newVersion); - gitDiffResponse = 'packages/plugin/pubspec.yaml'; + createFakePlugin('plugin', packagesDir, version: '0.6.1'); gitShowResponses = { 'abc123:packages/plugin/pubspec.yaml': 'version: 0.6.2', - 'HEAD:packages/plugin/pubspec.yaml': 'version: $newVersion', }; final List output = await runCapturingPrint(runner, ['version-check']); expect( output, - containsAllInOrder([ - '${indentation}New version is lower than previous version. This is assumed to be a revert.', + containsAllInOrder([ + contains('New version is lower than previous version. ' + 'This is assumed to be a revert.'), ]), ); }); test('denies lower version that could not be a simple revert', () async { - const String newVersion = '0.5.1'; - createFakePlugin('plugin', packagesDir, version: newVersion); - gitDiffResponse = 'packages/plugin/pubspec.yaml'; + createFakePlugin('plugin', packagesDir, version: '0.5.1'); gitShowResponses = { 'abc123:packages/plugin/pubspec.yaml': 'version: 0.6.2', - 'HEAD:packages/plugin/pubspec.yaml': 'version: $newVersion', }; final Future> result = runCapturingPrint(runner, ['version-check']); @@ -226,12 +193,9 @@ void main() { }); test('denies invalid version without explicit base-sha', () async { - const String newVersion = '0.2.0'; - createFakePlugin('plugin', packagesDir, version: newVersion); - gitDiffResponse = 'packages/plugin/pubspec.yaml'; + createFakePlugin('plugin', packagesDir, version: '0.2.0'); gitShowResponses = { 'abc123:packages/plugin/pubspec.yaml': 'version: 0.0.1', - 'HEAD:packages/plugin/pubspec.yaml': 'version: $newVersion', }; final Future> result = runCapturingPrint(runner, ['version-check']); @@ -242,73 +206,39 @@ void main() { ); }); - test('gracefully handles missing pubspec.yaml', () async { - final Directory pluginDir = - createFakePlugin('plugin', packagesDir, examples: []); - gitDiffResponse = 'packages/plugin/pubspec.yaml'; - pluginDir.childFile('pubspec.yaml').deleteSync(); - final List output = await runCapturingPrint( - runner, ['version-check', '--base-sha=master']); - - expect( - output, - orderedEquals([ - 'Determine diff with base sha: master', - 'Checking versions for packages/plugin/pubspec.yaml...', - ' Deleted; skipping.', - 'No version check errors found!', - ]), - ); - expect(gitDirCommands.length, equals(1)); - expect(gitDirCommands.first.join(' '), - equals('diff --name-only master HEAD')); - }); - test('allows minor changes to platform interfaces', () async { - const String newVersion = '1.1.0'; createFakePlugin('plugin_platform_interface', packagesDir, - version: newVersion); - gitDiffResponse = 'packages/plugin_platform_interface/pubspec.yaml'; + version: '1.1.0'); gitShowResponses = { 'master:packages/plugin_platform_interface/pubspec.yaml': 'version: 1.0.0', - 'HEAD:packages/plugin_platform_interface/pubspec.yaml': - 'version: $newVersion', }; final List output = await runCapturingPrint( runner, ['version-check', '--base-sha=master']); expect( output, - containsAllInOrder([ - 'No version check errors found!', + containsAllInOrder([ + contains('Running for plugin'), + contains('1.0.0 -> 1.1.0'), ]), ); - expect(gitDirCommands.length, equals(3)); + expect(gitDirCommands.length, equals(1)); expect( gitDirCommands, containsAll([ - equals(['diff', '--name-only', 'master', 'HEAD']), equals([ 'show', 'master:packages/plugin_platform_interface/pubspec.yaml' ]), - equals([ - 'show', - 'HEAD:packages/plugin_platform_interface/pubspec.yaml' - ]), ])); }); test('disallows breaking changes to platform interfaces', () async { - const String newVersion = '2.0.0'; createFakePlugin('plugin_platform_interface', packagesDir, - version: newVersion); - gitDiffResponse = 'packages/plugin_platform_interface/pubspec.yaml'; + version: '2.0.0'); gitShowResponses = { 'master:packages/plugin_platform_interface/pubspec.yaml': 'version: 1.0.0', - 'HEAD:packages/plugin_platform_interface/pubspec.yaml': - 'version: $newVersion', }; final Future> output = runCapturingPrint( runner, ['version-check', '--base-sha=master']); @@ -316,19 +246,14 @@ void main() { output, throwsA(const TypeMatcher()), ); - expect(gitDirCommands.length, equals(3)); + expect(gitDirCommands.length, equals(1)); expect( gitDirCommands, containsAll([ - equals(['diff', '--name-only', 'master', 'HEAD']), equals([ 'show', 'master:packages/plugin_platform_interface/pubspec.yaml' ]), - equals([ - 'show', - 'HEAD:packages/plugin_platform_interface/pubspec.yaml' - ]), ])); }); @@ -338,6 +263,7 @@ void main() { final Directory pluginDirectory = createFakePlugin('plugin', packagesDir, version: version); const String changelog = ''' + ## $version * Some changes. '''; @@ -346,10 +272,8 @@ void main() { runner, ['version-check', '--base-sha=master']); expect( output, - containsAllInOrder([ - 'Checking the first version listed in CHANGELOG.md matches the version in pubspec.yaml for plugin.', - 'plugin passed version check', - 'No version check errors found!' + containsAllInOrder([ + contains('Running for plugin'), ]), ); }); @@ -375,12 +299,8 @@ void main() { expect( output, - containsAllInOrder([ - _redColorString(''' -versions for plugin in CHANGELOG.md and pubspec.yaml do not match. -The version in pubspec.yaml is 1.0.1. -The first version listed in CHANGELOG.md is 1.0.2. -'''), + containsAllInOrder([ + contains('Versions in CHANGELOG.md and pubspec.yaml do not match.'), ]), ); }); @@ -399,10 +319,8 @@ The first version listed in CHANGELOG.md is 1.0.2. runner, ['version-check', '--base-sha=master']); expect( output, - containsAllInOrder([ - 'Checking the first version listed in CHANGELOG.md matches the version in pubspec.yaml for plugin.', - 'plugin passed version check', - 'No version check errors found!' + containsAllInOrder([ + contains('Running for plugin'), ]), ); }); @@ -433,14 +351,8 @@ The first version listed in CHANGELOG.md is 1.0.2. expect( output, - containsAllInOrder([ - _redColorString( - ''' -versions for plugin in CHANGELOG.md and pubspec.yaml do not match. -The version in pubspec.yaml is 1.0.0. -The first version listed in CHANGELOG.md is 1.0.1. -''', - ) + containsAllInOrder([ + contains('Versions in CHANGELOG.md and pubspec.yaml do not match.'), ]), ); }); @@ -462,10 +374,9 @@ The first version listed in CHANGELOG.md is 1.0.1. runner, ['version-check', '--base-sha=master']); await expectLater( output, - containsAllInOrder([ - 'Found NEXT; validating next version in the CHANGELOG.', - 'plugin passed version check', - 'No version check errors found!', + containsAllInOrder([ + contains('Running for plugin'), + contains('Found NEXT; validating next version in the CHANGELOG.'), ]), ); }); @@ -498,13 +409,9 @@ The first version listed in CHANGELOG.md is 1.0.1. expect( output, - containsAllInOrder([ - _redColorString( - ''' -When bumping the version for release, the NEXT section should be incorporated -into the new version's release notes. -''', - ) + containsAllInOrder([ + contains('When bumping the version for release, the NEXT section ' + 'should be incorporated into the new version\'s release notes.') ]), ); }); @@ -533,15 +440,9 @@ into the new version's release notes. expect( output, - containsAllInOrder([ - 'Found NEXT; validating next version in the CHANGELOG.', - _redColorString( - ''' -versions for plugin in CHANGELOG.md and pubspec.yaml do not match. -The version in pubspec.yaml is 1.0.1. -The first version listed in CHANGELOG.md is 1.0.0. -''', - ) + containsAllInOrder([ + contains('Found NEXT; validating next version in the CHANGELOG.'), + contains('Versions in CHANGELOG.md and pubspec.yaml do not match.'), ]), ); }); @@ -565,21 +466,17 @@ The first version listed in CHANGELOG.md is 1.0.0. 'version_check_command', 'Test for $VersionCheckCommand'); runner.addCommand(command); - const String newVersion = '2.0.0'; - createFakePlugin('plugin', packagesDir, version: newVersion); - gitDiffResponse = 'packages/plugin/pubspec.yaml'; + createFakePlugin('plugin', packagesDir, version: '2.0.0'); gitShowResponses = { 'master:packages/plugin/pubspec.yaml': 'version: 1.0.0', - 'HEAD:packages/plugin/pubspec.yaml': 'version: $newVersion', }; final List output = await runCapturingPrint(runner, ['version-check', '--base-sha=master', '--against-pub']); expect( output, - containsAllInOrder([ - '${indentation}plugin: Current largest version on pub: 1.0.0', - 'No version check errors found!', + containsAllInOrder([ + contains('plugin: Current largest version on pub: 1.0.0'), ]), ); }); @@ -602,12 +499,9 @@ The first version listed in CHANGELOG.md is 1.0.0. 'version_check_command', 'Test for $VersionCheckCommand'); runner.addCommand(command); - const String newVersion = '2.0.0'; - createFakePlugin('plugin', packagesDir, version: newVersion); - gitDiffResponse = 'packages/plugin/pubspec.yaml'; + createFakePlugin('plugin', packagesDir, version: '2.0.0'); gitShowResponses = { 'master:packages/plugin/pubspec.yaml': 'version: 1.0.0', - 'HEAD:packages/plugin/pubspec.yaml': 'version: $newVersion', }; bool hasError = false; @@ -623,13 +517,11 @@ The first version listed in CHANGELOG.md is 1.0.0. expect( result, - containsAllInOrder([ - _redColorString( - ''' + containsAllInOrder([ + contains(''' ${indentation}Incorrectly updated version. ${indentation}HEAD: 2.0.0, pub: 0.0.2. -${indentation}Allowed versions: {1.0.0: NextVersionType.BREAKING_MAJOR, 0.1.0: NextVersionType.MINOR, 0.0.3: NextVersionType.PATCH}''', - ) +${indentation}Allowed versions: {1.0.0: NextVersionType.BREAKING_MAJOR, 0.1.0: NextVersionType.MINOR, 0.0.3: NextVersionType.PATCH}''') ]), ); }); @@ -647,12 +539,9 @@ ${indentation}Allowed versions: {1.0.0: NextVersionType.BREAKING_MAJOR, 0.1.0: N 'version_check_command', 'Test for $VersionCheckCommand'); runner.addCommand(command); - const String newVersion = '2.0.0'; - createFakePlugin('plugin', packagesDir, version: newVersion); - gitDiffResponse = 'packages/plugin/pubspec.yaml'; + createFakePlugin('plugin', packagesDir, version: '2.0.0'); gitShowResponses = { 'master:packages/plugin/pubspec.yaml': 'version: 1.0.0', - 'HEAD:packages/plugin/pubspec.yaml': 'version: $newVersion', }; bool hasError = false; final List result = await runCapturingPrint(runner, [ @@ -667,14 +556,12 @@ ${indentation}Allowed versions: {1.0.0: NextVersionType.BREAKING_MAJOR, 0.1.0: N expect( result, - containsAllInOrder([ - _redColorString( - ''' + containsAllInOrder([ + contains(''' ${indentation}Error fetching version on pub for plugin. ${indentation}HTTP Status 400 ${indentation}HTTP response: xx -''', - ) +''') ]), ); }); @@ -691,21 +578,17 @@ ${indentation}HTTP response: xx 'version_check_command', 'Test for $VersionCheckCommand'); runner.addCommand(command); - const String newVersion = '2.0.0'; - createFakePlugin('plugin', packagesDir, version: newVersion); - gitDiffResponse = 'packages/plugin/pubspec.yaml'; + createFakePlugin('plugin', packagesDir, version: '2.0.0'); gitShowResponses = { 'master:packages/plugin/pubspec.yaml': 'version: 1.0.0', - 'HEAD:packages/plugin/pubspec.yaml': 'version: $newVersion', }; final List result = await runCapturingPrint(runner, ['version-check', '--base-sha=master', '--against-pub']); expect( result, - containsAllInOrder([ - '${indentation}Unable to find package on pub server. Safe to ignore if the project is new.', - 'No version check errors found!', + containsAllInOrder([ + contains('Unable to find previous version on pub server.'), ]), ); });