[tool] combine run and runAndExitOnError (#3827)

Author: Chris Yang

script/tool/lib/src/common.dart

@@ -500,17 +500,33 @@ class ProcessRunner {
   ///
   /// If [exitOnError] is set to `true`, then this will throw an error if
   /// the [executable] terminates with a non-zero exit code.
+  /// Defaults to `false`.
+  ///
+  /// If [logOnError] is set to `true`, it will print a formatted message about the error.
+  /// Defaults to `false`
   ///
   /// Returns the [io.ProcessResult] of the [executable].
   Future<io.ProcessResult> run(String executable, List<String> args,
       {Directory workingDir,
       bool exitOnError = false,
+      bool logOnError = false,
       Encoding stdoutEncoding = io.systemEncoding,
       Encoding stderrEncoding = io.systemEncoding}) async {
-    return io.Process.run(executable, args,
+    final io.ProcessResult result = await io.Process.run(executable, args,
         workingDirectory: workingDir?.path,
         stdoutEncoding: stdoutEncoding,
         stderrEncoding: stderrEncoding);
+    if (result.exitCode != 0) {
+      if (logOnError) {
+        final String error =
+            _getErrorString(executable, args, workingDir: workingDir);
+        print('$error Stderr:\n${result.stdout}');
+      }
+      if (exitOnError) {
+        throw ToolExit(result.exitCode);
+      }
+    }
+    return result;
   }
 
   /// Starts the [executable] with [args].
@@ -526,31 +542,6 @@ class ProcessRunner {
     return process;
   }
 
-  /// Run the [executable] with [args], throwing an error on non-zero exit code.
-  ///
-  /// Unlike [runAndStream], this does not stream the process output to stdout.
-  /// It also unconditionally throws an error on a non-zero exit code.
-  ///
-  /// The current working directory of [executable] can be overridden by
-  /// passing [workingDir].
-  ///
-  /// Returns the [io.ProcessResult] of running the [executable].
-  Future<io.ProcessResult> runAndExitOnError(
-    String executable,
-    List<String> args, {
-    Directory workingDir,
-  }) async {
-    final io.ProcessResult result = await io.Process.run(executable, args,
-        workingDirectory: workingDir?.path);
-    if (result.exitCode != 0) {
-      final String error =
-          _getErrorString(executable, args, workingDir: workingDir);
-      print('$error Stderr:\n${result.stdout}');
-      throw ToolExit(result.exitCode);
-    }
-    return result;
-  }
-
   String _getErrorString(String executable, List<String> args,
       {Directory workingDir}) {
     final String workdir = workingDir == null ? '' : ' in ${workingDir.path}';

script/tool/lib/src/firebase_test_lab_command.dart

@@ -73,11 +73,16 @@ class FirebaseTestLabCommand extends PluginCommand {
     } else {
       _firebaseProjectConfigured = Completer<void>();
     }
-    await processRunner.runAndExitOnError('gcloud', <String>[
-      'auth',
-      'activate-service-account',
-      '--key-file=${argResults['service-key']}',
-    ]);
+    await processRunner.run(
+      'gcloud',
+      <String>[
+        'auth',
+        'activate-service-account',
+        '--key-file=${argResults['service-key']}',
+      ],
+      exitOnError: true,
+      logOnError: true,
+    );
     final int exitCode = await processRunner.runAndStream('gcloud', <String>[
       'config',
       'set',

script/tool/lib/src/format_command.dart

@@ -56,9 +56,13 @@ class FormatCommand extends PluginCommand {
   }
 
   Future<bool> _didModifyAnything() async {
-    final io.ProcessResult modifiedFiles = await processRunner
-        .runAndExitOnError('git', <String>['ls-files', '--modified'],
-            workingDir: packagesDir);
+    final io.ProcessResult modifiedFiles = await processRunner.run(
+      'git',
+      <String>['ls-files', '--modified'],
+      workingDir: packagesDir,
+      exitOnError: true,
+      logOnError: true,
+    );
 
     print('\n\n');
 
@@ -76,8 +80,13 @@ class FormatCommand extends PluginCommand {
         'this command into your terminal:');
 
     print('patch -p1 <<DONE');
-    final io.ProcessResult diff = await processRunner
-        .runAndExitOnError('git', <String>['diff'], workingDir: packagesDir);
+    final io.ProcessResult diff = await processRunner.run(
+      'git',
+      <String>['diff'],
+      workingDir: packagesDir,
+      exitOnError: true,
+      logOnError: true,
+    );
     print(diff.stdout);
     print('DONE');
     return true;

script/tool/lib/src/lint_podspecs_command.dart

@@ -58,8 +58,13 @@ class LintPodspecsCommand extends PluginCommand {
       return;
     }
 
-    await processRunner.runAndExitOnError('which', <String>['pod'],
-        workingDir: packagesDir);
+    await processRunner.run(
+      'which',
+      <String>['pod'],
+      workingDir: packagesDir,
+      exitOnError: true,
+      logOnError: true,
+    );
 
     _print('Starting podspec lint test');
 

script/tool/lib/src/publish_plugin_command.dart

@@ -136,8 +136,13 @@ class PublishPluginCommand extends PluginCommand {
       @required bool shouldPushTag}) async {
     final String tag = _getTag(packageDir);
     _print('Tagging release $tag...');
-    await processRunner.runAndExitOnError('git', <String>['tag', tag],
-        workingDir: packageDir);
+    await processRunner.run(
+      'git',
+      <String>['tag', tag],
+      workingDir: packageDir,
+      exitOnError: true,
+      logOnError: true,
+    );
     if (!shouldPushTag) {
       return;
     }
@@ -163,15 +168,13 @@ class PublishPluginCommand extends PluginCommand {
   }
 
   Future<void> _checkGitStatus(Directory packageDir) async {
-    final ProcessResult statusResult = await processRunner.runAndExitOnError(
-        'git',
-        <String>[
-          'status',
-          '--porcelain',
-          '--ignored',
-          packageDir.absolute.path
-        ],
-        workingDir: packageDir);
+    final ProcessResult statusResult = await processRunner.run(
+      'git',
+      <String>['status', '--porcelain', '--ignored', packageDir.absolute.path],
+      workingDir: packageDir,
+      logOnError: true,
+      exitOnError: true,
+    );
 
     final String statusOutput = statusResult.stdout as String;
     if (statusOutput.isNotEmpty) {
@@ -184,9 +187,13 @@ class PublishPluginCommand extends PluginCommand {
   }
 
   Future<String> _verifyRemote(String remote) async {
-    final ProcessResult remoteInfo = await processRunner.runAndExitOnError(
-        'git', <String>['remote', 'get-url', remote],
-        workingDir: packagesDir);
+    final ProcessResult remoteInfo = await processRunner.run(
+      'git',
+      <String>['remote', 'get-url', remote],
+      workingDir: packagesDir,
+      exitOnError: true,
+      logOnError: true,
+    );
     return remoteInfo.stdout as String;
   }
 
@@ -239,7 +246,12 @@ class PublishPluginCommand extends PluginCommand {
       _print('Tag push canceled.');
       throw ToolExit(1);
     }
-    await processRunner.runAndExitOnError('git', <String>['push', remote, tag],
-        workingDir: packagesDir);
+    await processRunner.run(
+      'git',
+      <String>['push', remote, tag],
+      workingDir: packagesDir,
+      exitOnError: true,
+      logOnError: true,
+    );
   }
 }

script/tool/test/publish_plugin_command_test.dart

@@ -328,10 +328,14 @@ class TestProcessRunner extends ProcessRunner {
   final List<String> pushTagsArgs = <String>[];
 
   @override
-  Future<io.ProcessResult> runAndExitOnError(
+  Future<io.ProcessResult> run(
     String executable,
     List<String> args, {
     Directory workingDir,
+    bool exitOnError = false,
+    bool logOnError = false,
+    Encoding stdoutEncoding = io.systemEncoding,
+    Encoding stderrEncoding = io.systemEncoding,
   }) async {
     // Don't ever really push tags.
     if (executable == 'git' && args.isNotEmpty && args[0] == 'push') {

script/tool/test/util.dart

@@ -235,32 +235,18 @@ class RecordingProcessRunner extends ProcessRunner {
 
   /// Returns [io.ProcessResult] created from [processToReturn], [resultStdout], and [resultStderr].
   @override
-  Future<io.ProcessResult> run(String executable, List<String> args,
-      {Directory workingDir,
-      bool exitOnError = false,
-      Encoding stdoutEncoding = io.systemEncoding,
-      Encoding stderrEncoding = io.systemEncoding}) async {
-    recordedCalls.add(ProcessCall(executable, args, workingDir?.path));
-    io.ProcessResult result;
-
-    if (processToReturn != null) {
-      result = io.ProcessResult(
-          processToReturn.pid,
-          await processToReturn.exitCode,
-          resultStdout ?? processToReturn.stdout,
-          resultStderr ?? processToReturn.stderr);
-    }
-    return Future<io.ProcessResult>.value(result);
-  }
-
-  @override
-  Future<io.ProcessResult> runAndExitOnError(
+  Future<io.ProcessResult> run(
     String executable,
     List<String> args, {
     Directory workingDir,
+    bool exitOnError = false,
+    bool logOnError = false,
+    Encoding stdoutEncoding = io.systemEncoding,
+    Encoding stderrEncoding = io.systemEncoding,
   }) async {
     recordedCalls.add(ProcessCall(executable, args, workingDir?.path));
     io.ProcessResult result;
+
     if (processToReturn != null) {
       result = io.ProcessResult(
           processToReturn.pid,