diff --git a/script/tool/CHANGELOG.md b/script/tool/CHANGELOG.md index 5faa7c8201d9..9ed2a9278653 100644 --- a/script/tool/CHANGELOG.md +++ b/script/tool/CHANGELOG.md @@ -1,3 +1,7 @@ +## NEXT + +- Fixes changelog validation when reverting to a `NEXT` state. + ## 0.8.5 - Updates `test` to inculde the Dart unit tests of examples, if any. @@ -322,7 +326,7 @@ and `firebase-test-lab`. ## v.0.0.36+2 -- Default to showing podspec lint warnings. +- Default to showing podspec lint warnings ## v.0.0.36+1 diff --git a/script/tool/lib/src/version_check_command.dart b/script/tool/lib/src/version_check_command.dart index fb768c19b524..c0e67764360e 100644 --- a/script/tool/lib/src/version_check_command.dart +++ b/script/tool/lib/src/version_check_command.dart @@ -39,8 +39,11 @@ enum _CurrentVersionState { /// The version is unchanged. unchanged, - /// The version has changed, and the transition is valid. - validChange, + /// The version has increased, and the transition is valid. + validIncrease, + + /// The version has decrease, and the transition is a valid revert. + validRevert, /// The version has changed, and the transition is invalid. invalidChange, @@ -218,7 +221,8 @@ class VersionCheckCommand extends PackageLoopingCommand { case _CurrentVersionState.unchanged: versionChanged = false; break; - case _CurrentVersionState.validChange: + case _CurrentVersionState.validIncrease: + case _CurrentVersionState.validRevert: versionChanged = true; break; case _CurrentVersionState.invalidChange: @@ -232,7 +236,7 @@ class VersionCheckCommand extends PackageLoopingCommand { } if (!(await _validateChangelogVersion(package, - pubspec: pubspec, pubspecVersionChanged: versionChanged))) { + pubspec: pubspec, pubspecVersionState: versionState))) { errors.add('CHANGELOG.md failed validation.'); } @@ -322,7 +326,7 @@ ${indentation}HTTP response: ${pubVersionFinderResponse.httpResponse.body} '${getBoolArg(_againstPubFlag) ? 'on pub server' : 'at git base'}.'); logWarning( '${indentation}If this plugin is not new, something has gone wrong.'); - return _CurrentVersionState.validChange; // Assume new, thus valid. + return _CurrentVersionState.validIncrease; // Assume new, thus valid. } if (previousVersion == currentVersion) { @@ -340,7 +344,7 @@ ${indentation}HTTP response: ${pubVersionFinderResponse.httpResponse.body} if (possibleVersionsFromNewVersion.containsKey(previousVersion)) { logWarning('${indentation}New version is lower than previous version. ' 'This is assumed to be a revert.'); - return _CurrentVersionState.validChange; + return _CurrentVersionState.validRevert; } } @@ -367,7 +371,7 @@ ${indentation}HTTP response: ${pubVersionFinderResponse.httpResponse.body} return _CurrentVersionState.invalidChange; } - return _CurrentVersionState.validChange; + return _CurrentVersionState.validIncrease; } /// Checks whether or not [package]'s CHANGELOG's versioning is correct, @@ -378,7 +382,7 @@ ${indentation}HTTP response: ${pubVersionFinderResponse.httpResponse.body} Future _validateChangelogVersion( RepositoryPackage package, { required Pubspec pubspec, - required bool pubspecVersionChanged, + required _CurrentVersionState pubspecVersionState, }) async { // This method isn't called unless `version` is non-null. final Version fromPubspec = pubspec.version!; @@ -405,8 +409,9 @@ ${indentation}HTTP response: ${pubVersionFinderResponse.httpResponse.body} // changes that don't warrant publishing on their own. final bool hasNextSection = versionString == 'NEXT'; if (hasNextSection) { - // NEXT should not be present in a commit that changes the version. - if (pubspecVersionChanged) { + // NEXT should not be present in a commit that increases the version. + if (pubspecVersionState == _CurrentVersionState.validIncrease || + pubspecVersionState == _CurrentVersionState.invalidChange) { printError(badNextErrorMessage); return false; } diff --git a/script/tool/test/util.dart b/script/tool/test/util.dart index b0a8990e1300..5c38bd5f7033 100644 --- a/script/tool/test/util.dart +++ b/script/tool/test/util.dart @@ -319,14 +319,14 @@ String _pluginPlatformSection( return entry; } -typedef ErrorHandler = void Function(Error error); +typedef _ErrorHandler = void Function(Error error); /// Run the command [runner] with the given [args] and return /// what was printed. /// A custom [errorHandler] can be used to handle the runner error as desired without throwing. Future> runCapturingPrint( CommandRunner runner, List args, - {ErrorHandler? errorHandler}) async { + {_ErrorHandler? errorHandler}) async { final List prints = []; final ZoneSpecification spec = ZoneSpecification( print: (_, __, ___, String message) { diff --git a/script/tool/test/version_check_command_test.dart b/script/tool/test/version_check_command_test.dart index aeacd77635df..5b8ed97e20c5 100644 --- a/script/tool/test/version_check_command_test.dart +++ b/script/tool/test/version_check_command_test.dart @@ -44,7 +44,7 @@ class MockProcessResult extends Mock implements io.ProcessResult {} void main() { const String indentation = ' '; - group('$VersionCheckCommand', () { + group('VersionCheckCommand', () { late FileSystem fileSystem; late MockPlatform mockPlatform; late Directory packagesDir; @@ -602,7 +602,7 @@ This is necessary because of X, Y, and Z ); }); - test('Fail if the version changes without replacing NEXT', () async { + test('fails if the version increases without replacing NEXT', () async { final RepositoryPackage plugin = createFakePlugin('plugin', packagesDir, version: '1.0.1'); @@ -632,6 +632,33 @@ This is necessary because of X, Y, and Z ); }); + test('allows NEXT for a revert', () async { + final RepositoryPackage plugin = + createFakePlugin('plugin', packagesDir, version: '1.0.0'); + + const String changelog = ''' +## NEXT +* Some changes that should be listed as part of 1.0.1. +## 1.0.0 +* Some other changes. +'''; + createFakeCHANGELOG(plugin, changelog); + createFakeCHANGELOG(plugin, changelog); + processRunner.mockProcessesForExecutable['git-show'] = [ + MockProcess(stdout: 'version: 1.0.1'), + ]; + + final List output = await runCapturingPrint( + runner, ['version-check', '--base-sha=main']); + expect( + output, + containsAllInOrder([ + contains('New version is lower than previous version. ' + 'This is assumed to be a revert.'), + ]), + ); + }); + test( 'fails gracefully if the version headers are not found due to using the wrong style', () async {