Create abstraction layer for dart:io's Process commands (#7100)
With this change, they're run via instance methods on an object
obtained through the context. This will allow us to substitute
that object in tests with replay/record versions to allow us to
mock out the os-layer in tests.
diff --git a/packages/flutter_tools/lib/executable.dart b/packages/flutter_tools/lib/executable.dart
index 6508b3a..b25b5f1 100644
--- a/packages/flutter_tools/lib/executable.dart
+++ b/packages/flutter_tools/lib/executable.dart
@@ -14,6 +14,7 @@
import 'src/base/logger.dart';
import 'src/base/os.dart';
import 'src/base/process.dart';
+import 'src/base/process_manager.dart';
import 'src/base/utils.dart';
import 'src/cache.dart';
import 'src/commands/analyze.dart';
@@ -95,7 +96,11 @@
// Make the context current.
_executableContext.runInZone(() {
// Initialize the context with some defaults.
+ // Seed these context entries first since others depend on them
+ context.putIfAbsent(ProcessManager, () => new ProcessManager());
context.putIfAbsent(Logger, () => new StdoutLogger());
+
+ // Order-independent context entries
context.putIfAbsent(DeviceManager, () => new DeviceManager());
context.putIfAbsent(DevFSConfig, () => new DevFSConfig());
context.putIfAbsent(Doctor, () => new Doctor());
diff --git a/packages/flutter_tools/lib/src/android/android_device.dart b/packages/flutter_tools/lib/src/android/android_device.dart
index e8f9452..bf86f76 100644
--- a/packages/flutter_tools/lib/src/android/android_device.dart
+++ b/packages/flutter_tools/lib/src/android/android_device.dart
@@ -11,6 +11,7 @@
import '../base/os.dart';
import '../base/logger.dart';
import '../base/process.dart';
+import '../base/process_manager.dart';
import '../build_info.dart';
import '../commands/build_apk.dart';
import '../device.dart';
@@ -59,7 +60,7 @@
try {
// We pass an encoding of LATIN1 so that we don't try and interpret the
// `adb shell getprop` result as UTF8.
- ProcessResult result = Process.runSync(
+ ProcessResult result = processManager.runSync(
propCommand.first,
propCommand.sublist(1),
stdoutEncoding: LATIN1
diff --git a/packages/flutter_tools/lib/src/android/android_workflow.dart b/packages/flutter_tools/lib/src/android/android_workflow.dart
index 05bc39a..58f77db 100644
--- a/packages/flutter_tools/lib/src/android/android_workflow.dart
+++ b/packages/flutter_tools/lib/src/android/android_workflow.dart
@@ -6,6 +6,7 @@
import 'dart:io';
import '../base/os.dart';
+import '../base/process_manager.dart';
import '../doctor.dart';
import '../globals.dart';
import 'android_sdk.dart';
@@ -70,7 +71,7 @@
try {
printTrace('java -version');
- ProcessResult result = Process.runSync('java', <String>['-version']);
+ ProcessResult result = processManager.runSync('java', <String>['-version']);
if (result.exitCode == 0) {
javaVersion = result.stderr;
List<String> versionLines = javaVersion.split('\n');
diff --git a/packages/flutter_tools/lib/src/base/os.dart b/packages/flutter_tools/lib/src/base/os.dart
index f709d4d..01c46d3 100644
--- a/packages/flutter_tools/lib/src/base/os.dart
+++ b/packages/flutter_tools/lib/src/base/os.dart
@@ -10,6 +10,7 @@
import 'context.dart';
import 'process.dart';
+import 'process_manager.dart';
/// Returns [OperatingSystemUtils] active in the current app context (i.e. zone).
OperatingSystemUtils get os => context[OperatingSystemUtils];
@@ -49,14 +50,14 @@
@override
ProcessResult makeExecutable(File file) {
- return Process.runSync('chmod', <String>['a+x', file.path]);
+ return processManager.runSync('chmod', <String>['a+x', file.path]);
}
/// Return the path to the given executable, or `null` if `which` was not able
/// to locate the binary.
@override
File which(String execName) {
- ProcessResult result = Process.runSync('which', <String>[execName]);
+ ProcessResult result = processManager.runSync('which', <String>[execName]);
if (result.exitCode != 0)
return null;
String path = result.stdout.trim().split('\n').first.trim();
@@ -87,7 +88,7 @@
@override
File which(String execName) {
- ProcessResult result = Process.runSync('where', <String>[execName]);
+ ProcessResult result = processManager.runSync('where', <String>[execName]);
if (result.exitCode != 0)
return null;
return new File(result.stdout.trim().split('\n').first.trim());
diff --git a/packages/flutter_tools/lib/src/base/process.dart b/packages/flutter_tools/lib/src/base/process.dart
index 104db6c..8b20f90 100644
--- a/packages/flutter_tools/lib/src/base/process.dart
+++ b/packages/flutter_tools/lib/src/base/process.dart
@@ -6,6 +6,7 @@
import 'dart:convert';
import 'dart:io';
+import 'process_manager.dart';
import '../globals.dart';
typedef String StringConverter(String string);
@@ -44,7 +45,7 @@
_traceCommand(cmd, workingDirectory: workingDirectory);
String executable = cmd[0];
List<String> arguments = cmd.length > 1 ? cmd.sublist(1) : <String>[];
- Process process = await Process.start(
+ Process process = await processManager.start(
executable,
arguments,
workingDirectory: workingDirectory,
@@ -108,13 +109,13 @@
Future<Process> proc = runDetached(cmd);
return new Future<Null>.delayed(timeout, () async {
printTrace('Intentionally killing ${cmd[0]}');
- Process.killPid((await proc).pid);
+ processManager.killPid((await proc).pid);
});
}
Future<Process> runDetached(List<String> cmd) {
_traceCommand(cmd);
- Future<Process> proc = Process.start(
+ Future<Process> proc = processManager.start(
cmd[0], cmd.getRange(1, cmd.length).toList(),
mode: ProcessStartMode.DETACHED
);
@@ -126,7 +127,7 @@
bool allowReentrantFlutter: false
}) async {
_traceCommand(cmd, workingDirectory: workingDirectory);
- ProcessResult results = await Process.run(
+ ProcessResult results = await processManager.run(
cmd[0],
cmd.getRange(1, cmd.length).toList(),
workingDirectory: workingDirectory,
@@ -140,7 +141,7 @@
bool exitsHappy(List<String> cli) {
_traceCommand(cli);
try {
- return Process.runSync(cli.first, cli.sublist(1)).exitCode == 0;
+ return processManager.runSync(cli.first, cli.sublist(1)).exitCode == 0;
} catch (error) {
return false;
}
@@ -203,7 +204,7 @@
bool hideStdout: false,
}) {
_traceCommand(cmd, workingDirectory: workingDirectory);
- ProcessResult results = Process.runSync(
+ ProcessResult results = processManager.runSync(
cmd[0],
cmd.getRange(1, cmd.length).toList(),
workingDirectory: workingDirectory,
diff --git a/packages/flutter_tools/lib/src/base/process_manager.dart b/packages/flutter_tools/lib/src/base/process_manager.dart
new file mode 100644
index 0000000..0cfc1f5
--- /dev/null
+++ b/packages/flutter_tools/lib/src/base/process_manager.dart
@@ -0,0 +1,66 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// 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:io';
+
+import 'context.dart';
+
+ProcessManager get processManager => context[ProcessManager];
+
+class ProcessManager {
+ Future<Process> start(
+ String executable,
+ List<String> arguments,
+ {String workingDirectory,
+ Map<String, String> environment,
+ ProcessStartMode mode: ProcessStartMode.NORMAL}) {
+ return Process.start(
+ executable,
+ arguments,
+ workingDirectory: workingDirectory,
+ environment: environment,
+ mode: mode,
+ );
+ }
+
+ Future<ProcessResult> run(
+ String executable,
+ List<String> arguments,
+ {String workingDirectory,
+ Map<String, String> environment,
+ Encoding stdoutEncoding: SYSTEM_ENCODING,
+ Encoding stderrEncoding: SYSTEM_ENCODING}) {
+ return Process.run(
+ executable,
+ arguments,
+ workingDirectory: workingDirectory,
+ environment: environment,
+ stdoutEncoding: stdoutEncoding,
+ stderrEncoding: stderrEncoding,
+ );
+ }
+
+ ProcessResult runSync(
+ String executable,
+ List<String> arguments,
+ {String workingDirectory,
+ Map<String, String> environment,
+ Encoding stdoutEncoding: SYSTEM_ENCODING,
+ Encoding stderrEncoding: SYSTEM_ENCODING}) {
+ return Process.runSync(
+ executable,
+ arguments,
+ workingDirectory: workingDirectory,
+ environment: environment,
+ stdoutEncoding: stdoutEncoding,
+ stderrEncoding: stderrEncoding,
+ );
+ }
+
+ bool killPid(int pid, [ProcessSignal signal = ProcessSignal.SIGTERM]) {
+ return Process.killPid(pid, signal);
+ }
+}
diff --git a/packages/flutter_tools/lib/src/commands/analyze_continuously.dart b/packages/flutter_tools/lib/src/commands/analyze_continuously.dart
index b04d6f4..b8e5534 100644
--- a/packages/flutter_tools/lib/src/commands/analyze_continuously.dart
+++ b/packages/flutter_tools/lib/src/commands/analyze_continuously.dart
@@ -11,6 +11,7 @@
import '../base/common.dart';
import '../base/logger.dart';
+import '../base/process_manager.dart';
import '../base/utils.dart';
import '../cache.dart';
import '../dart/sdk.dart';
@@ -159,7 +160,7 @@
List<String> args = <String>[snapshot, '--sdk', sdk];
printTrace('dart ${args.join(' ')}');
- _process = await Process.start(path.join(dartSdkPath, 'bin', 'dart'), args);
+ _process = await processManager.start(path.join(dartSdkPath, 'bin', 'dart'), args);
_process.exitCode.whenComplete(() => _process = null);
Stream<String> errorStream = _process.stderr.transform(UTF8.decoder).transform(const LineSplitter());
diff --git a/packages/flutter_tools/lib/src/commands/test.dart b/packages/flutter_tools/lib/src/commands/test.dart
index cf421cd..f83f7a0 100644
--- a/packages/flutter_tools/lib/src/commands/test.dart
+++ b/packages/flutter_tools/lib/src/commands/test.dart
@@ -10,6 +10,7 @@
import '../base/common.dart';
import '../base/logger.dart';
+import '../base/process_manager.dart';
import '../base/os.dart';
import '../cache.dart';
import '../dart/package_map.dart';
@@ -126,7 +127,7 @@
Directory tempDir = Directory.systemTemp.createTempSync('flutter_tools');
try {
File sourceFile = coverageFile.copySync(path.join(tempDir.path, 'lcov.source.info'));
- ProcessResult result = Process.runSync('lcov', <String>[
+ ProcessResult result = processManager.runSync('lcov', <String>[
'--add-tracefile', baseCoverageData,
'--add-tracefile', sourceFile.path,
'--output-file', coverageFile.path,
diff --git a/packages/flutter_tools/lib/src/ios/devices.dart b/packages/flutter_tools/lib/src/ios/devices.dart
index d2aa790..9e05e19 100644
--- a/packages/flutter_tools/lib/src/ios/devices.dart
+++ b/packages/flutter_tools/lib/src/ios/devices.dart
@@ -9,6 +9,7 @@
import '../application_package.dart';
import '../base/os.dart';
import '../base/process.dart';
+import '../base/process_manager.dart';
import '../build_info.dart';
import '../device.dart';
import '../doctor.dart';
@@ -514,7 +515,7 @@
Process process = forwardedPort.context;
if (process != null) {
- Process.killPid(process.pid);
+ processManager.killPid(process.pid);
} else {
printError("Forwarded port did not have a valid process");
}
diff --git a/packages/flutter_tools/lib/src/ios/mac.dart b/packages/flutter_tools/lib/src/ios/mac.dart
index f727f38..b7eead4 100644
--- a/packages/flutter_tools/lib/src/ios/mac.dart
+++ b/packages/flutter_tools/lib/src/ios/mac.dart
@@ -11,6 +11,7 @@
import '../application_package.dart';
import '../base/context.dart';
import '../base/process.dart';
+import '../base/process_manager.dart';
import '../build_info.dart';
import '../flx.dart' as flx;
import '../globals.dart';
@@ -39,7 +40,7 @@
} else {
try {
printTrace('xcrun clang');
- ProcessResult result = Process.runSync('/usr/bin/xcrun', <String>['clang']);
+ ProcessResult result = processManager.runSync('/usr/bin/xcrun', <String>['clang']);
if (result.stdout != null && result.stdout.contains('license'))
_eulaSigned = false;
diff --git a/packages/flutter_tools/lib/src/ios/simulators.dart b/packages/flutter_tools/lib/src/ios/simulators.dart
index d0bae71..3c30dca 100644
--- a/packages/flutter_tools/lib/src/ios/simulators.dart
+++ b/packages/flutter_tools/lib/src/ios/simulators.dart
@@ -13,6 +13,7 @@
import '../base/common.dart';
import '../base/context.dart';
import '../base/process.dart';
+import '../base/process_manager.dart';
import '../build_info.dart';
import '../device.dart';
import '../flx.dart' as flx;
@@ -190,7 +191,7 @@
List<String> args = <String>['simctl', 'list', '--json', section.name];
printTrace('$_xcrunPath ${args.join(' ')}');
- ProcessResult results = Process.runSync(_xcrunPath, args);
+ ProcessResult results = processManager.runSync(_xcrunPath, args);
if (results.exitCode != 0) {
printError('Error executing simctl: ${results.exitCode}\n${results.stderr}');
return <String, Map<String, dynamic>>{};
diff --git a/packages/flutter_tools/lib/src/test/flutter_platform.dart b/packages/flutter_tools/lib/src/test/flutter_platform.dart
index 680df4c..6e85424 100644
--- a/packages/flutter_tools/lib/src/test/flutter_platform.dart
+++ b/packages/flutter_tools/lib/src/test/flutter_platform.dart
@@ -15,6 +15,7 @@
import 'package:test/src/runner/plugin/platform.dart'; // ignore: implementation_imports
import 'package:test/src/runner/plugin/hack_register_platform.dart' as hack; // ignore: implementation_imports
+import '../base/process_manager.dart';
import '../dart/package_map.dart';
import '../globals.dart';
import 'coverage_collector.dart';
@@ -74,7 +75,7 @@
'FLUTTER_TEST': 'true',
'FONTCONFIG_FILE': _fontConfigFile.path,
};
- return Process.start(executable, arguments, environment: environment);
+ return processManager.start(executable, arguments, environment: environment);
}
void _attachStandardStreams(Process process) {
diff --git a/packages/flutter_tools/lib/src/version.dart b/packages/flutter_tools/lib/src/version.dart
index 4ff3859..953d41e 100644
--- a/packages/flutter_tools/lib/src/version.dart
+++ b/packages/flutter_tools/lib/src/version.dart
@@ -5,6 +5,7 @@
import 'dart:io';
import 'base/process.dart';
+import 'base/process_manager.dart';
import 'cache.dart';
final Set<String> kKnownBranchNames = new Set<String>.from(<String>[
@@ -102,7 +103,7 @@
}
String _runSync(String executable, List<String> arguments, String cwd) {
- ProcessResult results = Process.runSync(executable, arguments, workingDirectory: cwd);
+ ProcessResult results = processManager.runSync(executable, arguments, workingDirectory: cwd);
return results.exitCode == 0 ? results.stdout.trim() : '';
}
diff --git a/packages/flutter_tools/test/analytics_test.dart b/packages/flutter_tools/test/analytics_test.dart
index 99c3108..c7c069e 100644
--- a/packages/flutter_tools/test/analytics_test.dart
+++ b/packages/flutter_tools/test/analytics_test.dart
@@ -49,8 +49,8 @@
runner = createTestCommandRunner(doctorCommand);
await runner.run(<String>['doctor']);
expect(count, 0);
- }, overrides: <Type, dynamic>{
- Usage: new Usage()
+ }, overrides: <Type, Generator>{
+ Usage: () => new Usage(),
});
// Ensure we con't send for the 'flutter config' command.
@@ -67,8 +67,8 @@
flutterUsage.enabled = true;
await runner.run(<String>['config']);
expect(count, 0);
- }, overrides: <Type, dynamic>{
- Usage: new Usage()
+ }, overrides: <Type, Generator>{
+ Usage: () => new Usage(),
});
});
@@ -79,8 +79,11 @@
await createTestCommandRunner().run(<String>['--version']);
expect(count, 0);
- }, overrides: <Type, dynamic>{
- Usage: new Usage(settingsName: 'flutter_bot_test', versionOverride: 'dev/unknown')
+ }, overrides: <Type, Generator>{
+ Usage: () => new Usage(
+ settingsName: 'flutter_bot_test',
+ versionOverride: 'dev/unknown',
+ ),
});
});
}
diff --git a/packages/flutter_tools/test/analyze_continuously_test.dart b/packages/flutter_tools/test/analyze_continuously_test.dart
index 88dc1dd..6017ba0 100644
--- a/packages/flutter_tools/test/analyze_continuously_test.dart
+++ b/packages/flutter_tools/test/analyze_continuously_test.dart
@@ -45,8 +45,8 @@
await onDone;
expect(errorCount, 0);
- }, overrides: <Type, dynamic>{
- OperatingSystemUtils: os
+ }, overrides: <Type, Generator>{
+ OperatingSystemUtils: () => os
});
});
@@ -65,8 +65,8 @@
await onDone;
expect(errorCount, 2);
- }, overrides: <Type, dynamic>{
- OperatingSystemUtils: os
+ }, overrides: <Type, Generator>{
+ OperatingSystemUtils: () => os
});
}
diff --git a/packages/flutter_tools/test/devices_test.dart b/packages/flutter_tools/test/devices_test.dart
index f6f6387..22245c4 100644
--- a/packages/flutter_tools/test/devices_test.dart
+++ b/packages/flutter_tools/test/devices_test.dart
@@ -21,9 +21,9 @@
DevicesCommand command = new DevicesCommand();
await createTestCommandRunner(command).run(<String>['devices']);
expect(testLogger.statusText, contains('No devices detected'));
- }, overrides: <Type, dynamic>{
- AndroidSdk: null,
- DeviceManager: new DeviceManager()
+ }, overrides: <Type, Generator>{
+ AndroidSdk: () => null,
+ DeviceManager: () => new DeviceManager(),
});
});
}
diff --git a/packages/flutter_tools/test/os_utils_test.dart b/packages/flutter_tools/test/os_utils_test.dart
index ac37d56..2443e1e 100644
--- a/packages/flutter_tools/test/os_utils_test.dart
+++ b/packages/flutter_tools/test/os_utils_test.dart
@@ -33,8 +33,8 @@
// rwxr--r--
expect(mode.substring(0, 3), endsWith('x'));
}
- }, overrides: <Type, dynamic> {
- OperatingSystemUtils: new OperatingSystemUtils(),
+ }, overrides: <Type, Generator> {
+ OperatingSystemUtils: () => new OperatingSystemUtils(),
});
});
}
diff --git a/packages/flutter_tools/test/src/context.dart b/packages/flutter_tools/test/src/context.dart
index 71af551..35a286d 100644
--- a/packages/flutter_tools/test/src/context.dart
+++ b/packages/flutter_tools/test/src/context.dart
@@ -8,6 +8,7 @@
import 'package:flutter_tools/src/base/context.dart';
import 'package:flutter_tools/src/base/logger.dart';
import 'package:flutter_tools/src/base/os.dart';
+import 'package:flutter_tools/src/base/process_manager.dart';
import 'package:flutter_tools/src/cache.dart';
import 'package:flutter_tools/src/device.dart';
import 'package:flutter_tools/src/devfs.dart';
@@ -27,20 +28,21 @@
MockDeviceManager get testDeviceManager => context[DeviceManager];
MockDoctor get testDoctor => context[Doctor];
+typedef dynamic Generator();
+
void testUsingContext(String description, dynamic testMethod(), {
Timeout timeout,
- Map<Type, dynamic> overrides: const <Type, dynamic>{}
+ Map<Type, Generator> overrides: const <Type, Generator>{}
}) {
test(description, () async {
AppContext testContext = new AppContext();
- // Apply all overrides to the test context.
- overrides.forEach((Type type, dynamic value) {
- testContext.setVariable(type, value);
- });
-
// Initialize the test context with some default mocks.
+ // Seed these context entries first since others depend on them
+ testContext.putIfAbsent(ProcessManager, () => new ProcessManager());
testContext.putIfAbsent(Logger, () => new BufferLogger());
+
+ // Order-independent context entries
testContext.putIfAbsent(DeviceManager, () => new MockDeviceManager());
testContext.putIfAbsent(DevFSConfig, () => new DevFSConfig());
testContext.putIfAbsent(Doctor, () => new MockDoctor());
@@ -63,7 +65,15 @@
testContext.putIfAbsent(Usage, () => new MockUsage());
try {
- return await testContext.runInZone(testMethod);
+ return await testContext.runInZone(() {
+ // Apply the overrides to the test context in the zone since their
+ // instantiation may reference items already stored on the context.
+ overrides.forEach((Type type, dynamic value()) {
+ context.setVariable(type, value());
+ });
+
+ return testMethod();
+ });
} catch (error) {
if (testContext[Logger] is BufferLogger) {
BufferLogger bufferLogger = testContext[Logger];
diff --git a/packages/flutter_tools/test/toolchain_test.dart b/packages/flutter_tools/test/toolchain_test.dart
index f54b906..25e7afe 100644
--- a/packages/flutter_tools/test/toolchain_test.dart
+++ b/packages/flutter_tools/test/toolchain_test.dart
@@ -29,8 +29,8 @@
);
expect(tempDir, isNotNull);
tempDir.deleteSync(recursive: true);
- }, overrides: <Type, dynamic> {
- Cache: new Cache(rootOverride: tempDir)
+ }, overrides: <Type, Generator> {
+ Cache: () => new Cache(rootOverride: tempDir),
});
testUsingContext('using enginePath', () {
