From bdc686767fba6a5aac535b9d8ece6df223e09d3a Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Fri, 3 Sep 2021 16:38:57 -0400 Subject: [PATCH] [flutter_plugin_tools] Adjust diff logging Moves diff logging out of a helper, which would not be expected to do logging, and into specific commands where logging the diff base is useful. Fixes some log spam during tests as a side effect. --- .../lib/src/common/git_version_finder.dart | 21 +++++++++++-------- .../tool/lib/src/common/plugin_command.dart | 3 +++ .../tool/lib/src/publish_plugin_command.dart | 3 +++ .../tool/test/common/plugin_command_test.dart | 9 +++++++- .../test/publish_plugin_command_test.dart | 2 ++ 5 files changed, 28 insertions(+), 10 deletions(-) diff --git a/script/tool/lib/src/common/git_version_finder.dart b/script/tool/lib/src/common/git_version_finder.dart index 2c9519e7a856..a0a7a32b5e0f 100644 --- a/script/tool/lib/src/common/git_version_finder.dart +++ b/script/tool/lib/src/common/git_version_finder.dart @@ -11,7 +11,7 @@ import 'package:yaml/yaml.dart'; /// Finding diffs based on `baseGitDir` and `baseSha`. class GitVersionFinder { /// Constructor - GitVersionFinder(this.baseGitDir, this.baseSha); + GitVersionFinder(this.baseGitDir, String? baseSha) : _baseSha = baseSha; /// The top level directory of the git repo. /// @@ -19,7 +19,7 @@ class GitVersionFinder { final GitDir baseGitDir; /// The base sha used to get diff. - final String? baseSha; + String? _baseSha; static bool _isPubspec(String file) { return file.trim().endsWith('pubspec.yaml'); @@ -32,10 +32,9 @@ class GitVersionFinder { /// Get a list of all the changed files. Future> getChangedFiles() async { - final String baseSha = await _getBaseSha(); + final String baseSha = await getBaseSha(); final io.ProcessResult changedFilesCommand = await baseGitDir .runCommand(['diff', '--name-only', baseSha, 'HEAD']); - print('Determine diff with base sha: $baseSha'); final String changedFilesStdout = changedFilesCommand.stdout.toString(); if (changedFilesStdout.isEmpty) { return []; @@ -49,7 +48,7 @@ class GitVersionFinder { /// at the revision of `gitRef` (defaulting to the base if not provided). Future getPackageVersion(String pubspecPath, {String? gitRef}) async { - final String ref = gitRef ?? (await _getBaseSha()); + final String ref = gitRef ?? (await getBaseSha()); io.ProcessResult gitShow; try { @@ -63,9 +62,11 @@ class GitVersionFinder { return versionString == null ? null : Version.parse(versionString); } - Future _getBaseSha() async { - if (baseSha != null && baseSha!.isNotEmpty) { - return baseSha!; + /// Returns the base used to diff against. + Future getBaseSha() async { + String? baseSha = _baseSha; + if (baseSha != null && baseSha.isNotEmpty) { + return baseSha; } io.ProcessResult baseShaFromMergeBase = await baseGitDir.runCommand( @@ -76,6 +77,8 @@ class GitVersionFinder { baseShaFromMergeBase = await baseGitDir .runCommand(['merge-base', 'FETCH_HEAD', 'HEAD']); } - return (baseShaFromMergeBase.stdout as String).trim(); + baseSha = (baseShaFromMergeBase.stdout as String).trim(); + _baseSha = baseSha; + return baseSha; } } diff --git a/script/tool/lib/src/common/plugin_command.dart b/script/tool/lib/src/common/plugin_command.dart index 514a90b85cc7..5d5cbd9abf6c 100644 --- a/script/tool/lib/src/common/plugin_command.dart +++ b/script/tool/lib/src/common/plugin_command.dart @@ -314,6 +314,9 @@ abstract class PluginCommand extends Command { if (runOnChangedPackages) { final GitVersionFinder gitVersionFinder = await retrieveVersionFinder(); + final String baseSha = await gitVersionFinder.getBaseSha(); + print( + 'Running for all packages that have changed relative to "$baseSha"\n'); final List changedFiles = await gitVersionFinder.getChangedFiles(); if (!_changesRequireFullTest(changedFiles)) { diff --git a/script/tool/lib/src/publish_plugin_command.dart b/script/tool/lib/src/publish_plugin_command.dart index 6da51706ef1e..769b9e8c8f00 100644 --- a/script/tool/lib/src/publish_plugin_command.dart +++ b/script/tool/lib/src/publish_plugin_command.dart @@ -159,6 +159,9 @@ class PublishPluginCommand extends PackageLoopingCommand { Stream getPackagesToProcess() async* { if (getBoolArg(_allChangedFlag)) { final GitVersionFinder gitVersionFinder = await retrieveVersionFinder(); + final String baseSha = await gitVersionFinder.getBaseSha(); + print( + 'Publishing all packages that have changed relative to "$baseSha"\n'); final List changedPubspecs = await gitVersionFinder.getChangedPubSpecs(); diff --git a/script/tool/test/common/plugin_command_test.dart b/script/tool/test/common/plugin_command_test.dart index 3ef0d3b3c005..13724e26e5f8 100644 --- a/script/tool/test/common/plugin_command_test.dart +++ b/script/tool/test/common/plugin_command_test.dart @@ -398,12 +398,19 @@ packages/plugin1/CHANGELOG ]; final Directory plugin1 = createFakePlugin('plugin1', packagesDir); createFakePlugin('plugin2', packagesDir); - await runCapturingPrint(runner, [ + final List output = await runCapturingPrint(runner, [ 'sample', '--base-sha=master', '--run-on-changed-packages' ]); + expect( + output, + containsAllInOrder([ + contains( + 'Running for all packages that have changed relative to "master"'), + ])); + expect(command.plugins, unorderedEquals([plugin1.path])); }); diff --git a/script/tool/test/publish_plugin_command_test.dart b/script/tool/test/publish_plugin_command_test.dart index 2ea4fc753460..14e99a10f365 100644 --- a/script/tool/test/publish_plugin_command_test.dart +++ b/script/tool/test/publish_plugin_command_test.dart @@ -466,6 +466,8 @@ void main() { expect( output, containsAllInOrder([ + contains( + 'Publishing all packages that have changed relative to "HEAD~"'), contains('Running `pub publish ` in ${pluginDir1.path}...'), contains('Running `pub publish ` in ${pluginDir2.path}...'), contains('plugin1 - \x1B[32mpublished\x1B[0m'),