diff --git a/dev/bots/run_command.dart b/dev/bots/run_command.dart index 413fdd4949..bc04450713 100644 --- a/dev/bots/run_command.dart +++ b/dev/bots/run_command.dart @@ -2,92 +2,113 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'dart:async'; import 'dart:convert'; import 'dart:core' hide print; -import 'dart:io' hide exit; +import 'dart:io' as io; import 'package:path/path.dart' as path; import 'utils.dart'; -// TODO(ianh): These two functions should be refactored into something that avoids all this code duplication. - +/// Runs the `executable` and returns standard output as a stream of lines. +/// +/// The returned stream reaches its end immediately after the command exits. +/// +/// If `expectNonZeroExit` is false and the process exits with a non-zero exit +/// code fails the test immediately by exiting the test process with exit code +/// 1. Stream runAndGetStdout(String executable, List arguments, { String workingDirectory, Map environment, bool expectNonZeroExit = false, - int expectedExitCode, - String failureMessage, - bool skip = false, }) async* { - final String commandDescription = '${path.relative(executable, from: workingDirectory)} ${arguments.join(' ')}'; - final String relativeWorkingDir = path.relative(workingDirectory); - if (skip) { - printProgress('SKIPPING', relativeWorkingDir, commandDescription); - return; - } - printProgress('RUNNING', relativeWorkingDir, commandDescription); - - final Stopwatch time = Stopwatch()..start(); - final Process process = await Process.start(executable, arguments, + final StreamController output = StreamController(); + final Future command = runCommand( + executable, + arguments, workingDirectory: workingDirectory, environment: environment, + expectNonZeroExit: expectNonZeroExit, + // Capture the output so it's not printed to the console by default. + outputMode: OutputMode.capture, + outputListener: (String line, io.Process process) { + output.add(line); + }, ); - stderr.addStream(process.stderr); - final Stream lines = process.stdout.transform(utf8.decoder).transform(const LineSplitter()); - yield* lines; + // Close the stream controller after the command is complete. Otherwise, + // the yield* will never finish. + command.whenComplete(output.close); - final int exitCode = await process.exitCode; - if ((exitCode == 0) == expectNonZeroExit || (expectedExitCode != null && exitCode != expectedExitCode)) { - exitWithError([ - if (failureMessage != null) - failureMessage - else - '${bold}ERROR: ${red}Last command exited with $exitCode (expected: ${expectNonZeroExit ? (expectedExitCode ?? 'non-zero') : 'zero'}).$reset', - '${bold}Command: $green$commandDescription$reset', - '${bold}Relative working directory: $cyan$relativeWorkingDir$reset', - ]); - } - print('$clock ELAPSED TIME: ${prettyPrintDuration(time.elapsed)} for $green$commandDescription$reset in $cyan$relativeWorkingDir$reset'); + yield* output.stream; } -/// Runs the `executable` and waits until the process exits. -/// -/// If the process exits with a non-zero exit code, exits this process with -/// exit code 1, unless `expectNonZeroExit` is set to true. +/// Represents a running process launched using [startCommand]. +class Command { + Command._(this.process, this._time, this._savedStdout, this._savedStderr); + + /// The raw process that was launched for this command. + final io.Process process; + + final Stopwatch _time; + final Future>> _savedStdout; + final Future>> _savedStderr; + + /// Evaluates when the [process] exits. + /// + /// Returns the result of running the command. + Future get onExit async { + final int exitCode = await process.exitCode; + _time.stop(); + + // Saved output is null when OutputMode.print is used. + final String flattenedStdout = _savedStdout != null ? _flattenToString(await _savedStdout) : null; + final String flattenedStderr = _savedStderr != null ? _flattenToString(await _savedStderr) : null; + return CommandResult._(exitCode, _time.elapsed, flattenedStdout, flattenedStderr); + } +} + +/// The result of running a command using [startCommand] and [runCommand]; +class CommandResult { + CommandResult._(this.exitCode, this.elapsedTime, this.flattenedStdout, this.flattenedStderr); + + /// The exit code of the process. + final int exitCode; + + /// The amount of time it took the process to complete. + final Duration elapsedTime; + + /// Standard output decoded as a string using UTF8 decoder. + final String flattenedStdout; + + /// Standard error output decoded as a string using UTF8 decoder. + final String flattenedStderr; +} + +/// Starts the `executable` and returns a command object representing the +/// running process. /// /// `outputListener` is called for every line of standard output from the /// process, and is given the [Process] object. This can be used to interrupt /// an indefinitely running process, for example, by waiting until the process /// emits certain output. -Future runCommand(String executable, List arguments, { +/// +/// `outputMode` controls where the standard output from the command process +/// goes. See [OutputMode]. +Future startCommand(String executable, List arguments, { String workingDirectory, Map environment, - bool expectNonZeroExit = false, - int expectedExitCode, - String failureMessage, OutputMode outputMode = OutputMode.print, - CapturedOutput output, - bool skip = false, bool Function(String) removeLine, - void Function(String, Process) outputListener, + void Function(String, io.Process) outputListener, }) async { - assert( - (outputMode == OutputMode.capture) == (output != null), - 'The output parameter must be non-null with and only with OutputMode.capture', - ); - final String commandDescription = '${path.relative(executable, from: workingDirectory)} ${arguments.join(' ')}'; - final String relativeWorkingDir = path.relative(workingDirectory ?? Directory.current.path); - if (skip) { - printProgress('SKIPPING', relativeWorkingDir, commandDescription); - return; - } + final String relativeWorkingDir = path.relative(workingDirectory ?? io.Directory.current.path); printProgress('RUNNING', relativeWorkingDir, commandDescription); final Stopwatch time = Stopwatch()..start(); - final Process process = await Process.start(executable, arguments, + final io.Process process = await io.Process.start(executable, arguments, workingDirectory: workingDirectory, environment: environment, ); @@ -108,56 +129,103 @@ Future runCommand(String executable, List arguments, { switch (outputMode) { case OutputMode.print: await Future.wait(>[ - stdout.addStream(stdoutSource), - stderr.addStream(process.stderr), + io.stdout.addStream(stdoutSource), + io.stderr.addStream(process.stderr), ]); break; case OutputMode.capture: - case OutputMode.discard: savedStdout = stdoutSource.toList(); savedStderr = process.stderr.toList(); break; } - final int exitCode = await process.exitCode; - if (output != null) { - output.stdout = _flattenToString(await savedStdout); - output.stderr = _flattenToString(await savedStderr); + return Command._(process, time, savedStdout, savedStderr); +} + +/// Runs the `executable` and waits until the process exits. +/// +/// If the process exits with a non-zero exit code, exits this process with +/// exit code 1, unless `expectNonZeroExit` is set to true. +/// +/// `outputListener` is called for every line of standard output from the +/// process, and is given the [Process] object. This can be used to interrupt +/// an indefinitely running process, for example, by waiting until the process +/// emits certain output. +/// +/// Returns the result of the finished process, or null if `skip` is true. +/// +/// `outputMode` controls where the standard output from the command process +/// goes. See [OutputMode]. +Future runCommand(String executable, List arguments, { + String workingDirectory, + Map environment, + bool expectNonZeroExit = false, + int expectedExitCode, + String failureMessage, + OutputMode outputMode = OutputMode.print, + bool skip = false, + bool Function(String) removeLine, + void Function(String, io.Process) outputListener, +}) async { + final String commandDescription = '${path.relative(executable, from: workingDirectory)} ${arguments.join(' ')}'; + final String relativeWorkingDir = path.relative(workingDirectory ?? io.Directory.current.path); + if (skip) { + printProgress('SKIPPING', relativeWorkingDir, commandDescription); + return null; } - if ((exitCode == 0) == expectNonZeroExit || (expectedExitCode != null && exitCode != expectedExitCode)) { + final Command command = await startCommand(executable, arguments, + workingDirectory: workingDirectory, + environment: environment, + outputMode: outputMode, + removeLine: removeLine, + outputListener: outputListener, + ); + + final CommandResult result = await command.onExit; + + if ((result.exitCode == 0) == expectNonZeroExit || (expectedExitCode != null && result.exitCode != expectedExitCode)) { // Print the output when we get unexpected results (unless output was // printed already). switch (outputMode) { case OutputMode.print: break; case OutputMode.capture: - case OutputMode.discard: - stdout.writeln(_flattenToString(await savedStdout)); - stderr.writeln(_flattenToString(await savedStderr)); + io.stdout.writeln(result.flattenedStdout); + io.stderr.writeln(result.flattenedStderr); break; } exitWithError([ if (failureMessage != null) failureMessage else - '${bold}ERROR: ${red}Last command exited with $exitCode (expected: ${expectNonZeroExit ? (expectedExitCode ?? 'non-zero') : 'zero'}).$reset', + '${bold}ERROR: ${red}Last command exited with ${result.exitCode} (expected: ${expectNonZeroExit ? (expectedExitCode ?? 'non-zero') : 'zero'}).$reset', '${bold}Command: $green$commandDescription$reset', '${bold}Relative working directory: $cyan$relativeWorkingDir$reset', ]); } - print('$clock ELAPSED TIME: ${prettyPrintDuration(time.elapsed)} for $green$commandDescription$reset in $cyan$relativeWorkingDir$reset'); + print('$clock ELAPSED TIME: ${prettyPrintDuration(result.elapsedTime)} for $green$commandDescription$reset in $cyan$relativeWorkingDir$reset'); + return result; } /// Flattens a nested list of UTF-8 code units into a single string. String _flattenToString(List> chunks) => utf8.decode(chunks.expand((List ints) => ints).toList()); -/// Specifies what to do with command output from [runCommand]. -enum OutputMode { print, capture, discard } +/// Specifies what to do with the command output from [runCommand] and [startCommand]. +enum OutputMode { + /// Forwards standard output and standard error streams to the test process' + /// respective standard streams. + /// + /// Use this mode if all you want is print the output of the command to the + /// console. The output is no longer available after the process exits. + print, -/// Stores command output from [runCommand] when used with [OutputMode.capture]. -class CapturedOutput { - String stdout; - String stderr; + /// Saves standard output and standard error streams in memory. + /// + /// Captured output can be retrieved from the [CommandResult] object. + /// + /// Use this mode in tests that need to inspect the output of a command, or + /// when the output should not be printed to console. + capture, } diff --git a/dev/bots/test.dart b/dev/bots/test.dart index 84ba99a81e..9b6bb83a39 100644 --- a/dev/bots/test.dart +++ b/dev/bots/test.dart @@ -26,7 +26,7 @@ typedef ShardRunner = Future Function(); /// /// If the output does not match expectations, the function shall return an /// appropriate error message. -typedef OutputChecker = String Function(CapturedOutput); +typedef OutputChecker = String Function(CommandResult); final String exe = Platform.isWindows ? '.exe' : ''; final String bat = Platform.isWindows ? '.bat' : ''; @@ -143,9 +143,8 @@ Future _validateEngineHash() async { return; } final String expectedVersion = File(engineVersionFile).readAsStringSync().trim(); - final CapturedOutput flutterTesterOutput = CapturedOutput(); - await runCommand(flutterTester, ['--help'], output: flutterTesterOutput, outputMode: OutputMode.capture); - final String actualVersion = flutterTesterOutput.stderr.split('\n').firstWhere((final String line) { + final CommandResult result = await runCommand(flutterTester, ['--help'], outputMode: OutputMode.capture); + final String actualVersion = result.flattenedStderr.split('\n').firstWhere((final String line) { return line.startsWith('Flutter Engine Version:'); }); if (!actualVersion.contains(expectedVersion)) { @@ -190,8 +189,8 @@ Future _runSmokeTests() async { script: path.join('test_smoke_test', 'pending_timer_fail_test.dart'), expectFailure: true, printOutput: false, - outputChecker: (CapturedOutput output) { - return output.stdout.contains('failingPendingTimerTest') + outputChecker: (CommandResult result) { + return result.flattenedStdout.contains('failingPendingTimerTest') ? null : 'Failed to find the stack trace for the pending Timer.'; } @@ -230,7 +229,7 @@ Future _runSmokeTests() async { ['drive', '--use-existing-app', '-t', path.join('test_driver', 'failure.dart')], workingDirectory: path.join(flutterRoot, 'packages', 'flutter_driver'), expectNonZeroExit: true, - outputMode: OutputMode.discard, + outputMode: OutputMode.capture, ), ], ); @@ -287,7 +286,7 @@ Future _runToolCoverage() async { '--report-on=lib/' ], workingDirectory: toolRoot, - outputMode: OutputMode.discard, + outputMode: OutputMode.capture, ); } @@ -642,11 +641,11 @@ Future _runFrameworkTests() async { script: path.join('test', 'bindings_test_failure.dart'), expectFailure: true, printOutput: false, - outputChecker: (CapturedOutput output) { - final Iterable matches = httpClientWarning.allMatches(output.stdout); + outputChecker: (CommandResult result) { + final Iterable matches = httpClientWarning.allMatches(result.flattenedStdout); if (matches == null || matches.isEmpty || matches.length > 1) { return 'Failed to print warning about HttpClientUsage, or printed it too many times.\n' - 'stdout:\n${output.stdout}'; + 'stdout:\n${result.flattenedStdout}'; } return null; }, @@ -868,9 +867,8 @@ Future _runWebDebugTest(String target, { List additionalArguments = const[], }) async { final String testAppDirectory = path.join(flutterRoot, 'dev', 'integration_tests', 'web'); - final CapturedOutput output = CapturedOutput(); bool success = false; - await runCommand( + final CommandResult result = await runCommand( flutter, [ 'run', @@ -889,7 +887,6 @@ Future _runWebDebugTest(String target, { '-t', target, ], - output: output, outputMode: OutputMode.capture, outputListener: (String line, Process process) { if (line.contains('--- TEST SUCCEEDED ---')) { @@ -908,7 +905,8 @@ Future _runWebDebugTest(String target, { if (success) { print('${green}Web stack trace integration test passed.$reset'); } else { - print(output.stdout); + print(result.flattenedStdout); + print(result.flattenedStderr); print('${red}Web stack trace integration test failed.$reset'); exit(1); } @@ -1125,29 +1123,22 @@ Future _runFlutterTest(String workingDirectory, { args.addAll(tests); if (!shouldProcessOutput) { - OutputMode outputMode = OutputMode.discard; - CapturedOutput output; + final OutputMode outputMode = outputChecker == null && printOutput + ? OutputMode.print + : OutputMode.capture; - if (outputChecker != null) { - outputMode = OutputMode.capture; - output = CapturedOutput(); - } else if (printOutput) { - outputMode = OutputMode.print; - } - - await runCommand( + final CommandResult result = await runCommand( flutter, args, workingDirectory: workingDirectory, expectNonZeroExit: expectFailure, outputMode: outputMode, - output: output, skip: skip, environment: environment, ); if (outputChecker != null) { - final String message = outputChecker(output); + final String message = outputChecker(result); if (message != null) exitWithError([message]); }