From 99ab7d0e45d288dea730e11f78afd7e6b74b3b2f Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Mon, 26 Apr 2021 11:52:24 -0700 Subject: [PATCH 1/4] [tool] combine run and runAndExitOnError --- script/tool/lib/src/common.dart | 43 +++++++----------- .../lib/src/firebase_test_lab_command.dart | 15 ++++--- script/tool/lib/src/format_command.dart | 19 +++++--- .../tool/lib/src/lint_podspecs_command.dart | 9 +++- .../tool/lib/src/publish_plugin_command.dart | 44 ++++++++++++------- script/tool/test/util.dart | 22 ++-------- 6 files changed, 79 insertions(+), 73 deletions(-) diff --git a/script/tool/lib/src/common.dart b/script/tool/lib/src/common.dart index d8826c793088..c4cf9f264a1e 100644 --- a/script/tool/lib/src/common.dart +++ b/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 [lonOnError] is set to `true`, it will print a formatted message about the error. + /// Defaults to `false` /// /// Returns the [io.ProcessResult] of the [executable]. Future run(String executable, List 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 runAndExitOnError( - String executable, - List 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 args, {Directory workingDir}) { final String workdir = workingDir == null ? '' : ' in ${workingDir.path}'; diff --git a/script/tool/lib/src/firebase_test_lab_command.dart b/script/tool/lib/src/firebase_test_lab_command.dart index 2998522da036..ff7d05bf4e89 100644 --- a/script/tool/lib/src/firebase_test_lab_command.dart +++ b/script/tool/lib/src/firebase_test_lab_command.dart @@ -73,11 +73,16 @@ class FirebaseTestLabCommand extends PluginCommand { } else { _firebaseProjectConfigured = Completer(); } - await processRunner.runAndExitOnError('gcloud', [ - 'auth', - 'activate-service-account', - '--key-file=${argResults['service-key']}', - ]); + await processRunner.run( + 'gcloud', + [ + 'auth', + 'activate-service-account', + '--key-file=${argResults['service-key']}', + ], + exitOnError: true, + logOnError: true, + ); final int exitCode = await processRunner.runAndStream('gcloud', [ 'config', 'set', diff --git a/script/tool/lib/src/format_command.dart b/script/tool/lib/src/format_command.dart index 19b6004d2446..9c29f0f8c684 100644 --- a/script/tool/lib/src/format_command.dart +++ b/script/tool/lib/src/format_command.dart @@ -56,9 +56,13 @@ class FormatCommand extends PluginCommand { } Future _didModifyAnything() async { - final io.ProcessResult modifiedFiles = await processRunner - .runAndExitOnError('git', ['ls-files', '--modified'], - workingDir: packagesDir); + final io.ProcessResult modifiedFiles = await processRunner.run( + 'git', + ['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 <['diff'], workingDir: packagesDir); + final io.ProcessResult diff = await processRunner.run( + 'git', + ['diff'], + workingDir: packagesDir, + exitOnError: true, + logOnError: true, + ); print(diff.stdout); print('DONE'); return true; diff --git a/script/tool/lib/src/lint_podspecs_command.dart b/script/tool/lib/src/lint_podspecs_command.dart index 1fe6b71cf11b..ebcefd6c2343 100644 --- a/script/tool/lib/src/lint_podspecs_command.dart +++ b/script/tool/lib/src/lint_podspecs_command.dart @@ -58,8 +58,13 @@ class LintPodspecsCommand extends PluginCommand { return; } - await processRunner.runAndExitOnError('which', ['pod'], - workingDir: packagesDir); + await processRunner.run( + 'which', + ['pod'], + workingDir: packagesDir, + exitOnError: true, + logOnError: true, + ); _print('Starting podspec lint test'); diff --git a/script/tool/lib/src/publish_plugin_command.dart b/script/tool/lib/src/publish_plugin_command.dart index 0dae3a502be1..201130db754f 100644 --- a/script/tool/lib/src/publish_plugin_command.dart +++ b/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', ['tag', tag], - workingDir: packageDir); + await processRunner.run( + 'git', + ['tag', tag], + workingDir: packageDir, + exitOnError: true, + logOnError: true, + ); if (!shouldPushTag) { return; } @@ -163,15 +168,13 @@ class PublishPluginCommand extends PluginCommand { } Future _checkGitStatus(Directory packageDir) async { - final ProcessResult statusResult = await processRunner.runAndExitOnError( - 'git', - [ - 'status', - '--porcelain', - '--ignored', - packageDir.absolute.path - ], - workingDir: packageDir); + final ProcessResult statusResult = await processRunner.run( + 'git', + ['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 _verifyRemote(String remote) async { - final ProcessResult remoteInfo = await processRunner.runAndExitOnError( - 'git', ['remote', 'get-url', remote], - workingDir: packagesDir); + final ProcessResult remoteInfo = await processRunner.run( + 'git', + ['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', ['push', remote, tag], - workingDir: packagesDir); + await processRunner.run( + 'git', + ['push', remote, tag], + workingDir: packagesDir, + exitOnError: true, + logOnError: true, + ); } } diff --git a/script/tool/test/util.dart b/script/tool/test/util.dart index 5a2c42bd3194..b0cf36269412 100644 --- a/script/tool/test/util.dart +++ b/script/tool/test/util.dart @@ -238,8 +238,10 @@ class RecordingProcessRunner extends ProcessRunner { Future run(String executable, List args, {Directory workingDir, bool exitOnError = false, + bool logOnError = false, Encoding stdoutEncoding = io.systemEncoding, - Encoding stderrEncoding = io.systemEncoding}) async { + Encoding stderrEncoding = io.systemEncoding, + }) async { recordedCalls.add(ProcessCall(executable, args, workingDir?.path)); io.ProcessResult result; @@ -253,24 +255,6 @@ class RecordingProcessRunner extends ProcessRunner { return Future.value(result); } - @override - Future runAndExitOnError( - String executable, - List args, { - Directory workingDir, - }) 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.value(result); - } - @override Future start(String executable, List args, {Directory workingDirectory}) async { From d327cd17f36507524c73d8db7366b4ac2d12de51 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Mon, 26 Apr 2021 11:55:01 -0700 Subject: [PATCH 2/4] format --- script/tool/test/util.dart | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/script/tool/test/util.dart b/script/tool/test/util.dart index b0cf36269412..4dc019c968d5 100644 --- a/script/tool/test/util.dart +++ b/script/tool/test/util.dart @@ -235,13 +235,15 @@ class RecordingProcessRunner extends ProcessRunner { /// Returns [io.ProcessResult] created from [processToReturn], [resultStdout], and [resultStderr]. @override - Future run(String executable, List args, - {Directory workingDir, - bool exitOnError = false, - bool logOnError = false, - Encoding stdoutEncoding = io.systemEncoding, - Encoding stderrEncoding = io.systemEncoding, - }) async { + Future run( + String executable, + List 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; From 2a13c584365434c8151a2e75b3a776c113d01b9f Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Mon, 26 Apr 2021 11:55:33 -0700 Subject: [PATCH 3/4] typo --- script/tool/lib/src/common.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/script/tool/lib/src/common.dart b/script/tool/lib/src/common.dart index c4cf9f264a1e..c16ba8e957e0 100644 --- a/script/tool/lib/src/common.dart +++ b/script/tool/lib/src/common.dart @@ -502,7 +502,7 @@ class ProcessRunner { /// the [executable] terminates with a non-zero exit code. /// Defaults to `false`. /// - /// If [lonOnError] is set to `true`, it will print a formatted message about the error. + /// 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]. From 8cc543164ba41c517b9e0feeaf079a1b7f9f3051 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Mon, 26 Apr 2021 13:50:15 -0700 Subject: [PATCH 4/4] fix test --- script/tool/test/publish_plugin_command_test.dart | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/script/tool/test/publish_plugin_command_test.dart b/script/tool/test/publish_plugin_command_test.dart index 03e7858d3bcd..0cf709adc0d4 100644 --- a/script/tool/test/publish_plugin_command_test.dart +++ b/script/tool/test/publish_plugin_command_test.dart @@ -328,10 +328,14 @@ class TestProcessRunner extends ProcessRunner { final List pushTagsArgs = []; @override - Future runAndExitOnError( + Future run( String executable, List 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') {