[tool] Consistent FakeProcessManager.run/runSync (#103947)

cbracken · web-flow · commit 928bb1229ba0 · 2022-05-17T11:15:54.000-07:00
`FakeProcessManager` is a test-oriented implementation of `ProcessManager` that simulates launching processes and returning `ProcessResult` objects whose `exitCode`, `stdout`, `stderr` can be used to write platform-portable, hermetic tests that don't rely on actually launching processes from executables on disk. Its `run` and `runSync` methods provide asynchronous and synchronous variants of this functionality. Previously, the behaviour of `run` and `runSync` were inconsistent with regards to the treatment of the `stdoutEncoding` (similarly, `stderrEncoding`) parameters: `run`: * if the encoding was null, `ProcessResult.stdout` was returned as a String in UTF-8 encoding. This was incorrect. The behaviour as specified in `ProcessResult.stdout` is that in this case, a raw `List<int>` should be returned. * If the encoding was unspecified, `ProcessResult.stdout` was returned as a `String` in the `io.systemEncoding` encoding. This was correct. * If the encoding was non-null, `ProcessResult.stdout` was returned as a `String` in the specified encoding. This was correct. `runSync`: * if the encoding was null, `ProcessResult.stdout` was returned as a `List<int>` in UTF-8 encoding. This was incorrect. The behaviour as specified in `ProcessResult.stdout` is that in this case, a raw `List<int>` should be returned. * If the encoding was unspecified, `ProcessResult.stdout` was returned as `List<int>` in UTF-8 encoding. This was incorrect. The behaviour as specified in `ProcessResult.stdout` is that in this case, a String a `String` in the `io.systemEncoding` encoding should be returned. * if the encoding was non-null, `ProcessResult.stdout` was returned as a `String` in unknown (but probably UTF-8) encoding. This was incorrect. The behaviour as specified in `ProcessResult.stdout` is that in this case, a `String` in the specified encoding should be returned. `_FakeProcess`, from which we obtain the fake stdout and stderr values now holds these fields as raw `List<int>` of bytes rather than as `String`s. It is up to the user to supply values that can be decoded with the encoding passed to `run`/`runAsync`. `run` and `runAsync` have been updated to set stdout (likewise, stderr) as specified in the `ProcessResult` documentation. This is pre-factoring for #102451, in which the tool throws an exception when processing the JSON output from stdout of the `vswhere.exe` tool, whose output was found to include the `U+FFFD` Unicode replacement character during UTF-8 decoding, which triggers a `toolExit` exception when decoded using our [Utf8Decoder][decoder] configured with `reportErrors` = true. Because `FakeProcessManager.runAsync` did not previously invoke `utf8.decode` on its output (behaviour which differs from the non-fake implementation), it was impossible to write tests to verify the fix. Ref: https://api.flutter.dev/flutter/dart-io/ProcessResult/stdout.html Issue: flutter/flutter#102451 [decoder]: https://github.com/flutter/flutter/blob/fd312f1ccff909fde28d2247a489bf210bbc6c48/packages/flutter_tools/lib/src/convert.dart#L51-L60
diff --git a/packages/flutter_tools/test/general.shard/ios/ios_device_port_forwarder_test.dart b/packages/flutter_tools/test/general.shard/ios/ios_device_port_forwarder_test.dart
@@ -2,8 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// @dart = 2.8
-
 import 'package:flutter_tools/src/base/logger.dart';
 import 'package:flutter_tools/src/ios/devices.dart';
 
@@ -25,8 +23,8 @@ void main() {
       // the FakeCommands below expect an exitCode of 0.
       const FakeCommand(
         command: <String>['iproxy', '12345:456', '--udid', '1234'],
-        stdout: null, // no stdout indicates failure.
         environment: kDyLdLibEntry,
+        // Empty stdout indicates failure.
       ),
       const FakeCommand(
         command: <String>['iproxy', '12346:456', '--udid', '1234'],
diff --git a/packages/flutter_tools/test/src/fake_process_manager.dart b/packages/flutter_tools/test/src/fake_process_manager.dart
@@ -100,8 +100,8 @@ class FakeCommand {
   /// If provided, this exception will be thrown when the fake command is run.
   final Object? exception;
 
-  /// Indicates that output will only be emitted after the `exitCode` [Future]
-  /// on [io.Process] completes.
+  /// When true, stdout and stderr will only be emitted after the `exitCode`
+  /// [Future] on [io.Process] completes.
   final bool outputFollowsExit;
 
   void _matches(
@@ -141,24 +141,26 @@ class _FakeProcess implements io.Process {
       }),
       stdin = stdin ?? IOSink(StreamController<List<int>>().sink)
   {
-    if (_stderr == null) {
+    if (_stderr.isEmpty) {
       stderr = const Stream<List<int>>.empty();
     } else if (outputFollowsExit) {
+      // Wait for the process to exit before emitting stderr.
       stderr = Stream<List<int>>.fromFuture(exitCode.then((_) {
-        return Future<List<int>>(() => utf8.encode(_stderr));
+        return Future<List<int>>(() => _stderr);
       }));
     } else {
-      stderr = Stream<List<int>>.value(utf8.encode(_stderr));
+      stderr = Stream<List<int>>.value(_stderr);
     }
 
-    if (_stdout == null) {
+    if (_stdout.isEmpty) {
       stdout = const Stream<List<int>>.empty();
     } else if (outputFollowsExit) {
+      // Wait for the process to exit before emitting stdout.
       stdout = Stream<List<int>>.fromFuture(exitCode.then((_) {
-        return Future<List<int>>(() => utf8.encode(_stdout));
+        return Future<List<int>>(() => _stdout);
       }));
     } else {
-      stdout = Stream<List<int>>.value(utf8.encode(_stdout));
+      stdout = Stream<List<int>>.value(_stdout);
     }
   }
 
@@ -171,7 +173,7 @@ class _FakeProcess implements io.Process {
   @override
   final int pid;
 
-  final String _stderr;
+  final List<int> _stderr;
 
   @override
   late final Stream<List<int>> stderr;
@@ -182,7 +184,7 @@ class _FakeProcess implements io.Process {
   @override
   late final Stream<List<int>> stdout;
 
-  final String _stdout;
+  final List<int> _stdout;
 
   @override
   bool kill([io.ProcessSignal signal = io.ProcessSignal.sigterm]) {
@@ -268,9 +270,9 @@ abstract class FakeProcessManager implements ProcessManager {
       fakeCommand.exitCode,
       fakeCommand.duration,
       _pid,
-      fakeCommand.stderr,
+      encoding?.encode(fakeCommand.stderr) ?? fakeCommand.stderr.codeUnits,
       fakeCommand.stdin,
-      fakeCommand.stdout,
+      encoding?.encode(fakeCommand.stdout) ?? fakeCommand.stdout.codeUnits,
       fakeCommand.completer,
       fakeCommand.outputFollowsExit,
     );
@@ -310,8 +312,8 @@ abstract class FakeProcessManager implements ProcessManager {
     return io.ProcessResult(
       process.pid,
       process._exitCode,
-      stdoutEncoding == null ? process.stdout : await stdoutEncoding.decodeStream(process.stdout),
-      stderrEncoding == null ? process.stderr : await stderrEncoding.decodeStream(process.stderr),
+      stdoutEncoding == null ? process._stdout : await stdoutEncoding.decodeStream(process.stdout),
+      stderrEncoding == null ? process._stderr : await stderrEncoding.decodeStream(process.stderr),
     );
   }
 
@@ -322,15 +324,15 @@ abstract class FakeProcessManager implements ProcessManager {
     Map<String, String>? environment,
     bool includeParentEnvironment = true, // ignored
     bool runInShell = false, // ignored
-    Encoding? stdoutEncoding = io.systemEncoding, // actual encoder is ignored
-    Encoding? stderrEncoding = io.systemEncoding, // actual encoder is ignored
+    Encoding? stdoutEncoding = io.systemEncoding,
+    Encoding? stderrEncoding = io.systemEncoding,
   }) {
     final _FakeProcess process = _runCommand(command.cast<String>(), workingDirectory, environment, stdoutEncoding);
     return io.ProcessResult(
       process.pid,
       process._exitCode,
-      stdoutEncoding == null ? utf8.encode(process._stdout) : process._stdout,
-      stderrEncoding == null ? utf8.encode(process._stderr) : process._stderr,
+      stdoutEncoding == null ? process._stdout : stdoutEncoding.decode(process._stdout),
+      stderrEncoding == null ? process._stderr : stderrEncoding.decode(process._stderr),
     );
   }
 
