From d6ec71d2c06247d8ab2c247a499f64d22727269c Mon Sep 17 00:00:00 2001 From: Chris Bracken Date: Thu, 15 Jun 2017 19:03:24 -0700 Subject: [PATCH] Extract libimobiledevice tools interface (#10759) Extract out IMobileDevice class, move class to idevice_id, ideviceinfo (and eventually other libimobiledevice tools such as iproxy) behind this interface. Add tests for the case where libimobiledevice is not installed, the case where it returns no devices, and the case where it returns device IDs. --- .../flutter_tools/lib/src/ios/devices.dart | 29 ++----- .../lib/src/ios/ios_workflow.dart | 18 +---- packages/flutter_tools/lib/src/ios/mac.dart | 39 +++++++++ .../flutter_tools/test/ios/devices_test.dart | 46 +++++++++++ .../test/ios/ios_workflow_test.dart | 80 ++++++++++++++----- 5 files changed, 152 insertions(+), 60 deletions(-) diff --git a/packages/flutter_tools/lib/src/ios/devices.dart b/packages/flutter_tools/lib/src/ios/devices.dart index dc170ec94f1..1c11cfa2440 100644 --- a/packages/flutter_tools/lib/src/ios/devices.dart +++ b/packages/flutter_tools/lib/src/ios/devices.dart @@ -43,7 +43,6 @@ class IOSDevice extends Device { IOSDevice(String id, { this.name }) : super(id) { _installerPath = _checkForCommand('ideviceinstaller'); _listerPath = _checkForCommand('idevice_id'); - _informerPath = _checkForCommand('ideviceinfo'); _iproxyPath = _checkForCommand('iproxy'); _debuggerPath = _checkForCommand('idevicedebug'); _loggerPath = _checkForCommand('idevicesyslog'); @@ -62,9 +61,6 @@ class IOSDevice extends Device { String _listerPath; String get listerPath => _listerPath; - String _informerPath; - String get informerPath => _informerPath; - String _iproxyPath; String get iproxyPath => _iproxyPath; @@ -97,32 +93,17 @@ class IOSDevice extends Device { bool get supportsStartPaused => false; static List getAttachedDevices() { - if (!iosWorkflow.hasIDeviceId) + if (!iMobileDevice.isInstalled) return []; final List devices = []; - for (String id in _getAttachedDeviceIDs()) { - final String name = IOSDevice._getDeviceInfo(id, 'DeviceName'); + for (String id in iMobileDevice.getAttachedDeviceIDs()) { + final String name = iMobileDevice.getInfoForDevice(id, 'DeviceName'); devices.add(new IOSDevice(id, name: name)); } return devices; } - static Iterable _getAttachedDeviceIDs() { - final String listerPath = _checkForCommand('idevice_id'); - try { - final String output = runSync([listerPath, '-l']); - return output.trim().split('\n').where((String s) => s != null && s.isNotEmpty); - } catch (e) { - return []; - } - } - - static String _getDeviceInfo(String deviceID, String infoKey) { - final String informerPath = _checkForCommand('ideviceinfo'); - return runSync([informerPath, '-k', infoKey, '-u', deviceID]).trim(); - } - static String _checkForCommand( String command, [ String macInstructions = _kIdeviceinstallerInstructions @@ -353,9 +334,9 @@ class IOSDevice extends Device { @override Future get sdkNameAndVersion async => 'iOS $_sdkVersion ($_buildVersion)'; - String get _sdkVersion => _getDeviceInfo(id, 'ProductVersion'); + String get _sdkVersion => iMobileDevice.getInfoForDevice(id, 'ProductVersion'); - String get _buildVersion => _getDeviceInfo(id, 'BuildVersion'); + String get _buildVersion => iMobileDevice.getInfoForDevice(id, 'BuildVersion'); @override DeviceLogReader getLogReader({ApplicationPackage app}) { diff --git a/packages/flutter_tools/lib/src/ios/ios_workflow.dart b/packages/flutter_tools/lib/src/ios/ios_workflow.dart index 4c640aade2d..9f6e0de02b4 100644 --- a/packages/flutter_tools/lib/src/ios/ios_workflow.dart +++ b/packages/flutter_tools/lib/src/ios/ios_workflow.dart @@ -7,7 +7,6 @@ import 'dart:async'; import '../base/common.dart'; import '../base/context.dart'; import '../base/file_system.dart'; -import '../base/io.dart'; import '../base/os.dart'; import '../base/platform.dart'; import '../base/process.dart'; @@ -34,21 +33,6 @@ class IOSWorkflow extends DoctorValidator implements Workflow { @override bool get canLaunchDevices => xcode.isInstalledAndMeetsVersionCheck; - bool get hasIDeviceId => exitsHappy(['idevice_id', '-h']); - - Future get hasWorkingLibimobiledevice async { - // Verify that libimobiledevice tools are installed. - if (!hasIDeviceId) - return false; - - // If a device is attached, verify that we can get its name. - final ProcessResult result = (await runAsync(['idevice_id', '-l'])).processResult; - if (result.exitCode == 0 && result.stdout.isNotEmpty && !await exitsHappyAsync(['idevicename'])) - return false; - - return true; - } - Future get hasIDeviceInstaller => exitsHappyAsync(['ideviceinstaller', '-h']); Future get hasIosDeploy => exitsHappyAsync(['ios-deploy', '--version']); @@ -154,7 +138,7 @@ class IOSWorkflow extends DoctorValidator implements Workflow { if (hasHomebrew) { brewStatus = ValidationType.installed; - if (!await hasWorkingLibimobiledevice) { + if (!await iMobileDevice.isWorking) { brewStatus = ValidationType.partial; messages.add(new ValidationMessage.error( 'libimobiledevice and ideviceinstaller are not installed or require updating. 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 ef9188af84c..5e2337a77f9 100644 --- a/packages/flutter_tools/lib/src/ios/mac.dart +++ b/packages/flutter_tools/lib/src/ios/mac.dart @@ -33,6 +33,8 @@ const int kXcodeRequiredVersionMinor = 0; // Homebrew. const PythonModule kPythonSix = const PythonModule('six'); +IMobileDevice get iMobileDevice => context.putIfAbsent(IMobileDevice, () => const IMobileDevice()); + class PythonModule { const PythonModule(this.name); @@ -45,6 +47,43 @@ class PythonModule { 'Install via \'pip install $name\' or \'sudo easy_install $name\'.'; } +class IMobileDevice { + const IMobileDevice(); + + bool get isInstalled => exitsHappy(['idevice_id', '-h']); + + /// Returns true if libimobiledevice is installed and working as expected. + /// + /// Older releases of libimobiledevice fail to work with iOS 10.3 and above. + Future get isWorking async { + if (!isInstalled) + return false; + + // If no device is attached, we're unable to detect any problems. Assume all is well. + final ProcessResult result = (await runAsync(['idevice_id', '-l'])).processResult; + if (result.exitCode != 0 || result.stdout.isEmpty) + return true; + + // Check that we can look up the names of any attached devices. + return await exitsHappyAsync(['idevicename']); + } + + List getAttachedDeviceIDs() { + return runSync(['idevice_id', '-l']) + .trim() + .split('\n') + .where((String line) => line.isNotEmpty) + .toList(); + } + + /// Returns the value associated with the specified `ideviceinfo` key for a device. + /// + /// If either the specified key or device does not exist, returns the empty string. + String getInfoForDevice(String deviceID, String key) { + return runSync(['ideviceinfo', '-k', key, '-u', deviceID]).trim(); + } +} + class Xcode { Xcode() { _eulaSigned = false; diff --git a/packages/flutter_tools/test/ios/devices_test.dart b/packages/flutter_tools/test/ios/devices_test.dart index ec1b995bca5..fda5250f7a0 100644 --- a/packages/flutter_tools/test/ios/devices_test.dart +++ b/packages/flutter_tools/test/ios/devices_test.dart @@ -8,6 +8,7 @@ import 'dart:io' show ProcessResult; import 'package:file/file.dart'; import 'package:flutter_tools/src/base/file_system.dart'; import 'package:flutter_tools/src/ios/devices.dart'; +import 'package:flutter_tools/src/ios/mac.dart'; import 'package:mockito/mockito.dart'; import 'package:platform/platform.dart'; import 'package:process/process.dart'; @@ -16,12 +17,57 @@ import 'package:test/test.dart'; import '../src/context.dart'; class MockProcessManager extends Mock implements ProcessManager {} +class MockIMobileDevice extends Mock implements IMobileDevice {} class MockFile extends Mock implements File {} void main() { final FakePlatform osx = new FakePlatform.fromPlatform(const LocalPlatform()); osx.operatingSystem = 'macos'; + group('getAttachedDevices', () { + MockIMobileDevice mockIMobileDevice; + + setUp(() { + mockIMobileDevice = new MockIMobileDevice(); + }); + + testUsingContext('return no devices if libimobiledevice is not installed', () async { + when(mockIMobileDevice.isInstalled).thenReturn(false); + expect(IOSDevice.getAttachedDevices(), isEmpty); + }, overrides: { + IMobileDevice: () => mockIMobileDevice, + }); + + testUsingContext('returns no devices if none are attached', () async { + when(mockIMobileDevice.isInstalled).thenReturn(true); + when(mockIMobileDevice.getAttachedDeviceIDs()).thenReturn([]); + final List devices = IOSDevice.getAttachedDevices(); + expect(devices, isEmpty); + }, overrides: { + IMobileDevice: () => mockIMobileDevice, + }); + + testUsingContext('returns attached devices', () async { + when(mockIMobileDevice.isInstalled).thenReturn(true); + when(mockIMobileDevice.getAttachedDeviceIDs()).thenReturn([ + '98206e7a4afd4aedaff06e687594e089dede3c44', + 'f577a7903cc54959be2e34bc4f7f80b7009efcf4', + ]); + when(mockIMobileDevice.getInfoForDevice('98206e7a4afd4aedaff06e687594e089dede3c44', 'DeviceName')) + .thenReturn('La tele me regarde'); + when(mockIMobileDevice.getInfoForDevice('f577a7903cc54959be2e34bc4f7f80b7009efcf4', 'DeviceName')) + .thenReturn('Puits sans fond'); + final List devices = IOSDevice.getAttachedDevices(); + expect(devices, hasLength(2)); + expect(devices[0].id, '98206e7a4afd4aedaff06e687594e089dede3c44'); + expect(devices[0].name, 'La tele me regarde'); + expect(devices[1].id, 'f577a7903cc54959be2e34bc4f7f80b7009efcf4'); + expect(devices[1].name, 'Puits sans fond'); + }, overrides: { + IMobileDevice: () => mockIMobileDevice, + }); + }); + group('screenshot', () { MockProcessManager mockProcessManager; MockFile mockOutputFile; diff --git a/packages/flutter_tools/test/ios/ios_workflow_test.dart b/packages/flutter_tools/test/ios/ios_workflow_test.dart index c7afec70768..b1142481a71 100644 --- a/packages/flutter_tools/test/ios/ios_workflow_test.dart +++ b/packages/flutter_tools/test/ios/ios_workflow_test.dart @@ -19,11 +19,13 @@ import '../src/context.dart'; void main() { group('iOS Workflow validation', () { + MockIMobileDevice iMobileDevice; MockXcode xcode; MockProcessManager processManager; FileSystem fs; setUp(() { + iMobileDevice = new MockIMobileDevice(); xcode = new MockXcode(); processManager = new MockProcessManager(); fs = new MemoryFileSystem(); @@ -39,7 +41,10 @@ void main() { ); final ValidationResult result = await workflow.validate(); expect(result.type, ValidationType.missing); - }, overrides: { Xcode: () => xcode }); + }, overrides: { + IMobileDevice: () => iMobileDevice, + Xcode: () => xcode, + }); testUsingContext('Emits partial status when Xcode is not installed', () async { when(xcode.isInstalled).thenReturn(false); @@ -47,7 +52,10 @@ void main() { final IOSWorkflowTestTarget workflow = new IOSWorkflowTestTarget(); final ValidationResult result = await workflow.validate(); expect(result.type, ValidationType.partial); - }, overrides: { Xcode: () => xcode }); + }, overrides: { + IMobileDevice: () => iMobileDevice, + Xcode: () => xcode, + }); testUsingContext('Emits partial status when Xcode is partially installed', () async { when(xcode.isInstalled).thenReturn(false); @@ -55,7 +63,10 @@ void main() { final IOSWorkflowTestTarget workflow = new IOSWorkflowTestTarget(); final ValidationResult result = await workflow.validate(); expect(result.type, ValidationType.partial); - }, overrides: { Xcode: () => xcode }); + }, overrides: { + IMobileDevice: () => iMobileDevice, + Xcode: () => xcode, + }); testUsingContext('Emits partial status when Xcode version too low', () async { when(xcode.isInstalled).thenReturn(true); @@ -66,7 +77,10 @@ void main() { final IOSWorkflowTestTarget workflow = new IOSWorkflowTestTarget(); final ValidationResult result = await workflow.validate(); expect(result.type, ValidationType.partial); - }, overrides: { Xcode: () => xcode }); + }, overrides: { + IMobileDevice: () => iMobileDevice, + Xcode: () => xcode, + }); testUsingContext('Emits partial status when Xcode EULA not signed', () async { when(xcode.isInstalled).thenReturn(true); @@ -77,7 +91,10 @@ void main() { final IOSWorkflowTestTarget workflow = new IOSWorkflowTestTarget(); final ValidationResult result = await workflow.validate(); expect(result.type, ValidationType.partial); - }, overrides: { Xcode: () => xcode }); + }, overrides: { + IMobileDevice: () => iMobileDevice, + Xcode: () => xcode, + }); testUsingContext('Emits partial status when python six not installed', () async { when(xcode.isInstalled).thenReturn(true); @@ -88,7 +105,10 @@ void main() { final IOSWorkflowTestTarget workflow = new IOSWorkflowTestTarget(hasPythonSixModule: false); final ValidationResult result = await workflow.validate(); expect(result.type, ValidationType.partial); - }, overrides: { Xcode: () => xcode }); + }, overrides: { + IMobileDevice: () => iMobileDevice, + Xcode: () => xcode, + }); testUsingContext('Emits partial status when homebrew not installed', () async { when(xcode.isInstalled).thenReturn(true); @@ -99,7 +119,10 @@ void main() { final IOSWorkflowTestTarget workflow = new IOSWorkflowTestTarget(hasHomebrew: false); final ValidationResult result = await workflow.validate(); expect(result.type, ValidationType.partial); - }, overrides: { Xcode: () => xcode }); + }, overrides: { + IMobileDevice: () => iMobileDevice, + Xcode: () => xcode, + }); testUsingContext('Emits partial status when libimobiledevice is not installed', () async { when(xcode.isInstalled).thenReturn(true); @@ -107,10 +130,13 @@ void main() { .thenReturn('Xcode 8.2.1\nBuild version 8C1002\n'); when(xcode.isInstalledAndMeetsVersionCheck).thenReturn(true); when(xcode.eulaSigned).thenReturn(true); - final IOSWorkflowTestTarget workflow = new IOSWorkflowTestTarget(hasWorkingLibimobiledevice: false); + final IOSWorkflowTestTarget workflow = new IOSWorkflowTestTarget(); final ValidationResult result = await workflow.validate(); expect(result.type, ValidationType.partial); - }, overrides: { Xcode: () => xcode }); + }, overrides: { + IMobileDevice: () => new MockIMobileDevice(isWorking: false), + Xcode: () => xcode, + }); testUsingContext('Emits partial status when ios-deploy is not installed', () async { when(xcode.isInstalled).thenReturn(true); @@ -121,7 +147,10 @@ void main() { final IOSWorkflowTestTarget workflow = new IOSWorkflowTestTarget(hasIosDeploy: false); final ValidationResult result = await workflow.validate(); expect(result.type, ValidationType.partial); - }, overrides: { Xcode: () => xcode }); + }, overrides: { + IMobileDevice: () => iMobileDevice, + Xcode: () => xcode, + }); testUsingContext('Emits partial status when ios-deploy version is too low', () async { when(xcode.isInstalled).thenReturn(true); @@ -132,7 +161,10 @@ void main() { final IOSWorkflowTestTarget workflow = new IOSWorkflowTestTarget(iosDeployVersionText: '1.8.0'); final ValidationResult result = await workflow.validate(); expect(result.type, ValidationType.partial); - }, overrides: { Xcode: () => xcode }); + }, overrides: { + IMobileDevice: () => iMobileDevice, + Xcode: () => xcode, + }); testUsingContext('Emits partial status when CocoaPods is not installed', () async { when(xcode.isInstalled).thenReturn(true); @@ -143,7 +175,10 @@ void main() { final IOSWorkflowTestTarget workflow = new IOSWorkflowTestTarget(hasCocoaPods: false); final ValidationResult result = await workflow.validate(); expect(result.type, ValidationType.partial); - }, overrides: { Xcode: () => xcode }); + }, overrides: { + IMobileDevice: () => iMobileDevice, + Xcode: () => xcode, + }); testUsingContext('Emits partial status when CocoaPods version is too low', () async { when(xcode.isInstalled).thenReturn(true); @@ -154,7 +189,10 @@ void main() { final IOSWorkflowTestTarget workflow = new IOSWorkflowTestTarget(cocoaPodsVersionText: '0.39.0'); final ValidationResult result = await workflow.validate(); expect(result.type, ValidationType.partial); - }, overrides: { Xcode: () => xcode }); + }, overrides: { + IMobileDevice: () => iMobileDevice, + Xcode: () => xcode, + }); testUsingContext('Emits partial status when CocoaPods is not initialized', () async { when(xcode.isInstalled).thenReturn(true); @@ -172,6 +210,7 @@ void main() { expect(result.type, ValidationType.partial); }, overrides: { FileSystem: () => fs, + IMobileDevice: () => iMobileDevice, Xcode: () => xcode, ProcessManager: () => processManager, }); @@ -194,6 +233,7 @@ void main() { expect(result.type, ValidationType.installed); }, overrides: { FileSystem: () => fs, + IMobileDevice: () => iMobileDevice, Xcode: () => xcode, ProcessManager: () => processManager, }); @@ -207,6 +247,13 @@ final ProcessResult exitsHappy = new ProcessResult( '', // stderr ); +class MockIMobileDevice extends IMobileDevice { + MockIMobileDevice({bool isWorking: true}) : isWorking = new Future.value(isWorking); + + @override + final Future isWorking; +} + class MockXcode extends Mock implements Xcode {} class MockProcessManager extends Mock implements ProcessManager {} @@ -214,14 +261,12 @@ class IOSWorkflowTestTarget extends IOSWorkflow { IOSWorkflowTestTarget({ this.hasPythonSixModule: true, this.hasHomebrew: true, - bool hasWorkingLibimobiledevice: true, bool hasIosDeploy: true, String iosDeployVersionText: '1.9.0', bool hasIDeviceInstaller: true, bool hasCocoaPods: true, String cocoaPodsVersionText: '1.2.0', - }) : hasWorkingLibimobiledevice = new Future.value(hasWorkingLibimobiledevice), - hasIosDeploy = new Future.value(hasIosDeploy), + }) : hasIosDeploy = new Future.value(hasIosDeploy), iosDeployVersionText = new Future.value(iosDeployVersionText), hasIDeviceInstaller = new Future.value(hasIDeviceInstaller), hasCocoaPods = new Future.value(hasCocoaPods), @@ -233,9 +278,6 @@ class IOSWorkflowTestTarget extends IOSWorkflow { @override final bool hasHomebrew; - @override - final Future hasWorkingLibimobiledevice; - @override final Future hasIosDeploy;