[flutter_plugin_tools] Check for missing version and CHANGELOG updates (#4530)
The currently documented repository policy is to:
- require version updates for packages changes that don't meet specific exemptions, and
- require CHANGELOG changes for essentially all changes.
This adds tooling that enforces that policy, with a mechanism for overriding it via PR descriptions, to avoid cases where they are accidentally omitted without reviewers catching it.
In order to facilitate testing (which require mocking another `git` command), this also updates the existing `version-check` tests:
- Replaces the custom git result injection/validating with the newer bind-to-process-mocks approach that is now used in the rest of the tool tests.
- Fixes some tests that were only checking for `ToolExit` to also check the error output, in order to ensure that failure tests are not accidentally passing for the wrong reason (as is being done in general as tests in the tooling are updated).
Fixes https://github.com/flutter/flutter/issues/93790
diff --git a/.cirrus.yml b/.cirrus.yml
index e559339..59f686d 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -75,16 +75,16 @@
CHANGE_DESC: "$TMPDIR/change-description.txt"
version_check_script:
# For pre-submit, pass the PR description to the script to allow for
- # platform version breaking version change justifications.
- # For post-submit, ignore platform version breaking version changes.
- # The PR description isn't reliably part of the commit message, so using
- # the same flags as for presubmit would likely result in false-positive
- # post-submit failures.
+ # version check overrides.
+ # For post-submit, ignore platform version breaking version changes and
+ # missing version/CHANGELOG detection; the PR description isn't reliably
+ # part of the commit message, so using the same flags as for presubmit
+ # would likely result in false-positive post-submit failures.
- if [[ $CIRRUS_PR == "" ]]; then
- ./script/tool_runner.sh version-check --ignore-platform-interface-breaks
- else
- echo "$CIRRUS_CHANGE_MESSAGE" > "$CHANGE_DESC"
- - ./script/tool_runner.sh version-check --change-description-file="$CHANGE_DESC"
+ - ./script/tool_runner.sh version-check --check-for-missing-changes --change-description-file="$CHANGE_DESC"
- fi
publish_check_script: ./script/tool_runner.sh publish-check
- name: format
diff --git a/script/tool/CHANGELOG.md b/script/tool/CHANGELOG.md
index 31efc28..ee23428 100644
--- a/script/tool/CHANGELOG.md
+++ b/script/tool/CHANGELOG.md
@@ -1,9 +1,12 @@
-## NEXT
+## 0.7.3
- `native-test` now builds unit tests before running them on Windows and Linux,
matching the behavior of other platforms.
-- Added `--log-timing` to add timing information to package headers in looping
+- Adds `--log-timing` to add timing information to package headers in looping
commands.
+- Adds a `--check-for-missing-changes` flag to `version-check` that requires
+ version updates (except for recognized exemptions) and CHANGELOG changes when
+ modifying packages, unless the PR description explains why it's not needed.
## 0.7.2
diff --git a/script/tool/lib/src/common/git_version_finder.dart b/script/tool/lib/src/common/git_version_finder.dart
index 1cdd2fc..3311873 100644
--- a/script/tool/lib/src/common/git_version_finder.dart
+++ b/script/tool/lib/src/common/git_version_finder.dart
@@ -75,8 +75,9 @@
io.ProcessResult baseShaFromMergeBase = await baseGitDir.runCommand(
<String>['merge-base', '--fork-point', 'FETCH_HEAD', 'HEAD'],
throwOnError: false);
- if (baseShaFromMergeBase.stderr != null ||
- baseShaFromMergeBase.stdout == null) {
+ final String stdout = (baseShaFromMergeBase.stdout as String? ?? '').trim();
+ final String stderr = (baseShaFromMergeBase.stdout as String? ?? '').trim();
+ if (stderr.isNotEmpty || stdout.isEmpty) {
baseShaFromMergeBase = await baseGitDir
.runCommand(<String>['merge-base', 'FETCH_HEAD', 'HEAD']);
}
diff --git a/script/tool/lib/src/version_check_command.dart b/script/tool/lib/src/version_check_command.dart
index dc664e5..1ec5dc4 100644
--- a/script/tool/lib/src/version_check_command.dart
+++ b/script/tool/lib/src/version_check_command.dart
@@ -120,6 +120,14 @@
'(e.g., PR description or commit message).\n\n'
'If supplied, this is used to allow overrides to some version '
'checks.');
+ argParser.addFlag(_checkForMissingChanges,
+ help: 'Validates that changes to packages include CHANGELOG and '
+ 'version changes unless they meet an established exemption.\n\n'
+ 'If used with --$_changeDescriptionFile, this is should only be '
+ 'used in pre-submit CI checks, to prevent the possibility of '
+ 'post-submit breakage if an override justification is not '
+ 'transferred into the commit message.',
+ hide: true);
argParser.addFlag(_ignorePlatformInterfaceBreaks,
help: 'Bypasses the check that platform interfaces do not contain '
'breaking changes.\n\n'
@@ -133,6 +141,7 @@
static const String _againstPubFlag = 'against-pub';
static const String _changeDescriptionFile = 'change-description-file';
+ static const String _checkForMissingChanges = 'check-for-missing-changes';
static const String _ignorePlatformInterfaceBreaks =
'ignore-platform-interface-breaks';
@@ -141,8 +150,26 @@
static const String _breakingChangeJustificationMarker =
'## Breaking change justification';
+ /// The string that must be at the start of a line in [_changeDescriptionFile]
+ /// to allow skipping a version change for a PR that would normally require
+ /// one.
+ static const String _missingVersionChangeJustificationMarker =
+ 'No version change:';
+
+ /// The string that must be at the start of a line in [_changeDescriptionFile]
+ /// to allow skipping a CHANGELOG change for a PR that would normally require
+ /// one.
+ static const String _missingChangelogChangeJustificationMarker =
+ 'No CHANGELOG change:';
+
final PubVersionFinder _pubVersionFinder;
+ late final GitVersionFinder _gitVersionFinder;
+ late final String _mergeBase;
+ late final List<String> _changedFiles;
+
+ late final String _changeDescription = _loadChangeDescription();
+
@override
final String name = 'version-check';
@@ -156,7 +183,11 @@
bool get hasLongOutput => false;
@override
- Future<void> initializeRun() async {}
+ Future<void> initializeRun() async {
+ _gitVersionFinder = await retrieveVersionFinder();
+ _mergeBase = await _gitVersionFinder.getBaseSha();
+ _changedFiles = await _gitVersionFinder.getChangedFiles();
+ }
@override
Future<PackageResult> runForPackage(RepositoryPackage package) async {
@@ -206,6 +237,17 @@
errors.add('CHANGELOG.md failed validation.');
}
+ // If there are no other issues, make sure that there isn't a missing
+ // change to the version and/or CHANGELOG.
+ if (getBoolArg(_checkForMissingChanges) &&
+ !versionChanged &&
+ errors.isEmpty) {
+ final String? error = await _checkForMissingChangeError(package);
+ if (error != null) {
+ errors.add(error);
+ }
+ }
+
return errors.isEmpty
? PackageResult.success()
: PackageResult.fail(errors);
@@ -239,10 +281,7 @@
}
/// Returns the version of [package] from git at the base comparison hash.
- Future<Version?> _getPreviousVersionFromGit(
- RepositoryPackage package, {
- required GitVersionFinder gitVersionFinder,
- }) async {
+ Future<Version?> _getPreviousVersionFromGit(RepositoryPackage package) async {
final File pubspecFile = package.pubspecFile;
final String relativePath =
path.relative(pubspecFile.absolute.path, from: (await gitDir).path);
@@ -250,7 +289,8 @@
final String gitPath = path.style == p.Style.windows
? p.posix.joinAll(path.split(relativePath))
: relativePath;
- return await gitVersionFinder.getPackageVersion(gitPath);
+ return await _gitVersionFinder.getPackageVersion(gitPath,
+ gitRef: _mergeBase);
}
/// Returns the state of the verison of [package] relative to the comparison
@@ -274,11 +314,9 @@
'$indentation${pubspec.name}: Current largest version on pub: $previousVersion');
}
} else {
- final GitVersionFinder gitVersionFinder = await retrieveVersionFinder();
- previousVersionSource = await gitVersionFinder.getBaseSha();
- previousVersion = await _getPreviousVersionFromGit(package,
- gitVersionFinder: gitVersionFinder) ??
- Version.none;
+ previousVersionSource = _mergeBase;
+ previousVersion =
+ await _getPreviousVersionFromGit(package) ?? Version.none;
}
if (previousVersion == Version.none) {
print('${indentation}Unable to find previous version '
@@ -462,9 +500,11 @@
return false;
}
+ String _getChangeDescription() => _changeDescription;
+
/// Returns the contents of the file pointed to by [_changeDescriptionFile],
/// or an empty string if that flag is not provided.
- String _getChangeDescription() {
+ String _loadChangeDescription() {
final String path = getStringArg(_changeDescriptionFile);
if (path.isEmpty) {
return '';
@@ -476,4 +516,91 @@
}
return file.readAsStringSync();
}
+
+ /// Returns an error string if the changes to this package should have
+ /// resulted in a version change, or shoud have resulted in a CHANGELOG change
+ /// but didn't.
+ ///
+ /// This should only be called if the version did not change.
+ Future<String?> _checkForMissingChangeError(RepositoryPackage package) async {
+ // Find the relative path to the current package, as it would appear at the
+ // beginning of a path reported by getChangedFiles() (which always uses
+ // Posix paths).
+ final Directory gitRoot =
+ packagesDir.fileSystem.directory((await gitDir).path);
+ final String relativePackagePath =
+ getRelativePosixPath(package.directory, from: gitRoot) + '/';
+ bool hasChanges = false;
+ bool needsVersionChange = false;
+ bool hasChangelogChange = false;
+ for (final String path in _changedFiles) {
+ // Only consider files within the package.
+ if (!path.startsWith(relativePackagePath)) {
+ continue;
+ }
+ hasChanges = true;
+
+ final List<String> components = p.posix.split(path);
+ final bool isChangelog = components.last == 'CHANGELOG.md';
+ if (isChangelog) {
+ hasChangelogChange = true;
+ }
+
+ if (!needsVersionChange &&
+ !isChangelog &&
+ // The example's main.dart is shown on pub.dev, but for anything else
+ // in the example publishing has no purpose.
+ !(components.contains('example') && components.last != 'main.dart') &&
+ // Changes to tests don't need to be published.
+ !components.contains('test') &&
+ !components.contains('androidTest') &&
+ !components.contains('RunnerTests') &&
+ !components.contains('RunnerUITests') &&
+ // Ignoring lints doesn't affect clients.
+ !components.contains('lint-baseline.xml')) {
+ needsVersionChange = true;
+ }
+ }
+
+ if (!hasChanges) {
+ return null;
+ }
+
+ if (needsVersionChange) {
+ if (_getChangeDescription().split('\n').any((String line) =>
+ line.startsWith(_missingVersionChangeJustificationMarker))) {
+ logWarning('Ignoring lack of version change due to '
+ '"$_missingVersionChangeJustificationMarker" in the '
+ 'change description.');
+ } else {
+ printError(
+ 'No version change found, but the change to this package could '
+ 'not be verified to be exempt from version changes according to '
+ 'repository policy. If this is a false positive, please '
+ 'add a line starting with\n'
+ '$_missingVersionChangeJustificationMarker\n'
+ 'to your PR description with an explanation of why it is exempt.');
+ return 'Missing version change';
+ }
+ }
+
+ if (!hasChangelogChange) {
+ if (_getChangeDescription().split('\n').any((String line) =>
+ line.startsWith(_missingChangelogChangeJustificationMarker))) {
+ logWarning('Ignoring lack of CHANGELOG update due to '
+ '"$_missingChangelogChangeJustificationMarker" in the '
+ 'change description.');
+ } else {
+ printError(
+ 'No CHANGELOG change found. If this PR needs an exemption from'
+ 'the standard policy of listing all changes in the CHANGELOG, '
+ 'please add a line starting with\n'
+ '$_missingChangelogChangeJustificationMarker\n'
+ 'to your PR description with an explanation of why.');
+ return 'Missing CHANGELOG change';
+ }
+ }
+
+ return null;
+ }
}
diff --git a/script/tool/test/common/git_version_finder_test.dart b/script/tool/test/common/git_version_finder_test.dart
index f1f40b5..fa8b1c4 100644
--- a/script/tool/test/common/git_version_finder_test.dart
+++ b/script/tool/test/common/git_version_finder_test.dart
@@ -14,7 +14,7 @@
late List<List<String>?> gitDirCommands;
late String gitDiffResponse;
late MockGitDir gitDir;
- String? mergeBaseResponse;
+ String mergeBaseResponse = '';
setUp(() {
gitDirCommands = <List<String>?>[];
@@ -74,7 +74,7 @@
final GitVersionFinder finder = GitVersionFinder(gitDir, null);
await finder.getChangedFiles();
verify(gitDir.runCommand(
- <String>['diff', '--name-only', mergeBaseResponse!, 'HEAD']));
+ <String>['diff', '--name-only', mergeBaseResponse, 'HEAD']));
});
test('use correct base sha if specified', () async {
diff --git a/script/tool/test/version_check_command_test.dart b/script/tool/test/version_check_command_test.dart
index 30b8855..5a5a0a1 100644
--- a/script/tool/test/version_check_command_test.dart
+++ b/script/tool/test/version_check_command_test.dart
@@ -2,7 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-import 'dart:async';
import 'dart:convert';
import 'dart:io' as io;
@@ -51,8 +50,6 @@
late Directory packagesDir;
late CommandRunner<void> runner;
late RecordingProcessRunner processRunner;
- late List<List<String>> gitDirCommands;
- Map<String, String> gitShowResponses;
late MockGitDir gitDir;
// Ignored if mockHttpResponse is set.
int mockHttpStatus;
@@ -63,27 +60,17 @@
mockPlatform = MockPlatform();
packagesDir = createPackagesDirectory(fileSystem: fileSystem);
- gitDirCommands = <List<String>>[];
- gitShowResponses = <String, String>{};
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<String>);
- final MockProcessResult mockProcessResult = MockProcessResult();
- if (invocation.positionalArguments[0][0] == 'show') {
- final String? response =
- gitShowResponses[invocation.positionalArguments[0][1]];
- if (response == null) {
- throw const io.ProcessException('git', <String>['show']);
- }
- when<String?>(mockProcessResult.stdout as String?)
- .thenReturn(response);
- } else if (invocation.positionalArguments[0][0] == 'merge-base') {
- when<String?>(mockProcessResult.stdout as String?)
- .thenReturn('abc123');
- }
- return Future<io.ProcessResult>.value(mockProcessResult);
+ final List<String> arguments =
+ invocation.positionalArguments[0]! as List<String>;
+ // Route git calls through the process runner, to make mock output
+ // consistent with other processes. Attach the first argument to the
+ // command to make targeting the mock results easier.
+ final String gitCommand = arguments.removeAt(0);
+ return processRunner.run('git-$gitCommand', arguments);
});
// Default to simulating the plugin never having been published.
@@ -108,9 +95,9 @@
test('allows valid version', () async {
createFakePlugin('plugin', packagesDir, version: '2.0.0');
- gitShowResponses = <String, String>{
- 'main:packages/plugin/pubspec.yaml': 'version: 1.0.0',
- };
+ processRunner.mockProcessesForExecutable['git-show'] = <io.Process>[
+ MockProcess(stdout: 'version: 1.0.0'),
+ ];
final List<String> output = await runCapturingPrint(
runner, <String>['version-check', '--base-sha=main']);
@@ -121,39 +108,49 @@
contains('1.0.0 -> 2.0.0'),
]),
);
- expect(gitDirCommands.length, equals(1));
expect(
- gitDirCommands,
- containsAll(<Matcher>[
- equals(<String>['show', 'main:packages/plugin/pubspec.yaml']),
+ processRunner.recordedCalls,
+ containsAllInOrder(const <ProcessCall>[
+ ProcessCall(
+ 'git-show', <String>['main:packages/plugin/pubspec.yaml'], null)
]));
});
test('denies invalid version', () async {
createFakePlugin('plugin', packagesDir, version: '0.2.0');
- gitShowResponses = <String, String>{
- 'main:packages/plugin/pubspec.yaml': 'version: 0.0.1',
- };
- final Future<List<String>> result = runCapturingPrint(
- runner, <String>['version-check', '--base-sha=main']);
+ processRunner.mockProcessesForExecutable['git-show'] = <io.Process>[
+ MockProcess(stdout: 'version: 0.0.1'),
+ ];
+ Error? commandError;
+ final List<String> output = await runCapturingPrint(
+ runner, <String>['version-check', '--base-sha=main'],
+ errorHandler: (Error e) {
+ commandError = e;
+ });
- await expectLater(
- result,
- throwsA(isA<ToolExit>()),
- );
- expect(gitDirCommands.length, equals(1));
+ expect(commandError, isA<ToolExit>());
expect(
- gitDirCommands,
- containsAll(<Matcher>[
- equals(<String>['show', 'main:packages/plugin/pubspec.yaml']),
+ output,
+ containsAllInOrder(<Matcher>[
+ contains('Incorrectly updated version.'),
+ ]));
+ expect(
+ processRunner.recordedCalls,
+ containsAllInOrder(const <ProcessCall>[
+ ProcessCall(
+ 'git-show', <String>['main:packages/plugin/pubspec.yaml'], null)
]));
});
- test('allows valid version without explicit base-sha', () async {
+ test('uses merge-base without explicit base-sha', () async {
createFakePlugin('plugin', packagesDir, version: '2.0.0');
- gitShowResponses = <String, String>{
- 'abc123:packages/plugin/pubspec.yaml': 'version: 1.0.0',
- };
+ processRunner.mockProcessesForExecutable['git-merge-base'] = <io.Process>[
+ MockProcess(stdout: 'abc123'),
+ MockProcess(stdout: 'abc123'),
+ ];
+ processRunner.mockProcessesForExecutable['git-show'] = <io.Process>[
+ MockProcess(stdout: 'version: 1.0.0'),
+ ];
final List<String> output =
await runCapturingPrint(runner, <String>['version-check']);
@@ -164,6 +161,14 @@
contains('1.0.0 -> 2.0.0'),
]),
);
+ expect(
+ processRunner.recordedCalls,
+ containsAllInOrder(const <ProcessCall>[
+ ProcessCall('git-merge-base',
+ <String>['--fork-point', 'FETCH_HEAD', 'HEAD'], null),
+ ProcessCall('git-show',
+ <String>['abc123:packages/plugin/pubspec.yaml'], null),
+ ]));
});
test('allows valid version for new package.', () async {
@@ -182,11 +187,11 @@
test('allows likely reverts.', () async {
createFakePlugin('plugin', packagesDir, version: '0.6.1');
- gitShowResponses = <String, String>{
- 'abc123:packages/plugin/pubspec.yaml': 'version: 0.6.2',
- };
- final List<String> output =
- await runCapturingPrint(runner, <String>['version-check']);
+ processRunner.mockProcessesForExecutable['git-show'] = <io.Process>[
+ MockProcess(stdout: 'version: 0.6.2'),
+ ];
+ final List<String> output = await runCapturingPrint(
+ runner, <String>['version-check', '--base-sha=main']);
expect(
output,
@@ -195,43 +200,47 @@
'This is assumed to be a revert.'),
]),
);
+ expect(
+ processRunner.recordedCalls,
+ containsAllInOrder(const <ProcessCall>[
+ ProcessCall(
+ 'git-show', <String>['main:packages/plugin/pubspec.yaml'], null)
+ ]));
});
test('denies lower version that could not be a simple revert', () async {
createFakePlugin('plugin', packagesDir, version: '0.5.1');
- gitShowResponses = <String, String>{
- 'abc123:packages/plugin/pubspec.yaml': 'version: 0.6.2',
- };
- final Future<List<String>> result =
- runCapturingPrint(runner, <String>['version-check']);
+ processRunner.mockProcessesForExecutable['git-show'] = <io.Process>[
+ MockProcess(stdout: 'version: 0.6.2'),
+ ];
- await expectLater(
- result,
- throwsA(isA<ToolExit>()),
- );
- });
+ Error? commandError;
+ final List<String> output = await runCapturingPrint(
+ runner, <String>['version-check', '--base-sha=main'],
+ errorHandler: (Error e) {
+ commandError = e;
+ });
- test('denies invalid version without explicit base-sha', () async {
- createFakePlugin('plugin', packagesDir, version: '0.2.0');
- gitShowResponses = <String, String>{
- 'abc123:packages/plugin/pubspec.yaml': 'version: 0.0.1',
- };
- final Future<List<String>> result =
- runCapturingPrint(runner, <String>['version-check']);
-
- await expectLater(
- result,
- throwsA(isA<ToolExit>()),
- );
+ expect(commandError, isA<ToolExit>());
+ expect(
+ output,
+ containsAllInOrder(<Matcher>[
+ contains('Incorrectly updated version.'),
+ ]));
+ expect(
+ processRunner.recordedCalls,
+ containsAllInOrder(const <ProcessCall>[
+ ProcessCall(
+ 'git-show', <String>['main:packages/plugin/pubspec.yaml'], null)
+ ]));
});
test('allows minor changes to platform interfaces', () async {
createFakePlugin('plugin_platform_interface', packagesDir,
version: '1.1.0');
- gitShowResponses = <String, String>{
- 'main:packages/plugin_platform_interface/pubspec.yaml':
- 'version: 1.0.0',
- };
+ processRunner.mockProcessesForExecutable['git-show'] = <io.Process>[
+ MockProcess(stdout: 'version: 1.0.0'),
+ ];
final List<String> output = await runCapturingPrint(
runner, <String>['version-check', '--base-sha=main']);
expect(
@@ -241,14 +250,15 @@
contains('1.0.0 -> 1.1.0'),
]),
);
- expect(gitDirCommands.length, equals(1));
expect(
- gitDirCommands,
- containsAll(<Matcher>[
- equals(<String>[
- 'show',
- 'main:packages/plugin_platform_interface/pubspec.yaml'
- ]),
+ processRunner.recordedCalls,
+ containsAllInOrder(const <ProcessCall>[
+ ProcessCall(
+ 'git-show',
+ <String>[
+ 'main:packages/plugin_platform_interface/pubspec.yaml'
+ ],
+ null)
]));
});
@@ -256,24 +266,35 @@
() async {
createFakePlugin('plugin_platform_interface', packagesDir,
version: '2.0.0');
- gitShowResponses = <String, String>{
- 'main:packages/plugin_platform_interface/pubspec.yaml':
- 'version: 1.0.0',
- };
- final Future<List<String>> output = runCapturingPrint(
- runner, <String>['version-check', '--base-sha=main']);
- await expectLater(
- output,
- throwsA(isA<ToolExit>()),
- );
- expect(gitDirCommands.length, equals(1));
+ processRunner.mockProcessesForExecutable['git-show'] = <io.Process>[
+ MockProcess(stdout: 'version: 1.0.0'),
+ ];
+ Error? commandError;
+ final List<String> output = await runCapturingPrint(
+ runner, <String>['version-check', '--base-sha=main'],
+ errorHandler: (Error e) {
+ commandError = e;
+ });
+
+ expect(commandError, isA<ToolExit>());
expect(
- gitDirCommands,
- containsAll(<Matcher>[
- equals(<String>[
- 'show',
- 'main:packages/plugin_platform_interface/pubspec.yaml'
- ]),
+ output,
+ containsAllInOrder(<Matcher>[
+ contains(
+ ' Breaking changes to platform interfaces are not allowed '
+ 'without explicit justification.\n'
+ ' See https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages '
+ 'for more information.'),
+ ]));
+ expect(
+ processRunner.recordedCalls,
+ containsAllInOrder(const <ProcessCall>[
+ ProcessCall(
+ 'git-show',
+ <String>[
+ 'main:packages/plugin_platform_interface/pubspec.yaml'
+ ],
+ null)
]));
});
@@ -281,10 +302,9 @@
() async {
createFakePlugin('plugin_platform_interface', packagesDir,
version: '2.0.0');
- gitShowResponses = <String, String>{
- 'main:packages/plugin_platform_interface/pubspec.yaml':
- 'version: 1.0.0',
- };
+ processRunner.mockProcessesForExecutable['git-show'] = <io.Process>[
+ MockProcess(stdout: 'version: 1.0.0'),
+ ];
final File changeDescriptionFile =
fileSystem.file('change_description.txt');
changeDescriptionFile.writeAsStringSync('''
@@ -310,16 +330,25 @@
contains('Ran for 1 package(s) (1 with warnings)'),
]),
);
+ expect(
+ processRunner.recordedCalls,
+ containsAllInOrder(const <ProcessCall>[
+ ProcessCall(
+ 'git-show',
+ <String>[
+ 'main:packages/plugin_platform_interface/pubspec.yaml'
+ ],
+ null)
+ ]));
});
test('throws if a nonexistent change description file is specified',
() async {
createFakePlugin('plugin_platform_interface', packagesDir,
version: '2.0.0');
- gitShowResponses = <String, String>{
- 'main:packages/plugin_platform_interface/pubspec.yaml':
- 'version: 1.0.0',
- };
+ processRunner.mockProcessesForExecutable['git-show'] = <io.Process>[
+ MockProcess(stdout: 'version: 1.0.0'),
+ ];
Error? commandError;
final List<String> output = await runCapturingPrint(runner, <String>[
@@ -337,16 +366,25 @@
contains('No such file: a_missing_file.txt'),
]),
);
+ expect(
+ processRunner.recordedCalls,
+ containsAllInOrder(const <ProcessCall>[
+ ProcessCall(
+ 'git-show',
+ <String>[
+ 'main:packages/plugin_platform_interface/pubspec.yaml'
+ ],
+ null)
+ ]));
});
test('allows breaking changes to platform interfaces with bypass flag',
() async {
createFakePlugin('plugin_platform_interface', packagesDir,
version: '2.0.0');
- gitShowResponses = <String, String>{
- 'main:packages/plugin_platform_interface/pubspec.yaml':
- 'version: 1.0.0',
- };
+ processRunner.mockProcessesForExecutable['git-show'] = <io.Process>[
+ MockProcess(stdout: 'version: 1.0.0'),
+ ];
final List<String> output = await runCapturingPrint(runner, <String>[
'version-check',
'--base-sha=main',
@@ -361,6 +399,16 @@
contains('Ran for 1 package(s) (1 with warnings)'),
]),
);
+ expect(
+ processRunner.recordedCalls,
+ containsAllInOrder(const <ProcessCall>[
+ ProcessCall(
+ 'git-show',
+ <String>[
+ 'main:packages/plugin_platform_interface/pubspec.yaml'
+ ],
+ null)
+ ]));
});
test('Allow empty lines in front of the first version in CHANGELOG',
@@ -392,15 +440,14 @@
* Some changes.
''';
createFakeCHANGELOG(pluginDirectory, changelog);
- bool hasError = false;
+ Error? commandError;
final List<String> output = await runCapturingPrint(
runner, <String>['version-check', '--base-sha=main', '--against-pub'],
errorHandler: (Error e) {
- expect(e, isA<ToolExit>());
- hasError = true;
+ commandError = e;
});
- expect(hasError, isTrue);
+ expect(commandError, isA<ToolExit>());
expect(
output,
containsAllInOrder(<Matcher>[
@@ -472,13 +519,13 @@
* Some other changes.
''';
createFakeCHANGELOG(pluginDirectory, changelog);
- gitShowResponses = <String, String>{
- 'main:packages/plugin/pubspec.yaml': 'version: 1.0.0',
- };
+ processRunner.mockProcessesForExecutable['git-show'] = <io.Process>[
+ MockProcess(stdout: 'version: 1.0.0'),
+ ];
final List<String> output = await runCapturingPrint(
runner, <String>['version-check', '--base-sha=main']);
- await expectLater(
+ expect(
output,
containsAllInOrder(<Matcher>[
contains('Running for plugin'),
@@ -534,9 +581,6 @@
* Some other changes.
''';
createFakeCHANGELOG(pluginDirectory, changelog);
- gitShowResponses = <String, String>{
- 'main:packages/plugin/pubspec.yaml': 'version: 1.0.0',
- };
bool hasError = false;
final List<String> output = await runCapturingPrint(
@@ -569,9 +613,6 @@
* Some other changes.
''';
createFakeCHANGELOG(pluginDirectory, changelog);
- gitShowResponses = <String, String>{
- 'main:packages/plugin/pubspec.yaml': 'version: 1.0.0',
- };
bool hasError = false;
final List<String> output = await runCapturingPrint(
@@ -604,9 +645,9 @@
* Some other changes.
''';
createFakeCHANGELOG(pluginDirectory, changelog);
- gitShowResponses = <String, String>{
- 'main:packages/plugin/pubspec.yaml': 'version: 1.0.0',
- };
+ processRunner.mockProcessesForExecutable['git-show'] = <io.Process>[
+ MockProcess(stdout: 'version: 1.0.0'),
+ ];
Error? commandError;
final List<String> output = await runCapturingPrint(runner, <String>[
@@ -637,9 +678,9 @@
* Some changes.
''';
createFakeCHANGELOG(pluginDirectory, changelog);
- gitShowResponses = <String, String>{
- 'main:packages/plugin/pubspec.yaml': 'version: 1.0.0',
- };
+ processRunner.mockProcessesForExecutable['git-show'] = <io.Process>[
+ MockProcess(stdout: 'version: 1.0.0'),
+ ];
Error? commandError;
final List<String> output = await runCapturingPrint(runner, <String>[
@@ -658,6 +699,360 @@
);
});
+ group('missing change detection', () {
+ Future<List<String>> _runWithMissingChangeDetection(
+ List<String> extraArgs,
+ {void Function(Error error)? errorHandler}) async {
+ return runCapturingPrint(
+ runner,
+ <String>[
+ 'version-check',
+ '--base-sha=main',
+ '--check-for-missing-changes',
+ ...extraArgs,
+ ],
+ errorHandler: errorHandler);
+ }
+
+ test('passes for unchanged packages', () async {
+ final Directory pluginDirectory =
+ createFakePlugin('plugin', packagesDir, version: '1.0.0');
+
+ const String changelog = '''
+## 1.0.0
+* Some changes.
+''';
+ createFakeCHANGELOG(pluginDirectory, changelog);
+ processRunner.mockProcessesForExecutable['git-show'] = <io.Process>[
+ MockProcess(stdout: 'version: 1.0.0'),
+ ];
+ processRunner.mockProcessesForExecutable['git-diff'] = <io.Process>[
+ MockProcess(stdout: ''),
+ ];
+
+ final List<String> output =
+ await _runWithMissingChangeDetection(<String>[]);
+
+ expect(
+ output,
+ containsAllInOrder(<Matcher>[
+ contains('Running for plugin'),
+ ]),
+ );
+ });
+
+ test(
+ 'fails if a version change is missing from a change that does not '
+ 'pass the exemption check', () async {
+ final Directory pluginDirectory =
+ createFakePlugin('plugin', packagesDir, version: '1.0.0');
+
+ const String changelog = '''
+## 1.0.0
+* Some changes.
+''';
+ createFakeCHANGELOG(pluginDirectory, changelog);
+ processRunner.mockProcessesForExecutable['git-show'] = <io.Process>[
+ MockProcess(stdout: 'version: 1.0.0'),
+ ];
+ processRunner.mockProcessesForExecutable['git-diff'] = <io.Process>[
+ MockProcess(stdout: '''
+packages/plugin/lib/plugin.dart
+'''),
+ ];
+
+ Error? commandError;
+ final List<String> output = await _runWithMissingChangeDetection(
+ <String>[], errorHandler: (Error e) {
+ commandError = e;
+ });
+
+ expect(commandError, isA<ToolExit>());
+ expect(
+ output,
+ containsAllInOrder(<Matcher>[
+ contains('No version change found'),
+ contains('plugin:\n'
+ ' Missing version change'),
+ ]),
+ );
+ });
+
+ test('passes version change requirement when version changes', () async {
+ final Directory pluginDirectory =
+ createFakePlugin('plugin', packagesDir, version: '1.0.1');
+
+ const String changelog = '''
+## 1.0.1
+* Some changes.
+''';
+ createFakeCHANGELOG(pluginDirectory, changelog);
+ processRunner.mockProcessesForExecutable['git-show'] = <io.Process>[
+ MockProcess(stdout: 'version: 1.0.0'),
+ ];
+ processRunner.mockProcessesForExecutable['git-diff'] = <io.Process>[
+ MockProcess(stdout: '''
+packages/plugin/lib/plugin.dart
+packages/plugin/CHANGELOG.md
+packages/plugin/pubspec.yaml
+'''),
+ ];
+
+ final List<String> output =
+ await _runWithMissingChangeDetection(<String>[]);
+
+ expect(
+ output,
+ containsAllInOrder(<Matcher>[
+ contains('Running for plugin'),
+ ]),
+ );
+ });
+
+ test('version change check ignores files outside the package', () async {
+ final Directory pluginDirectory =
+ createFakePlugin('plugin', packagesDir, version: '1.0.0');
+
+ const String changelog = '''
+## 1.0.0
+* Some changes.
+''';
+ createFakeCHANGELOG(pluginDirectory, changelog);
+ processRunner.mockProcessesForExecutable['git-show'] = <io.Process>[
+ MockProcess(stdout: 'version: 1.0.0'),
+ ];
+ processRunner.mockProcessesForExecutable['git-diff'] = <io.Process>[
+ MockProcess(stdout: '''
+packages/plugin_a/lib/plugin.dart
+tool/plugin/lib/plugin.dart
+'''),
+ ];
+
+ final List<String> output =
+ await _runWithMissingChangeDetection(<String>[]);
+
+ expect(
+ output,
+ containsAllInOrder(<Matcher>[
+ contains('Running for plugin'),
+ ]),
+ );
+ });
+
+ test('allows missing version change for exempt changes', () async {
+ final Directory pluginDirectory =
+ createFakePlugin('plugin', packagesDir, version: '1.0.0');
+
+ const String changelog = '''
+## 1.0.0
+* Some changes.
+''';
+ createFakeCHANGELOG(pluginDirectory, changelog);
+ processRunner.mockProcessesForExecutable['git-show'] = <io.Process>[
+ MockProcess(stdout: 'version: 1.0.0'),
+ ];
+ processRunner.mockProcessesForExecutable['git-diff'] = <io.Process>[
+ MockProcess(stdout: '''
+packages/plugin/example/android/lint-baseline.xml
+packages/plugin/example/android/src/androidTest/foo/bar/FooTest.java
+packages/plugin/example/ios/RunnerTests/Foo.m
+packages/plugin/example/ios/RunnerUITests/info.plist
+packages/plugin/CHANGELOG.md
+'''),
+ ];
+
+ final List<String> output =
+ await _runWithMissingChangeDetection(<String>[]);
+
+ expect(
+ output,
+ containsAllInOrder(<Matcher>[
+ contains('Running for plugin'),
+ ]),
+ );
+ });
+
+ test('allows missing version change with justification', () async {
+ final Directory pluginDirectory =
+ createFakePlugin('plugin', packagesDir, version: '1.0.0');
+
+ const String changelog = '''
+## 1.0.0
+* Some changes.
+''';
+ createFakeCHANGELOG(pluginDirectory, changelog);
+ processRunner.mockProcessesForExecutable['git-show'] = <io.Process>[
+ MockProcess(stdout: 'version: 1.0.0'),
+ ];
+ processRunner.mockProcessesForExecutable['git-diff'] = <io.Process>[
+ MockProcess(stdout: '''
+packages/plugin/lib/plugin.dart
+packages/plugin/CHANGELOG.md
+packages/plugin/pubspec.yaml
+'''),
+ ];
+
+ final File changeDescriptionFile =
+ fileSystem.file('change_description.txt');
+ changeDescriptionFile.writeAsStringSync('''
+Some general PR description
+
+No version change: Code change is only to implementation comments.
+''');
+ final List<String> output =
+ await _runWithMissingChangeDetection(<String>[
+ '--change-description-file=${changeDescriptionFile.path}'
+ ]);
+
+ expect(
+ output,
+ containsAllInOrder(<Matcher>[
+ contains('Ignoring lack of version change due to '
+ '"No version change:" in the change description.'),
+ ]),
+ );
+ });
+
+ test('fails if a CHANGELOG change is missing', () async {
+ final Directory pluginDirectory =
+ createFakePlugin('plugin', packagesDir, version: '1.0.0');
+
+ const String changelog = '''
+## 1.0.0
+* Some changes.
+''';
+ createFakeCHANGELOG(pluginDirectory, changelog);
+ processRunner.mockProcessesForExecutable['git-show'] = <io.Process>[
+ MockProcess(stdout: 'version: 1.0.0'),
+ ];
+ processRunner.mockProcessesForExecutable['git-diff'] = <io.Process>[
+ MockProcess(stdout: '''
+packages/plugin/example/lib/foo.dart
+'''),
+ ];
+
+ Error? commandError;
+ final List<String> output = await _runWithMissingChangeDetection(
+ <String>[], errorHandler: (Error e) {
+ commandError = e;
+ });
+
+ expect(commandError, isA<ToolExit>());
+ expect(
+ output,
+ containsAllInOrder(<Matcher>[
+ contains('No CHANGELOG change found'),
+ contains('plugin:\n'
+ ' Missing CHANGELOG change'),
+ ]),
+ );
+ });
+
+ test('passes CHANGELOG check when the CHANGELOG is changed', () async {
+ final Directory pluginDirectory =
+ createFakePlugin('plugin', packagesDir, version: '1.0.0');
+
+ const String changelog = '''
+## 1.0.0
+* Some changes.
+''';
+ createFakeCHANGELOG(pluginDirectory, changelog);
+ processRunner.mockProcessesForExecutable['git-show'] = <io.Process>[
+ MockProcess(stdout: 'version: 1.0.0'),
+ ];
+ processRunner.mockProcessesForExecutable['git-diff'] = <io.Process>[
+ MockProcess(stdout: '''
+packages/plugin/example/lib/foo.dart
+packages/plugin/CHANGELOG.md
+'''),
+ ];
+
+ final List<String> output =
+ await _runWithMissingChangeDetection(<String>[]);
+
+ expect(
+ output,
+ containsAllInOrder(<Matcher>[
+ contains('Running for plugin'),
+ ]),
+ );
+ });
+
+ test('fails CHANGELOG check if only another package CHANGELOG chages',
+ () async {
+ final Directory pluginDirectory =
+ createFakePlugin('plugin', packagesDir, version: '1.0.0');
+
+ const String changelog = '''
+## 1.0.0
+* Some changes.
+''';
+ createFakeCHANGELOG(pluginDirectory, changelog);
+ processRunner.mockProcessesForExecutable['git-show'] = <io.Process>[
+ MockProcess(stdout: 'version: 1.0.0'),
+ ];
+ processRunner.mockProcessesForExecutable['git-diff'] = <io.Process>[
+ MockProcess(stdout: '''
+packages/plugin/example/lib/foo.dart
+packages/another_plugin/CHANGELOG.md
+'''),
+ ];
+
+ Error? commandError;
+ final List<String> output = await _runWithMissingChangeDetection(
+ <String>[], errorHandler: (Error e) {
+ commandError = e;
+ });
+
+ expect(commandError, isA<ToolExit>());
+ expect(
+ output,
+ containsAllInOrder(<Matcher>[
+ contains('No CHANGELOG change found'),
+ ]),
+ );
+ });
+
+ test('allows missing CHANGELOG change with justification', () async {
+ final Directory pluginDirectory =
+ createFakePlugin('plugin', packagesDir, version: '1.0.0');
+
+ const String changelog = '''
+## 1.0.0
+* Some changes.
+''';
+ createFakeCHANGELOG(pluginDirectory, changelog);
+ processRunner.mockProcessesForExecutable['git-show'] = <io.Process>[
+ MockProcess(stdout: 'version: 1.0.0'),
+ ];
+ processRunner.mockProcessesForExecutable['git-diff'] = <io.Process>[
+ MockProcess(stdout: '''
+packages/plugin/example/lib/foo.dart
+'''),
+ ];
+
+ final File changeDescriptionFile =
+ fileSystem.file('change_description.txt');
+ changeDescriptionFile.writeAsStringSync('''
+Some general PR description
+
+No CHANGELOG change: Code change is only to implementation comments.
+''');
+ final List<String> output =
+ await _runWithMissingChangeDetection(<String>[
+ '--change-description-file=${changeDescriptionFile.path}'
+ ]);
+
+ expect(
+ output,
+ containsAllInOrder(<Matcher>[
+ contains('Ignoring lack of CHANGELOG update due to '
+ '"No CHANGELOG change:" in the change description.'),
+ ]),
+ );
+ });
+ });
+
test('allows valid against pub', () async {
mockHttpResponse = <String, dynamic>{
'name': 'some_package',
@@ -669,9 +1064,6 @@
};
createFakePlugin('plugin', packagesDir, version: '2.0.0');
- gitShowResponses = <String, String>{
- 'main:packages/plugin/pubspec.yaml': 'version: 1.0.0',
- };
final List<String> output = await runCapturingPrint(runner,
<String>['version-check', '--base-sha=main', '--against-pub']);
@@ -693,9 +1085,6 @@
};
createFakePlugin('plugin', packagesDir, version: '2.0.0');
- gitShowResponses = <String, String>{
- 'main:packages/plugin/pubspec.yaml': 'version: 1.0.0',
- };
bool hasError = false;
final List<String> result = await runCapturingPrint(
@@ -723,9 +1112,6 @@
mockHttpStatus = 400;
createFakePlugin('plugin', packagesDir, version: '2.0.0');
- gitShowResponses = <String, String>{
- 'main:packages/plugin/pubspec.yaml': 'version: 1.0.0',
- };
bool hasError = false;
final List<String> result = await runCapturingPrint(
runner, <String>['version-check', '--base-sha=main', '--against-pub'],
@@ -752,9 +1138,9 @@
mockHttpStatus = 404;
createFakePlugin('plugin', packagesDir, version: '2.0.0');
- gitShowResponses = <String, String>{
- 'main:packages/plugin/pubspec.yaml': 'version: 1.0.0',
- };
+ processRunner.mockProcessesForExecutable['git-show'] = <io.Process>[
+ MockProcess(stdout: 'version: 1.0.0'),
+ ];
final List<String> result = await runCapturingPrint(runner,
<String>['version-check', '--base-sha=main', '--against-pub']);
