Merge pull request #2123 from devoncarew/device_commands

additional validation for device commands

packages/flutter_tools/lib/src/base/file_system.dart

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 import 'dart:io' as dart_io;
+
 import 'package:file/io.dart';
 import 'package:file/sync_io.dart';
 import 'package:path/path.dart' as path;

packages/flutter_tools/lib/src/commands/drive.dart

@@ -83,6 +83,8 @@ class DriveCommand extends RunCommand {
 
   DriveCommand() : this.custom();
 
+  bool get requiresDevice => true;
+
   @override
   Future<int> runInProject() async {
     String testFile = _getTestFile();

packages/flutter_tools/lib/src/commands/install.dart

@@ -3,41 +3,29 @@
 // found in the LICENSE file.
 
 import 'dart:async';
-import 'dart:io';
 
 import '../application_package.dart';
 import '../device.dart';
-import '../ios/simulators.dart';
 import '../runner/flutter_command.dart';
 
 class InstallCommand extends FlutterCommand {
   final String name = 'install';
   final String description = 'Install Flutter apps on attached devices.';
 
-  InstallCommand() {
-    argParser.addFlag('boot', help: 'Boot the iOS Simulator if it isn\'t already running.');
-  }
+  bool get requiresDevice => true;
 
   @override
   Future<int> runInProject() async {
     await downloadApplicationPackagesAndConnectToDevices();
-    bool installedAny = await installApp(
-      devices,
-      applicationPackages,
-      boot: argResults['boot']
-    );
+    bool installedAny = await installApp(devices, applicationPackages);
     return installedAny ? 0 : 2;
   }
 }
 
 Future<bool> installApp(
   DeviceStore devices,
-  ApplicationPackageStore applicationPackages, {
-  bool boot: false
-}) async {
-  if (boot && Platform.isMacOS)
-    await SimControl.boot();
-
+  ApplicationPackageStore applicationPackages
+) async {
   bool installedSomewhere = false;
 
   for (Device device in devices.all) {

packages/flutter_tools/lib/src/commands/listen.dart

@@ -22,6 +22,10 @@ class ListenCommand extends RunCommandBase {
 
   ListenCommand({ this.singleRun: false });
 
+  bool get androidOnly => true;
+
+  bool get requiresDevice => true;
+
   @override
   Future<int> runInProject() async {
     await downloadApplicationPackagesAndConnectToDevices();

packages/flutter_tools/lib/src/commands/logs.dart

@@ -22,6 +22,8 @@ class LogsCommand extends FlutterCommand {
 
   bool get requiresProjectRoot => false;
 
+  bool get requiresDevice => true;
+
   Future<int> runInProject() async {
     List<Device> devices = await deviceManager.getDevices();
 

packages/flutter_tools/lib/src/commands/refresh.dart

@@ -23,6 +23,10 @@ class RefreshCommand extends FlutterCommand {
     );
   }
 
+  bool get androidOnly => true;
+
+  bool get requiresDevice => true;
+
   @override
   Future<int> runInProject() async {
     printTrace('Downloading toolchain.');

packages/flutter_tools/lib/src/commands/run.dart

@@ -55,8 +55,7 @@ abstract class RunCommandBase extends FlutterCommand {
 
 class RunCommand extends RunCommandBase {
   final String name = 'run';
-  final String description =
-    'Run your Flutter app on an attached device (defaults to checked/debug mode).';
+  final String description = 'Run your Flutter app on an attached device.';
   final List<String> aliases = <String>['start'];
 
   RunCommand() {
@@ -78,6 +77,8 @@ class RunCommand extends RunCommandBase {
         help: 'Listen to the given port for a debug connection.');
   }
 
+  bool get requiresDevice => true;
+
   @override
   Future<int> run() async {
     if (argResults['pub'])

packages/flutter_tools/lib/src/commands/test.dart

@@ -51,12 +51,17 @@ class TestCommand extends FlutterCommand {
   }
 
   TestCommand() {
-    argParser.addFlag('flutter-repo', help: 'Run tests from the \'flutter\' package in the Flutter repository instead of the current directory.', defaultsTo: false);
+    argParser.addFlag(
+      'flutter-repo',
+      help: 'Run tests from the \'flutter\' package in the Flutter repository instead of the current directory.',
+      defaultsTo: false
+    );
   }
 
   Iterable<String> _findTests(Directory directory) {
     return directory.listSync(recursive: true, followLinks: false)
-                    .where((FileSystemEntity entity) => entity.path.endsWith('_test.dart') && FileSystemEntity.isFileSync(entity.path))
+                    .where((FileSystemEntity entity) => entity.path.endsWith('_test.dart') &&
+                      FileSystemEntity.isFileSync(entity.path))
                     .map((FileSystemEntity entity) => path.absolute(entity.path));
   }
 

packages/flutter_tools/lib/src/commands/trace.dart

@@ -25,6 +25,10 @@ class TraceCommand extends FlutterCommand {
         defaultsTo: '10', abbr: 'd', help: 'Duration in seconds to trace.');
   }
 
+  bool get androidOnly => true;
+
+  bool get requiresDevice => true;
+
   @override
   Future<int> runInProject() async {
     await downloadApplicationPackagesAndConnectToDevices();

packages/flutter_tools/lib/src/doctor.dart

@@ -39,7 +39,9 @@ class Doctor {
 
   List<DoctorValidator> _validators = <DoctorValidator>[];
 
-  Iterable<Workflow> get workflows => _validators.where((DoctorValidator validator) => validator is Workflow);
+  List<Workflow> get workflows {
+    return new List<Workflow>.from(_validators.where((DoctorValidator validator) => validator is Workflow));
+  }
 
   /// Print a summary of the state of the tooling, as well as how to get more info.
   void summary() => printStatus(summaryText);

packages/flutter_tools/lib/src/runner/flutter_command.dart

@@ -22,31 +22,68 @@ abstract class FlutterCommand extends Command {
   /// Whether this command needs to be run from the root of a project.
   bool get requiresProjectRoot => true;
 
+  /// Whether this command requires a (single) Flutter target device to be connected.
+  bool get requiresDevice => false;
+
+  /// Whether this command only applies to Android devices.
+  bool get androidOnly => false;
+
   List<BuildConfiguration> get buildConfigurations => runner.buildConfigurations;
 
-  Future downloadApplicationPackages() async {
-    if (applicationPackages == null)
-      applicationPackages = await ApplicationPackageStore.forConfigs(buildConfigurations);
+  Future downloadToolchain() async {
+    toolchain ??= await Toolchain.forConfigs(buildConfigurations);
   }
 
-  Future downloadToolchain() async {
-    if (toolchain == null)
-      toolchain = await Toolchain.forConfigs(buildConfigurations);
+  Future downloadApplicationPackagesAndConnectToDevices() async {
+    await _downloadApplicationPackages();
+    _connectToDevices();
   }
 
-  void connectToDevices() {
-    if (devices == null)
-      devices = new DeviceStore.forConfigs(buildConfigurations);
+  Future _downloadApplicationPackages() async {
+    applicationPackages ??= await ApplicationPackageStore.forConfigs(buildConfigurations);
   }
 
-  Future downloadApplicationPackagesAndConnectToDevices() async {
-    await downloadApplicationPackages();
-    connectToDevices();
+  void _connectToDevices() {
+    devices ??= new DeviceStore.forConfigs(buildConfigurations);
   }
 
   Future<int> run() async {
     if (requiresProjectRoot && !projectRootValidator())
       return 1;
+
+    // Ensure at least one toolchain is installed.
+    if (requiresDevice && !doctor.canLaunchAnything) {
+      printError("Unable to locate a development device; please run 'flutter doctor' "
+        "for information about installing additional components.");
+      return 1;
+    }
+
+    // Validate devices.
+    if (requiresDevice) {
+      List<Device> devices = await deviceManager.getDevices();
+
+      if (devices.isEmpty && deviceManager.hasSpecifiedDeviceId) {
+        printError("No device found with id '${deviceManager.specifiedDeviceId}'.");
+        return 1;
+      } else if (devices.isEmpty) {
+        printStatus('No connected devices.');
+        return 1;
+      }
+
+      devices = devices.where((Device device) => device.isSupported()).toList();
+
+      if (androidOnly)
+        devices = devices.where((Device device) => device.platform == TargetPlatform.android).toList();
+
+      // TODO(devoncarew): Switch this to just supporting one connected device?
+      if (devices.isEmpty) {
+        printStatus('No supported devices connected.');
+        return 1;
+      }
+
+      _devicesForCommand = await _getDevicesForCommand();
+    }
+
     return await runInProject();
   }
 
@@ -63,7 +100,36 @@ abstract class FlutterCommand extends Command {
 
   Future<int> runInProject();
 
+  List<Device> get devicesForCommand => _devicesForCommand;
+
+  Device get deviceForCommand {
+    // TODO(devoncarew): Switch this to just supporting one connected device?
+    return devicesForCommand.isNotEmpty ? devicesForCommand.first : null;
+  }
+
+  // This is caculated in run() if the command has [requiresDevice] specified.
+  List<Device> _devicesForCommand;
+
   ApplicationPackageStore applicationPackages;
   Toolchain toolchain;
   DeviceStore devices;
+
+  Future<List<Device>> _getDevicesForCommand() async {
+    List<Device> devices = await deviceManager.getDevices();
+
+    if (devices.isEmpty)
+      return null;
+
+    if (deviceManager.hasSpecifiedDeviceId) {
+      Device device = await deviceManager.getDeviceById(deviceManager.specifiedDeviceId);
+      return device == null ? <Device>[] : <Device>[device];
+    }
+
+    devices = devices.where((Device device) => device.isSupported()).toList();
+
+    if (androidOnly)
+      devices = devices.where((Device device) => device.platform == TargetPlatform.android).toList();
+
+    return devices;
+  }
 }

packages/flutter_tools/test/daemon_test.dart

@@ -8,12 +8,14 @@ import 'dart:io';
 import 'package:flutter_tools/src/base/context.dart';
 import 'package:flutter_tools/src/base/logger.dart';
 import 'package:flutter_tools/src/commands/daemon.dart';
+import 'package:flutter_tools/src/device.dart';
 import 'package:flutter_tools/src/doctor.dart';
 import 'package:flutter_tools/src/globals.dart';
 import 'package:flutter_tools/src/ios/mac.dart';
 import 'package:mockito/mockito.dart';
 import 'package:test/test.dart';
 
+import 'src/context.dart';
 import 'src/mocks.dart';
 
 main() => defineTests();
@@ -37,6 +39,7 @@ defineTests() {
       appContext[Doctor] = new Doctor();
       if (Platform.isMacOS)
         appContext[XCode] = new XCode();
+      appContext[DeviceManager] = new MockDeviceManager();
     });
 
     tearDown(() {

packages/flutter_tools/test/install_test.dart

@@ -31,6 +31,8 @@ defineTests() {
       when(mockDevices.iOSSimulator.isAppInstalled(any)).thenReturn(false);
       when(mockDevices.iOSSimulator.installApp(any)).thenReturn(false);
 
+      testDeviceManager.addDevice(mockDevices.android);
+
       return createTestCommandRunner(command).run(['install']).then((int code) {
         expect(code, equals(0));
       });
@@ -53,6 +55,8 @@ defineTests() {
       when(mockDevices.iOSSimulator.isAppInstalled(any)).thenReturn(false);
       when(mockDevices.iOSSimulator.installApp(any)).thenReturn(false);
 
+      testDeviceManager.addDevice(mockDevices.iOS);
+
       return createTestCommandRunner(command).run(['install']).then((int code) {
         expect(code, equals(0));
       });

packages/flutter_tools/test/listen_test.dart

@@ -3,7 +3,6 @@
 // found in the LICENSE file.
 
 import 'package:flutter_tools/src/commands/listen.dart';
-import 'package:mockito/mockito.dart';
 import 'package:test/test.dart';
 
 import 'src/common.dart';
@@ -14,17 +13,11 @@ main() => defineTests();
 
 defineTests() {
   group('listen', () {
-    testUsingContext('returns 0 when no device is connected', () {
+    testUsingContext('returns 1 when no device is connected', () {
       ListenCommand command = new ListenCommand(singleRun: true);
-      applyMocksToCommand(command);
-      MockDeviceStore mockDevices = command.devices;
-
-      when(mockDevices.android.isConnected()).thenReturn(false);
-      when(mockDevices.iOS.isConnected()).thenReturn(false);
-      when(mockDevices.iOSSimulator.isConnected()).thenReturn(false);
-
+      applyMocksToCommand(command, noDevices: true);
       return createTestCommandRunner(command).run(['listen']).then((int code) {
-        expect(code, equals(0));
+        expect(code, equals(1));
       });
     });
   });

packages/flutter_tools/test/src/context.dart

@@ -15,6 +15,9 @@ import 'package:test/test.dart';
 /// Return the test logger. This assumes that the current Logger is a BufferLogger.
 BufferLogger get testLogger => context[Logger];
 
+MockDeviceManager get testDeviceManager => context[DeviceManager];
+MockDoctor get testDoctor => context[Doctor];
+
 void testUsingContext(String description, dynamic testMethod(), {
   Timeout timeout,
   Map<Type, dynamic> overrides: const <Type, dynamic>{}
@@ -33,7 +36,7 @@ void testUsingContext(String description, dynamic testMethod(), {
       testContext[DeviceManager] = new MockDeviceManager();
 
     if (!overrides.containsKey(Doctor))
-      testContext[Doctor] = new Doctor();
+      testContext[Doctor] = new MockDoctor();
 
     if (Platform.isMacOS) {
       if (!overrides.containsKey(XCode))
@@ -45,10 +48,31 @@ void testUsingContext(String description, dynamic testMethod(), {
 }
 
 class MockDeviceManager implements DeviceManager {
+  List<Device> devices = <Device>[];
+
   String specifiedDeviceId;
   bool get hasSpecifiedDeviceId => specifiedDeviceId != null;
 
-  Future<List<Device>> getAllConnectedDevices() => new Future.value(<Device>[]);
-  Future<Device> getDeviceById(String deviceId) => new Future.value(null);
-  Future<List<Device>> getDevices() => getAllConnectedDevices();
+  Future<List<Device>> getAllConnectedDevices() => new Future.value(devices);
+
+  Future<Device> getDeviceById(String deviceId) {
+    Device device = devices.firstWhere((Device device) => device.id == deviceId, orElse: () => null);
+    return new Future.value(device);
+  }
+
+  Future<List<Device>> getDevices() async {
+    if (specifiedDeviceId == null) {
+      return getAllConnectedDevices();
+    } else {
+      Device device = await getDeviceById(specifiedDeviceId);
+      return device == null ? <Device>[] : <Device>[device];
+    }
+  }
+
+  void addDevice(Device device) => devices.add(device);
+}
+
+class MockDoctor extends Doctor {
+  // True for testing.
+  bool get canLaunchAnything => true;
 }

packages/flutter_tools/test/src/mocks.dart

@@ -12,6 +12,8 @@ import 'package:flutter_tools/src/runner/flutter_command.dart';
 import 'package:flutter_tools/src/toolchain.dart';
 import 'package:mockito/mockito.dart';
 
+import 'context.dart';
+
 class MockApplicationPackageStore extends ApplicationPackageStore {
   MockApplicationPackageStore() : super(
     android: new AndroidApk(localPath: '/mock/path/to/android/SkyShell.apk'),
@@ -31,14 +33,17 @@ class MockToolchain extends Toolchain {
 
 class MockAndroidDevice extends Mock implements AndroidDevice {
   TargetPlatform get platform => TargetPlatform.android;
+  bool isSupported() => true;
 }
 
 class MockIOSDevice extends Mock implements IOSDevice {
   TargetPlatform get platform => TargetPlatform.iOS;
+  bool isSupported() => true;
 }
 
 class MockIOSSimulator extends Mock implements IOSSimulator {
   TargetPlatform get platform => TargetPlatform.iOSSimulator;
+  bool isSupported() => true;
 }
 
 class MockDeviceStore extends DeviceStore {
@@ -48,10 +53,13 @@ class MockDeviceStore extends DeviceStore {
     iOSSimulator: new MockIOSSimulator());
 }
 
-void applyMocksToCommand(FlutterCommand command) {
+void applyMocksToCommand(FlutterCommand command, { bool noDevices: false }) {
   command
     ..applicationPackages = new MockApplicationPackageStore()
     ..toolchain = new MockToolchain()
     ..devices = new MockDeviceStore()
     ..projectRootValidator = () => true;
+
+  if (!noDevices)
+    testDeviceManager.addDevice(command.devices.android);
 }