From b89c690d6ae02988173a12e9aeafc00425010744 Mon Sep 17 00:00:00 2001 From: Todd Volkert Date: Tue, 25 Apr 2017 13:46:55 -0700 Subject: [PATCH 1/5] Switch many `Device` methods to be async `adb` can sometimes hang, which will in turn hang the Dart isolate if we're using `Process.runSync()`. This changes many of the `Device` methods to return `Future` in order to allow them to use the async process methods. A future change will add timeouts to the associated calls so that we can properly alert the user to the hung `adb` process. This is work towards #7102, #9567 --- .../lib/src/android/android_device.dart | 49 +++++------ .../flutter_tools/lib/src/base/process.dart | 9 ++ .../lib/src/commands/config.dart | 2 +- .../lib/src/commands/daemon.dart | 31 +++---- .../lib/src/commands/devices.dart | 4 +- .../flutter_tools/lib/src/commands/drive.dart | 32 +++++--- .../lib/src/commands/install.dart | 6 +- .../flutter_tools/lib/src/commands/run.dart | 6 +- .../flutter_tools/lib/src/commands/stop.dart | 5 +- packages/flutter_tools/lib/src/device.dart | 82 +++++++++---------- packages/flutter_tools/lib/src/doctor.dart | 4 +- .../lib/src/fuchsia/fuchsia_device.dart | 10 +-- .../flutter_tools/lib/src/ios/devices.dart | 16 ++-- .../flutter_tools/lib/src/ios/simulators.dart | 47 +++++------ packages/flutter_tools/lib/src/run_cold.dart | 7 +- packages/flutter_tools/lib/src/run_hot.dart | 7 +- .../lib/src/runner/flutter_command.dart | 17 ++-- packages/flutter_tools/test/device_test.dart | 8 +- packages/flutter_tools/test/src/context.dart | 17 ++-- packages/flutter_tools/test/src/mocks.dart | 6 +- 20 files changed, 191 insertions(+), 174 deletions(-) diff --git a/packages/flutter_tools/lib/src/android/android_device.dart b/packages/flutter_tools/lib/src/android/android_device.dart index c83ad4a7f97d2..c1a20843c30bc 100644 --- a/packages/flutter_tools/lib/src/android/android_device.dart +++ b/packages/flutter_tools/lib/src/android/android_device.dart @@ -51,7 +51,7 @@ class AndroidDevice extends Device { bool _isLocalEmulator; TargetPlatform _platform; - String _getProperty(String name) { + Future _getProperty(String name) async { if (_properties == null) { _properties = {}; @@ -79,19 +79,19 @@ class AndroidDevice extends Device { } @override - bool get isLocalEmulator { + Future get isLocalEmulator async { if (_isLocalEmulator == null) { - final String characteristics = _getProperty('ro.build.characteristics'); + final String characteristics = await _getProperty('ro.build.characteristics'); _isLocalEmulator = characteristics != null && characteristics.contains('emulator'); } return _isLocalEmulator; } @override - TargetPlatform get targetPlatform { + Future get targetPlatform async { if (_platform == null) { // http://developer.android.com/ndk/guides/abis.html (x86, armeabi-v7a, ...) - switch (_getProperty('ro.product.cpu.abi')) { + switch (await _getProperty('ro.product.cpu.abi')) { case 'x86_64': _platform = TargetPlatform.android_x64; break; @@ -110,9 +110,9 @@ class AndroidDevice extends Device { @override String get sdkNameAndVersion => 'Android $_sdkVersion (API $_apiVersion)'; - String get _sdkVersion => _getProperty('ro.build.version.release'); + Future get _sdkVersion => _getProperty('ro.build.version.release'); - String get _apiVersion => _getProperty('ro.build.version.sdk'); + Future get _apiVersion => _getProperty('ro.build.version.sdk'); _AdbLogReader _logReader; _AndroidDevicePortForwarder _portForwarder; @@ -160,7 +160,7 @@ class AndroidDevice extends Device { return false; } - bool _checkForSupportedAndroidVersion() { + Future _checkForSupportedAndroidVersion() async { try { // If the server is automatically restarted, then we get irrelevant // output lines like this, which we want to ignore: @@ -169,7 +169,7 @@ class AndroidDevice extends Device { runCheckedSync([getAdbPath(androidSdk), 'start-server']); // Sample output: '22' - final String sdkVersion = _getProperty('ro.build.version.sdk'); + final String sdkVersion = await _getProperty('ro.build.version.sdk'); final int sdkVersionParsed = int.parse(sdkVersion, onError: (String source) => null); if (sdkVersionParsed == null) { @@ -195,8 +195,9 @@ class AndroidDevice extends Device { return '/data/local/tmp/sky.${app.id}.sha1'; } - String _getDeviceApkSha1(ApplicationPackage app) { - return runSync(adbCommandForDevice(['shell', 'cat', _getDeviceSha1Path(app)])); + Future _getDeviceApkSha1(ApplicationPackage app) async { + final RunResult result = await runAsync(adbCommandForDevice(['shell', 'cat', _getDeviceSha1Path(app)])); + return result.stdout; } String _getSourceSha1(ApplicationPackage app) { @@ -209,15 +210,15 @@ class AndroidDevice extends Device { String get name => modelID; @override - bool isAppInstalled(ApplicationPackage app) { + Future isAppInstalled(ApplicationPackage app) async { // This call takes 400ms - 600ms. - final String listOut = runCheckedSync(adbCommandForDevice(['shell', 'pm', 'list', 'packages', app.id])); - return LineSplitter.split(listOut).contains("package:${app.id}"); + final RunResult listOut = await runCheckedAsync(adbCommandForDevice(['shell', 'pm', 'list', 'packages', app.id])); + return LineSplitter.split(listOut.stdout).contains("package:${app.id}"); } @override - bool isLatestBuildInstalled(ApplicationPackage app) { - final String installedSha1 = _getDeviceApkSha1(app); + Future isLatestBuildInstalled(ApplicationPackage app) async { + final String installedSha1 = await _getDeviceApkSha1(app); return installedSha1.isNotEmpty && installedSha1 == _getSourceSha1(app); } @@ -229,7 +230,7 @@ class AndroidDevice extends Device { return false; } - if (!_checkForSupportedAdbVersion() || !_checkForSupportedAndroidVersion()) + if (!_checkForSupportedAdbVersion() || !await _checkForSupportedAndroidVersion()) return false; final Status status = logger.startProgress('Installing ${apk.apkPath}...', expectSlowOperation: true); @@ -249,8 +250,8 @@ class AndroidDevice extends Device { } @override - bool uninstallApp(ApplicationPackage app) { - if (!_checkForSupportedAdbVersion() || !_checkForSupportedAndroidVersion()) + Future uninstallApp(ApplicationPackage app) async { + if (!_checkForSupportedAdbVersion() || !await _checkForSupportedAndroidVersion()) return false; final String uninstallOut = runCheckedSync(adbCommandForDevice(['uninstall', app.id])); @@ -265,9 +266,9 @@ class AndroidDevice extends Device { } Future _installLatestApp(ApplicationPackage package) async { - final bool wasInstalled = isAppInstalled(package); + final bool wasInstalled = await isAppInstalled(package); if (wasInstalled) { - if (isLatestBuildInstalled(package)) { + if (await isLatestBuildInstalled(package)) { printTrace('Latest build already installed.'); return true; } @@ -277,7 +278,7 @@ class AndroidDevice extends Device { printTrace('Warning: Failed to install APK.'); if (wasInstalled) { printStatus('Uninstalling old version...'); - if (!uninstallApp(package)) { + if (!await uninstallApp(package)) { printError('Error: Uninstalling old version failed.'); return false; } @@ -304,10 +305,10 @@ class AndroidDevice extends Device { bool prebuiltApplication: false, bool applicationNeedsRebuild: false, }) async { - if (!_checkForSupportedAdbVersion() || !_checkForSupportedAndroidVersion()) + if (!_checkForSupportedAdbVersion() || !await _checkForSupportedAndroidVersion()) return new LaunchResult.failed(); - if (targetPlatform != TargetPlatform.android_arm && mode != BuildMode.debug) { + if (await targetPlatform != TargetPlatform.android_arm && mode != BuildMode.debug) { printError('Profile and release builds are only supported on ARM targets.'); return new LaunchResult.failed(); } diff --git a/packages/flutter_tools/lib/src/base/process.dart b/packages/flutter_tools/lib/src/base/process.dart index 8ded4f6955828..24142debe5023 100644 --- a/packages/flutter_tools/lib/src/base/process.dart +++ b/packages/flutter_tools/lib/src/base/process.dart @@ -221,6 +221,15 @@ bool exitsHappy(List cli) { } } +Future exitsHappyAsync(List cli) async { + _traceCommand(cli); + try { + return (await processManager.run(cli)).exitCode == 0; + } catch (error) { + return false; + } +} + /// Run cmd and return stdout. /// /// Throws an error if cmd exits with a non-zero value. diff --git a/packages/flutter_tools/lib/src/commands/config.dart b/packages/flutter_tools/lib/src/commands/config.dart index 59484e581c3fd..3c872883b5003 100644 --- a/packages/flutter_tools/lib/src/commands/config.dart +++ b/packages/flutter_tools/lib/src/commands/config.dart @@ -44,7 +44,7 @@ class ConfigCommand extends FlutterCommand { /// Return `null` to disable tracking of the `config` command. @override - String get usagePath => null; + Future get usagePath async => null; @override Future runCommand() async { diff --git a/packages/flutter_tools/lib/src/commands/daemon.dart b/packages/flutter_tools/lib/src/commands/daemon.dart index 50483fe87a557..951a04495be61 100644 --- a/packages/flutter_tools/lib/src/commands/daemon.dart +++ b/packages/flutter_tools/lib/src/commands/daemon.dart @@ -346,7 +346,7 @@ class AppDomain extends Domain { String packagesFilePath, String projectAssets, }) async { - if (device.isLocalEmulator && !isEmulatorBuildMode(options.buildMode)) + if (await device.isLocalEmulator && !isEmulatorBuildMode(options.buildMode)) throw '${toTitleCase(getModeName(options.buildMode))} mode is not supported for emulators.'; // We change the current working directory for the duration of the `start` command. @@ -381,7 +381,7 @@ class AppDomain extends Domain { _sendAppEvent(app, 'start', { 'deviceId': device.id, 'directory': projectDirectory, - 'supportsRestart': isRestartSupported(enableHotReload, device) + 'supportsRestart': isRestartSupported(enableHotReload, device), }); Completer connectionInfoCompleter; @@ -505,6 +505,8 @@ class AppDomain extends Domain { } } +typedef void _DeviceEventHandler(Device device); + /// This domain lets callers list and monitor connected devices. /// /// It exports a `getDevices()` call, as well as firing `device.added` and @@ -530,15 +532,20 @@ class DeviceDomain extends Domain { _discoverers.add(deviceDiscovery); for (PollingDeviceDiscovery discoverer in _discoverers) { - discoverer.onAdded.listen((Device device) { - sendEvent('device.added', _deviceToMap(device)); - }); - discoverer.onRemoved.listen((Device device) { - sendEvent('device.removed', _deviceToMap(device)); - }); + discoverer.onAdded.listen(_onDeviceEvent('device.added')); + discoverer.onRemoved.listen(_onDeviceEvent('device.removed')); } } + Future _deviceEvents = new Future.value(); + _DeviceEventHandler _onDeviceEvent(String eventName) { + return (Device device) { + _deviceEvents = _deviceEvents.then((_) async { + sendEvent(eventName, await _deviceToMap(device)); + }); + }; + } + final List _discoverers = []; Future> getDevices([Map args]) { @@ -638,18 +645,16 @@ void stdoutCommandResponse(Map command) { } dynamic _jsonEncodeObject(dynamic object) { - if (object is Device) - return _deviceToMap(object); if (object is OperationResult) return _operationResultToMap(object); return object; } -Map _deviceToMap(Device device) { +Future> _deviceToMap(Device device) async { return { 'id': device.id, 'name': device.name, - 'platform': getNameForTargetPlatform(device.targetPlatform), + 'platform': getNameForTargetPlatform(await device.targetPlatform), 'emulator': device.isLocalEmulator }; } @@ -664,8 +669,6 @@ Map _operationResultToMap(OperationResult result) { dynamic _toJsonable(dynamic obj) { if (obj is String || obj is int || obj is bool || obj is Map || obj is List || obj == null) return obj; - if (obj is Device) - return obj; if (obj is OperationResult) return obj; return '$obj'; diff --git a/packages/flutter_tools/lib/src/commands/devices.dart b/packages/flutter_tools/lib/src/commands/devices.dart index 2982216f3cdfb..51d7aeda94c30 100644 --- a/packages/flutter_tools/lib/src/commands/devices.dart +++ b/packages/flutter_tools/lib/src/commands/devices.dart @@ -27,7 +27,7 @@ class DevicesCommand extends FlutterCommand { exitCode: 1); } - final List devices = await deviceManager.getAllConnectedDevices(); + final List devices = await deviceManager.getAllConnectedDevices().toList(); if (devices.isEmpty) { printStatus( @@ -36,7 +36,7 @@ class DevicesCommand extends FlutterCommand { 'potential issues, or visit https://flutter.io/setup/ for troubleshooting tips.'); } else { printStatus('${devices.length} connected ${pluralize('device', devices.length)}:\n'); - Device.printDevices(devices); + await Device.printDevices(devices); } } } diff --git a/packages/flutter_tools/lib/src/commands/drive.dart b/packages/flutter_tools/lib/src/commands/drive.dart index 4e7d2bd1302f2..bdc0b9f4dcf68 100644 --- a/packages/flutter_tools/lib/src/commands/drive.dart +++ b/packages/flutter_tools/lib/src/commands/drive.dart @@ -183,7 +183,7 @@ void restoreTargetDeviceFinder() { } Future findTargetDevice() async { - final List devices = await deviceManager.getDevices(); + final List devices = await deviceManager.getDevices().toList(); if (deviceManager.hasSpecifiedDeviceId) { if (devices.isEmpty) { @@ -192,7 +192,7 @@ Future findTargetDevice() async { } if (devices.length > 1) { printStatus("Found ${devices.length} devices with name or id matching '${deviceManager.specifiedDeviceId}':"); - Device.printDevices(devices); + await Device.printDevices(devices); return null; } return devices.first; @@ -203,16 +203,24 @@ Future findTargetDevice() async { // On Mac we look for the iOS Simulator. If available, we use that. Then // we look for an Android device. If there's one, we use that. Otherwise, // we launch a new iOS Simulator. - final Device reusableDevice = devices.firstWhere( - (Device d) => d.isLocalEmulator, - orElse: () { - return devices.firstWhere((Device d) => d is AndroidDevice, - orElse: () => null); + Device reusableDevice; + for (Device device in devices) { + if (await device.isLocalEmulator) { + reusableDevice = device; + break; } - ); + } + if (reusableDevice == null) { + for (Device device in devices) { + if (device is AndroidDevice) { + reusableDevice = device; + break; + } + } + } if (reusableDevice != null) { - printStatus('Found connected ${reusableDevice.isLocalEmulator ? "emulator" : "device"} "${reusableDevice.name}"; will reuse it.'); + printStatus('Found connected ${await reusableDevice.isLocalEmulator ? "emulator" : "device"} "${reusableDevice.name}"; will reuse it.'); return reusableDevice; } @@ -262,8 +270,8 @@ Future _startApp(DriveCommand command) async { printTrace('Installing application package.'); final ApplicationPackage package = command.applicationPackages - .getPackageForPlatform(command.device.targetPlatform); - if (command.device.isAppInstalled(package)) + .getPackageForPlatform(await command.device.targetPlatform); + if (await command.device.isAppInstalled(package)) command.device.uninstallApp(package); command.device.installApp(package); @@ -335,7 +343,7 @@ void restoreAppStopper() { Future _stopApp(DriveCommand command) async { printTrace('Stopping application.'); - final ApplicationPackage package = command.applicationPackages.getPackageForPlatform(command.device.targetPlatform); + final ApplicationPackage package = command.applicationPackages.getPackageForPlatform(await command.device.targetPlatform); final bool stopped = await command.device.stopApp(package); await command._deviceLogSubscription?.cancel(); return stopped; diff --git a/packages/flutter_tools/lib/src/commands/install.dart b/packages/flutter_tools/lib/src/commands/install.dart index 063585ed6065e..3651c01d1b6f1 100644 --- a/packages/flutter_tools/lib/src/commands/install.dart +++ b/packages/flutter_tools/lib/src/commands/install.dart @@ -31,7 +31,7 @@ class InstallCommand extends FlutterCommand { @override Future runCommand() async { - final ApplicationPackage package = applicationPackages.getPackageForPlatform(device.targetPlatform); + final ApplicationPackage package = applicationPackages.getPackageForPlatform(await device.targetPlatform); Cache.releaseLockEarly(); @@ -46,9 +46,9 @@ Future installApp(Device device, ApplicationPackage package, { bool uninst if (package == null) return false; - if (uninstall && device.isAppInstalled(package)) { + if (uninstall && await device.isAppInstalled(package)) { printStatus('Uninstalling old version...'); - if (!device.uninstallApp(package)) + if (!await device.uninstallApp(package)) printError('Warning: uninstalling old version failed'); } diff --git a/packages/flutter_tools/lib/src/commands/run.dart b/packages/flutter_tools/lib/src/commands/run.dart index 50fe3ed67c060..aad16e6aa474c 100644 --- a/packages/flutter_tools/lib/src/commands/run.dart +++ b/packages/flutter_tools/lib/src/commands/run.dart @@ -153,14 +153,14 @@ class RunCommand extends RunCommandBase { Device device; @override - String get usagePath { + Future get usagePath async { final String command = shouldUseHotMode() ? 'hotrun' : name; if (device == null) return command; // Return 'run/ios'. - return '$command/${getNameForTargetPlatform(device.targetPlatform)}'; + return '$command/${getNameForTargetPlatform(await device.targetPlatform)}'; } @override @@ -249,7 +249,7 @@ class RunCommand extends RunCommandBase { return null; } - if (device.isLocalEmulator && !isEmulatorBuildMode(getBuildMode())) + if (await device.isLocalEmulator && !isEmulatorBuildMode(getBuildMode())) throwToolExit('${toTitleCase(getModeName(getBuildMode()))} mode is not supported for emulators.'); if (hotMode) { diff --git a/packages/flutter_tools/lib/src/commands/stop.dart b/packages/flutter_tools/lib/src/commands/stop.dart index c506ee2c6ad74..836538f361ce0 100644 --- a/packages/flutter_tools/lib/src/commands/stop.dart +++ b/packages/flutter_tools/lib/src/commands/stop.dart @@ -31,9 +31,10 @@ class StopCommand extends FlutterCommand { @override Future runCommand() async { - final ApplicationPackage app = applicationPackages.getPackageForPlatform(device.targetPlatform); + final TargetPlatform targetPlatform = await device.targetPlatform; + final ApplicationPackage app = applicationPackages.getPackageForPlatform(targetPlatform); if (app == null) { - final String platformName = getNameForTargetPlatform(device.targetPlatform); + final String platformName = getNameForTargetPlatform(targetPlatform); throwToolExit('No Flutter application for $platformName found in the current directory.'); } printStatus('Stopping apps on ${device.name}.'); diff --git a/packages/flutter_tools/lib/src/device.dart b/packages/flutter_tools/lib/src/device.dart index 9a76b7e7a955a..c03a0bc9155b7 100644 --- a/packages/flutter_tools/lib/src/device.dart +++ b/packages/flutter_tools/lib/src/device.dart @@ -37,42 +37,40 @@ class DeviceManager { bool get hasSpecifiedDeviceId => specifiedDeviceId != null; - /// Return the devices with a name or id matching [deviceId]. - /// This does a case insentitive compare with [deviceId]. - Future> getDevicesById(String deviceId) async { + Stream getDevicesById(String deviceId) async* { + final Stream devices = getAllConnectedDevices(); deviceId = deviceId.toLowerCase(); - final List devices = await getAllConnectedDevices(); - final Device device = devices.firstWhere( - (Device device) => - device.id.toLowerCase() == deviceId || - device.name.toLowerCase() == deviceId, - orElse: () => null); - - if (device != null) - return [device]; + bool exactlyMatchesDeviceId(Device device) => + device.id.toLowerCase() == deviceId || + device.name.toLowerCase() == deviceId; + bool startsWithDeviceId(Device device) => + device.id.toLowerCase().startsWith(deviceId) || + device.name.toLowerCase().startsWith(deviceId); + + final Device exactMatch = await devices.firstWhere( + exactlyMatchesDeviceId, defaultValue: () => null); + if (exactMatch != null) { + yield exactMatch; + return; + } // Match on a id or name starting with [deviceId]. - return devices.where((Device device) { - return (device.id.toLowerCase().startsWith(deviceId) || - device.name.toLowerCase().startsWith(deviceId)); - }).toList(); + await for (Device device in devices.where(startsWithDeviceId)) + yield device; } /// Return the list of connected devices, filtered by any user-specified device id. - Future> getDevices() async { - if (specifiedDeviceId == null) { - return getAllConnectedDevices(); - } else { - return getDevicesById(specifiedDeviceId); - } + Stream getDevices() { + return hasSpecifiedDeviceId + ? getDevicesById(specifiedDeviceId) + : getAllConnectedDevices(); } /// Return the list of all connected devices. - Future> getAllConnectedDevices() async { - return _deviceDiscoverers + Stream getAllConnectedDevices() { + return new Stream.fromIterable(_deviceDiscoverers .where((DeviceDiscovery discoverer) => discoverer.supportsPlatform) - .expand((DeviceDiscovery discoverer) => discoverer.devices) - .toList(); + .expand((DeviceDiscovery discoverer) => discoverer.devices)); } } @@ -141,19 +139,19 @@ abstract class Device { bool get supportsStartPaused => true; /// Whether it is an emulated device running on localhost. - bool get isLocalEmulator; + Future get isLocalEmulator; /// Check if a version of the given app is already installed - bool isAppInstalled(ApplicationPackage app); + Future isAppInstalled(ApplicationPackage app); /// Check if the latest build of the [app] is already installed. - bool isLatestBuildInstalled(ApplicationPackage app); + Future isLatestBuildInstalled(ApplicationPackage app); /// Install an app package on the current device Future installApp(ApplicationPackage app); /// Uninstall an app package from the current device - bool uninstallApp(ApplicationPackage app); + Future uninstallApp(ApplicationPackage app); /// Check if the device is supported by Flutter bool isSupported(); @@ -162,7 +160,8 @@ abstract class Device { // supported by Flutter, and, if not, why. String supportMessage() => isSupported() ? "Supported" : "Unsupported"; - TargetPlatform get targetPlatform; + /// The device's platform. + Future get targetPlatform; String get sdkNameAndVersion; @@ -238,22 +237,23 @@ abstract class Device { @override String toString() => name; - static Iterable descriptions(List devices) { + static Stream descriptions(List devices) async* { if (devices.isEmpty) - return []; + return; // Extract device information final List> table = >[]; for (Device device in devices) { String supportIndicator = device.isSupported() ? '' : ' (unsupported)'; - if (device.isLocalEmulator) { - final String type = device.targetPlatform == TargetPlatform.ios ? 'simulator' : 'emulator'; + final TargetPlatform targetPlatform = await device.targetPlatform; + if (await device.isLocalEmulator) { + final String type = targetPlatform == TargetPlatform.ios ? 'simulator' : 'emulator'; supportIndicator += ' ($type)'; } table.add([ device.name, device.id, - '${getNameForTargetPlatform(device.targetPlatform)}', + '${getNameForTargetPlatform(targetPlatform)}', '${device.sdkNameAndVersion}$supportIndicator', ]); } @@ -266,13 +266,13 @@ abstract class Device { } // Join columns into lines of text - return table.map((List row) => - indices.map((int i) => row[i].padRight(widths[i])).join(' • ') + - ' • ${row.last}'); + for (List row in table) { + yield indices.map((int i) => row[i].padRight(widths[i])).join(' • ') + ' • ${row.last}'; + } } - static void printDevices(List devices) { - descriptions(devices).forEach(printStatus); + static Future printDevices(List devices) async { + await descriptions(devices).forEach(printStatus); } } diff --git a/packages/flutter_tools/lib/src/doctor.dart b/packages/flutter_tools/lib/src/doctor.dart index 3bac2cc124a8f..ba28a7be883e7 100644 --- a/packages/flutter_tools/lib/src/doctor.dart +++ b/packages/flutter_tools/lib/src/doctor.dart @@ -486,12 +486,12 @@ class DeviceValidator extends DoctorValidator { @override Future validate() async { - final List devices = await deviceManager.getAllConnectedDevices(); + final List devices = await deviceManager.getAllConnectedDevices().toList(); List messages; if (devices.isEmpty) { messages = [new ValidationMessage('None')]; } else { - messages = Device.descriptions(devices) + messages = await Device.descriptions(devices) .map((String msg) => new ValidationMessage(msg)).toList(); } return new ValidationResult(ValidationType.installed, messages); diff --git a/packages/flutter_tools/lib/src/fuchsia/fuchsia_device.dart b/packages/flutter_tools/lib/src/fuchsia/fuchsia_device.dart index 0c4ce203c9db4..9010e0f5f0945 100644 --- a/packages/flutter_tools/lib/src/fuchsia/fuchsia_device.dart +++ b/packages/flutter_tools/lib/src/fuchsia/fuchsia_device.dart @@ -37,22 +37,22 @@ class FuchsiaDevice extends Device { final String name; @override - bool get isLocalEmulator => false; + Future get isLocalEmulator async => false; @override bool get supportsStartPaused => false; @override - bool isAppInstalled(ApplicationPackage app) => false; + Future isAppInstalled(ApplicationPackage app) async => false; @override - bool isLatestBuildInstalled(ApplicationPackage app) => false; + Future isLatestBuildInstalled(ApplicationPackage app) async => false; @override Future installApp(ApplicationPackage app) => new Future.value(false); @override - bool uninstallApp(ApplicationPackage app) => false; + Future uninstallApp(ApplicationPackage app) async => false; @override bool isSupported() => true; @@ -77,7 +77,7 @@ class FuchsiaDevice extends Device { } @override - TargetPlatform get targetPlatform => TargetPlatform.fuchsia; + Future get targetPlatform async => TargetPlatform.fuchsia; @override String get sdkNameAndVersion => 'Fuchsia'; diff --git a/packages/flutter_tools/lib/src/ios/devices.dart b/packages/flutter_tools/lib/src/ios/devices.dart index fb6bc3d5c9f8f..388265c346af1 100644 --- a/packages/flutter_tools/lib/src/ios/devices.dart +++ b/packages/flutter_tools/lib/src/ios/devices.dart @@ -87,7 +87,7 @@ class IOSDevice extends Device { _IOSDevicePortForwarder _portForwarder; @override - bool get isLocalEmulator => false; + Future get isLocalEmulator async => false; @override bool get supportsStartPaused => false; @@ -139,10 +139,10 @@ class IOSDevice extends Device { } @override - bool isAppInstalled(ApplicationPackage app) { + Future isAppInstalled(ApplicationPackage app) async { try { - final String apps = runCheckedSync([installerPath, '--list-apps']); - if (new RegExp(app.id, multiLine: true).hasMatch(apps)) { + final RunResult apps = await runCheckedAsync([installerPath, '--list-apps']); + if (new RegExp(app.id, multiLine: true).hasMatch(apps.stdout)) { return true; } } catch (e) { @@ -152,7 +152,7 @@ class IOSDevice extends Device { } @override - bool isLatestBuildInstalled(ApplicationPackage app) => false; + Future isLatestBuildInstalled(ApplicationPackage app) async => false; @override Future installApp(ApplicationPackage app) async { @@ -172,9 +172,9 @@ class IOSDevice extends Device { } @override - bool uninstallApp(ApplicationPackage app) { + Future uninstallApp(ApplicationPackage app) async { try { - runCheckedSync([installerPath, '-U', app.id]); + await runCheckedAsync([installerPath, '-U', app.id]); return true; } catch (e) { return false; @@ -343,7 +343,7 @@ class IOSDevice extends Device { } @override - TargetPlatform get targetPlatform => TargetPlatform.ios; + Future get targetPlatform async => TargetPlatform.ios; @override String get sdkNameAndVersion => 'iOS $_sdkVersion ($_buildVersion)'; diff --git a/packages/flutter_tools/lib/src/ios/simulators.dart b/packages/flutter_tools/lib/src/ios/simulators.dart index 6fd46b8906076..8aa0a417516ec 100644 --- a/packages/flutter_tools/lib/src/ios/simulators.dart +++ b/packages/flutter_tools/lib/src/ios/simulators.dart @@ -224,8 +224,8 @@ class SimControl { bool _isAnyConnected() => getConnectedDevices().isNotEmpty; - bool isInstalled(String appId) { - return exitsHappy([ + Future isInstalled(String appId) { + return exitsHappyAsync([ _xcrunPath, 'simctl', 'get_app_container', @@ -234,23 +234,23 @@ class SimControl { ]); } - void install(String deviceId, String appPath) { - runCheckedSync([_xcrunPath, 'simctl', 'install', deviceId, appPath]); + Future install(String deviceId, String appPath) { + return runCheckedAsync([_xcrunPath, 'simctl', 'install', deviceId, appPath]); } - void uninstall(String deviceId, String appId) { - runCheckedSync([_xcrunPath, 'simctl', 'uninstall', deviceId, appId]); + Future uninstall(String deviceId, String appId) { + return runCheckedAsync([_xcrunPath, 'simctl', 'uninstall', deviceId, appId]); } - void launch(String deviceId, String appIdentifier, [List launchArgs]) { + Future launch(String deviceId, String appIdentifier, [List launchArgs]) { final List args = [_xcrunPath, 'simctl', 'launch', deviceId, appIdentifier]; if (launchArgs != null) args.addAll(launchArgs); - runCheckedSync(args); + return runCheckedAsync(args); } - void takeScreenshot(String outputPath) { - runCheckedSync([_xcrunPath, 'simctl', 'io', 'booted', 'screenshot', outputPath]); + Future takeScreenshot(String outputPath) { + return runCheckedAsync([_xcrunPath, 'simctl', 'io', 'booted', 'screenshot', outputPath]); } } @@ -313,7 +313,7 @@ class IOSSimulator extends Device { final String category; @override - bool get isLocalEmulator => true; + Future get isLocalEmulator async => true; @override bool get supportsHotMode => true; @@ -335,12 +335,12 @@ class IOSSimulator extends Device { } @override - bool isAppInstalled(ApplicationPackage app) { + Future isAppInstalled(ApplicationPackage app) { return SimControl.instance.isInstalled(app.id); } @override - bool isLatestBuildInstalled(ApplicationPackage app) => false; + Future isLatestBuildInstalled(ApplicationPackage app) async => false; @override Future installApp(ApplicationPackage app) async { @@ -354,9 +354,9 @@ class IOSSimulator extends Device { } @override - bool uninstallApp(ApplicationPackage app) { + Future uninstallApp(ApplicationPackage app) async { try { - SimControl.instance.uninstall(id, app.id); + await SimControl.instance.uninstall(id, app.id); return true; } catch (e) { return false; @@ -495,21 +495,18 @@ class IOSSimulator extends Device { } } - bool _applicationIsInstalledAndRunning(ApplicationPackage app) { - final bool isInstalled = isAppInstalled(app); - - final bool isRunning = exitsHappy([ - '/usr/bin/killall', - 'Runner', + Future _applicationIsInstalledAndRunning(ApplicationPackage app) async { + final List criteria = await Future.wait(>[ + isAppInstalled(app), + exitsHappyAsync(['/usr/bin/killall', 'Runner']), ]); - - return isInstalled && isRunning; + return criteria.reduce((bool a, bool b) => a && b); } Future _setupUpdatedApplicationBundle(ApplicationPackage app) async { await _sideloadUpdatedAssetsForInstalledApplicationBundle(app); - if (!_applicationIsInstalledAndRunning(app)) + if (!await _applicationIsInstalledAndRunning(app)) return _buildAndInstallApplicationBundle(app); } @@ -555,7 +552,7 @@ class IOSSimulator extends Device { } @override - TargetPlatform get targetPlatform => TargetPlatform.ios; + Future get targetPlatform async => TargetPlatform.ios; @override String get sdkNameAndVersion => category; diff --git a/packages/flutter_tools/lib/src/run_cold.dart b/packages/flutter_tools/lib/src/run_cold.dart index d114a7fb29a9c..8614f93eacd82 100644 --- a/packages/flutter_tools/lib/src/run_cold.dart +++ b/packages/flutter_tools/lib/src/run_cold.dart @@ -61,11 +61,12 @@ class ColdRunner extends ResidentRunner { printStatus('Launching ${getDisplayPath(mainPath)} on ${device.name} in $modeName mode...'); } - package = getApplicationPackageForPlatform(device.targetPlatform, applicationBinary: applicationBinary); + final TargetPlatform targetPlatform = await device.targetPlatform; + package = getApplicationPackageForPlatform(targetPlatform, applicationBinary: applicationBinary); if (package == null) { - String message = 'No application found for ${device.targetPlatform}.'; - final String hint = getMissingPackageHintForPlatform(device.targetPlatform); + String message = 'No application found for ${targetPlatform}.'; + final String hint = getMissingPackageHintForPlatform(targetPlatform); if (hint != null) message += '\n$hint'; printError(message); diff --git a/packages/flutter_tools/lib/src/run_hot.dart b/packages/flutter_tools/lib/src/run_hot.dart index 823ff7e0b29bd..04d9181f23e71 100644 --- a/packages/flutter_tools/lib/src/run_hot.dart +++ b/packages/flutter_tools/lib/src/run_hot.dart @@ -177,11 +177,12 @@ class HotRunner extends ResidentRunner { final String modeName = getModeName(debuggingOptions.buildMode); printStatus('Launching ${getDisplayPath(mainPath)} on ${device.name} in $modeName mode...'); - package = getApplicationPackageForPlatform(device.targetPlatform, applicationBinary: applicationBinary); + final TargetPlatform targetPlatform = await device.targetPlatform; + package = getApplicationPackageForPlatform(targetPlatform, applicationBinary: applicationBinary); if (package == null) { - String message = 'No application found for ${device.targetPlatform}.'; - final String hint = getMissingPackageHintForPlatform(device.targetPlatform); + String message = 'No application found for ${targetPlatform}.'; + final String hint = getMissingPackageHintForPlatform(targetPlatform); if (hint != null) message += '\n$hint'; printError(message); diff --git a/packages/flutter_tools/lib/src/runner/flutter_command.dart b/packages/flutter_tools/lib/src/runner/flutter_command.dart index 9485618db3761..3fd1e0fa3319f 100644 --- a/packages/flutter_tools/lib/src/runner/flutter_command.dart +++ b/packages/flutter_tools/lib/src/runner/flutter_command.dart @@ -102,7 +102,7 @@ abstract class FlutterCommand extends Command { /// The path to send to Google Analytics. Return `null` here to disable /// tracking of the command. - String get usagePath => name; + Future get usagePath async => name; /// Runs this command. /// @@ -144,9 +144,9 @@ abstract class FlutterCommand extends Command { setupApplicationPackages(); - final String commandPath = usagePath; + final String commandPath = await usagePath; if (commandPath != null) - flutterUsage.sendCommand(usagePath); + flutterUsage.sendCommand(commandPath); await runCommand(); } @@ -158,14 +158,14 @@ abstract class FlutterCommand extends Command { /// devices and criteria entered by the user on the command line. /// If a device cannot be found that meets specified criteria, /// then print an error message and return `null`. - Future findTargetDevice({bool androidOnly: false}) async { + Future findTargetDevice() async { if (!doctor.canLaunchAnything) { printError("Unable to locate a development device; please run 'flutter doctor' " "for information about installing additional components."); return null; } - List devices = await deviceManager.getDevices(); + List devices = await deviceManager.getDevices().toList(); if (devices.isEmpty && deviceManager.hasSpecifiedDeviceId) { printStatus("No devices found with name or id " @@ -178,9 +178,6 @@ abstract class FlutterCommand extends Command { devices = devices.where((Device device) => device.isSupported()).toList(); - if (androidOnly) - devices = devices.where((Device device) => device.targetPlatform == TargetPlatform.android_arm).toList(); - if (devices.isEmpty) { printStatus('No supported devices connected.'); return null; @@ -191,10 +188,10 @@ abstract class FlutterCommand extends Command { } else { printStatus("More than one device connected; please specify a device with " "the '-d ' flag."); - devices = await deviceManager.getAllConnectedDevices(); + devices = await deviceManager.getAllConnectedDevices().toList(); } printStatus(''); - Device.printDevices(devices); + await Device.printDevices(devices); return null; } return devices.single; diff --git a/packages/flutter_tools/test/device_test.dart b/packages/flutter_tools/test/device_test.dart index f16d2e462da1a..7f26d4a32f390 100644 --- a/packages/flutter_tools/test/device_test.dart +++ b/packages/flutter_tools/test/device_test.dart @@ -14,7 +14,7 @@ void main() { testUsingContext('getDevices', () async { // Test that DeviceManager.getDevices() doesn't throw. final DeviceManager deviceManager = new DeviceManager(); - final List devices = await deviceManager.getDevices(); + final List devices = await deviceManager.getDevices().toList(); expect(devices, isList); }); @@ -26,7 +26,7 @@ void main() { final DeviceManager deviceManager = new TestDeviceManager(devices); Future expectDevice(String id, List expected) async { - expect(await deviceManager.getDevicesById(id), expected); + expect(await deviceManager.getDevicesById(id).toList(), expected); } expectDevice('01abfc49119c410e', [device2]); expectDevice('Nexus 5X', [device2]); @@ -44,8 +44,8 @@ class TestDeviceManager extends DeviceManager { TestDeviceManager(this.allDevices); @override - Future> getAllConnectedDevices() async { - return allDevices; + Stream getAllConnectedDevices() { + return new Stream.fromIterable(allDevices); } } diff --git a/packages/flutter_tools/test/src/context.dart b/packages/flutter_tools/test/src/context.dart index 97fbd03dcf811..42bf7eb5545f6 100644 --- a/packages/flutter_tools/test/src/context.dart +++ b/packages/flutter_tools/test/src/context.dart @@ -134,20 +134,19 @@ class MockDeviceManager implements DeviceManager { bool get hasSpecifiedDeviceId => specifiedDeviceId != null; @override - Future> getAllConnectedDevices() => new Future>.value(devices); + Stream getAllConnectedDevices() => new Stream.fromIterable(devices); @override - Future> getDevicesById(String deviceId) async { - return devices.where((Device device) => device.id == deviceId).toList(); + Stream getDevicesById(String deviceId) { + return new Stream.fromIterable( + devices.where((Device device) => device.id == deviceId)); } @override - Future> getDevices() async { - if (specifiedDeviceId == null) { - return getAllConnectedDevices(); - } else { - return getDevicesById(specifiedDeviceId); - } + Stream getDevices() { + return hasSpecifiedDeviceId + ? getDevicesById(specifiedDeviceId) + : getAllConnectedDevices(); } void addDevice(Device device) => devices.add(device); diff --git a/packages/flutter_tools/test/src/mocks.dart b/packages/flutter_tools/test/src/mocks.dart index 3fbc21d21ff7a..6bd31ef36cf34 100644 --- a/packages/flutter_tools/test/src/mocks.dart +++ b/packages/flutter_tools/test/src/mocks.dart @@ -31,7 +31,7 @@ class MockApplicationPackageStore extends ApplicationPackageStore { class MockAndroidDevice extends Mock implements AndroidDevice { @override - TargetPlatform get targetPlatform => TargetPlatform.android_arm; + Future get targetPlatform async => TargetPlatform.android_arm; @override bool isSupported() => true; @@ -39,7 +39,7 @@ class MockAndroidDevice extends Mock implements AndroidDevice { class MockIOSDevice extends Mock implements IOSDevice { @override - TargetPlatform get targetPlatform => TargetPlatform.ios; + Future get targetPlatform async => TargetPlatform.ios; @override bool isSupported() => true; @@ -47,7 +47,7 @@ class MockIOSDevice extends Mock implements IOSDevice { class MockIOSSimulator extends Mock implements IOSSimulator { @override - TargetPlatform get targetPlatform => TargetPlatform.ios; + Future get targetPlatform async => TargetPlatform.ios; @override bool isSupported() => true; From 98935673f964ab73afdd2ef4847a620fa6e680db Mon Sep 17 00:00:00 2001 From: Todd Volkert Date: Tue, 25 Apr 2017 15:26:15 -0700 Subject: [PATCH 2/5] Fix tests and analyzer errors --- packages/flutter_tools/lib/src/commands/config.dart | 2 +- packages/flutter_tools/lib/src/ios/simulators.dart | 3 +-- packages/flutter_tools/lib/src/run_cold.dart | 2 +- packages/flutter_tools/lib/src/run_hot.dart | 2 +- packages/flutter_tools/lib/src/usage.dart | 4 +++- packages/flutter_tools/test/analytics_test.dart | 2 +- .../flutter_tools/test/src/ios/simulators_test.dart | 11 ++++++----- 7 files changed, 14 insertions(+), 12 deletions(-) diff --git a/packages/flutter_tools/lib/src/commands/config.dart b/packages/flutter_tools/lib/src/commands/config.dart index 3c872883b5003..11f7e078aaa68 100644 --- a/packages/flutter_tools/lib/src/commands/config.dart +++ b/packages/flutter_tools/lib/src/commands/config.dart @@ -44,7 +44,7 @@ class ConfigCommand extends FlutterCommand { /// Return `null` to disable tracking of the `config` command. @override - Future get usagePath async => null; + Future get usagePath => null; @override Future runCommand() async { diff --git a/packages/flutter_tools/lib/src/ios/simulators.dart b/packages/flutter_tools/lib/src/ios/simulators.dart index 8aa0a417516ec..ae7e229e24be1 100644 --- a/packages/flutter_tools/lib/src/ios/simulators.dart +++ b/packages/flutter_tools/lib/src/ios/simulators.dart @@ -592,8 +592,7 @@ class IOSSimulator extends Device { @override Future takeScreenshot(File outputFile) { - SimControl.instance.takeScreenshot(outputFile.path); - return new Future.value(); + return SimControl.instance.takeScreenshot(outputFile.path); } } diff --git a/packages/flutter_tools/lib/src/run_cold.dart b/packages/flutter_tools/lib/src/run_cold.dart index 8614f93eacd82..0a7b671b335e1 100644 --- a/packages/flutter_tools/lib/src/run_cold.dart +++ b/packages/flutter_tools/lib/src/run_cold.dart @@ -65,7 +65,7 @@ class ColdRunner extends ResidentRunner { package = getApplicationPackageForPlatform(targetPlatform, applicationBinary: applicationBinary); if (package == null) { - String message = 'No application found for ${targetPlatform}.'; + String message = 'No application found for $targetPlatform.'; final String hint = getMissingPackageHintForPlatform(targetPlatform); if (hint != null) message += '\n$hint'; diff --git a/packages/flutter_tools/lib/src/run_hot.dart b/packages/flutter_tools/lib/src/run_hot.dart index 04d9181f23e71..3b05d16faab9d 100644 --- a/packages/flutter_tools/lib/src/run_hot.dart +++ b/packages/flutter_tools/lib/src/run_hot.dart @@ -181,7 +181,7 @@ class HotRunner extends ResidentRunner { package = getApplicationPackageForPlatform(targetPlatform, applicationBinary: applicationBinary); if (package == null) { - String message = 'No application found for ${targetPlatform}.'; + String message = 'No application found for $targetPlatform.'; final String hint = getMissingPackageHintForPlatform(targetPlatform); if (hint != null) message += '\n$hint'; diff --git a/packages/flutter_tools/lib/src/usage.dart b/packages/flutter_tools/lib/src/usage.dart index 4f72326011549..bad768dc1d127 100644 --- a/packages/flutter_tools/lib/src/usage.dart +++ b/packages/flutter_tools/lib/src/usage.dart @@ -4,6 +4,7 @@ import 'dart:async'; +import 'package:meta/meta.dart'; import 'package:usage/usage_io.dart'; import 'base/context.dart'; @@ -96,7 +97,8 @@ class Usage { _analytics.sendException('${exception.runtimeType}\n${sanitizeStacktrace(trace)}'); } - /// Fires whenever analytics data is sent over the network; public for testing. + /// Fires whenever analytics data is sent over the network. + @visibleForTesting Stream> get onSend => _analytics.onSend; /// Returns when the last analytics event has been sent, or after a fixed diff --git a/packages/flutter_tools/test/analytics_test.dart b/packages/flutter_tools/test/analytics_test.dart index 26f9f63178129..5619f977e467a 100644 --- a/packages/flutter_tools/test/analytics_test.dart +++ b/packages/flutter_tools/test/analytics_test.dart @@ -56,7 +56,7 @@ void main() { Usage: () => new Usage(), }); - // Ensure we con't send for the 'flutter config' command. + // Ensure we don't send for the 'flutter config' command. testUsingContext('config doesn\'t send', () async { int count = 0; flutterUsage.onSend.listen((Map data) => count++); diff --git a/packages/flutter_tools/test/src/ios/simulators_test.dart b/packages/flutter_tools/test/src/ios/simulators_test.dart index 610c9e8e71027..a731197799dae 100644 --- a/packages/flutter_tools/test/src/ios/simulators_test.dart +++ b/packages/flutter_tools/test/src/ios/simulators_test.dart @@ -1,3 +1,4 @@ +import 'dart:async'; import 'dart:io' show ProcessResult; import 'package:file/file.dart'; @@ -128,9 +129,9 @@ void main() { mockProcessManager = new MockProcessManager(); // Let everything else return exit code 0 so process.dart doesn't crash. when( - mockProcessManager.runSync(any, environment: null, workingDirectory: null) + mockProcessManager.run(any, environment: null, workingDirectory: null) ).thenReturn( - new ProcessResult(2, 0, '', null) + new Future.value(new ProcessResult(2, 0, '', '')) ); // Doesn't matter what the device is. deviceUnderTest = new IOSSimulator('x', name: 'iPhone SE'); @@ -148,14 +149,14 @@ void main() { testUsingContext( 'Xcode 8.2+ supports screenshots', - () { + () async { when(mockXcode.xcodeMajorVersion).thenReturn(8); when(mockXcode.xcodeMinorVersion).thenReturn(2); expect(deviceUnderTest.supportsScreenshot, true); final MockFile mockFile = new MockFile(); when(mockFile.path).thenReturn(fs.path.join('some', 'path', 'to', 'screenshot.png')); - deviceUnderTest.takeScreenshot(mockFile); - verify(mockProcessManager.runSync( + await deviceUnderTest.takeScreenshot(mockFile); + verify(mockProcessManager.run( [ '/usr/bin/xcrun', 'simctl', From 328bc089646a891aa88a30ec8da3d97f5edc0db4 Mon Sep 17 00:00:00 2001 From: Todd Volkert Date: Tue, 25 Apr 2017 15:47:46 -0700 Subject: [PATCH 3/5] Convert some `runCheckedSync()` calls into `runCheckedAsync()` where trivial to do so --- .../lib/src/android/android_device.dart | 30 +++++++++---------- .../flutter_tools/lib/src/base/process.dart | 10 ------- .../lib/src/commands/build_aot.dart | 16 +++++----- .../lib/src/commands/upgrade.dart | 2 +- .../flutter_tools/lib/src/ios/devices.dart | 5 ++-- .../lib/src/ios/ios_workflow.dart | 2 +- packages/flutter_tools/lib/src/ios/mac.dart | 2 +- .../flutter_tools/lib/src/ios/simulators.dart | 2 +- packages/flutter_tools/lib/src/zip.dart | 4 +-- 9 files changed, 30 insertions(+), 43 deletions(-) diff --git a/packages/flutter_tools/lib/src/android/android_device.dart b/packages/flutter_tools/lib/src/android/android_device.dart index c1a20843c30bc..fce979c3cce93 100644 --- a/packages/flutter_tools/lib/src/android/android_device.dart +++ b/packages/flutter_tools/lib/src/android/android_device.dart @@ -166,7 +166,7 @@ class AndroidDevice extends Device { // output lines like this, which we want to ignore: // adb server is out of date. killing.. // * daemon started successfully * - runCheckedSync([getAdbPath(androidSdk), 'start-server']); + await runCheckedAsync([getAdbPath(androidSdk), 'start-server']); // Sample output: '22' final String sdkVersion = await _getProperty('ro.build.version.sdk'); @@ -254,7 +254,7 @@ class AndroidDevice extends Device { if (!_checkForSupportedAdbVersion() || !await _checkForSupportedAndroidVersion()) return false; - final String uninstallOut = runCheckedSync(adbCommandForDevice(['uninstall', app.id])); + final String uninstallOut = (await runCheckedAsync(adbCommandForDevice(['uninstall', app.id]))).stdout; final RegExp failureExp = new RegExp(r'^Failure.*$', multiLine: true); final String failure = failureExp.stringMatch(uninstallOut); if (failure != null) { @@ -370,7 +370,7 @@ class AndroidDevice extends Device { cmd.addAll(['--ez', 'use-test-fonts', 'true']); } cmd.add(apk.launchActivity); - final String result = runCheckedSync(cmd); + final String result = (await runCheckedAsync(cmd)).stdout; // This invocation returns 0 even when it fails. if (result.contains('Error: ')) { printError(result.trim()); @@ -456,16 +456,15 @@ class AndroidDevice extends Device { bool get supportsScreenshot => true; @override - Future takeScreenshot(File outputFile) { + Future takeScreenshot(File outputFile) async { const String remotePath = '/data/local/tmp/flutter_screenshot.png'; - runCheckedSync(adbCommandForDevice(['shell', 'screencap', '-p', remotePath])); - runCheckedSync(adbCommandForDevice(['pull', remotePath, outputFile.path])); - runCheckedSync(adbCommandForDevice(['shell', 'rm', remotePath])); - return new Future.value(); + await runCheckedAsync(adbCommandForDevice(['shell', 'screencap', '-p', remotePath])); + await runCheckedAsync(adbCommandForDevice(['pull', remotePath, outputFile.path])); + await runCheckedAsync(adbCommandForDevice(['shell', 'rm', remotePath])); } @override - Future> discoverApps() { + Future> discoverApps() async { final RegExp discoverExp = new RegExp(r'DISCOVER: (.*)'); final List result = []; final StreamSubscription logs = getLogReader().logLines.listen((String line) { @@ -476,14 +475,13 @@ class AndroidDevice extends Device { } }); - runCheckedSync(adbCommandForDevice([ + await runCheckedAsync(adbCommandForDevice([ 'shell', 'am', 'broadcast', '-a', 'io.flutter.view.DISCOVER' ])); - return new Future>.delayed(const Duration(seconds: 1), () { - logs.cancel(); - return result; - }); + await new Future.delayed(const Duration(seconds: 1)); + logs.cancel(); + return result; } } @@ -745,7 +743,7 @@ class _AndroidDevicePortForwarder extends DevicePortForwarder { hostPort = await portScanner.findAvailablePort(); } - runCheckedSync(device.adbCommandForDevice( + await runCheckedAsync(device.adbCommandForDevice( ['forward', 'tcp:$hostPort', 'tcp:$devicePort'] )); @@ -754,7 +752,7 @@ class _AndroidDevicePortForwarder extends DevicePortForwarder { @override Future unforward(ForwardedPort forwardedPort) async { - runCheckedSync(device.adbCommandForDevice( + await runCheckedAsync(device.adbCommandForDevice( ['forward', '--remove', 'tcp:${forwardedPort.hostPort}'] )); } diff --git a/packages/flutter_tools/lib/src/base/process.dart b/packages/flutter_tools/lib/src/base/process.dart index 24142debe5023..7b589c4e67b94 100644 --- a/packages/flutter_tools/lib/src/base/process.dart +++ b/packages/flutter_tools/lib/src/base/process.dart @@ -250,16 +250,6 @@ String runCheckedSync(List cmd, { ); } -/// Run cmd and return stdout on success. -/// -/// Throws the standard error output if cmd exits with a non-zero value. -String runSyncAndThrowStdErrOnError(List cmd) { - return _runWithLoggingSync(cmd, - checked: true, - throwStandardErrorOnError: true, - hideStdout: true); -} - /// Run cmd and return stdout. String runSync(List cmd, { String workingDirectory, diff --git a/packages/flutter_tools/lib/src/commands/build_aot.dart b/packages/flutter_tools/lib/src/commands/build_aot.dart index de0dce60527a4..10f79bb9cc4bc 100644 --- a/packages/flutter_tools/lib/src/commands/build_aot.dart +++ b/packages/flutter_tools/lib/src/commands/build_aot.dart @@ -273,24 +273,24 @@ Future _buildAotSnapshot( final List commonBuildOptions = ['-arch', 'arm64', '-miphoneos-version-min=8.0']; if (interpreter) { - runCheckedSync(['mv', vmSnapshotData, fs.path.join(outputDir.path, kVmSnapshotData)]); - runCheckedSync(['mv', isolateSnapshotData, fs.path.join(outputDir.path, kIsolateSnapshotData)]); + await runCheckedAsync(['mv', vmSnapshotData, fs.path.join(outputDir.path, kVmSnapshotData)]); + await runCheckedAsync(['mv', isolateSnapshotData, fs.path.join(outputDir.path, kIsolateSnapshotData)]); - runCheckedSync([ + await runCheckedAsync([ 'xxd', '--include', kVmSnapshotData, fs.path.basename(kVmSnapshotDataC) ], workingDirectory: outputDir.path); - runCheckedSync([ + await runCheckedAsync([ 'xxd', '--include', kIsolateSnapshotData, fs.path.basename(kIsolateSnapshotDataC) ], workingDirectory: outputDir.path); - runCheckedSync(['xcrun', 'cc'] + await runCheckedAsync(['xcrun', 'cc'] ..addAll(commonBuildOptions) ..addAll(['-c', kVmSnapshotDataC, '-o', kVmSnapshotDataO])); - runCheckedSync(['xcrun', 'cc'] + await runCheckedAsync(['xcrun', 'cc'] ..addAll(commonBuildOptions) ..addAll(['-c', kIsolateSnapshotDataC, '-o', kIsolateSnapshotDataO])); } else { - runCheckedSync(['xcrun', 'cc'] + await runCheckedAsync(['xcrun', 'cc'] ..addAll(commonBuildOptions) ..addAll(['-c', assembly, '-o', assemblyO])); } @@ -313,7 +313,7 @@ Future _buildAotSnapshot( } else { linkCommand.add(assemblyO); } - runCheckedSync(linkCommand); + await runCheckedAsync(linkCommand); } return outputPath; diff --git a/packages/flutter_tools/lib/src/commands/upgrade.dart b/packages/flutter_tools/lib/src/commands/upgrade.dart index d86864c2a359d..7722dc85e9dbd 100644 --- a/packages/flutter_tools/lib/src/commands/upgrade.dart +++ b/packages/flutter_tools/lib/src/commands/upgrade.dart @@ -28,7 +28,7 @@ class UpgradeCommand extends FlutterCommand { @override Future runCommand() async { try { - runCheckedSync([ + await runCheckedAsync([ 'git', 'rev-parse', '@{u}' ], workingDirectory: Cache.flutterRoot); } catch (e) { diff --git a/packages/flutter_tools/lib/src/ios/devices.dart b/packages/flutter_tools/lib/src/ios/devices.dart index 388265c346af1..bfa6379931ffb 100644 --- a/packages/flutter_tools/lib/src/ios/devices.dart +++ b/packages/flutter_tools/lib/src/ios/devices.dart @@ -164,7 +164,7 @@ class IOSDevice extends Device { } try { - runCheckedSync([installerPath, '-i', iosApp.deviceBundlePath]); + await runCheckedAsync([installerPath, '-i', iosApp.deviceBundlePath]); return true; } catch (e) { return false; @@ -370,8 +370,7 @@ class IOSDevice extends Device { @override Future takeScreenshot(File outputFile) { - runCheckedSync([screenshotPath, outputFile.path]); - return new Future.value(); + return runCheckedAsync([screenshotPath, outputFile.path]); } } diff --git a/packages/flutter_tools/lib/src/ios/ios_workflow.dart b/packages/flutter_tools/lib/src/ios/ios_workflow.dart index 018469d612afd..70bb70273896e 100644 --- a/packages/flutter_tools/lib/src/ios/ios_workflow.dart +++ b/packages/flutter_tools/lib/src/ios/ios_workflow.dart @@ -165,7 +165,7 @@ class IOSWorkflow extends DoctorValidator implements Workflow { // Check for compatibility between libimobiledevice and Xcode. // TODO(cbracken) remove this check once libimobiledevice > 1.2.0 is released. final ProcessResult result = (await runAsync(['idevice_id', '-l'])).processResult; - if (result.exitCode == 0 && result.stdout.isNotEmpty && !exitsHappy(['ideviceName'])) { + if (result.exitCode == 0 && result.stdout.isNotEmpty && !await exitsHappyAsync(['ideviceName'])) { brewStatus = ValidationType.partial; messages.add(new ValidationMessage.error( 'libimobiledevice is incompatible with the installed Xcode version. To update, run:\n' diff --git a/packages/flutter_tools/lib/src/ios/mac.dart b/packages/flutter_tools/lib/src/ios/mac.dart index 9baf97fe09bd0..cf675ac5a28d4 100644 --- a/packages/flutter_tools/lib/src/ios/mac.dart +++ b/packages/flutter_tools/lib/src/ios/mac.dart @@ -399,7 +399,7 @@ Future _copyServiceFrameworks(List> services, Director continue; } // Shell out so permissions on the dylib are preserved. - runCheckedSync(['/bin/cp', dylib.path, frameworksDirectory.path]); + await runCheckedAsync(['/bin/cp', dylib.path, frameworksDirectory.path]); } } diff --git a/packages/flutter_tools/lib/src/ios/simulators.dart b/packages/flutter_tools/lib/src/ios/simulators.dart index ae7e229e24be1..82b594c1280d3 100644 --- a/packages/flutter_tools/lib/src/ios/simulators.dart +++ b/packages/flutter_tools/lib/src/ios/simulators.dart @@ -541,7 +541,7 @@ class IOSSimulator extends Device { ApplicationPackage app, String localFile, String targetFile) async { if (platform.isMacOS) { final String simulatorHomeDirectory = _getSimulatorAppHomeDirectory(app); - runCheckedSync(['cp', localFile, fs.path.join(simulatorHomeDirectory, targetFile)]); + await runCheckedAsync(['cp', localFile, fs.path.join(simulatorHomeDirectory, targetFile)]); return true; } return false; diff --git a/packages/flutter_tools/lib/src/zip.dart b/packages/flutter_tools/lib/src/zip.dart index a7b11611cfad1..f4107f052bf47 100644 --- a/packages/flutter_tools/lib/src/zip.dart +++ b/packages/flutter_tools/lib/src/zip.dart @@ -86,7 +86,7 @@ class _ZipToolBuilder extends ZipBuilder { final Iterable compressedNames = _getCompressedNames(); if (compressedNames.isNotEmpty) { - runCheckedSync( + await runCheckedAsync( ['zip', '-q', outFile.absolute.path]..addAll(compressedNames), workingDirectory: zipBuildDir.path ); @@ -94,7 +94,7 @@ class _ZipToolBuilder extends ZipBuilder { final Iterable storedNames = _getStoredNames(); if (storedNames.isNotEmpty) { - runCheckedSync( + await runCheckedAsync( ['zip', '-q', '-0', outFile.absolute.path]..addAll(storedNames), workingDirectory: zipBuildDir.path ); From ed9193ecd4a15561d2e29078672eefc93e9ef7b2 Mon Sep 17 00:00:00 2001 From: Todd Volkert Date: Tue, 25 Apr 2017 16:37:07 -0700 Subject: [PATCH 4/5] Fix test --- .../test/src/ios/devices_test.dart | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/packages/flutter_tools/test/src/ios/devices_test.dart b/packages/flutter_tools/test/src/ios/devices_test.dart index 37a280dbe11e6..20f9723e13d09 100644 --- a/packages/flutter_tools/test/src/ios/devices_test.dart +++ b/packages/flutter_tools/test/src/ios/devices_test.dart @@ -2,6 +2,7 @@ // 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:io' show ProcessResult; import 'package:file/file.dart'; @@ -38,23 +39,23 @@ void main() { // Let everything else return exit code 0 so process.dart doesn't crash. // The matcher order is important. when( - mockProcessManager.runSync(any, environment: null, workingDirectory: null) + mockProcessManager.run(any, environment: null, workingDirectory: null) ).thenReturn( - new ProcessResult(2, 0, '', null) + new Future.value(new ProcessResult(2, 0, '', '')) ); // Let `which idevicescreenshot` fail with exit code 1. when( mockProcessManager.runSync( ['which', 'idevicescreenshot'], environment: null, workingDirectory: null) ).thenReturn( - new ProcessResult(1, 1, '', null) + new ProcessResult(1, 1, '', '') ); iosDeviceUnderTest = new IOSDevice('1234'); - iosDeviceUnderTest.takeScreenshot(mockOutputFile); + await iosDeviceUnderTest.takeScreenshot(mockOutputFile); verify(mockProcessManager.runSync( ['which', 'idevicescreenshot'], environment: null, workingDirectory: null)); - verifyNever(mockProcessManager.runSync( + verifyNever(mockProcessManager.run( ['idevicescreenshot', fs.path.join('some', 'test', 'path', 'image.png')], environment: null, workingDirectory: null @@ -74,23 +75,23 @@ void main() { // Let everything else return exit code 0. // The matcher order is important. when( - mockProcessManager.runSync(any, environment: null, workingDirectory: null) + mockProcessManager.run(any, environment: null, workingDirectory: null) ).thenReturn( - new ProcessResult(4, 0, '', null) + new Future.value(new ProcessResult(4, 0, '', '')) ); // Let there be idevicescreenshot in the PATH. when( mockProcessManager.runSync( ['which', 'idevicescreenshot'], environment: null, workingDirectory: null) ).thenReturn( - new ProcessResult(3, 0, fs.path.join('some', 'path', 'to', 'iscreenshot'), null) + new ProcessResult(3, 0, fs.path.join('some', 'path', 'to', 'iscreenshot'), '') ); iosDeviceUnderTest = new IOSDevice('1234'); - iosDeviceUnderTest.takeScreenshot(mockOutputFile); + await iosDeviceUnderTest.takeScreenshot(mockOutputFile); verify(mockProcessManager.runSync( ['which', 'idevicescreenshot'], environment: null, workingDirectory: null)); - verify(mockProcessManager.runSync( + verify(mockProcessManager.run( [ fs.path.join('some', 'path', 'to', 'iscreenshot'), fs.path.join('some', 'test', 'path', 'image.png') From f3b91476409df7f155a272a53b170c5775098f8c Mon Sep 17 00:00:00 2001 From: Todd Volkert Date: Tue, 25 Apr 2017 16:43:05 -0700 Subject: [PATCH 5/5] Review comments --- packages/flutter_tools/lib/src/android/android_device.dart | 3 ++- packages/flutter_tools/lib/src/device.dart | 4 ++-- packages/flutter_tools/lib/src/fuchsia/fuchsia_device.dart | 2 +- packages/flutter_tools/lib/src/ios/devices.dart | 2 +- packages/flutter_tools/lib/src/ios/simulators.dart | 2 +- 5 files changed, 7 insertions(+), 6 deletions(-) diff --git a/packages/flutter_tools/lib/src/android/android_device.dart b/packages/flutter_tools/lib/src/android/android_device.dart index fce979c3cce93..0d3044c5d54d7 100644 --- a/packages/flutter_tools/lib/src/android/android_device.dart +++ b/packages/flutter_tools/lib/src/android/android_device.dart @@ -108,7 +108,8 @@ class AndroidDevice extends Device { } @override - String get sdkNameAndVersion => 'Android $_sdkVersion (API $_apiVersion)'; + Future get sdkNameAndVersion async => + 'Android ${await _sdkVersion} (API ${await _apiVersion})'; Future get _sdkVersion => _getProperty('ro.build.version.release'); diff --git a/packages/flutter_tools/lib/src/device.dart b/packages/flutter_tools/lib/src/device.dart index c03a0bc9155b7..b4e0fbcf8426f 100644 --- a/packages/flutter_tools/lib/src/device.dart +++ b/packages/flutter_tools/lib/src/device.dart @@ -163,7 +163,7 @@ abstract class Device { /// The device's platform. Future get targetPlatform; - String get sdkNameAndVersion; + Future get sdkNameAndVersion; /// Get a log reader for this device. /// If [app] is specified, this will return a log reader specific to that @@ -254,7 +254,7 @@ abstract class Device { device.name, device.id, '${getNameForTargetPlatform(targetPlatform)}', - '${device.sdkNameAndVersion}$supportIndicator', + '${await device.sdkNameAndVersion}$supportIndicator', ]); } diff --git a/packages/flutter_tools/lib/src/fuchsia/fuchsia_device.dart b/packages/flutter_tools/lib/src/fuchsia/fuchsia_device.dart index 9010e0f5f0945..9b1a6a5861897 100644 --- a/packages/flutter_tools/lib/src/fuchsia/fuchsia_device.dart +++ b/packages/flutter_tools/lib/src/fuchsia/fuchsia_device.dart @@ -80,7 +80,7 @@ class FuchsiaDevice extends Device { Future get targetPlatform async => TargetPlatform.fuchsia; @override - String get sdkNameAndVersion => 'Fuchsia'; + Future get sdkNameAndVersion async => 'Fuchsia'; _FuchsiaLogReader _logReader; @override diff --git a/packages/flutter_tools/lib/src/ios/devices.dart b/packages/flutter_tools/lib/src/ios/devices.dart index bfa6379931ffb..f61d48679ce9a 100644 --- a/packages/flutter_tools/lib/src/ios/devices.dart +++ b/packages/flutter_tools/lib/src/ios/devices.dart @@ -346,7 +346,7 @@ class IOSDevice extends Device { Future get targetPlatform async => TargetPlatform.ios; @override - String get sdkNameAndVersion => 'iOS $_sdkVersion ($_buildVersion)'; + Future get sdkNameAndVersion async => 'iOS $_sdkVersion ($_buildVersion)'; String get _sdkVersion => _getDeviceInfo(id, 'ProductVersion'); diff --git a/packages/flutter_tools/lib/src/ios/simulators.dart b/packages/flutter_tools/lib/src/ios/simulators.dart index 82b594c1280d3..1f5f3e79bacad 100644 --- a/packages/flutter_tools/lib/src/ios/simulators.dart +++ b/packages/flutter_tools/lib/src/ios/simulators.dart @@ -555,7 +555,7 @@ class IOSSimulator extends Device { Future get targetPlatform async => TargetPlatform.ios; @override - String get sdkNameAndVersion => category; + Future get sdkNameAndVersion async => category; @override DeviceLogReader getLogReader({ApplicationPackage app}) {