Unverified Commit 928bb122 authored by Chris Bracken's avatar Chris Bracken Committed by GitHub

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

`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: https://github.com/flutter/flutter/issues/102451

[decoder]: https://github.com/flutter/flutter/blob/fd312f1ccff909fde28d2247a489bf210bbc6c48/packages/flutter_tools/lib/src/convert.dart#L51-L60
parent 19940279
...@@ -2,8 +2,6 @@ ...@@ -2,8 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
// @dart = 2.8
import 'package:flutter_tools/src/base/logger.dart'; import 'package:flutter_tools/src/base/logger.dart';
import 'package:flutter_tools/src/ios/devices.dart'; import 'package:flutter_tools/src/ios/devices.dart';
...@@ -25,8 +23,8 @@ void main() { ...@@ -25,8 +23,8 @@ void main() {
// the FakeCommands below expect an exitCode of 0. // the FakeCommands below expect an exitCode of 0.
const FakeCommand( const FakeCommand(
command: <String>['iproxy', '12345:456', '--udid', '1234'], command: <String>['iproxy', '12345:456', '--udid', '1234'],
stdout: null, // no stdout indicates failure.
environment: kDyLdLibEntry, environment: kDyLdLibEntry,
// Empty stdout indicates failure.
), ),
const FakeCommand( const FakeCommand(
command: <String>['iproxy', '12346:456', '--udid', '1234'], command: <String>['iproxy', '12346:456', '--udid', '1234'],
......
...@@ -100,8 +100,8 @@ class FakeCommand { ...@@ -100,8 +100,8 @@ class FakeCommand {
/// If provided, this exception will be thrown when the fake command is run. /// If provided, this exception will be thrown when the fake command is run.
final Object? exception; final Object? exception;
/// Indicates that output will only be emitted after the `exitCode` [Future] /// When true, stdout and stderr will only be emitted after the `exitCode`
/// on [io.Process] completes. /// [Future] on [io.Process] completes.
final bool outputFollowsExit; final bool outputFollowsExit;
void _matches( void _matches(
...@@ -141,24 +141,26 @@ class _FakeProcess implements io.Process { ...@@ -141,24 +141,26 @@ class _FakeProcess implements io.Process {
}), }),
stdin = stdin ?? IOSink(StreamController<List<int>>().sink) stdin = stdin ?? IOSink(StreamController<List<int>>().sink)
{ {
if (_stderr == null) { if (_stderr.isEmpty) {
stderr = const Stream<List<int>>.empty(); stderr = const Stream<List<int>>.empty();
} else if (outputFollowsExit) { } else if (outputFollowsExit) {
// Wait for the process to exit before emitting stderr.
stderr = Stream<List<int>>.fromFuture(exitCode.then((_) { stderr = Stream<List<int>>.fromFuture(exitCode.then((_) {
return Future<List<int>>(() => utf8.encode(_stderr)); return Future<List<int>>(() => _stderr);
})); }));
} else { } 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(); stdout = const Stream<List<int>>.empty();
} else if (outputFollowsExit) { } else if (outputFollowsExit) {
// Wait for the process to exit before emitting stdout.
stdout = Stream<List<int>>.fromFuture(exitCode.then((_) { stdout = Stream<List<int>>.fromFuture(exitCode.then((_) {
return Future<List<int>>(() => utf8.encode(_stdout)); return Future<List<int>>(() => _stdout);
})); }));
} else { } else {
stdout = Stream<List<int>>.value(utf8.encode(_stdout)); stdout = Stream<List<int>>.value(_stdout);
} }
} }
...@@ -171,7 +173,7 @@ class _FakeProcess implements io.Process { ...@@ -171,7 +173,7 @@ class _FakeProcess implements io.Process {
@override @override
final int pid; final int pid;
final String _stderr; final List<int> _stderr;
@override @override
late final Stream<List<int>> stderr; late final Stream<List<int>> stderr;
...@@ -182,7 +184,7 @@ class _FakeProcess implements io.Process { ...@@ -182,7 +184,7 @@ class _FakeProcess implements io.Process {
@override @override
late final Stream<List<int>> stdout; late final Stream<List<int>> stdout;
final String _stdout; final List<int> _stdout;
@override @override
bool kill([io.ProcessSignal signal = io.ProcessSignal.sigterm]) { bool kill([io.ProcessSignal signal = io.ProcessSignal.sigterm]) {
...@@ -268,9 +270,9 @@ abstract class FakeProcessManager implements ProcessManager { ...@@ -268,9 +270,9 @@ abstract class FakeProcessManager implements ProcessManager {
fakeCommand.exitCode, fakeCommand.exitCode,
fakeCommand.duration, fakeCommand.duration,
_pid, _pid,
fakeCommand.stderr, encoding?.encode(fakeCommand.stderr) ?? fakeCommand.stderr.codeUnits,
fakeCommand.stdin, fakeCommand.stdin,
fakeCommand.stdout, encoding?.encode(fakeCommand.stdout) ?? fakeCommand.stdout.codeUnits,
fakeCommand.completer, fakeCommand.completer,
fakeCommand.outputFollowsExit, fakeCommand.outputFollowsExit,
); );
...@@ -310,8 +312,8 @@ abstract class FakeProcessManager implements ProcessManager { ...@@ -310,8 +312,8 @@ abstract class FakeProcessManager implements ProcessManager {
return io.ProcessResult( return io.ProcessResult(
process.pid, process.pid,
process._exitCode, process._exitCode,
stdoutEncoding == null ? process.stdout : await stdoutEncoding.decodeStream(process.stdout), stdoutEncoding == null ? process._stdout : await stdoutEncoding.decodeStream(process.stdout),
stderrEncoding == null ? process.stderr : await stderrEncoding.decodeStream(process.stderr), stderrEncoding == null ? process._stderr : await stderrEncoding.decodeStream(process.stderr),
); );
} }
...@@ -322,15 +324,15 @@ abstract class FakeProcessManager implements ProcessManager { ...@@ -322,15 +324,15 @@ abstract class FakeProcessManager implements ProcessManager {
Map<String, String>? environment, Map<String, String>? environment,
bool includeParentEnvironment = true, // ignored bool includeParentEnvironment = true, // ignored
bool runInShell = false, // ignored bool runInShell = false, // ignored
Encoding? stdoutEncoding = io.systemEncoding, // actual encoder is ignored Encoding? stdoutEncoding = io.systemEncoding,
Encoding? stderrEncoding = io.systemEncoding, // actual encoder is ignored Encoding? stderrEncoding = io.systemEncoding,
}) { }) {
final _FakeProcess process = _runCommand(command.cast<String>(), workingDirectory, environment, stdoutEncoding); final _FakeProcess process = _runCommand(command.cast<String>(), workingDirectory, environment, stdoutEncoding);
return io.ProcessResult( return io.ProcessResult(
process.pid, process.pid,
process._exitCode, process._exitCode,
stdoutEncoding == null ? utf8.encode(process._stdout) : process._stdout, stdoutEncoding == null ? process._stdout : stdoutEncoding.decode(process._stdout),
stderrEncoding == null ? utf8.encode(process._stderr) : process._stderr, stderrEncoding == null ? process._stderr : stderrEncoding.decode(process._stderr),
); );
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment