diff --git a/.cirrus.yml b/.cirrus.yml index 59f686dbf5d6..453d01b89f07 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -109,13 +109,25 @@ task: 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 a2a2ed824295..234700ab5a7d 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 331187335f51..32d30e60feb5 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 @@ class GitVersionFinder { } /// Get a list of all the changed files. - Future> getChangedFiles() async { + Future> getChangedFiles( + {bool includeUncommitted = false}) async { final String baseSha = await getBaseSha(); final io.ProcessResult changedFilesCommand = await baseGitDir - .runCommand(['diff', '--name-only', baseSha, 'HEAD']); + .runCommand([ + 'diff', + '--name-only', + baseSha, + if (!includeUncommitted) 'HEAD' + ]); final String changedFilesStdout = changedFilesCommand.stdout.toString(); if (changedFilesStdout.isEmpty) { return []; diff --git a/script/tool/lib/src/common/plugin_command.dart b/script/tool/lib/src/common/plugin_command.dart index f40a102dfbc0..7166c754e129 100644 --- a/script/tool/lib/src/common/plugin_command.dart +++ b/script/tool/lib/src/common/plugin_command.dart @@ -75,9 +75,15 @@ abstract class PluginCommand extends Command { 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 @@ abstract class PluginCommand extends Command { 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 @@ abstract class PluginCommand extends Command { final Set packageSelectionFlags = { _packagesArg, _runOnChangedPackagesArg, + _runOnDirtyPackagesArg, _packagesForBranchArg, }; if (packageSelectionFlags @@ -300,7 +308,7 @@ abstract class PluginCommand extends Command { throw ToolExit(exitInvalidArguments); } - Set plugins = Set.from(getStringListArg(_packagesArg)); + Set packages = Set.from(getStringListArg(_packagesArg)); final bool runOnChangedPackages; if (getBoolArg(_runOnChangedPackagesArg)) { @@ -331,7 +339,21 @@ abstract class PluginCommand extends Command { final List 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 @@ abstract class PluginCommand extends Command { 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 @@ abstract class PluginCommand extends Command { 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 3b4417ac8182..e0c4e4a83bfe 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 @@ class RepositoryPackage { /// 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 5d9b4ed9c728..82f29bd501f3 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 @@ class CreateAllPluginsAppCommand extends PluginCommand { 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 70a6ab516037..3e8f19b044dd 100644 --- a/script/tool/lib/src/main.dart +++ b/script/tool/lib/src/main.dart @@ -20,6 +20,7 @@ import 'license_check_command.dart'; 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 @@ void main(List args) { ..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 000000000000..04869639cf74 --- /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 run() async { + final Set 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 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 _findLocalPackages(Set packageNames) { + final Map targets = + {}; + for (final String packageName in packageNames) { + final Directory topLevelCandidate = + packagesDir.childDirectory(packageName); + // If packages// exists, then either that directory is the + // package, or packages/// 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/ 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 _addDependencyOverridesIfNecessary(File pubspecFile, + Map 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 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 relativeBasePathComponents = + List.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 repoRelativePathComponents = path.split( + path.relative(localDependencies[packageName]!.directory.path, + from: commonBasePath)); + newPubspecContents += ''' + $packageName: + path: ${p.posix.joinAll([ + ...relativeBasePathComponents, + ...repoRelativePathComponents, + ])} +'''; + } + pubspecFile.writeAsStringSync(newPubspecContents); + return true; + } + return false; + } + + /// Returns all pubspecs anywhere under the packages directory. + Future> _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> _getNonBreakingUpdatePackages() async { + final GitVersionFinder gitVersionFinder = await retrieveVersionFinder(); + final String baseSha = await gitVersionFinder.getBaseSha(); + print('Finding changed packages relative to "$baseSha"\n'); + + final Set changedPackages = {}; + for (final String path in await gitVersionFinder.getChangedFiles()) { + // Git output always uses Posix paths. + final List 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 _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 563e0904552a..8fd96b818c1d 100644 --- a/script/tool/lib/src/publish_check_command.dart +++ b/script/tool/lib/src/publish_check_command.dart @@ -123,13 +123,12 @@ class PublishCheckCommand extends PackageLoopingCommand { } 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 4fdecf603eec..28d17a3a2487 100644 --- a/script/tool/lib/src/publish_plugin_command.dart +++ b/script/tool/lib/src/publish_plugin_command.dart @@ -217,16 +217,15 @@ class PublishPluginCommand extends PackageLoopingCommand { /// In cases where a non-null result is returned, that should be returned /// as the final result for the package, without further processing. Future _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 1ec5dc4f2490..fcaea335920f 100644 --- a/script/tool/lib/src/version_check_command.dart +++ b/script/tool/lib/src/version_check_command.dart @@ -463,10 +463,8 @@ ${indentation}The first version listed in CHANGELOG.md is $fromChangeLog. } 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 fa8b1c410dd5..ad1a26ffc165 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 @@ file2/file2.cc verify(gitDir .runCommand(['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(['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 6d586e416b7d..222df544f344 100644 --- a/script/tool/test/common/plugin_command_test.dart +++ b/script/tool/test/common/plugin_command_test.dart @@ -486,6 +486,104 @@ packages/plugin3/plugin3.dart expect(command.plugins, unorderedEquals([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, ['sample', '--run-on-dirty-packages']); + + expect(command.plugins, unorderedEquals([])); + }); + + test( + 'no packages should be tested if there are no plugin related changes.', + () async { + processRunner.mockProcessesForExecutable['git-diff'] = [ + MockProcess(stdout: 'AUTHORS'), + ]; + createFakePackage('a_package', packagesDir); + await runCapturingPrint( + runner, ['sample', '--run-on-dirty-packages']); + + expect(command.plugins, unorderedEquals([])); + }); + + test('no packages should be tested even if special repo files change.', + () async { + processRunner.mockProcessesForExecutable['git-diff'] = [ + MockProcess(stdout: ''' +.cirrus.yml +.ci.yaml +.ci/Dockerfile +.clang-format +analysis_options.yaml +script/tool_runner.sh +'''), + ]; + createFakePackage('a_package', packagesDir); + await runCapturingPrint( + runner, ['sample', '--run-on-dirty-packages']); + + expect(command.plugins, unorderedEquals([])); + }); + + test('Only changed packages should be tested.', () async { + processRunner.mockProcessesForExecutable['git-diff'] = [ + MockProcess(stdout: 'packages/a_package/lib/a_package.dart'), + ]; + final Directory packageA = createFakePackage('a_package', packagesDir); + createFakePlugin('b_package', packagesDir); + final List output = await runCapturingPrint( + runner, ['sample', '--run-on-dirty-packages']); + + expect( + output, + containsAllInOrder([ + contains( + 'Running for all packages that have uncommitted changes'), + ])); + + expect(command.plugins, unorderedEquals([packageA.path])); + }); + + test('multiple packages changed should test all the changed packages', + () async { + processRunner.mockProcessesForExecutable['git-diff'] = [ + 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, ['sample', '--run-on-dirty-packages']); + + expect(command.plugins, + unorderedEquals([packageA.path, packageB.path])); + }); + + test('honors --exclude flag', () async { + processRunner.mockProcessesForExecutable['git-diff'] = [ + 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, [ + 'sample', + '--exclude=b_package', + '--run-on-dirty-packages' + ]); + + expect(command.plugins, unorderedEquals([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 4c20389ae4be..29e3b5832127 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 @@ void main() { 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: ['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 000000000000..29e4f724b338 --- /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 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 arguments = + invocation.positionalArguments[0]! as List; + // 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( + '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 dependencies) { + final List lines = package.pubspecFile.readAsLinesSync(); + final int dependenciesStartIndex = lines.indexOf('dependencies:'); + assert(dependenciesStartIndex != -1); + lines.insertAll(dependenciesStartIndex + 1, [ + 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, ['foo']); + final String originalPubspecContents = + packageBar.pubspecFile.readAsStringSync(); + + final List output = + await runCapturingPrint(runner, ['make-deps-path-based']); + + expect( + output, + containsAllInOrder([ + 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, [ + 'bar', + 'bar_android', + 'bar_platform_interface', + ]); + _addDependencies(pluginAppFacing, [ + 'bar_platform_interface', + 'bar_android', + ]); + _addDependencies(pluginImplementation, [ + 'bar_platform_interface', + ]); + + final List output = await runCapturingPrint(runner, [ + 'make-deps-path-based', + '--target-dependencies=bar,bar_platform_interface' + ]); + + expect( + output, + containsAll([ + '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([ + '# 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([ + '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 = [ + package.childFile('pubspec.yaml'), + ].map((File file) => file.path).join('\n'); + processRunner.mockProcessesForExecutable['git-diff'] = [ + MockProcess(stdout: changedFileOutput), + ]; + // Simulate no change to the version in the interface's pubspec.yaml. + processRunner.mockProcessesForExecutable['git-show'] = [ + MockProcess( + stdout: RepositoryPackage(package).pubspecFile.readAsStringSync()), + ]; + + final List output = await runCapturingPrint(runner, [ + 'make-deps-path-based', + '--target-dependencies-with-non-breaking-updates' + ]); + + expect( + output, + containsAllInOrder([ + 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 = [ + pubspecFile, + ].map((File file) => file.path).join('\n'); + processRunner.mockProcessesForExecutable['git-diff'] = [ + 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'] = [ + MockProcess(stdout: gitPubspecContents), + ]; + + final List output = await runCapturingPrint(runner, [ + 'make-deps-path-based', + '--target-dependencies-with-non-breaking-updates' + ]); + + expect( + output, + containsAllInOrder([ + 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 = [ + pubspecFile, + ].map((File file) => file.path).join('\n'); + processRunner.mockProcessesForExecutable['git-diff'] = [ + 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'] = [ + MockProcess(stdout: gitPubspecContents), + ]; + + final List output = await runCapturingPrint(runner, [ + 'make-deps-path-based', + '--target-dependencies-with-non-breaking-updates' + ]); + + expect( + output, + containsAllInOrder([ + 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 = [ + pubspecFile, + ].map((File file) => file.path).join('\n'); + processRunner.mockProcessesForExecutable['git-diff'] = [ + 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'] = [ + MockProcess(stdout: gitPubspecContents), + ]; + + final List output = await runCapturingPrint(runner, [ + 'make-deps-path-based', + '--target-dependencies-with-non-breaking-updates' + ]); + + expect( + output, + containsAllInOrder([ + 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 = [ + pubspecFile, + ].map((File file) => file.path).join('\n'); + processRunner.mockProcessesForExecutable['git-diff'] = [ + 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'] = [ + MockProcess(stdout: gitPubspecContents), + ]; + + final List output = await runCapturingPrint(runner, [ + 'make-deps-path-based', + '--target-dependencies-with-non-breaking-updates' + ]); + + expect( + output, + containsAllInOrder([ + 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 14e99a10f365..2cb3fc25af2e 100644 --- a/script/tool/test/publish_plugin_command_test.dart +++ b/script/tool/test/publish_plugin_command_test.dart @@ -675,7 +675,7 @@ void main() { 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)'), ]));