From 9ba2d7da3caf966f6cd5ad387e50ba4104bdcf70 Mon Sep 17 00:00:00 2001
From: Chris Yang <ychris@google.com>
Date: Thu, 4 Mar 2021 19:36:48 -0800
Subject: [PATCH 1/3] add --all and --dry-run to publish-plugin

---
 .../tool/lib/src/publish_plugin_command.dart  | 198 ++++++++--
 .../test/publish_plugin_command_test.dart     | 363 +++++++++++++++++-
 script/tool/test/util.dart                    |  13 +-
 3 files changed, 548 insertions(+), 26 deletions(-)

diff --git a/script/tool/lib/src/publish_plugin_command.dart b/script/tool/lib/src/publish_plugin_command.dart
index 0dae3a502be1..e9b9d821f5d6 100644
--- a/script/tool/lib/src/publish_plugin_command.dart
+++ b/script/tool/lib/src/publish_plugin_command.dart
@@ -4,12 +4,14 @@
 
 import 'dart:async';
 import 'dart:convert';
-import 'dart:io';
+import 'dart:io' as io;
 
 import 'package:file/file.dart';
 import 'package:git/git.dart';
 import 'package:meta/meta.dart';
 import 'package:path/path.dart' as p;
+import 'package:pub_semver/pub_semver.dart';
+import 'package:pubspec_parse/pubspec_parse.dart';
 import 'package:yaml/yaml.dart';
 
 import 'common.dart';
@@ -32,10 +34,12 @@ class PublishPluginCommand extends PluginCommand {
     FileSystem fileSystem, {
     ProcessRunner processRunner = const ProcessRunner(),
     Print print = print,
-    Stdin stdinput,
+    io.Stdin stdinput,
+    GitDir gitDir,
   })  : _print = print,
-        _stdin = stdinput ?? stdin,
-        super(packagesDir, fileSystem, processRunner: processRunner) {
+        _stdin = stdinput ?? io.stdin,
+        super(packagesDir, fileSystem,
+            processRunner: processRunner, gitDir: gitDir) {
     argParser.addOption(
       _packageOption,
       help: 'The package to publish.'
@@ -64,6 +68,22 @@ class PublishPluginCommand extends PluginCommand {
       // Flutter convention is to use "upstream" for the single source of truth, and "origin" for personal forks.
       defaultsTo: 'upstream',
     );
+    argParser.addFlag(
+      _allFlag,
+      help:
+          'Release all plugins that contains pubspec changes at the current commit compares to the base-sha.\n'
+          'The $_packageOption option is ignored if this is on.',
+      defaultsTo: false,
+    );
+    argParser.addFlag(
+      _dryRunFlag,
+      help:
+          'Skips the real `pub publish` and `git tag` commands and assumes both commands are successful.\n'
+          'This does not run `pub publish --dry-run`.\n'
+          'If you want to run the command with `pub publish --dry-run`, use `pub-publish-flags=--dry-run`',
+      defaultsTo: false,
+      negatable: true,
+    );
   }
 
   static const String _packageOption = 'package';
@@ -71,6 +91,8 @@ class PublishPluginCommand extends PluginCommand {
   static const String _pushTagsOption = 'push-tags';
   static const String _pubFlagsOption = 'pub-publish-flags';
   static const String _remoteOption = 'remote';
+  static const String _allFlag = 'all';
+  static const String _dryRunFlag = 'dry-run';
 
   // Version tags should follow <package-name>-v<semantic-version>. For example,
   // `flutter_plugin_tools-v0.0.24`.
@@ -84,14 +106,15 @@ class PublishPluginCommand extends PluginCommand {
       'Attempts to publish the given plugin and tag its release on GitHub.';
 
   final Print _print;
-  final Stdin _stdin;
-  // The directory of the actual package that we are publishing.
+  final io.Stdin _stdin;
   StreamSubscription<String> _stdinSubscription;
+  bool _startedListenToStdStream = false;
 
   @override
   Future<void> run() async {
     final String package = argResults[_packageOption] as String;
-    if (package == null) {
+    final bool all = argResults[_allFlag] as bool;
+    if (package == null && !all) {
       _print(
           'Must specify a package to publish. See `plugin_tools help publish-plugin`.');
       throw ToolExit(1);
@@ -102,6 +125,8 @@ class PublishPluginCommand extends PluginCommand {
       _print('$packagesDir is not a valid Git repository.');
       throw ToolExit(1);
     }
+    final GitDir baseGitDir =
+        await GitDir.fromExisting(packagesDir.path, allowSubdirectory: true);
 
     final bool shouldPushTag = argResults[_pushTagsOption] == true;
     final String remote = argResults[_remoteOption] as String;
@@ -111,7 +136,74 @@ class PublishPluginCommand extends PluginCommand {
     }
     _print('Local repo is ready!');
 
-    final Directory packageDir = _getPackageDir(package);
+    if (all) {
+      await _publishAllPackages(
+          remote: remote,
+          remoteUrl: remoteUrl,
+          shouldPushTag: shouldPushTag,
+          baseGitDir: baseGitDir);
+    } else {
+      await _publishAndReleasePackage(
+          packageDir: _getPackageDir(package),
+          remote: remote,
+          remoteUrl: remoteUrl,
+          shouldPushTag: shouldPushTag);
+    }
+    await _finishSuccesfully();
+  }
+
+  Future<void> _publishAllPackages(
+      {String remote,
+      String remoteUrl,
+      bool shouldPushTag,
+      GitDir baseGitDir}) async {
+    final List<String> packagesReleased = <String>[];
+    final GitVersionFinder gitVersionFinder = await retrieveVersionFinder();
+    final List<String> changedPubspecs =
+        await gitVersionFinder.getChangedPubSpecs();
+    if (changedPubspecs.isEmpty) {
+      _print('No version updates in this commit, exiting...');
+      return;
+    }
+    _print('Getting existing tags...');
+    final io.ProcessResult existingTagsResult =
+        await baseGitDir.runCommand(<String>['tag', '--sort=-committerdate']);
+    final List<String> existingTags = (existingTagsResult.stdout as String)
+        .split('\n')
+          ..removeWhere((element) => element == '');
+    for (final String pubspecPath in changedPubspecs) {
+      final File pubspecFile =
+          fileSystem.directory(baseGitDir.path).childFile(pubspecPath);
+      if (!pubspecFile.existsSync()) {
+        printErrorAndExit(
+            errorMessage:
+                'Fatal: The pubspec file at ${pubspecFile.path} does not exist.');
+      }
+      final bool needsRelease = await _checkNeedsRelease(
+          pubspecPath: pubspecPath,
+          pubspecFile: pubspecFile,
+          gitVersionFinder: gitVersionFinder,
+          existingTags: existingTags);
+      if (!needsRelease) {
+        continue;
+      }
+      _print('\n');
+      await _publishAndReleasePackage(
+          packageDir: pubspecFile.parent,
+          remote: remote,
+          remoteUrl: remoteUrl,
+          shouldPushTag: shouldPushTag);
+      packagesReleased.add(pubspecFile.parent.basename);
+      _print('\n');
+    }
+    _print('Packages released: ${packagesReleased.join(', ')}');
+  }
+
+  Future<void> _publishAndReleasePackage(
+      {@required Directory packageDir,
+      @required String remote,
+      @required String remoteUrl,
+      @required bool shouldPushTag}) async {
     await _publishPlugin(packageDir: packageDir);
     if (argResults[_tagReleaseOption] as bool) {
       await _tagRelease(
@@ -120,7 +212,48 @@ class PublishPluginCommand extends PluginCommand {
           remoteUrl: remoteUrl,
           shouldPushTag: shouldPushTag);
     }
-    await _finishSuccesfully();
+    _print('Release ${packageDir.basename} successful.');
+  }
+
+  // Returns `true` if needs to release the version, `false` if needs to skip
+  Future<bool> _checkNeedsRelease({
+    @required String pubspecPath,
+    @required File pubspecFile,
+    @required GitVersionFinder gitVersionFinder,
+    @required List<String> existingTags,
+  }) async {
+    final Pubspec pubspec = Pubspec.parse(pubspecFile.readAsStringSync());
+    if (pubspec.publishTo == 'none') {
+      return false;
+    }
+
+    final Version headVersion =
+        await gitVersionFinder.getPackageVersion(pubspecPath, gitRef: 'HEAD');
+    if (headVersion == null) {
+      printErrorAndExit(
+          errorMessage: 'No version found. A package that '
+              'intentionally has no version should be marked '
+              '"publish_to: none".');
+    }
+
+    if (pubspec.name == null) {
+      printErrorAndExit(errorMessage: 'Fatal: Package name is null.');
+    }
+    // Get latest tagged version and compare with the current version.
+    final String latestTag = existingTags.isNotEmpty
+        ? existingTags
+            .firstWhere((String tag) => tag.split('-v').first == pubspec.name)
+        : '';
+    if (latestTag.isNotEmpty) {
+      final String latestTaggedVersion = latestTag.split('-v').last;
+      final Version latestVersion = Version.parse(latestTaggedVersion);
+      if (pubspec.version < latestVersion) {
+        _print(
+            'The new version (${pubspec.version}) is lower than the current version ($latestVersion) for ${pubspec.name}.\nThis git commit is a revert, no release is tagged.');
+        return false;
+      }
+    }
+    return true;
   }
 
   Future<void> _publishPlugin({@required Directory packageDir}) async {
@@ -135,6 +268,14 @@ class PublishPluginCommand extends PluginCommand {
       @required String remoteUrl,
       @required bool shouldPushTag}) async {
     final String tag = _getTag(packageDir);
+    if (argResults[_dryRunFlag] as bool) {
+      _print('DRY RUN: Tagging release $tag...');
+      if (!shouldPushTag) {
+        return;
+      }
+      _print('DRY RUN: Pushing tag to $remote...');
+      return;
+    }
     _print('Tagging release $tag...');
     await processRunner.runAndExitOnError('git', <String>['tag', tag],
         workingDir: packageDir);
@@ -147,7 +288,9 @@ class PublishPluginCommand extends PluginCommand {
   }
 
   Future<void> _finishSuccesfully() async {
-    await _stdinSubscription.cancel();
+    if (_stdinSubscription != null) {
+      await _stdinSubscription.cancel();
+    }
     _print('Done!');
   }
 
@@ -163,7 +306,7 @@ class PublishPluginCommand extends PluginCommand {
   }
 
   Future<void> _checkGitStatus(Directory packageDir) async {
-    final ProcessResult statusResult = await processRunner.runAndExitOnError(
+    final io.ProcessResult statusResult = await processRunner.runAndExitOnError(
         'git',
         <String>[
           'status',
@@ -184,7 +327,7 @@ class PublishPluginCommand extends PluginCommand {
   }
 
   Future<String> _verifyRemote(String remote) async {
-    final ProcessResult remoteInfo = await processRunner.runAndExitOnError(
+    final io.ProcessResult remoteInfo = await processRunner.runAndExitOnError(
         'git', <String>['remote', 'get-url', remote],
         workingDir: packagesDir);
     return remoteInfo.stdout as String;
@@ -193,20 +336,31 @@ class PublishPluginCommand extends PluginCommand {
   Future<void> _publish(Directory packageDir) async {
     final List<String> publishFlags =
         argResults[_pubFlagsOption] as List<String>;
+    if (argResults[_dryRunFlag] as bool) {
+      _print(
+          'DRY RUN: Running `pub publish ${publishFlags.join(' ')}` in ${packageDir.absolute.path}...\n');
+      return;
+    }
+
     _print(
         'Running `pub publish ${publishFlags.join(' ')}` in ${packageDir.absolute.path}...\n');
-    final Process publish = await processRunner.start(
+    final io.Process publish = await processRunner.start(
         'flutter', <String>['pub', 'publish'] + publishFlags,
         workingDirectory: packageDir);
-    publish.stdout
-        .transform(utf8.decoder)
-        .listen((String data) => _print(data));
-    publish.stderr
-        .transform(utf8.decoder)
-        .listen((String data) => _print(data));
-    _stdinSubscription = _stdin
-        .transform(utf8.decoder)
-        .listen((String data) => publish.stdin.writeln(data));
+
+    if (!_startedListenToStdStream) {
+      publish.stdout
+          .transform(utf8.decoder)
+          .listen((String data) => _print(data));
+      publish.stderr
+          .transform(utf8.decoder)
+          .listen((String data) => _print(data));
+      _stdinSubscription = _stdin
+          .transform(utf8.decoder)
+          .listen((String data) => publish.stdin.writeln(data));
+
+      _startedListenToStdStream = true;
+    }
     final int result = await publish.exitCode;
     if (result != 0) {
       _print('Publish failed. Exiting.');
diff --git a/script/tool/test/publish_plugin_command_test.dart b/script/tool/test/publish_plugin_command_test.dart
index 03e7858d3bcd..6b89286700d7 100644
--- a/script/tool/test/publish_plugin_command_test.dart
+++ b/script/tool/test/publish_plugin_command_test.dart
@@ -53,7 +53,8 @@ void main() {
           mockPackagesDir, mockPackagesDir.fileSystem,
           processRunner: processRunner,
           print: (Object message) => printedMessages.add(message.toString()),
-          stdinput: mockStdin));
+          stdinput: mockStdin,
+          gitDir: await GitDir.fromExisting(mockPackagesDir.path)));
   });
 
   tearDown(() {
@@ -212,6 +213,27 @@ void main() {
 
       expect(printedMessages, contains('Publish failed. Exiting.'));
     });
+
+    test('publish, dry run', () async {
+      // Immediately return 1 when running `pub publish`. If dry-run does not work, test should throw.
+      processRunner.mockPublishProcess.exitCodeCompleter.complete(1);
+      await commandRunner.run(<String>[
+        'publish-plugin',
+        '--package',
+        testPluginName,
+        '--dry-run',
+        '--no-push-tags',
+        '--no-tag-release',
+      ]);
+
+      expect(processRunner.pushTagsArgs, isEmpty);
+      expect(
+          printedMessages,
+          containsAllInOrder(<String>[
+            'DRY RUN: Running `pub publish ` in ${pluginDir.path}...\n',
+            'Done!'
+          ]));
+    });
   });
 
   group('Tags release', () {
@@ -286,6 +308,25 @@ void main() {
       expect(printedMessages.last, 'Done!');
     });
 
+    test('to upstream by default, dry run', () async {
+      await gitDir.runCommand(<String>['tag', 'garbage']);
+      // Immediately return 1 when running `pub publish`. If dry-run does not work, test should throw.
+      processRunner.mockPublishProcess.exitCodeCompleter.complete(1);
+      mockStdin.readLineOutput = 'y';
+      await commandRunner.run(
+          <String>['publish-plugin', '--package', testPluginName, '--dry-run']);
+
+      expect(processRunner.pushTagsArgs, isEmpty);
+      expect(
+          printedMessages,
+          containsAllInOrder(<String>[
+            'DRY RUN: Running `pub publish ` in ${pluginDir.path}...\n',
+            'DRY RUN: Tagging release fake_package-v0.0.1...',
+            'DRY RUN: Pushing tag to upstream...',
+            'Done!'
+          ]));
+    });
+
     test('to different remotes based on a flag', () async {
       await gitDir.runCommand(
           <String>['remote', 'add', 'origin', 'http://localhost:8001']);
@@ -318,6 +359,326 @@ void main() {
       expect(printedMessages.last, 'Done!');
     });
   });
+
+  group('Auto release (all flag)', () {
+    setUp(() async {
+      io.Process.runSync('git', <String>['init'],
+          workingDirectory: mockPackagesDir.path);
+      gitDir = await GitDir.fromExisting(mockPackagesDir.path);
+      await gitDir.runCommand(
+          <String>['remote', 'add', 'upstream', 'http://localhost:8000']);
+    });
+
+    test('can release newly created plugins', () async {
+      // Non-federated
+      final Directory pluginDir1 = createFakePlugin('plugin1',
+          withSingleExample: true, packagesDirectory: mockPackagesDir);
+      // federated
+      final Directory pluginDir2 = createFakePlugin('plugin2',
+          withSingleExample: true,
+          parentDirectoryName: 'plugin2',
+          packagesDirectory: mockPackagesDir);
+      createFakePubspec(pluginDir1,
+          name: 'plugin1',
+          includeVersion: true,
+          isFlutter: false,
+          version: '0.0.1');
+      createFakePubspec(pluginDir2,
+          name: 'plugin2',
+          includeVersion: true,
+          isFlutter: false,
+          version: '0.0.1');
+      await gitDir.runCommand(<String>['add', '-A']);
+      await gitDir.runCommand(<String>['commit', '-m', 'Add plugins']);
+      // Immediately return 0 when running `pub publish`.
+      processRunner.mockPublishProcess.exitCodeCompleter.complete(0);
+      mockStdin.readLineOutput = 'y';
+      await commandRunner
+          .run(<String>['publish-plugin', '--all', '--base-sha=HEAD~']);
+      expect(
+          printedMessages,
+          containsAllInOrder(<String>[
+            'Checking local repo...',
+            'Local repo is ready!',
+            'Getting existing tags...',
+            'Running `pub publish ` in ${pluginDir1.path}...\n',
+            'Running `pub publish ` in ${pluginDir2.path}...\n',
+            'Packages released: plugin1, plugin2',
+            'Done!'
+          ]));
+      expect(processRunner.pushTagsArgs, isNotEmpty);
+      expect(processRunner.pushTagsArgs[0], 'push');
+      expect(processRunner.pushTagsArgs[1], 'upstream');
+      expect(processRunner.pushTagsArgs[2], 'plugin1-v0.0.1');
+      expect(processRunner.pushTagsArgs[3], 'push');
+      expect(processRunner.pushTagsArgs[4], 'upstream');
+      expect(processRunner.pushTagsArgs[5], 'plugin2-v0.0.1');
+    });
+
+    test('can release newly created plugins, dry run', () async {
+      // Non-federated
+      final Directory pluginDir1 = createFakePlugin('plugin1',
+          withSingleExample: true, packagesDirectory: mockPackagesDir);
+      // federated
+      final Directory pluginDir2 = createFakePlugin('plugin2',
+          withSingleExample: true,
+          parentDirectoryName: 'plugin2',
+          packagesDirectory: mockPackagesDir);
+      createFakePubspec(pluginDir1,
+          name: 'plugin1',
+          includeVersion: true,
+          isFlutter: false,
+          version: '0.0.1');
+      createFakePubspec(pluginDir2,
+          name: 'plugin2',
+          includeVersion: true,
+          isFlutter: false,
+          version: '0.0.1');
+      await gitDir.runCommand(<String>['add', '-A']);
+      await gitDir.runCommand(<String>['commit', '-m', 'Add plugins']);
+      // Immediately return 1 when running `pub publish`. If dry-run does not work, test should throw.
+      processRunner.mockPublishProcess.exitCodeCompleter.complete(1);
+      mockStdin.readLineOutput = 'y';
+      await commandRunner.run(
+          <String>['publish-plugin', '--all', '--base-sha=HEAD~', '--dry-run']);
+      expect(
+          printedMessages,
+          containsAllInOrder(<String>[
+            'Checking local repo...',
+            'Local repo is ready!',
+            'Getting existing tags...',
+            'DRY RUN: Running `pub publish ` in ${pluginDir1.path}...\n',
+            'DRY RUN: Tagging release plugin1-v0.0.1...',
+            'DRY RUN: Pushing tag to upstream...',
+            'DRY RUN: Running `pub publish ` in ${pluginDir2.path}...\n',
+            'DRY RUN: Tagging release plugin2-v0.0.1...',
+            'DRY RUN: Pushing tag to upstream...',
+            'Packages released: plugin1, plugin2',
+            'Done!'
+          ]));
+      expect(processRunner.pushTagsArgs, isEmpty);
+    });
+
+    test('version change triggers releases.', () async {
+      // Non-federated
+      final Directory pluginDir1 = createFakePlugin('plugin1',
+          withSingleExample: true, packagesDirectory: mockPackagesDir);
+      // federated
+      final Directory pluginDir2 = createFakePlugin('plugin2',
+          withSingleExample: true,
+          parentDirectoryName: 'plugin2',
+          packagesDirectory: mockPackagesDir);
+      createFakePubspec(pluginDir1,
+          name: 'plugin1',
+          includeVersion: true,
+          isFlutter: false,
+          version: '0.0.1');
+      createFakePubspec(pluginDir2,
+          name: 'plugin2',
+          includeVersion: true,
+          isFlutter: false,
+          version: '0.0.1');
+      await gitDir.runCommand(<String>['add', '-A']);
+      await gitDir.runCommand(<String>['commit', '-m', 'Add plugins']);
+      // Immediately return 0 when running `pub publish`.
+      processRunner.mockPublishProcess.exitCodeCompleter.complete(0);
+      mockStdin.readLineOutput = 'y';
+      await commandRunner
+          .run(<String>['publish-plugin', '--all', '--base-sha=HEAD~']);
+      expect(
+          printedMessages,
+          containsAllInOrder(<String>[
+            'Checking local repo...',
+            'Local repo is ready!',
+            'Getting existing tags...',
+            'Running `pub publish ` in ${pluginDir1.path}...\n',
+            'Running `pub publish ` in ${pluginDir2.path}...\n',
+            'Packages released: plugin1, plugin2',
+            'Done!'
+          ]));
+      expect(processRunner.pushTagsArgs, isNotEmpty);
+      expect(processRunner.pushTagsArgs[0], 'push');
+      expect(processRunner.pushTagsArgs[1], 'upstream');
+      expect(processRunner.pushTagsArgs[2], 'plugin1-v0.0.1');
+      expect(processRunner.pushTagsArgs[3], 'push');
+      expect(processRunner.pushTagsArgs[4], 'upstream');
+      expect(processRunner.pushTagsArgs[5], 'plugin2-v0.0.1');
+
+      processRunner.pushTagsArgs.clear();
+      printedMessages.clear();
+
+      final List<String> plugin1Pubspec =
+          pluginDir1.childFile('pubspec.yaml').readAsLinesSync();
+      plugin1Pubspec[plugin1Pubspec.indexWhere(
+          (element) => element.contains('version:'))] = 'version: 0.0.2';
+      pluginDir1
+          .childFile('pubspec.yaml')
+          .writeAsStringSync(plugin1Pubspec.join('\n'));
+      final List<String> plugin2Pubspec =
+          pluginDir2.childFile('pubspec.yaml').readAsLinesSync();
+      plugin2Pubspec[plugin2Pubspec.indexWhere(
+          (element) => element.contains('version:'))] = 'version: 0.0.2';
+      pluginDir2
+          .childFile('pubspec.yaml')
+          .writeAsStringSync(plugin2Pubspec.join('\n'));
+      await gitDir.runCommand(<String>['add', '-A']);
+      await gitDir
+          .runCommand(<String>['commit', '-m', 'Update versions to 0.0.2']);
+
+      await commandRunner
+          .run(<String>['publish-plugin', '--all', '--base-sha=HEAD~']);
+      expect(
+          printedMessages,
+          containsAllInOrder(<String>[
+            'Checking local repo...',
+            'Local repo is ready!',
+            'Getting existing tags...',
+            'Running `pub publish ` in ${pluginDir1.path}...\n',
+            'Running `pub publish ` in ${pluginDir2.path}...\n',
+            'Packages released: plugin1, plugin2',
+            'Done!'
+          ]));
+
+      expect(processRunner.pushTagsArgs, isNotEmpty);
+      expect(processRunner.pushTagsArgs[0], 'push');
+      expect(processRunner.pushTagsArgs[1], 'upstream');
+      expect(processRunner.pushTagsArgs[2], 'plugin1-v0.0.2');
+      expect(processRunner.pushTagsArgs[3], 'push');
+      expect(processRunner.pushTagsArgs[4], 'upstream');
+      expect(processRunner.pushTagsArgs[5], 'plugin2-v0.0.2');
+    });
+
+    test(
+        'versions revert do not trigger releases. Also prints out warning message.',
+        () async {
+      // Non-federated
+      final Directory pluginDir1 = createFakePlugin('plugin1',
+          withSingleExample: true, packagesDirectory: mockPackagesDir);
+      // federated
+      final Directory pluginDir2 = createFakePlugin('plugin2',
+          withSingleExample: true,
+          parentDirectoryName: 'plugin2',
+          packagesDirectory: mockPackagesDir);
+      createFakePubspec(pluginDir1,
+          name: 'plugin1',
+          includeVersion: true,
+          isFlutter: false,
+          version: '0.0.2');
+      createFakePubspec(pluginDir2,
+          name: 'plugin2',
+          includeVersion: true,
+          isFlutter: false,
+          version: '0.0.2');
+      await gitDir.runCommand(<String>['add', '-A']);
+      await gitDir.runCommand(<String>['commit', '-m', 'Add plugins']);
+      // Immediately return 0 when running `pub publish`.
+      processRunner.mockPublishProcess.exitCodeCompleter.complete(0);
+      mockStdin.readLineOutput = 'y';
+      await commandRunner
+          .run(<String>['publish-plugin', '--all', '--base-sha=HEAD~']);
+      expect(
+          printedMessages,
+          containsAllInOrder(<String>[
+            'Checking local repo...',
+            'Local repo is ready!',
+            'Getting existing tags...',
+            'Running `pub publish ` in ${pluginDir1.path}...\n',
+            'Running `pub publish ` in ${pluginDir2.path}...\n',
+            'Packages released: plugin1, plugin2',
+            'Done!'
+          ]));
+      expect(processRunner.pushTagsArgs, isNotEmpty);
+      expect(processRunner.pushTagsArgs[0], 'push');
+      expect(processRunner.pushTagsArgs[1], 'upstream');
+      expect(processRunner.pushTagsArgs[2], 'plugin1-v0.0.2');
+      expect(processRunner.pushTagsArgs[3], 'push');
+      expect(processRunner.pushTagsArgs[4], 'upstream');
+      expect(processRunner.pushTagsArgs[5], 'plugin2-v0.0.2');
+
+      processRunner.pushTagsArgs.clear();
+      printedMessages.clear();
+
+      final List<String> plugin1Pubspec =
+          pluginDir1.childFile('pubspec.yaml').readAsLinesSync();
+      plugin1Pubspec[plugin1Pubspec.indexWhere(
+          (element) => element.contains('version:'))] = 'version: 0.0.1';
+      pluginDir1
+          .childFile('pubspec.yaml')
+          .writeAsStringSync(plugin1Pubspec.join('\n'));
+      final List<String> plugin2Pubspec =
+          pluginDir2.childFile('pubspec.yaml').readAsLinesSync();
+      plugin2Pubspec[plugin2Pubspec.indexWhere(
+          (element) => element.contains('version:'))] = 'version: 0.0.1';
+      pluginDir2
+          .childFile('pubspec.yaml')
+          .writeAsStringSync(plugin2Pubspec.join('\n'));
+      await gitDir.runCommand(<String>['add', '-A']);
+      await gitDir
+          .runCommand(<String>['commit', '-m', 'Update versions to 0.0.1']);
+
+      await commandRunner
+          .run(<String>['publish-plugin', '--all', '--base-sha=HEAD~']);
+      expect(
+          printedMessages,
+          containsAllInOrder(<String>[
+            'Checking local repo...',
+            'Local repo is ready!',
+            'Getting existing tags...',
+            'The new version (0.0.1) is lower than the current version (0.0.2) for plugin1.\nThis git commit is a revert, no release is tagged.',
+            'The new version (0.0.1) is lower than the current version (0.0.2) for plugin2.\nThis git commit is a revert, no release is tagged.',
+            'Done!'
+          ]));
+
+      expect(processRunner.pushTagsArgs, isEmpty);
+    });
+
+    test('No version change does not release any plugins', () async {
+      // Non-federated
+      final Directory pluginDir1 = createFakePlugin('plugin1',
+          withSingleExample: true, packagesDirectory: mockPackagesDir);
+      // federated
+      final Directory pluginDir2 = createFakePlugin('plugin2',
+          withSingleExample: true,
+          parentDirectoryName: 'plugin2',
+          packagesDirectory: mockPackagesDir);
+      createFakePubspec(pluginDir1,
+          name: 'plugin1',
+          includeVersion: true,
+          isFlutter: false,
+          version: '0.0.1');
+      createFakePubspec(pluginDir2,
+          name: 'plugin2',
+          includeVersion: true,
+          isFlutter: false,
+          version: '0.0.1');
+
+      io.Process.runSync('git', <String>['init'],
+          workingDirectory: mockPackagesDir.path);
+      gitDir = await GitDir.fromExisting(mockPackagesDir.path);
+      await gitDir.runCommand(<String>['add', '-A']);
+      await gitDir.runCommand(<String>['commit', '-m', 'Add plugins']);
+
+      pluginDir1.childFile('plugin1.dart').createSync();
+      pluginDir2.childFile('plugin2.dart').createSync();
+      await gitDir.runCommand(<String>['add', '-A']);
+      await gitDir.runCommand(<String>['commit', '-m', 'Add dart files']);
+
+      // Immediately return 0 when running `pub publish`.
+      processRunner.mockPublishProcess.exitCodeCompleter.complete(0);
+      mockStdin.readLineOutput = 'y';
+      await commandRunner
+          .run(<String>['publish-plugin', '--all', '--base-sha=HEAD~']);
+      expect(
+          printedMessages,
+          containsAllInOrder(<String>[
+            'Checking local repo...',
+            'Local repo is ready!',
+            'No version updates in this commit, exiting...',
+            'Done!'
+          ]));
+      expect(processRunner.pushTagsArgs, isEmpty);
+    });
+  });
 }
 
 class TestProcessRunner extends ProcessRunner {
diff --git a/script/tool/test/util.dart b/script/tool/test/util.dart
index 5a2c42bd3194..4dd27f6dea85 100644
--- a/script/tool/test/util.dart
+++ b/script/tool/test/util.dart
@@ -49,6 +49,7 @@ Directory createFakePlugin(
   bool isWindowsPlugin = false,
   bool includeChangeLog = false,
   bool includeVersion = false,
+  String version = '0.0.1',
   String parentDirectoryName = '',
   Directory packagesDirectory,
 }) {
@@ -73,6 +74,7 @@ Directory createFakePlugin(
     isMacOsPlugin: isMacOsPlugin,
     isWindowsPlugin: isWindowsPlugin,
     includeVersion: includeVersion,
+    version: version
   );
   if (includeChangeLog) {
     createFakeCHANGELOG(pluginDirectory, '''
@@ -85,14 +87,14 @@ Directory createFakePlugin(
     final Directory exampleDir = pluginDirectory.childDirectory('example')
       ..createSync();
     createFakePubspec(exampleDir,
-        name: '${name}_example', isFlutter: isFlutter);
+        name: '${name}_example', isFlutter: isFlutter, includeVersion: false, publish_to: 'none');
   } else if (withExamples.isNotEmpty) {
     final Directory exampleDir = pluginDirectory.childDirectory('example')
       ..createSync();
     for (final String example in withExamples) {
       final Directory currentExample = exampleDir.childDirectory(example)
         ..createSync();
-      createFakePubspec(currentExample, name: example, isFlutter: isFlutter);
+      createFakePubspec(currentExample, name: example, isFlutter: isFlutter, includeVersion: false, publish_to: 'none');
     }
   }
 
@@ -123,6 +125,7 @@ void createFakePubspec(
   bool isLinuxPlugin = false,
   bool isMacOsPlugin = false,
   bool isWindowsPlugin = false,
+  String publish_to = 'http://no_pub_server.com',
   String version = '0.0.1',
 }) {
   parent.childFile('pubspec.yaml').createSync();
@@ -180,7 +183,11 @@ dependencies:
   if (includeVersion) {
     yaml += '''
 version: $version
-publish_to: http://no_pub_server.com # Hardcoded safeguard to prevent this from somehow being published by a broken test.
+''';
+  }
+  if (publish_to.isNotEmpty) {
+     yaml += '''
+publish_to: $publish_to # Hardcoded safeguard to prevent this from somehow being published by a broken test.
 ''';
   }
   parent.childFile('pubspec.yaml').writeAsStringSync(yaml);

From 9a760ee4ea3b3e067b94d31c56be447822b7e78b Mon Sep 17 00:00:00 2001
From: Chris Yang <ychris@google.com>
Date: Wed, 21 Apr 2021 14:21:31 -0700
Subject: [PATCH 2/3] review fixes

---
 .../tool/lib/src/publish_plugin_command.dart  | 341 +++++++++++-------
 .../test/publish_plugin_command_test.dart     | 286 ++++++++++++---
 2 files changed, 450 insertions(+), 177 deletions(-)

diff --git a/script/tool/lib/src/publish_plugin_command.dart b/script/tool/lib/src/publish_plugin_command.dart
index e9b9d821f5d6..35b9284771e5 100644
--- a/script/tool/lib/src/publish_plugin_command.dart
+++ b/script/tool/lib/src/publish_plugin_command.dart
@@ -69,7 +69,7 @@ class PublishPluginCommand extends PluginCommand {
       defaultsTo: 'upstream',
     );
     argParser.addFlag(
-      _allFlag,
+      _allChangedFlag,
       help:
           'Release all plugins that contains pubspec changes at the current commit compares to the base-sha.\n'
           'The $_packageOption option is ignored if this is on.',
@@ -91,7 +91,7 @@ class PublishPluginCommand extends PluginCommand {
   static const String _pushTagsOption = 'push-tags';
   static const String _pubFlagsOption = 'pub-publish-flags';
   static const String _remoteOption = 'remote';
-  static const String _allFlag = 'all';
+  static const String _allChangedFlag = 'all-changed';
   static const String _dryRunFlag = 'dry-run';
 
   // Version tags should follow <package-name>-v<semantic-version>. For example,
@@ -108,13 +108,12 @@ class PublishPluginCommand extends PluginCommand {
   final Print _print;
   final io.Stdin _stdin;
   StreamSubscription<String> _stdinSubscription;
-  bool _startedListenToStdStream = false;
 
   @override
   Future<void> run() async {
     final String package = argResults[_packageOption] as String;
-    final bool all = argResults[_allFlag] as bool;
-    if (package == null && !all) {
+    final bool publishAllChanged = argResults[_allChangedFlag] as bool;
+    if (package == null && !publishAllChanged) {
       _print(
           'Must specify a package to publish. See `plugin_tools help publish-plugin`.');
       throw ToolExit(1);
@@ -135,163 +134,230 @@ class PublishPluginCommand extends PluginCommand {
       remoteUrl = await _verifyRemote(remote);
     }
     _print('Local repo is ready!');
+    if (argResults[_dryRunFlag] as bool) {
+      _print('===============  DRY RUN ===============');
+    }
 
-    if (all) {
-      await _publishAllPackages(
-          remote: remote,
-          remoteUrl: remoteUrl,
-          shouldPushTag: shouldPushTag,
-          baseGitDir: baseGitDir);
+    bool successful;
+    if (publishAllChanged) {
+      successful = await _publishAllChangedPackages(
+        remote: remote,
+        remoteUrl: remoteUrl,
+        shouldPushTag: shouldPushTag,
+        baseGitDir: baseGitDir,
+      );
     } else {
-      await _publishAndReleasePackage(
-          packageDir: _getPackageDir(package),
-          remote: remote,
-          remoteUrl: remoteUrl,
-          shouldPushTag: shouldPushTag);
+      successful = await _publishAndTagPackage(
+        packageDir: _getPackageDir(package),
+        remote: remote,
+        remoteUrl: remoteUrl,
+        shouldPushTag: shouldPushTag,
+      );
     }
-    await _finishSuccesfully();
+    await _finish(successful);
   }
 
-  Future<void> _publishAllPackages(
-      {String remote,
-      String remoteUrl,
-      bool shouldPushTag,
-      GitDir baseGitDir}) async {
-    final List<String> packagesReleased = <String>[];
+  Future<bool> _publishAllChangedPackages({
+    String remote,
+    String remoteUrl,
+    bool shouldPushTag,
+    GitDir baseGitDir,
+  }) async {
     final GitVersionFinder gitVersionFinder = await retrieveVersionFinder();
     final List<String> changedPubspecs =
         await gitVersionFinder.getChangedPubSpecs();
     if (changedPubspecs.isEmpty) {
-      _print('No version updates in this commit, exiting...');
-      return;
+      _print('No version updates in this commit.');
+      return true;
     }
     _print('Getting existing tags...');
     final io.ProcessResult existingTagsResult =
         await baseGitDir.runCommand(<String>['tag', '--sort=-committerdate']);
     final List<String> existingTags = (existingTagsResult.stdout as String)
         .split('\n')
-          ..removeWhere((element) => element == '');
+          ..removeWhere((element) => element.isEmpty);
+
+    final List<String> packagesReleased = <String>[];
+    final List<String> packagesFailed = <String>[];
+
     for (final String pubspecPath in changedPubspecs) {
       final File pubspecFile =
           fileSystem.directory(baseGitDir.path).childFile(pubspecPath);
-      if (!pubspecFile.existsSync()) {
-        printErrorAndExit(
-            errorMessage:
-                'Fatal: The pubspec file at ${pubspecFile.path} does not exist.');
-      }
-      final bool needsRelease = await _checkNeedsRelease(
-          pubspecPath: pubspecPath,
-          pubspecFile: pubspecFile,
-          gitVersionFinder: gitVersionFinder,
-          existingTags: existingTags);
-      if (!needsRelease) {
-        continue;
+      final _CheckNeedsReleaseResult result = await _checkNeedsRelease(
+        pubspecFile: pubspecFile,
+        gitVersionFinder: gitVersionFinder,
+        existingTags: existingTags,
+      );
+      switch (result) {
+        case _CheckNeedsReleaseResult.release:
+          break;
+        case _CheckNeedsReleaseResult.noRelease:
+          continue;
+        case _CheckNeedsReleaseResult.failure:
+          packagesFailed.add(pubspecFile.parent.basename);
+          continue;
       }
       _print('\n');
-      await _publishAndReleasePackage(
-          packageDir: pubspecFile.parent,
-          remote: remote,
-          remoteUrl: remoteUrl,
-          shouldPushTag: shouldPushTag);
-      packagesReleased.add(pubspecFile.parent.basename);
+      if (await _publishAndTagPackage(
+        packageDir: pubspecFile.parent,
+        remote: remote,
+        remoteUrl: remoteUrl,
+        shouldPushTag: shouldPushTag,
+      )) {
+        packagesReleased.add(pubspecFile.parent.basename);
+      } else {
+        packagesFailed.add(pubspecFile.parent.basename);
+      }
       _print('\n');
     }
-    _print('Packages released: ${packagesReleased.join(', ')}');
+    if (packagesReleased.isNotEmpty) {
+      _print('Packages released: ${packagesReleased.join(', ')}');
+    }
+    if (packagesFailed.isNotEmpty) {
+      _print(
+          'Packages failed to release: ${packagesFailed.join(', ')}, see above for details.');
+    }
+    return packagesFailed.isEmpty;
   }
 
-  Future<void> _publishAndReleasePackage(
-      {@required Directory packageDir,
-      @required String remote,
-      @required String remoteUrl,
-      @required bool shouldPushTag}) async {
-    await _publishPlugin(packageDir: packageDir);
+  // Publish the package to pub with `pub publish`.
+  // If `_tagReleaseOption` is on, git tag the release.
+  // If `shouldPushTag` is `true`, the tag will be pushed to `remote`.
+  // Returns `true` if publishing and tag are successful.
+  Future<bool> _publishAndTagPackage({
+    @required Directory packageDir,
+    @required String remote,
+    @required String remoteUrl,
+    @required bool shouldPushTag,
+  }) async {
+    if (!await _publishPlugin(packageDir: packageDir)) {
+      return false;
+    }
     if (argResults[_tagReleaseOption] as bool) {
-      await _tagRelease(
-          packageDir: packageDir,
-          remote: remote,
-          remoteUrl: remoteUrl,
-          shouldPushTag: shouldPushTag);
+      if (!await _tagRelease(
+        packageDir: packageDir,
+        remote: remote,
+        remoteUrl: remoteUrl,
+        shouldPushTag: shouldPushTag,
+      )) {
+        return false;
+      }
     }
-    _print('Release ${packageDir.basename} successful.');
+    _print('Released [${packageDir.basename}] successfully.');
+    return true;
   }
 
   // Returns `true` if needs to release the version, `false` if needs to skip
-  Future<bool> _checkNeedsRelease({
-    @required String pubspecPath,
+  Future<_CheckNeedsReleaseResult> _checkNeedsRelease({
     @required File pubspecFile,
     @required GitVersionFinder gitVersionFinder,
     @required List<String> existingTags,
   }) async {
+    if (!pubspecFile.existsSync()) {
+      _print('''
+The file at The pubspec file at ${pubspecFile.path} does not exist. Publishing will not happen for ${pubspecFile.parent.basename}.
+Safe to ignore if the package is deleted in this commit.
+''');
+      return _CheckNeedsReleaseResult.noRelease;
+    }
+
     final Pubspec pubspec = Pubspec.parse(pubspecFile.readAsStringSync());
     if (pubspec.publishTo == 'none') {
-      return false;
+      return _CheckNeedsReleaseResult.noRelease;
     }
 
-    final Version headVersion =
-        await gitVersionFinder.getPackageVersion(pubspecPath, gitRef: 'HEAD');
-    if (headVersion == null) {
-      printErrorAndExit(
-          errorMessage: 'No version found. A package that '
-              'intentionally has no version should be marked '
-              '"publish_to: none".');
+    if (pubspec.version == null) {
+      _print('No version found. A package that '
+          'intentionally has no version should be marked '
+          '"publish_to: none".');
+      return _CheckNeedsReleaseResult.failure;
     }
 
     if (pubspec.name == null) {
-      printErrorAndExit(errorMessage: 'Fatal: Package name is null.');
+      _print('Fatal: Package name is null.');
+      return _CheckNeedsReleaseResult.failure;
     }
     // Get latest tagged version and compare with the current version.
-    final String latestTag = existingTags.isNotEmpty
-        ? existingTags
-            .firstWhere((String tag) => tag.split('-v').first == pubspec.name)
-        : '';
+    // TODO(cyanglaz): Check latest version of the package on pub instead of git
+    // https://github.com/flutter/flutter/issues/81047
+
+    final String latestTag = existingTags.firstWhere(
+        (String tag) => tag.split('-v').first == pubspec.name,
+        orElse: () => '');
     if (latestTag.isNotEmpty) {
       final String latestTaggedVersion = latestTag.split('-v').last;
       final Version latestVersion = Version.parse(latestTaggedVersion);
       if (pubspec.version < latestVersion) {
         _print(
             'The new version (${pubspec.version}) is lower than the current version ($latestVersion) for ${pubspec.name}.\nThis git commit is a revert, no release is tagged.');
-        return false;
+        return _CheckNeedsReleaseResult.noRelease;
       }
     }
-    return true;
+    return _CheckNeedsReleaseResult.release;
   }
 
-  Future<void> _publishPlugin({@required Directory packageDir}) async {
-    await _checkGitStatus(packageDir);
-    await _publish(packageDir);
+  // Publish the plugin.
+  //
+  // Returns `true` if successful, `false` otherwise.
+  Future<bool> _publishPlugin({@required Directory packageDir}) async {
+    final bool gitStatusOK = await _checkGitStatus(packageDir);
+    if (!gitStatusOK) {
+      return false;
+    }
+    final bool publishOK = await _publish(packageDir);
+    if (!publishOK) {
+      return false;
+    }
     _print('Package published!');
+    return true;
   }
 
-  Future<void> _tagRelease(
-      {@required Directory packageDir,
-      @required String remote,
-      @required String remoteUrl,
-      @required bool shouldPushTag}) async {
+  // Tag the release with <plugin-name>-v<version>
+  //
+  // Return `true` if successful, `false` otherwise.
+  Future<bool> _tagRelease({
+    @required Directory packageDir,
+    @required String remote,
+    @required String remoteUrl,
+    @required bool shouldPushTag,
+  }) async {
     final String tag = _getTag(packageDir);
-    if (argResults[_dryRunFlag] as bool) {
-      _print('DRY RUN: Tagging release $tag...');
-      if (!shouldPushTag) {
-        return;
+    _print('Tagging release $tag...');
+    try {
+      if (!(argResults[_dryRunFlag] as bool)) {
+        await processRunner.runAndExitOnError(
+          'git',
+          <String>['tag', tag],
+          workingDir: packageDir,
+        );
       }
-      _print('DRY RUN: Pushing tag to $remote...');
-      return;
+    } on ToolExit catch (e) {
+      _print(e);
+      return false;
     }
-    _print('Tagging release $tag...');
-    await processRunner.runAndExitOnError('git', <String>['tag', tag],
-        workingDir: packageDir);
     if (!shouldPushTag) {
-      return;
+      return true;
     }
 
     _print('Pushing tag to $remote...');
-    await _pushTagToRemote(remote: remote, tag: tag, remoteUrl: remoteUrl);
+    return await _pushTagToRemote(
+      remote: remote,
+      tag: tag,
+      remoteUrl: remoteUrl,
+    );
   }
 
-  Future<void> _finishSuccesfully() async {
+  Future<void> _finish(bool successful) async {
     if (_stdinSubscription != null) {
       await _stdinSubscription.cancel();
+      _stdinSubscription = null;
+    }
+    if (successful) {
+      _print('Done!');
+    } else {
+      _print('Failed, see above for details.');
+      throw ToolExit(1);
     }
-    _print('Done!');
   }
 
   // Returns the packageDirectory based on the package name.
@@ -305,8 +371,10 @@ class PublishPluginCommand extends PluginCommand {
     return packageDir;
   }
 
-  Future<void> _checkGitStatus(Directory packageDir) async {
-    final io.ProcessResult statusResult = await processRunner.runAndExitOnError(
+  Future<bool> _checkGitStatus(Directory packageDir) async {
+    io.ProcessResult statusResult;
+    try {
+      statusResult = await processRunner.runAndExitOnError(
         'git',
         <String>[
           'status',
@@ -314,7 +382,12 @@ class PublishPluginCommand extends PluginCommand {
           '--ignored',
           packageDir.absolute.path
         ],
-        workingDir: packageDir);
+        workingDir: packageDir,
+      );
+    } on ToolExit catch (e) {
+      _print(e);
+      return false;
+    }
 
     final String statusOutput = statusResult.stdout as String;
     if (statusOutput.isNotEmpty) {
@@ -322,8 +395,8 @@ class PublishPluginCommand extends PluginCommand {
           "There are files in the package directory that haven't been saved in git. Refusing to publish these files:\n\n"
           '$statusOutput\n'
           'If the directory should be clean, you can run `git clean -xdf && git reset --hard HEAD` to wipe all local changes.');
-      throw ToolExit(1);
     }
+    return statusOutput.isEmpty;
   }
 
   Future<String> _verifyRemote(String remote) async {
@@ -333,39 +406,31 @@ class PublishPluginCommand extends PluginCommand {
     return remoteInfo.stdout as String;
   }
 
-  Future<void> _publish(Directory packageDir) async {
+  Future<bool> _publish(Directory packageDir) async {
     final List<String> publishFlags =
         argResults[_pubFlagsOption] as List<String>;
-    if (argResults[_dryRunFlag] as bool) {
-      _print(
-          'DRY RUN: Running `pub publish ${publishFlags.join(' ')}` in ${packageDir.absolute.path}...\n');
-      return;
-    }
-
     _print(
         'Running `pub publish ${publishFlags.join(' ')}` in ${packageDir.absolute.path}...\n');
-    final io.Process publish = await processRunner.start(
-        'flutter', <String>['pub', 'publish'] + publishFlags,
-        workingDirectory: packageDir);
-
-    if (!_startedListenToStdStream) {
+    if (!(argResults[_dryRunFlag] as bool)) {
+      final io.Process publish = await processRunner.start(
+          'flutter', <String>['pub', 'publish'] + publishFlags,
+          workingDirectory: packageDir);
       publish.stdout
           .transform(utf8.decoder)
           .listen((String data) => _print(data));
       publish.stderr
           .transform(utf8.decoder)
           .listen((String data) => _print(data));
-      _stdinSubscription = _stdin
+      _stdinSubscription ??= _stdin
           .transform(utf8.decoder)
           .listen((String data) => publish.stdin.writeln(data));
-
-      _startedListenToStdStream = true;
-    }
-    final int result = await publish.exitCode;
-    if (result != 0) {
-      _print('Publish failed. Exiting.');
-      throw ToolExit(result);
+      final int result = await publish.exitCode;
+      if (result != 0) {
+        _print('Publish ${packageDir.basename} failed.');
+        return false;
+      }
     }
+    return true;
   }
 
   String _getTag(Directory packageDir) {
@@ -382,18 +447,42 @@ class PublishPluginCommand extends PluginCommand {
         .replaceAll('%VERSION%', version);
   }
 
-  Future<void> _pushTagToRemote(
-      {@required String remote,
-      @required String tag,
-      @required String remoteUrl}) async {
+  // Pushes the `tag` to `remote`
+  //
+  // Return `true` if successful, `false` otherwise.
+  Future<bool> _pushTagToRemote({
+    @required String remote,
+    @required String tag,
+    @required String remoteUrl,
+  }) async {
     assert(remote != null && tag != null && remoteUrl != null);
     _print('Ready to push $tag to $remoteUrl (y/n)?');
     final String input = _stdin.readLineSync();
     if (input.toLowerCase() != 'y') {
       _print('Tag push canceled.');
-      throw ToolExit(1);
+      return false;
     }
-    await processRunner.runAndExitOnError('git', <String>['push', remote, tag],
-        workingDir: packagesDir);
+    try {
+      if (!(argResults[_dryRunFlag] as bool)) {
+        await processRunner.runAndExitOnError(
+            'git', <String>['push', remote, tag],
+            workingDir: packagesDir);
+      }
+    } on ToolExit catch (e) {
+      _print(e);
+      return false;
+    }
+    return true;
   }
 }
+
+enum _CheckNeedsReleaseResult {
+  // The package needs to be released.
+  release,
+
+  // The package does not need to be released.
+  noRelease,
+
+  // There's an error when trying to access if the package needs to be released.
+  failure,
+}
diff --git a/script/tool/test/publish_plugin_command_test.dart b/script/tool/test/publish_plugin_command_test.dart
index 6b89286700d7..27071c9f75a0 100644
--- a/script/tool/test/publish_plugin_command_test.dart
+++ b/script/tool/test/publish_plugin_command_test.dart
@@ -96,9 +96,11 @@ void main() {
           throwsA(const TypeMatcher<ToolExit>()));
 
       expect(
-          printedMessages.last,
-          contains(
-              "There are files in the package directory that haven't been saved in git."));
+          printedMessages,
+          containsAllInOrder(<String>[
+            'There are files in the package directory that haven\'t been saved in git. Refusing to publish these files:\n\n?? foo/tmp\n\nIf the directory should be clean, you can run `git clean -xdf && git reset --hard HEAD` to wipe all local changes.',
+            'Failed, see above for details.',
+          ]));
     });
 
     test('fails immediately if the remote doesn\'t exist', () async {
@@ -111,7 +113,7 @@ void main() {
 
     test("doesn't validate the remote if it's not pushing tags", () async {
       // Immediately return 0 when running `pub publish`.
-      processRunner.mockPublishProcess.exitCodeCompleter.complete(0);
+      processRunner.mockPublishCompleteCode = 0;
 
       await commandRunner.run(<String>[
         'publish-plugin',
@@ -132,7 +134,7 @@ void main() {
       await gitDir.runCommand(<String>['add', '-A']);
       await gitDir.runCommand(<String>['commit', '-m', 'Initial commit']);
       // Immediately return 0 when running `pub publish`.
-      processRunner.mockPublishProcess.exitCodeCompleter.complete(0);
+      processRunner.mockPublishCompleteCode = 0;
       await commandRunner.run(<String>[
         'publish-plugin',
         '--package',
@@ -153,9 +155,10 @@ void main() {
         '--no-push-tags',
         '--no-tag-release'
       ]);
-      processRunner.mockPublishProcess.stdoutController.add(utf8.encode('Foo'));
-      processRunner.mockPublishProcess.stderrController.add(utf8.encode('Bar'));
-      processRunner.mockPublishProcess.exitCodeCompleter.complete(0);
+
+      processRunner.mockPublishStdout = 'Foo';
+      processRunner.mockPublishStderr = 'Bar';
+      processRunner.mockPublishCompleteCode = 0;
 
       await publishCommand;
 
@@ -171,8 +174,8 @@ void main() {
         '--no-push-tags',
         '--no-tag-release'
       ]);
-      mockStdin.controller.add(utf8.encode('user input'));
-      processRunner.mockPublishProcess.exitCodeCompleter.complete(0);
+      mockStdin.mockUserInputs.add(utf8.encode('user input'));
+      processRunner.mockPublishCompleteCode = 0;
 
       await publishCommand;
 
@@ -181,7 +184,7 @@ void main() {
     });
 
     test('forwards --pub-publish-flags to pub publish', () async {
-      processRunner.mockPublishProcess.exitCodeCompleter.complete(0);
+      processRunner.mockPublishCompleteCode = 0;
       await commandRunner.run(<String>[
         'publish-plugin',
         '--package',
@@ -200,7 +203,7 @@ void main() {
     });
 
     test('throws if pub publish fails', () async {
-      processRunner.mockPublishProcess.exitCodeCompleter.complete(128);
+      processRunner.mockPublishCompleteCode = 128;
       await expectLater(
           () => commandRunner.run(<String>[
                 'publish-plugin',
@@ -211,12 +214,12 @@ void main() {
               ]),
           throwsA(const TypeMatcher<ToolExit>()));
 
-      expect(printedMessages, contains('Publish failed. Exiting.'));
+      expect(printedMessages, contains('Publish foo failed.'));
     });
 
     test('publish, dry run', () async {
       // Immediately return 1 when running `pub publish`. If dry-run does not work, test should throw.
-      processRunner.mockPublishProcess.exitCodeCompleter.complete(1);
+      processRunner.mockPublishCompleteCode = 1;
       await commandRunner.run(<String>[
         'publish-plugin',
         '--package',
@@ -230,7 +233,8 @@ void main() {
       expect(
           printedMessages,
           containsAllInOrder(<String>[
-            'DRY RUN: Running `pub publish ` in ${pluginDir.path}...\n',
+            '===============  DRY RUN ===============',
+            'Running `pub publish ` in ${pluginDir.path}...\n',
             'Done!'
           ]));
     });
@@ -238,7 +242,7 @@ void main() {
 
   group('Tags release', () {
     test('with the version and name from the pubspec.yaml', () async {
-      processRunner.mockPublishProcess.exitCodeCompleter.complete(0);
+      processRunner.mockPublishCompleteCode = 0;
       await commandRunner.run(<String>[
         'publish-plugin',
         '--package',
@@ -253,7 +257,7 @@ void main() {
     });
 
     test('only if publishing succeeded', () async {
-      processRunner.mockPublishProcess.exitCodeCompleter.complete(128);
+      processRunner.mockPublishCompleteCode = 128;
       await expectLater(
           () => commandRunner.run(<String>[
                 'publish-plugin',
@@ -263,7 +267,7 @@ void main() {
               ]),
           throwsA(const TypeMatcher<ToolExit>()));
 
-      expect(printedMessages, contains('Publish failed. Exiting.'));
+      expect(printedMessages, contains('Publish foo failed.'));
       final String tag = (await gitDir.runCommand(
               <String>['show-ref', 'fake_package-v0.0.1'],
               throwOnError: false))
@@ -279,7 +283,7 @@ void main() {
     });
 
     test('requires user confirmation', () async {
-      processRunner.mockPublishProcess.exitCodeCompleter.complete(0);
+      processRunner.mockPublishCompleteCode = 0;
       mockStdin.readLineOutput = 'help';
       await expectLater(
           () => commandRunner.run(<String>[
@@ -294,7 +298,7 @@ void main() {
 
     test('to upstream by default', () async {
       await gitDir.runCommand(<String>['tag', 'garbage']);
-      processRunner.mockPublishProcess.exitCodeCompleter.complete(0);
+      processRunner.mockPublishCompleteCode = 0;
       mockStdin.readLineOutput = 'y';
       await commandRunner.run(<String>[
         'publish-plugin',
@@ -311,7 +315,7 @@ void main() {
     test('to upstream by default, dry run', () async {
       await gitDir.runCommand(<String>['tag', 'garbage']);
       // Immediately return 1 when running `pub publish`. If dry-run does not work, test should throw.
-      processRunner.mockPublishProcess.exitCodeCompleter.complete(1);
+      processRunner.mockPublishCompleteCode = 1;
       mockStdin.readLineOutput = 'y';
       await commandRunner.run(
           <String>['publish-plugin', '--package', testPluginName, '--dry-run']);
@@ -320,9 +324,10 @@ void main() {
       expect(
           printedMessages,
           containsAllInOrder(<String>[
-            'DRY RUN: Running `pub publish ` in ${pluginDir.path}...\n',
-            'DRY RUN: Tagging release fake_package-v0.0.1...',
-            'DRY RUN: Pushing tag to upstream...',
+            '===============  DRY RUN ===============',
+            'Running `pub publish ` in ${pluginDir.path}...\n',
+            'Tagging release fake_package-v0.0.1...',
+            'Pushing tag to upstream...',
             'Done!'
           ]));
     });
@@ -330,7 +335,7 @@ void main() {
     test('to different remotes based on a flag', () async {
       await gitDir.runCommand(
           <String>['remote', 'add', 'origin', 'http://localhost:8001']);
-      processRunner.mockPublishProcess.exitCodeCompleter.complete(0);
+      processRunner.mockPublishCompleteCode = 0;
       mockStdin.readLineOutput = 'y';
       await commandRunner.run(<String>[
         'publish-plugin',
@@ -347,7 +352,7 @@ void main() {
     });
 
     test('only if tagging and pushing to remotes are both enabled', () async {
-      processRunner.mockPublishProcess.exitCodeCompleter.complete(0);
+      processRunner.mockPublishCompleteCode = 0;
       await commandRunner.run(<String>[
         'publish-plugin',
         '--package',
@@ -360,7 +365,7 @@ void main() {
     });
   });
 
-  group('Auto release (all flag)', () {
+  group('Auto release (all-changed flag)', () {
     setUp(() async {
       io.Process.runSync('git', <String>['init'],
           workingDirectory: mockPackagesDir.path);
@@ -391,10 +396,72 @@ void main() {
       await gitDir.runCommand(<String>['add', '-A']);
       await gitDir.runCommand(<String>['commit', '-m', 'Add plugins']);
       // Immediately return 0 when running `pub publish`.
-      processRunner.mockPublishProcess.exitCodeCompleter.complete(0);
+      processRunner.mockPublishCompleteCode = 0;
       mockStdin.readLineOutput = 'y';
       await commandRunner
-          .run(<String>['publish-plugin', '--all', '--base-sha=HEAD~']);
+          .run(<String>['publish-plugin', '--all-changed', '--base-sha=HEAD~']);
+      expect(
+          printedMessages,
+          containsAllInOrder(<String>[
+            'Checking local repo...',
+            'Local repo is ready!',
+            'Getting existing tags...',
+            'Running `pub publish ` in ${pluginDir1.path}...\n',
+            'Running `pub publish ` in ${pluginDir2.path}...\n',
+            'Packages released: plugin1, plugin2',
+            'Done!'
+          ]));
+      expect(processRunner.pushTagsArgs, isNotEmpty);
+      expect(processRunner.pushTagsArgs[0], 'push');
+      expect(processRunner.pushTagsArgs[1], 'upstream');
+      expect(processRunner.pushTagsArgs[2], 'plugin1-v0.0.1');
+      expect(processRunner.pushTagsArgs[3], 'push');
+      expect(processRunner.pushTagsArgs[4], 'upstream');
+      expect(processRunner.pushTagsArgs[5], 'plugin2-v0.0.1');
+    });
+
+    test('can release newly created plugins, while there are existing plugins',
+        () async {
+      // Prepare an exiting plugin and tag it
+      final Directory pluginDir0 = createFakePlugin('plugin0',
+          withSingleExample: true, packagesDirectory: mockPackagesDir);
+      createFakePubspec(pluginDir0,
+          name: 'plugin0',
+          includeVersion: true,
+          isFlutter: false,
+          version: '0.0.1');
+      await gitDir.runCommand(<String>['add', '-A']);
+      await gitDir.runCommand(<String>['commit', '-m', 'Add plugins']);
+      // Immediately return 0 when running `pub publish`.
+      processRunner.mockPublishCompleteCode = 0;
+      mockStdin.readLineOutput = 'y';
+      await commandRunner
+          .run(<String>['publish-plugin', '--all-changed', '--base-sha=HEAD~']);
+      processRunner.pushTagsArgs.clear();
+
+      // Non-federated
+      final Directory pluginDir1 = createFakePlugin('plugin1',
+          withSingleExample: true, packagesDirectory: mockPackagesDir);
+      // federated
+      final Directory pluginDir2 = createFakePlugin('plugin2',
+          withSingleExample: true,
+          parentDirectoryName: 'plugin2',
+          packagesDirectory: mockPackagesDir);
+      createFakePubspec(pluginDir1,
+          name: 'plugin1',
+          includeVersion: true,
+          isFlutter: false,
+          version: '0.0.1');
+      createFakePubspec(pluginDir2,
+          name: 'plugin2',
+          includeVersion: true,
+          isFlutter: false,
+          version: '0.0.1');
+      await gitDir.runCommand(<String>['add', '-A']);
+      await gitDir.runCommand(<String>['commit', '-m', 'Add plugins']);
+      // Immediately return 0 when running `pub publish`.
+      await commandRunner
+          .run(<String>['publish-plugin', '--all-changed', '--base-sha=HEAD~']);
       expect(
           printedMessages,
           containsAllInOrder(<String>[
@@ -437,22 +504,27 @@ void main() {
       await gitDir.runCommand(<String>['add', '-A']);
       await gitDir.runCommand(<String>['commit', '-m', 'Add plugins']);
       // Immediately return 1 when running `pub publish`. If dry-run does not work, test should throw.
-      processRunner.mockPublishProcess.exitCodeCompleter.complete(1);
+      processRunner.mockPublishCompleteCode = 1;
       mockStdin.readLineOutput = 'y';
-      await commandRunner.run(
-          <String>['publish-plugin', '--all', '--base-sha=HEAD~', '--dry-run']);
+      await commandRunner.run(<String>[
+        'publish-plugin',
+        '--all-changed',
+        '--base-sha=HEAD~',
+        '--dry-run'
+      ]);
       expect(
           printedMessages,
           containsAllInOrder(<String>[
             'Checking local repo...',
             'Local repo is ready!',
+            '===============  DRY RUN ===============',
             'Getting existing tags...',
-            'DRY RUN: Running `pub publish ` in ${pluginDir1.path}...\n',
-            'DRY RUN: Tagging release plugin1-v0.0.1...',
-            'DRY RUN: Pushing tag to upstream...',
-            'DRY RUN: Running `pub publish ` in ${pluginDir2.path}...\n',
-            'DRY RUN: Tagging release plugin2-v0.0.1...',
-            'DRY RUN: Pushing tag to upstream...',
+            'Running `pub publish ` in ${pluginDir1.path}...\n',
+            'Tagging release plugin1-v0.0.1...',
+            'Pushing tag to upstream...',
+            'Running `pub publish ` in ${pluginDir2.path}...\n',
+            'Tagging release plugin2-v0.0.1...',
+            'Pushing tag to upstream...',
             'Packages released: plugin1, plugin2',
             'Done!'
           ]));
@@ -481,10 +553,10 @@ void main() {
       await gitDir.runCommand(<String>['add', '-A']);
       await gitDir.runCommand(<String>['commit', '-m', 'Add plugins']);
       // Immediately return 0 when running `pub publish`.
-      processRunner.mockPublishProcess.exitCodeCompleter.complete(0);
+      processRunner.mockPublishCompleteCode = 0;
       mockStdin.readLineOutput = 'y';
       await commandRunner
-          .run(<String>['publish-plugin', '--all', '--base-sha=HEAD~']);
+          .run(<String>['publish-plugin', '--all-changed', '--base-sha=HEAD~']);
       expect(
           printedMessages,
           containsAllInOrder(<String>[
@@ -526,7 +598,7 @@ void main() {
           .runCommand(<String>['commit', '-m', 'Update versions to 0.0.2']);
 
       await commandRunner
-          .run(<String>['publish-plugin', '--all', '--base-sha=HEAD~']);
+          .run(<String>['publish-plugin', '--all-changed', '--base-sha=HEAD~']);
       expect(
           printedMessages,
           containsAllInOrder(<String>[
@@ -548,6 +620,94 @@ void main() {
       expect(processRunner.pushTagsArgs[5], 'plugin2-v0.0.2');
     });
 
+    test(
+        'delete package will not trigger publish but exit the command successfully.',
+        () async {
+      // Non-federated
+      final Directory pluginDir1 = createFakePlugin('plugin1',
+          withSingleExample: true, packagesDirectory: mockPackagesDir);
+      // federated
+      final Directory pluginDir2 = createFakePlugin('plugin2',
+          withSingleExample: true,
+          parentDirectoryName: 'plugin2',
+          packagesDirectory: mockPackagesDir);
+      createFakePubspec(pluginDir1,
+          name: 'plugin1',
+          includeVersion: true,
+          isFlutter: false,
+          version: '0.0.1');
+      createFakePubspec(pluginDir2,
+          name: 'plugin2',
+          includeVersion: true,
+          isFlutter: false,
+          version: '0.0.1');
+      await gitDir.runCommand(<String>['add', '-A']);
+      await gitDir.runCommand(<String>['commit', '-m', 'Add plugins']);
+      // Immediately return 0 when running `pub publish`.
+      processRunner.mockPublishCompleteCode = 0;
+      mockStdin.readLineOutput = 'y';
+      await commandRunner
+          .run(<String>['publish-plugin', '--all-changed', '--base-sha=HEAD~']);
+      expect(
+          printedMessages,
+          containsAllInOrder(<String>[
+            'Checking local repo...',
+            'Local repo is ready!',
+            'Getting existing tags...',
+            'Running `pub publish ` in ${pluginDir1.path}...\n',
+            'Running `pub publish ` in ${pluginDir2.path}...\n',
+            'Packages released: plugin1, plugin2',
+            'Done!'
+          ]));
+      expect(processRunner.pushTagsArgs, isNotEmpty);
+      expect(processRunner.pushTagsArgs[0], 'push');
+      expect(processRunner.pushTagsArgs[1], 'upstream');
+      expect(processRunner.pushTagsArgs[2], 'plugin1-v0.0.1');
+      expect(processRunner.pushTagsArgs[3], 'push');
+      expect(processRunner.pushTagsArgs[4], 'upstream');
+      expect(processRunner.pushTagsArgs[5], 'plugin2-v0.0.1');
+
+      processRunner.pushTagsArgs.clear();
+      printedMessages.clear();
+
+      final List<String> plugin1Pubspec =
+          pluginDir1.childFile('pubspec.yaml').readAsLinesSync();
+      plugin1Pubspec[plugin1Pubspec.indexWhere(
+          (element) => element.contains('version:'))] = 'version: 0.0.2';
+      pluginDir1
+          .childFile('pubspec.yaml')
+          .writeAsStringSync(plugin1Pubspec.join('\n'));
+
+      pluginDir2.deleteSync(recursive: true);
+
+      await gitDir.runCommand(<String>['add', '-A']);
+      await gitDir.runCommand(<String>[
+        'commit',
+        '-m',
+        'Update plugin1 versions to 0.0.2, delete plugin2'
+      ]);
+
+      await commandRunner
+          .run(<String>['publish-plugin', '--all-changed', '--base-sha=HEAD~']);
+      expect(
+          printedMessages,
+          containsAllInOrder(<String>[
+            'Checking local repo...',
+            'Local repo is ready!',
+            'Getting existing tags...',
+            'Running `pub publish ` in ${pluginDir1.path}...\n',
+            'The file at 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',
+            'Packages released: plugin1',
+            'Done!'
+          ]));
+
+      expect(processRunner.pushTagsArgs, isNotEmpty);
+      expect(processRunner.pushTagsArgs.length, 3);
+      expect(processRunner.pushTagsArgs[0], 'push');
+      expect(processRunner.pushTagsArgs[1], 'upstream');
+      expect(processRunner.pushTagsArgs[2], 'plugin1-v0.0.2');
+    });
+
     test(
         'versions revert do not trigger releases. Also prints out warning message.',
         () async {
@@ -572,10 +732,10 @@ void main() {
       await gitDir.runCommand(<String>['add', '-A']);
       await gitDir.runCommand(<String>['commit', '-m', 'Add plugins']);
       // Immediately return 0 when running `pub publish`.
-      processRunner.mockPublishProcess.exitCodeCompleter.complete(0);
+      processRunner.mockPublishCompleteCode = 0;
       mockStdin.readLineOutput = 'y';
       await commandRunner
-          .run(<String>['publish-plugin', '--all', '--base-sha=HEAD~']);
+          .run(<String>['publish-plugin', '--all-changed', '--base-sha=HEAD~']);
       expect(
           printedMessages,
           containsAllInOrder(<String>[
@@ -617,7 +777,7 @@ void main() {
           .runCommand(<String>['commit', '-m', 'Update versions to 0.0.1']);
 
       await commandRunner
-          .run(<String>['publish-plugin', '--all', '--base-sha=HEAD~']);
+          .run(<String>['publish-plugin', '--all-changed', '--base-sha=HEAD~']);
       expect(
           printedMessages,
           containsAllInOrder(<String>[
@@ -664,16 +824,16 @@ void main() {
       await gitDir.runCommand(<String>['commit', '-m', 'Add dart files']);
 
       // Immediately return 0 when running `pub publish`.
-      processRunner.mockPublishProcess.exitCodeCompleter.complete(0);
+      processRunner.mockPublishCompleteCode = 0;
       mockStdin.readLineOutput = 'y';
       await commandRunner
-          .run(<String>['publish-plugin', '--all', '--base-sha=HEAD~']);
+          .run(<String>['publish-plugin', '--all-changed', '--base-sha=HEAD~']);
       expect(
           printedMessages,
           containsAllInOrder(<String>[
             'Checking local repo...',
             'Local repo is ready!',
-            'No version updates in this commit, exiting...',
+            'No version updates in this commit.',
             'Done!'
           ]));
       expect(processRunner.pushTagsArgs, isEmpty);
@@ -683,11 +843,16 @@ void main() {
 
 class TestProcessRunner extends ProcessRunner {
   final List<io.ProcessResult> results = <io.ProcessResult>[];
-  final MockProcess mockPublishProcess = MockProcess();
+  // Most recent returned publish process.
+  MockProcess mockPublishProcess;
   final List<String> mockPublishArgs = <String>[];
   final MockProcessResult mockPushTagsResult = MockProcessResult();
   final List<String> pushTagsArgs = <String>[];
 
+  String mockPublishStdout;
+  String mockPublishStderr;
+  int mockPublishCompleteCode;
+
   @override
   Future<io.ProcessResult> runAndExitOnError(
     String executable,
@@ -719,23 +884,42 @@ class TestProcessRunner extends ProcessRunner {
         args[0] == 'pub' &&
         args[1] == 'publish');
     mockPublishArgs.addAll(args);
+    mockPublishProcess = MockProcess();
+    if (mockPublishStdout != null) {
+      mockPublishProcess.stdoutController.add(utf8.encode(mockPublishStdout));
+    }
+    if (mockPublishStderr != null) {
+      mockPublishProcess.stderrController.add(utf8.encode(mockPublishStderr));
+    }
+    if (mockPublishCompleteCode != null) {
+      mockPublishProcess.exitCodeCompleter.complete(mockPublishCompleteCode);
+    }
+
     return mockPublishProcess;
   }
 }
 
 class MockStdin extends Mock implements io.Stdin {
-  final StreamController<List<int>> controller = StreamController<List<int>>();
+  List<List<int>> mockUserInputs = <List<int>>[];
+  StreamController<List<int>> _controller;
   String readLineOutput;
 
   @override
   Stream<S> transform<S>(StreamTransformer<List<int>, S> streamTransformer) {
-    return controller.stream.transform(streamTransformer);
+    // In the test context, only one `PublishPluginCommand` object is created for a single test case.
+    // However, sometimes, we need to run multiple commands in a single test case.
+    // In such situation, this `MockStdin`'s StreamController might be listened to more than once, which is not allowed.
+    //
+    // Create a new controller every time so this Stdin could be listened to multiple times.
+    _controller = StreamController<List<int>>();
+    mockUserInputs.forEach((List<int> element) => _controller.add(element));
+    return _controller.stream.transform(streamTransformer);
   }
 
   @override
   StreamSubscription<List<int>> listen(void onData(List<int> event),
       {Function onError, void onDone(), bool cancelOnError}) {
-    return controller.stream.listen(onData,
+    return _controller.stream.listen(onData,
         onError: onError, onDone: onDone, cancelOnError: cancelOnError);
   }
 

From b54688b91a019d4c7089f6d0145c11712e4cf9c3 Mon Sep 17 00:00:00 2001
From: Chris Yang <ychris@google.com>
Date: Fri, 30 Apr 2021 14:26:52 -0700
Subject: [PATCH 3/3] review

---
 script/tool/lib/src/publish_plugin_command.dart | 10 ++++------
 script/tool/test/util.dart                      | 10 +++++-----
 2 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/script/tool/lib/src/publish_plugin_command.dart b/script/tool/lib/src/publish_plugin_command.dart
index 031de58f0bf2..9bfa0e71743a 100644
--- a/script/tool/lib/src/publish_plugin_command.dart
+++ b/script/tool/lib/src/publish_plugin_command.dart
@@ -215,7 +215,7 @@ class PublishPluginCommand extends PluginCommand {
     }
     if (packagesFailed.isNotEmpty) {
       _print(
-          'Packages failed to release: ${packagesFailed.join(', ')}, see above for details.');
+          'Failed to release the following packages: ${packagesFailed.join(', ')}, see above for details.');
     }
     return packagesFailed.isEmpty;
   }
@@ -247,7 +247,7 @@ class PublishPluginCommand extends PluginCommand {
     return true;
   }
 
-  // Returns `true` if needs to release the version, `false` if needs to skip
+  // Returns a [_CheckNeedsReleaseResult] that indicates the result.
   Future<_CheckNeedsReleaseResult> _checkNeedsRelease({
     @required File pubspecFile,
     @required GitVersionFinder gitVersionFinder,
@@ -267,9 +267,7 @@ Safe to ignore if the package is deleted in this commit.
     }
 
     if (pubspec.version == null) {
-      _print('No version found. A package that '
-          'intentionally has no version should be marked '
-          '"publish_to: none".');
+      _print('No version found. A package that intentionally has no version should be marked "publish_to: none"');
       return _CheckNeedsReleaseResult.failure;
     }
 
@@ -484,6 +482,6 @@ enum _CheckNeedsReleaseResult {
   // The package does not need to be released.
   noRelease,
 
-  // There's an error when trying to access if the package needs to be released.
+  // There's an error when trying to determine whether the package needs to be released.
   failure,
 }
diff --git a/script/tool/test/util.dart b/script/tool/test/util.dart
index 40d3db2964bc..96e00d3bb5ec 100644
--- a/script/tool/test/util.dart
+++ b/script/tool/test/util.dart
@@ -87,14 +87,14 @@ Directory createFakePlugin(
     final Directory exampleDir = pluginDirectory.childDirectory('example')
       ..createSync();
     createFakePubspec(exampleDir,
-        name: '${name}_example', isFlutter: isFlutter, includeVersion: false, publish_to: 'none');
+        name: '${name}_example', isFlutter: isFlutter, includeVersion: false, publishTo: 'none');
   } else if (withExamples.isNotEmpty) {
     final Directory exampleDir = pluginDirectory.childDirectory('example')
       ..createSync();
     for (final String example in withExamples) {
       final Directory currentExample = exampleDir.childDirectory(example)
         ..createSync();
-      createFakePubspec(currentExample, name: example, isFlutter: isFlutter, includeVersion: false, publish_to: 'none');
+      createFakePubspec(currentExample, name: example, isFlutter: isFlutter, includeVersion: false, publishTo: 'none');
     }
   }
 
@@ -125,7 +125,7 @@ void createFakePubspec(
   bool isLinuxPlugin = false,
   bool isMacOsPlugin = false,
   bool isWindowsPlugin = false,
-  String publish_to = 'http://no_pub_server.com',
+  String publishTo = 'http://no_pub_server.com',
   String version = '0.0.1',
 }) {
   parent.childFile('pubspec.yaml').createSync();
@@ -185,9 +185,9 @@ dependencies:
 version: $version
 ''';
   }
-  if (publish_to.isNotEmpty) {
+  if (publishTo.isNotEmpty) {
      yaml += '''
-publish_to: $publish_to # Hardcoded safeguard to prevent this from somehow being published by a broken test.
+publish_to: $publishTo # Hardcoded safeguard to prevent this from somehow being published by a broken test.
 ''';
   }
   parent.childFile('pubspec.yaml').writeAsStringSync(yaml);