[flutter_plugin_tools] Add a new 'make-deps-path-based' command (#4575)
Adds a new command that adds `dependency_overrides` to any packages in the repository that depend on a list of target packages, including an option to target packages that will publish a non-breaking change in a given diff.
Adds a new CI step that uses the above in conjunction with a new `--run-on-dirty-packages` to adjust the dependencies of anything in the repository that uses a to-be-published package and then re-run analysis on just those packages. This will allow us to catch in presubmit any changes that are not breaking from a semver standpoint, but will break us due to our strict analysis in CI.
Fixes https://github.com/flutter/flutter/issues/89862
diff --git a/.cirrus.yml b/.cirrus.yml
index 59f686d..453d01b 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -109,13 +109,25 @@
matrix:
CHANNEL: "master"
CHANNEL: "stable"
- tool_script:
+ analyze_tool_script:
- cd script/tool
- dart analyze --fatal-infos
- script:
+ analyze_script:
# DO NOT change the custom-analysis argument here without changing the Dart repo.
# See the comment in script/configs/custom_analysis.yaml for details.
- ./script/tool_runner.sh analyze --custom-analysis=script/configs/custom_analysis.yaml
+ pathified_analyze_script:
+ # Re-run analysis with path-based dependencies to ensure that publishing
+ # the changes won't break analysis of other packages in the respository
+ # that depend on it.
+ - ./script/tool_runner.sh make-deps-path-based --target-dependencies-with-non-breaking-updates
+ # This uses --run-on-dirty-packages rather than --packages-for-branch
+ # since only the packages changed by 'make-deps-path-based' need to be
+ # checked.
+ - dart $PLUGIN_TOOL analyze --run-on-dirty-packages --log-timing --custom-analysis=script/configs/custom_analysis.yaml
+ # Restore the tree to a clean state, to avoid accidental issues if
+ # other script steps are added to this task.
+ - git checkout .
### Android tasks ###
- name: android-build_all_plugins
env:
diff --git a/script/tool/CHANGELOG.md b/script/tool/CHANGELOG.md
index a2a2ed8..234700a 100644
--- a/script/tool/CHANGELOG.md
+++ b/script/tool/CHANGELOG.md
@@ -1,6 +1,10 @@
## NEXT
- Ensures that `firebase-test-lab` runs include an `integration_test` runner.
+- Adds a `make-deps-path-based` command to convert inter-repo package
+ dependencies to path-based dependencies.
+- Adds a (hidden) `--run-on-dirty-packages` flag for use with
+ `make-deps-path-based` in CI.
## 0.7.3
diff --git a/script/tool/lib/src/common/git_version_finder.dart b/script/tool/lib/src/common/git_version_finder.dart
index 3311873..32d30e6 100644
--- a/script/tool/lib/src/common/git_version_finder.dart
+++ b/script/tool/lib/src/common/git_version_finder.dart
@@ -31,10 +31,16 @@
}
/// Get a list of all the changed files.
- Future<List<String>> getChangedFiles() async {
+ Future<List<String>> getChangedFiles(
+ {bool includeUncommitted = false}) async {
final String baseSha = await getBaseSha();
final io.ProcessResult changedFilesCommand = await baseGitDir
- .runCommand(<String>['diff', '--name-only', baseSha, 'HEAD']);
+ .runCommand(<String>[
+ 'diff',
+ '--name-only',
+ baseSha,
+ if (!includeUncommitted) 'HEAD'
+ ]);
final String changedFilesStdout = changedFilesCommand.stdout.toString();
if (changedFilesStdout.isEmpty) {
return <String>[];
diff --git a/script/tool/lib/src/common/plugin_command.dart b/script/tool/lib/src/common/plugin_command.dart
index f40a102..7166c75 100644
--- a/script/tool/lib/src/common/plugin_command.dart
+++ b/script/tool/lib/src/common/plugin_command.dart
@@ -75,9 +75,15 @@
help: 'Run the command on changed packages/plugins.\n'
'If no packages have changed, or if there have been changes that may\n'
'affect all packages, the command runs on all packages.\n'
- 'The packages excluded with $_excludeArg is also excluded even if changed.\n'
+ 'Packages excluded with $_excludeArg are excluded even if changed.\n'
'See $_baseShaArg if a custom base is needed to determine the diff.\n\n'
'Cannot be combined with $_packagesArg.\n');
+ argParser.addFlag(_runOnDirtyPackagesArg,
+ help:
+ 'Run the command on packages with changes that have not been committed.\n'
+ 'Packages excluded with $_excludeArg are excluded even if changed.\n'
+ 'Cannot be combined with $_packagesArg.\n',
+ hide: true);
argParser.addFlag(_packagesForBranchArg,
help:
'This runs on all packages (equivalent to no package selection flag)\n'
@@ -103,6 +109,7 @@
static const String _packagesForBranchArg = 'packages-for-branch';
static const String _pluginsArg = 'plugins';
static const String _runOnChangedPackagesArg = 'run-on-changed-packages';
+ static const String _runOnDirtyPackagesArg = 'run-on-dirty-packages';
static const String _shardCountArg = 'shardCount';
static const String _shardIndexArg = 'shardIndex';
@@ -289,6 +296,7 @@
final Set<String> packageSelectionFlags = <String>{
_packagesArg,
_runOnChangedPackagesArg,
+ _runOnDirtyPackagesArg,
_packagesForBranchArg,
};
if (packageSelectionFlags
@@ -300,7 +308,7 @@
throw ToolExit(exitInvalidArguments);
}
- Set<String> plugins = Set<String>.from(getStringListArg(_packagesArg));
+ Set<String> packages = Set<String>.from(getStringListArg(_packagesArg));
final bool runOnChangedPackages;
if (getBoolArg(_runOnChangedPackagesArg)) {
@@ -331,7 +339,21 @@
final List<String> changedFiles =
await gitVersionFinder.getChangedFiles();
if (!_changesRequireFullTest(changedFiles)) {
- plugins = _getChangedPackages(changedFiles);
+ packages = _getChangedPackages(changedFiles);
+ }
+ } else if (getBoolArg(_runOnDirtyPackagesArg)) {
+ final GitVersionFinder gitVersionFinder =
+ GitVersionFinder(await gitDir, 'HEAD');
+ print('Running for all packages that have uncommitted changes\n');
+ // _changesRequireFullTest is deliberately not used here, as this flag is
+ // intended for use in CI to re-test packages changed by
+ // 'make-deps-path-based'.
+ packages = _getChangedPackages(
+ await gitVersionFinder.getChangedFiles(includeUncommitted: true));
+ // For the same reason, empty is not treated as "all packages" as it is
+ // for other flags.
+ if (packages.isEmpty) {
+ return;
}
}
@@ -347,7 +369,7 @@
in dir.list(followLinks: false)) {
// A top-level Dart package is a plugin package.
if (_isDartPackage(entity)) {
- if (plugins.isEmpty || plugins.contains(p.basename(entity.path))) {
+ if (packages.isEmpty || packages.contains(p.basename(entity.path))) {
yield PackageEnumerationEntry(
RepositoryPackage(entity as Directory),
excluded: excludedPluginNames.contains(entity.basename));
@@ -364,9 +386,9 @@
path.relative(subdir.path, from: dir.path);
final String packageName = path.basename(subdir.path);
final String basenamePath = path.basename(entity.path);
- if (plugins.isEmpty ||
- plugins.contains(relativePath) ||
- plugins.contains(basenamePath)) {
+ if (packages.isEmpty ||
+ packages.contains(relativePath) ||
+ packages.contains(basenamePath)) {
yield PackageEnumerationEntry(
RepositoryPackage(subdir as Directory),
excluded: excludedPluginNames.contains(basenamePath) ||
diff --git a/script/tool/lib/src/common/repository_package.dart b/script/tool/lib/src/common/repository_package.dart
index 3b4417a..e0c4e4a 100644
--- a/script/tool/lib/src/common/repository_package.dart
+++ b/script/tool/lib/src/common/repository_package.dart
@@ -4,6 +4,7 @@
import 'package:file/file.dart';
import 'package:path/path.dart' as p;
+import 'package:pubspec_parse/pubspec_parse.dart';
import 'core.dart';
@@ -47,6 +48,14 @@
/// The package's top-level pubspec.yaml.
File get pubspecFile => directory.childFile('pubspec.yaml');
+ late final Pubspec _parsedPubspec =
+ Pubspec.parse(pubspecFile.readAsStringSync());
+
+ /// Returns the parsed [pubspecFile].
+ ///
+ /// Caches for future use.
+ Pubspec parsePubspec() => _parsedPubspec;
+
/// True if this appears to be a federated plugin package, according to
/// repository conventions.
bool get isFederated =>
diff --git a/script/tool/lib/src/create_all_plugins_app_command.dart b/script/tool/lib/src/create_all_plugins_app_command.dart
index 5d9b4ed..82f29bd 100644
--- a/script/tool/lib/src/create_all_plugins_app_command.dart
+++ b/script/tool/lib/src/create_all_plugins_app_command.dart
@@ -178,8 +178,7 @@
final RepositoryPackage package = entry.package;
final Directory pluginDirectory = package.directory;
final String pluginName = pluginDirectory.basename;
- final File pubspecFile = package.pubspecFile;
- final Pubspec pubspec = Pubspec.parse(pubspecFile.readAsStringSync());
+ final Pubspec pubspec = package.parsePubspec();
if (pubspec.publishTo != 'none') {
pathDependencies[pluginName] = PathDependency(pluginDirectory.path);
diff --git a/script/tool/lib/src/main.dart b/script/tool/lib/src/main.dart
index 70a6ab5..3e8f19b 100644
--- a/script/tool/lib/src/main.dart
+++ b/script/tool/lib/src/main.dart
@@ -20,6 +20,7 @@
import 'lint_android_command.dart';
import 'lint_podspecs_command.dart';
import 'list_command.dart';
+import 'make_deps_path_based_command.dart';
import 'native_test_command.dart';
import 'publish_check_command.dart';
import 'publish_plugin_command.dart';
@@ -58,6 +59,7 @@
..addCommand(LintPodspecsCommand(packagesDir))
..addCommand(ListCommand(packagesDir))
..addCommand(NativeTestCommand(packagesDir))
+ ..addCommand(MakeDepsPathBasedCommand(packagesDir))
..addCommand(PublishCheckCommand(packagesDir))
..addCommand(PublishPluginCommand(packagesDir))
..addCommand(PubspecCheckCommand(packagesDir))
diff --git a/script/tool/lib/src/make_deps_path_based_command.dart b/script/tool/lib/src/make_deps_path_based_command.dart
new file mode 100644
index 0000000..0486963
--- /dev/null
+++ b/script/tool/lib/src/make_deps_path_based_command.dart
@@ -0,0 +1,249 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// 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:flutter_plugin_tools/src/common/repository_package.dart';
+import 'package:git/git.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';
+
+const int _exitPackageNotFound = 3;
+const int _exitCannotUpdatePubspec = 4;
+
+/// Converts all dependencies on target packages to path-based dependencies.
+///
+/// This is to allow for pre-publish testing of changes that could affect other
+/// packages in the repository. For instance, this allows for catching cases
+/// where a non-breaking change to a platform interface package of a federated
+/// plugin would cause post-publish analyzer failures in another package of that
+/// plugin.
+class MakeDepsPathBasedCommand extends PluginCommand {
+ /// Creates an instance of the command to convert selected dependencies to
+ /// path-based.
+ MakeDepsPathBasedCommand(
+ Directory packagesDir, {
+ GitDir? gitDir,
+ }) : super(packagesDir, gitDir: gitDir) {
+ argParser.addMultiOption(_targetDependenciesArg,
+ help:
+ 'The names of the packages to convert to path-based dependencies.\n'
+ 'Ignored if --$_targetDependenciesWithNonBreakingUpdatesArg is '
+ 'passed.',
+ valueHelp: 'some_package');
+ argParser.addFlag(
+ _targetDependenciesWithNonBreakingUpdatesArg,
+ help: 'Causes all packages that have non-breaking version changes '
+ 'when compared against the git base to be treated as target '
+ 'packages.',
+ );
+ }
+
+ static const String _targetDependenciesArg = 'target-dependencies';
+ static const String _targetDependenciesWithNonBreakingUpdatesArg =
+ 'target-dependencies-with-non-breaking-updates';
+
+ @override
+ final String name = 'make-deps-path-based';
+
+ @override
+ final String description =
+ 'Converts package dependencies to path-based references.';
+
+ @override
+ Future<void> run() async {
+ final Set<String> targetDependencies =
+ getBoolArg(_targetDependenciesWithNonBreakingUpdatesArg)
+ ? await _getNonBreakingUpdatePackages()
+ : getStringListArg(_targetDependenciesArg).toSet();
+
+ if (targetDependencies.isEmpty) {
+ print('No target dependencies; nothing to do.');
+ return;
+ }
+ print('Rewriting references to: ${targetDependencies.join(', ')}...');
+
+ final Map<String, RepositoryPackage> localDependencyPackages =
+ _findLocalPackages(targetDependencies);
+
+ final String repoRootPath = (await gitDir).path;
+ for (final File pubspec in await _getAllPubspecs()) {
+ if (await _addDependencyOverridesIfNecessary(
+ pubspec, localDependencyPackages)) {
+ // Print the relative path of the changed pubspec.
+ final String displayPath = p.posix.joinAll(path
+ .split(path.relative(pubspec.absolute.path, from: repoRootPath)));
+ print(' Modified $displayPath');
+ }
+ }
+ }
+
+ Map<String, RepositoryPackage> _findLocalPackages(Set<String> packageNames) {
+ final Map<String, RepositoryPackage> targets =
+ <String, RepositoryPackage>{};
+ for (final String packageName in packageNames) {
+ final Directory topLevelCandidate =
+ packagesDir.childDirectory(packageName);
+ // If packages/<packageName>/ exists, then either that directory is the
+ // package, or packages/<packageName>/<packageName>/ exists and is the
+ // package (in the case of a federated plugin).
+ if (topLevelCandidate.existsSync()) {
+ final Directory appFacingCandidate =
+ topLevelCandidate.childDirectory(packageName);
+ targets[packageName] = RepositoryPackage(appFacingCandidate.existsSync()
+ ? appFacingCandidate
+ : topLevelCandidate);
+ continue;
+ }
+ // If there is no packages/<packageName> directory, then either the
+ // packages doesn't exist, or it is a sub-package of a federated plugin.
+ // If it's the latter, it will be a directory whose name is a prefix.
+ for (final FileSystemEntity entity in packagesDir.listSync()) {
+ if (entity is Directory && packageName.startsWith(entity.basename)) {
+ final Directory subPackageCandidate =
+ entity.childDirectory(packageName);
+ if (subPackageCandidate.existsSync()) {
+ targets[packageName] = RepositoryPackage(subPackageCandidate);
+ break;
+ }
+ }
+ }
+
+ if (!targets.containsKey(packageName)) {
+ printError('Unable to find package "$packageName"');
+ throw ToolExit(_exitPackageNotFound);
+ }
+ }
+ return targets;
+ }
+
+ /// If [pubspecFile] has any dependencies on packages in [localDependencies],
+ /// adds dependency_overrides entries to redirect them to the local version
+ /// using path-based dependencies.
+ ///
+ /// Returns true if any changes were made.
+ Future<bool> _addDependencyOverridesIfNecessary(File pubspecFile,
+ Map<String, RepositoryPackage> localDependencies) async {
+ final String pubspecContents = pubspecFile.readAsStringSync();
+ final Pubspec pubspec = Pubspec.parse(pubspecContents);
+ // Fail if there are any dependency overrides already. If support for that
+ // is needed at some point, it can be added, but currently it's not and
+ // relying on that makes the logic here much simpler.
+ if (pubspec.dependencyOverrides.isNotEmpty) {
+ printError(
+ 'Plugins with dependency overrides are not currently supported.');
+ throw ToolExit(_exitCannotUpdatePubspec);
+ }
+
+ final Iterable<String> packagesToOverride = pubspec.dependencies.keys.where(
+ (String packageName) => localDependencies.containsKey(packageName));
+ if (packagesToOverride.isNotEmpty) {
+ final String commonBasePath = packagesDir.path;
+ // Find the relative path to the common base.
+ final int packageDepth = path
+ .split(path.relative(pubspecFile.parent.absolute.path,
+ from: commonBasePath))
+ .length;
+ final List<String> relativeBasePathComponents =
+ List<String>.filled(packageDepth, '..');
+ // This is done via strings rather than by manipulating the Pubspec and
+ // then re-serialiazing so that it's a localized change, rather than
+ // rewriting the whole file (e.g., destroying comments), which could be
+ // more disruptive for local use.
+ String newPubspecContents = pubspecContents +
+ '''
+
+# FOR TESTING ONLY. DO NOT MERGE.
+dependency_overrides:
+''';
+ for (final String packageName in packagesToOverride) {
+ // Find the relative path from the common base to the local package.
+ final List<String> repoRelativePathComponents = path.split(
+ path.relative(localDependencies[packageName]!.directory.path,
+ from: commonBasePath));
+ newPubspecContents += '''
+ $packageName:
+ path: ${p.posix.joinAll(<String>[
+ ...relativeBasePathComponents,
+ ...repoRelativePathComponents,
+ ])}
+''';
+ }
+ pubspecFile.writeAsStringSync(newPubspecContents);
+ return true;
+ }
+ return false;
+ }
+
+ /// Returns all pubspecs anywhere under the packages directory.
+ Future<List<File>> _getAllPubspecs() => packagesDir.parent
+ .list(recursive: true, followLinks: false)
+ .where((FileSystemEntity entity) =>
+ entity is File && p.basename(entity.path) == 'pubspec.yaml')
+ .map((FileSystemEntity file) => file as File)
+ .toList();
+
+ /// Returns all packages that have non-breaking published changes (i.e., a
+ /// minor or bugfix version change) relative to the git comparison base.
+ ///
+ /// Prints status information about what was checked for ease of auditing logs
+ /// in CI.
+ Future<Set<String>> _getNonBreakingUpdatePackages() async {
+ final GitVersionFinder gitVersionFinder = await retrieveVersionFinder();
+ final String baseSha = await gitVersionFinder.getBaseSha();
+ print('Finding changed packages relative to "$baseSha"\n');
+
+ final Set<String> changedPackages = <String>{};
+ for (final String path in await gitVersionFinder.getChangedFiles()) {
+ // Git output always uses Posix paths.
+ final List<String> allComponents = p.posix.split(path);
+ // Only pubspec changes are potential publishing events.
+ if (allComponents.last != 'pubspec.yaml' ||
+ allComponents.contains('example')) {
+ continue;
+ }
+ final RepositoryPackage package =
+ RepositoryPackage(packagesDir.fileSystem.file(path).parent);
+ final String packageName = package.parsePubspec().name;
+ if (!await _hasNonBreakingVersionChange(package)) {
+ // Log packages that had pubspec changes but weren't included for ease
+ // of auditing CI.
+ print(' Skipping $packageName; no non-breaking version change.');
+ continue;
+ }
+ changedPackages.add(packageName);
+ }
+ return changedPackages;
+ }
+
+ Future<bool> _hasNonBreakingVersionChange(RepositoryPackage package) async {
+ final Pubspec pubspec = package.parsePubspec();
+ if (pubspec.publishTo == 'none') {
+ return false;
+ }
+
+ final String pubspecGitPath = p.posix.joinAll(path.split(path.relative(
+ package.pubspecFile.absolute.path,
+ from: (await gitDir).path)));
+ final GitVersionFinder gitVersionFinder = await retrieveVersionFinder();
+ final Version? previousVersion =
+ await gitVersionFinder.getPackageVersion(pubspecGitPath);
+ if (previousVersion == null) {
+ // The plugin is new, so nothing can be depending on it yet.
+ return false;
+ }
+ final Version newVersion = pubspec.version!;
+ if ((newVersion.major > 0 && newVersion.major != previousVersion.major) ||
+ (newVersion.major == 0 && newVersion.minor != previousVersion.minor)) {
+ // Breaking changes aren't targetted since they won't be picked up
+ // automatically.
+ return false;
+ }
+ return newVersion != previousVersion;
+ }
+}
diff --git a/script/tool/lib/src/publish_check_command.dart b/script/tool/lib/src/publish_check_command.dart
index 563e090..8fd96b8 100644
--- a/script/tool/lib/src/publish_check_command.dart
+++ b/script/tool/lib/src/publish_check_command.dart
@@ -123,13 +123,12 @@
}
Pubspec? _tryParsePubspec(RepositoryPackage package) {
- final File pubspecFile = package.pubspecFile;
-
try {
- return Pubspec.parse(pubspecFile.readAsStringSync());
+ return package.parsePubspec();
} on Exception catch (exception) {
print(
- 'Failed to parse `pubspec.yaml` at ${pubspecFile.path}: $exception}',
+ 'Failed to parse `pubspec.yaml` at ${package.pubspecFile.path}: '
+ '$exception',
);
return null;
}
diff --git a/script/tool/lib/src/publish_plugin_command.dart b/script/tool/lib/src/publish_plugin_command.dart
index 4fdecf6..28d17a3 100644
--- a/script/tool/lib/src/publish_plugin_command.dart
+++ b/script/tool/lib/src/publish_plugin_command.dart
@@ -217,16 +217,15 @@
/// In cases where a non-null result is returned, that should be returned
/// as the final result for the package, without further processing.
Future<PackageResult?> _checkNeedsRelease(RepositoryPackage package) async {
- final File pubspecFile = package.pubspecFile;
- if (!pubspecFile.existsSync()) {
+ if (!package.pubspecFile.existsSync()) {
logWarning('''
-The pubspec file at ${pubspecFile.path} does not exist. Publishing will not happen for ${pubspecFile.parent.basename}.
+The pubspec file for ${package.displayName} does not exist, so no publishing will happen.
Safe to ignore if the package is deleted in this commit.
''');
return PackageResult.skip('package deleted');
}
- final Pubspec pubspec = Pubspec.parse(pubspecFile.readAsStringSync());
+ final Pubspec pubspec = package.parsePubspec();
if (pubspec.name == 'flutter_plugin_tools') {
// Ignore flutter_plugin_tools package when running publishing through flutter_plugin_tools.
diff --git a/script/tool/lib/src/version_check_command.dart b/script/tool/lib/src/version_check_command.dart
index 1ec5dc4..fcaea33 100644
--- a/script/tool/lib/src/version_check_command.dart
+++ b/script/tool/lib/src/version_check_command.dart
@@ -463,10 +463,8 @@
}
Pubspec? _tryParsePubspec(RepositoryPackage package) {
- final File pubspecFile = package.pubspecFile;
-
try {
- final Pubspec pubspec = Pubspec.parse(pubspecFile.readAsStringSync());
+ final Pubspec pubspec = package.parsePubspec();
return pubspec;
} on Exception catch (exception) {
printError('${indentation}Failed to parse `pubspec.yaml`: $exception}');
diff --git a/script/tool/test/common/git_version_finder_test.dart b/script/tool/test/common/git_version_finder_test.dart
index fa8b1c4..ad1a26f 100644
--- a/script/tool/test/common/git_version_finder_test.dart
+++ b/script/tool/test/common/git_version_finder_test.dart
@@ -88,6 +88,18 @@
verify(gitDir
.runCommand(<String>['diff', '--name-only', customBaseSha, 'HEAD']));
});
+
+ test('include uncommitted files if requested', () async {
+ const String customBaseSha = 'aklsjdcaskf12312';
+ gitDiffResponse = '''
+file1/pubspec.yaml
+file2/file2.cc
+''';
+ final GitVersionFinder finder = GitVersionFinder(gitDir, customBaseSha);
+ await finder.getChangedFiles(includeUncommitted: true);
+ // The call should not have HEAD as a final argument like the default diff.
+ verify(gitDir.runCommand(<String>['diff', '--name-only', customBaseSha]));
+ });
}
class MockProcessResult extends Mock implements ProcessResult {}
diff --git a/script/tool/test/common/plugin_command_test.dart b/script/tool/test/common/plugin_command_test.dart
index 6d586e4..222df54 100644
--- a/script/tool/test/common/plugin_command_test.dart
+++ b/script/tool/test/common/plugin_command_test.dart
@@ -486,6 +486,104 @@
expect(command.plugins, unorderedEquals(<String>[plugin1.path]));
});
});
+
+ group('test run-on-dirty-packages', () {
+ test('no packages should be tested if there are no changes.', () async {
+ createFakePackage('a_package', packagesDir);
+ await runCapturingPrint(
+ runner, <String>['sample', '--run-on-dirty-packages']);
+
+ expect(command.plugins, unorderedEquals(<String>[]));
+ });
+
+ test(
+ 'no packages should be tested if there are no plugin related changes.',
+ () async {
+ processRunner.mockProcessesForExecutable['git-diff'] = <Process>[
+ MockProcess(stdout: 'AUTHORS'),
+ ];
+ createFakePackage('a_package', packagesDir);
+ await runCapturingPrint(
+ runner, <String>['sample', '--run-on-dirty-packages']);
+
+ expect(command.plugins, unorderedEquals(<String>[]));
+ });
+
+ test('no packages should be tested even if special repo files change.',
+ () async {
+ processRunner.mockProcessesForExecutable['git-diff'] = <Process>[
+ MockProcess(stdout: '''
+.cirrus.yml
+.ci.yaml
+.ci/Dockerfile
+.clang-format
+analysis_options.yaml
+script/tool_runner.sh
+'''),
+ ];
+ createFakePackage('a_package', packagesDir);
+ await runCapturingPrint(
+ runner, <String>['sample', '--run-on-dirty-packages']);
+
+ expect(command.plugins, unorderedEquals(<String>[]));
+ });
+
+ test('Only changed packages should be tested.', () async {
+ processRunner.mockProcessesForExecutable['git-diff'] = <Process>[
+ MockProcess(stdout: 'packages/a_package/lib/a_package.dart'),
+ ];
+ final Directory packageA = createFakePackage('a_package', packagesDir);
+ createFakePlugin('b_package', packagesDir);
+ final List<String> output = await runCapturingPrint(
+ runner, <String>['sample', '--run-on-dirty-packages']);
+
+ expect(
+ output,
+ containsAllInOrder(<Matcher>[
+ contains(
+ 'Running for all packages that have uncommitted changes'),
+ ]));
+
+ expect(command.plugins, unorderedEquals(<String>[packageA.path]));
+ });
+
+ test('multiple packages changed should test all the changed packages',
+ () async {
+ processRunner.mockProcessesForExecutable['git-diff'] = <Process>[
+ MockProcess(stdout: '''
+packages/a_package/lib/a_package.dart
+packages/b_package/lib/src/foo.dart
+'''),
+ ];
+ final Directory packageA = createFakePackage('a_package', packagesDir);
+ final Directory packageB = createFakePackage('b_package', packagesDir);
+ createFakePackage('c_package', packagesDir);
+ await runCapturingPrint(
+ runner, <String>['sample', '--run-on-dirty-packages']);
+
+ expect(command.plugins,
+ unorderedEquals(<String>[packageA.path, packageB.path]));
+ });
+
+ test('honors --exclude flag', () async {
+ processRunner.mockProcessesForExecutable['git-diff'] = <Process>[
+ MockProcess(stdout: '''
+packages/a_package/lib/a_package.dart
+packages/b_package/lib/src/foo.dart
+'''),
+ ];
+ final Directory packageA = createFakePackage('a_package', packagesDir);
+ createFakePackage('b_package', packagesDir);
+ createFakePackage('c_package', packagesDir);
+ await runCapturingPrint(runner, <String>[
+ 'sample',
+ '--exclude=b_package',
+ '--run-on-dirty-packages'
+ ]);
+
+ expect(command.plugins, unorderedEquals(<String>[packageA.path]));
+ });
+ });
});
group('--packages-for-branch', () {
diff --git a/script/tool/test/common/repository_package_test.dart b/script/tool/test/common/repository_package_test.dart
index 4c20389..29e3b58 100644
--- a/script/tool/test/common/repository_package_test.dart
+++ b/script/tool/test/common/repository_package_test.dart
@@ -5,6 +5,7 @@
import 'package:file/file.dart';
import 'package:file/memory.dart';
import 'package:flutter_plugin_tools/src/common/repository_package.dart';
+import 'package:pubspec_parse/pubspec_parse.dart';
import 'package:test/test.dart';
import '../util.dart';
@@ -155,4 +156,23 @@
expect(RepositoryPackage(plugin).isPlatformImplementation, true);
});
});
+
+ group('pubspec', () {
+ test('file', () async {
+ final Directory plugin = createFakePlugin('a_plugin', packagesDir);
+
+ final File pubspecFile = RepositoryPackage(plugin).pubspecFile;
+
+ expect(pubspecFile.path, plugin.childFile('pubspec.yaml').path);
+ });
+
+ test('parsing', () async {
+ final Directory plugin = createFakePlugin('a_plugin', packagesDir,
+ examples: <String>['example1', 'example2']);
+
+ final Pubspec pubspec = RepositoryPackage(plugin).parsePubspec();
+
+ expect(pubspec.name, 'a_plugin');
+ });
+ });
}
diff --git a/script/tool/test/make_deps_path_based_command_test.dart b/script/tool/test/make_deps_path_based_command_test.dart
new file mode 100644
index 0000000..29e4f72
--- /dev/null
+++ b/script/tool/test/make_deps_path_based_command_test.dart
@@ -0,0 +1,304 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'dart:io' as io;
+
+import 'package:args/command_runner.dart';
+import 'package:file/file.dart';
+import 'package:file/memory.dart';
+import 'package:flutter_plugin_tools/src/common/repository_package.dart';
+import 'package:flutter_plugin_tools/src/make_deps_path_based_command.dart';
+import 'package:mockito/mockito.dart';
+import 'package:test/test.dart';
+
+import 'common/plugin_command_test.mocks.dart';
+import 'mocks.dart';
+import 'util.dart';
+
+void main() {
+ FileSystem fileSystem;
+ late Directory packagesDir;
+ late CommandRunner<void> runner;
+ late RecordingProcessRunner processRunner;
+
+ setUp(() {
+ fileSystem = MemoryFileSystem();
+ packagesDir = createPackagesDirectory(fileSystem: fileSystem);
+
+ final MockGitDir gitDir = MockGitDir();
+ when(gitDir.path).thenReturn(packagesDir.parent.path);
+ when(gitDir.runCommand(any, throwOnError: anyNamed('throwOnError')))
+ .thenAnswer((Invocation invocation) {
+ 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);
+ });
+
+ processRunner = RecordingProcessRunner();
+ final MakeDepsPathBasedCommand command =
+ MakeDepsPathBasedCommand(packagesDir, gitDir: gitDir);
+
+ runner = CommandRunner<void>(
+ 'make-deps-path-based_command', 'Test for $MakeDepsPathBasedCommand');
+ runner.addCommand(command);
+ });
+
+ /// Adds dummy 'dependencies:' entries for each package in [dependencies]
+ /// to [package].
+ void _addDependencies(
+ RepositoryPackage package, Iterable<String> dependencies) {
+ final List<String> lines = package.pubspecFile.readAsLinesSync();
+ final int dependenciesStartIndex = lines.indexOf('dependencies:');
+ assert(dependenciesStartIndex != -1);
+ lines.insertAll(dependenciesStartIndex + 1, <String>[
+ for (final String dependency in dependencies) ' $dependency: ^1.0.0',
+ ]);
+ package.pubspecFile.writeAsStringSync(lines.join('\n'));
+ }
+
+ test('no-ops for no plugins', () async {
+ RepositoryPackage(createFakePackage('foo', packagesDir, isFlutter: true));
+ final RepositoryPackage packageBar = RepositoryPackage(
+ createFakePackage('bar', packagesDir, isFlutter: true));
+ _addDependencies(packageBar, <String>['foo']);
+ final String originalPubspecContents =
+ packageBar.pubspecFile.readAsStringSync();
+
+ final List<String> output =
+ await runCapturingPrint(runner, <String>['make-deps-path-based']);
+
+ expect(
+ output,
+ containsAllInOrder(<Matcher>[
+ contains('No target dependencies'),
+ ]),
+ );
+ // The 'foo' reference should not have been modified.
+ expect(packageBar.pubspecFile.readAsStringSync(), originalPubspecContents);
+ });
+
+ test('rewrites references', () async {
+ final RepositoryPackage simplePackage = RepositoryPackage(
+ createFakePackage('foo', packagesDir, isFlutter: true));
+ final Directory pluginGroup = packagesDir.childDirectory('bar');
+
+ RepositoryPackage(createFakePackage('bar_platform_interface', pluginGroup,
+ isFlutter: true));
+ final RepositoryPackage pluginImplementation =
+ RepositoryPackage(createFakePlugin('bar_android', pluginGroup));
+ final RepositoryPackage pluginAppFacing =
+ RepositoryPackage(createFakePlugin('bar', pluginGroup));
+
+ _addDependencies(simplePackage, <String>[
+ 'bar',
+ 'bar_android',
+ 'bar_platform_interface',
+ ]);
+ _addDependencies(pluginAppFacing, <String>[
+ 'bar_platform_interface',
+ 'bar_android',
+ ]);
+ _addDependencies(pluginImplementation, <String>[
+ 'bar_platform_interface',
+ ]);
+
+ final List<String> output = await runCapturingPrint(runner, <String>[
+ 'make-deps-path-based',
+ '--target-dependencies=bar,bar_platform_interface'
+ ]);
+
+ expect(
+ output,
+ containsAll(<String>[
+ 'Rewriting references to: bar, bar_platform_interface...',
+ ' Modified packages/bar/bar/pubspec.yaml',
+ ' Modified packages/bar/bar_android/pubspec.yaml',
+ ' Modified packages/foo/pubspec.yaml',
+ ]));
+ expect(
+ output,
+ isNot(contains(
+ ' Modified packages/bar/bar_platform_interface/pubspec.yaml')));
+
+ expect(
+ simplePackage.pubspecFile.readAsLinesSync(),
+ containsAllInOrder(<String>[
+ '# FOR TESTING ONLY. DO NOT MERGE.',
+ 'dependency_overrides:',
+ ' bar:',
+ ' path: ../bar/bar',
+ ' bar_platform_interface:',
+ ' path: ../bar/bar_platform_interface',
+ ]));
+ expect(
+ pluginAppFacing.pubspecFile.readAsLinesSync(),
+ containsAllInOrder(<String>[
+ 'dependency_overrides:',
+ ' bar_platform_interface:',
+ ' path: ../../bar/bar_platform_interface',
+ ]));
+ });
+
+ group('target-dependencies-with-non-breaking-updates', () {
+ test('no-ops for no published changes', () async {
+ final Directory package = createFakePackage('foo', packagesDir);
+
+ final String changedFileOutput = <File>[
+ package.childFile('pubspec.yaml'),
+ ].map((File file) => file.path).join('\n');
+ processRunner.mockProcessesForExecutable['git-diff'] = <io.Process>[
+ MockProcess(stdout: changedFileOutput),
+ ];
+ // Simulate no change to the version in the interface's pubspec.yaml.
+ processRunner.mockProcessesForExecutable['git-show'] = <io.Process>[
+ MockProcess(
+ stdout: RepositoryPackage(package).pubspecFile.readAsStringSync()),
+ ];
+
+ final List<String> output = await runCapturingPrint(runner, <String>[
+ 'make-deps-path-based',
+ '--target-dependencies-with-non-breaking-updates'
+ ]);
+
+ expect(
+ output,
+ containsAllInOrder(<Matcher>[
+ contains('No target dependencies'),
+ ]),
+ );
+ });
+
+ test('includes bugfix version changes as targets', () async {
+ const String newVersion = '1.0.1';
+ final Directory package =
+ createFakePackage('foo', packagesDir, version: newVersion);
+
+ final File pubspecFile = RepositoryPackage(package).pubspecFile;
+ final String changedFileOutput = <File>[
+ pubspecFile,
+ ].map((File file) => file.path).join('\n');
+ processRunner.mockProcessesForExecutable['git-diff'] = <io.Process>[
+ MockProcess(stdout: changedFileOutput),
+ ];
+ final String gitPubspecContents =
+ pubspecFile.readAsStringSync().replaceAll(newVersion, '1.0.0');
+ // Simulate no change to the version in the interface's pubspec.yaml.
+ processRunner.mockProcessesForExecutable['git-show'] = <io.Process>[
+ MockProcess(stdout: gitPubspecContents),
+ ];
+
+ final List<String> output = await runCapturingPrint(runner, <String>[
+ 'make-deps-path-based',
+ '--target-dependencies-with-non-breaking-updates'
+ ]);
+
+ expect(
+ output,
+ containsAllInOrder(<Matcher>[
+ contains('Rewriting references to: foo...'),
+ ]),
+ );
+ });
+
+ test('includes minor version changes to 1.0+ as targets', () async {
+ const String newVersion = '1.1.0';
+ final Directory package =
+ createFakePackage('foo', packagesDir, version: newVersion);
+
+ final File pubspecFile = RepositoryPackage(package).pubspecFile;
+ final String changedFileOutput = <File>[
+ pubspecFile,
+ ].map((File file) => file.path).join('\n');
+ processRunner.mockProcessesForExecutable['git-diff'] = <io.Process>[
+ MockProcess(stdout: changedFileOutput),
+ ];
+ final String gitPubspecContents =
+ pubspecFile.readAsStringSync().replaceAll(newVersion, '1.0.0');
+ // Simulate no change to the version in the interface's pubspec.yaml.
+ processRunner.mockProcessesForExecutable['git-show'] = <io.Process>[
+ MockProcess(stdout: gitPubspecContents),
+ ];
+
+ final List<String> output = await runCapturingPrint(runner, <String>[
+ 'make-deps-path-based',
+ '--target-dependencies-with-non-breaking-updates'
+ ]);
+
+ expect(
+ output,
+ containsAllInOrder(<Matcher>[
+ contains('Rewriting references to: foo...'),
+ ]),
+ );
+ });
+
+ test('does not include major version changes as targets', () async {
+ const String newVersion = '2.0.0';
+ final Directory package =
+ createFakePackage('foo', packagesDir, version: newVersion);
+
+ final File pubspecFile = RepositoryPackage(package).pubspecFile;
+ final String changedFileOutput = <File>[
+ pubspecFile,
+ ].map((File file) => file.path).join('\n');
+ processRunner.mockProcessesForExecutable['git-diff'] = <io.Process>[
+ MockProcess(stdout: changedFileOutput),
+ ];
+ final String gitPubspecContents =
+ pubspecFile.readAsStringSync().replaceAll(newVersion, '1.0.0');
+ // Simulate no change to the version in the interface's pubspec.yaml.
+ processRunner.mockProcessesForExecutable['git-show'] = <io.Process>[
+ MockProcess(stdout: gitPubspecContents),
+ ];
+
+ final List<String> output = await runCapturingPrint(runner, <String>[
+ 'make-deps-path-based',
+ '--target-dependencies-with-non-breaking-updates'
+ ]);
+
+ expect(
+ output,
+ containsAllInOrder(<Matcher>[
+ contains('No target dependencies'),
+ ]),
+ );
+ });
+
+ test('does not include minor version changes to 0.x as targets', () async {
+ const String newVersion = '0.8.0';
+ final Directory package =
+ createFakePackage('foo', packagesDir, version: newVersion);
+
+ final File pubspecFile = RepositoryPackage(package).pubspecFile;
+ final String changedFileOutput = <File>[
+ pubspecFile,
+ ].map((File file) => file.path).join('\n');
+ processRunner.mockProcessesForExecutable['git-diff'] = <io.Process>[
+ MockProcess(stdout: changedFileOutput),
+ ];
+ final String gitPubspecContents =
+ pubspecFile.readAsStringSync().replaceAll(newVersion, '0.7.0');
+ // Simulate no change to the version in the interface's pubspec.yaml.
+ processRunner.mockProcessesForExecutable['git-show'] = <io.Process>[
+ MockProcess(stdout: gitPubspecContents),
+ ];
+
+ final List<String> output = await runCapturingPrint(runner, <String>[
+ 'make-deps-path-based',
+ '--target-dependencies-with-non-breaking-updates'
+ ]);
+
+ expect(
+ output,
+ containsAllInOrder(<Matcher>[
+ contains('No target dependencies'),
+ ]),
+ );
+ });
+ });
+}
diff --git a/script/tool/test/publish_plugin_command_test.dart b/script/tool/test/publish_plugin_command_test.dart
index 14e99a1..2cb3fc2 100644
--- a/script/tool/test/publish_plugin_command_test.dart
+++ b/script/tool/test/publish_plugin_command_test.dart
@@ -675,7 +675,7 @@
contains('Running `pub publish ` in ${pluginDir1.path}...'),
contains('Published plugin1 successfully!'),
contains(
- 'The pubspec file at ${pluginDir2.childFile('pubspec.yaml').path} does not exist. Publishing will not happen for plugin2.\nSafe to ignore if the package is deleted in this commit.\n'),
+ 'The pubspec file for plugin2/plugin2 does not exist, so no publishing will happen.\nSafe to ignore if the package is deleted in this commit.\n'),
contains('SKIPPING: package deleted'),
contains('skipped (with warning)'),
]));
