[flutter_tools]: reland android device cleanup (#52542)

packages/flutter_tools/lib/src/android/android_device.dart

@@ -9,7 +9,6 @@ import 'package:process/process.dart';
 
 import '../android/android_builder.dart';
 import '../android/android_sdk.dart';
-import '../android/android_workflow.dart';
 import '../application_package.dart';
 import '../base/common.dart' show throwToolExit, unawaited;
 import '../base/file_system.dart';
@@ -23,11 +22,13 @@ import '../globals.dart' as globals;
 import '../project.dart';
 import '../protocol_discovery.dart';
 
-import 'adb.dart';
 import 'android.dart';
 import 'android_console.dart';
 import 'android_sdk.dart';
 
+// TODO(jonahwilliams): update google3 client after roll to remove export.
+export 'android_device_discovery.dart';
+
 enum _HardwareType { emulator, physical }
 
 /// Map to help our `isLocalEmulator` detection.
@@ -52,22 +53,6 @@ bool allowHeapCorruptionOnWindows(int exitCode) {
   return exitCode == -1073740940 && globals.platform.isWindows;
 }
 
-class AndroidDevices extends PollingDeviceDiscovery {
-  AndroidDevices() : super('Android devices');
-
-  @override
-  bool get supportsPlatform => true;
-
-  @override
-  bool get canListAnything => androidWorkflow.canListDevices;
-
-  @override
-  Future<List<Device>> pollingGetDevices() async => getAdbDevices();
-
-  @override
-  Future<List<String>> getDiagnostics() async => getAdbDeviceDiagnostics();
-}
-
 class AndroidDevice extends Device {
   AndroidDevice(
     String id, {
@@ -855,30 +840,6 @@ AndroidMemoryInfo parseMeminfoDump(String input) {
   return androidMemoryInfo;
 }
 
-/// Return the list of connected ADB devices.
-List<AndroidDevice> getAdbDevices() {
-  final String adbPath = getAdbPath(androidSdk);
-  if (adbPath == null) {
-    return <AndroidDevice>[];
-  }
-  String text;
-  try {
-    text = processUtils.runSync(
-      <String>[adbPath, 'devices', '-l'],
-      throwOnError: true,
-    ).stdout.trim();
-  } on ArgumentError catch (exception) {
-    throwToolExit('Unable to find "adb", check your Android SDK installation and '
-      'ANDROID_HOME environment variable: ${exception.message}');
-  } on ProcessException catch (exception) {
-    throwToolExit('Unable to run "adb", check your Android SDK installation and '
-      'ANDROID_HOME environment variable: ${exception.executable}');
-  }
-  final List<AndroidDevice> devices = <AndroidDevice>[];
-  parseADBDeviceOutput(text, devices: devices);
-  return devices;
-}
-
 /// Android specific implementation of memory info.
 class AndroidMemoryInfo extends MemoryInfo {
   static const String _kUpTimeKey = 'Uptime';
@@ -922,104 +883,6 @@ class AndroidMemoryInfo extends MemoryInfo {
   }
 }
 
-/// Get diagnostics about issues with any connected devices.
-Future<List<String>> getAdbDeviceDiagnostics() async {
-  final String adbPath = getAdbPath(androidSdk);
-  if (adbPath == null) {
-    return <String>[];
-  }
-
-  final RunResult result = await processUtils.run(<String>[adbPath, 'devices', '-l']);
-  if (result.exitCode != 0) {
-    return <String>[];
-  } else {
-    final String text = result.stdout;
-    final List<String> diagnostics = <String>[];
-    parseADBDeviceOutput(text, diagnostics: diagnostics);
-    return diagnostics;
-  }
-}
-
-// 015d172c98400a03       device usb:340787200X product:nakasi model:Nexus_7 device:grouper
-final RegExp _kDeviceRegex = RegExp(r'^(\S+)\s+(\S+)(.*)');
-
-/// Parse the given `adb devices` output in [text], and fill out the given list
-/// of devices and possible device issue diagnostics. Either argument can be null,
-/// in which case information for that parameter won't be populated.
-@visibleForTesting
-void parseADBDeviceOutput(
-  String text, {
-  List<AndroidDevice> devices,
-  List<String> diagnostics,
-}) {
-  // Check for error messages from adb
-  if (!text.contains('List of devices')) {
-    diagnostics?.add(text);
-    return;
-  }
-
-  for (final String line in text.trim().split('\n')) {
-    // Skip lines like: * daemon started successfully *
-    if (line.startsWith('* daemon ')) {
-      continue;
-    }
-
-    // Skip lines about adb server and client version not matching
-    if (line.startsWith(RegExp(r'adb server (version|is out of date)'))) {
-      diagnostics?.add(line);
-      continue;
-    }
-
-    if (line.startsWith('List of devices')) {
-      continue;
-    }
-
-    if (_kDeviceRegex.hasMatch(line)) {
-      final Match match = _kDeviceRegex.firstMatch(line);
-
-      final String deviceID = match[1];
-      final String deviceState = match[2];
-      String rest = match[3];
-
-      final Map<String, String> info = <String, String>{};
-      if (rest != null && rest.isNotEmpty) {
-        rest = rest.trim();
-        for (final String data in rest.split(' ')) {
-          if (data.contains(':')) {
-            final List<String> fields = data.split(':');
-            info[fields[0]] = fields[1];
-          }
-        }
-      }
-
-      if (info['model'] != null) {
-        info['model'] = cleanAdbDeviceName(info['model']);
-      }
-
-      if (deviceState == 'unauthorized') {
-        diagnostics?.add(
-          'Device $deviceID is not authorized.\n'
-          'You might need to check your device for an authorization dialog.'
-        );
-      } else if (deviceState == 'offline') {
-        diagnostics?.add('Device $deviceID is offline.');
-      } else {
-        devices?.add(AndroidDevice(
-          deviceID,
-          productID: info['product'],
-          modelID: info['model'] ?? deviceID,
-          deviceCodeName: info['device'],
-        ));
-      }
-    } else {
-      diagnostics?.add(
-        'Unexpected failure parsing device information from adb output:\n'
-        '$line\n'
-        'Please report a bug at https://github.com/flutter/flutter/issues/new/choose');
-    }
-  }
-}
-
 /// A log reader that logs from `adb logcat`.
 class AdbLogReader extends DeviceLogReader {
   AdbLogReader._(this._adbProcess, this.name)  {

packages/flutter_tools/lib/src/android/android_device_discovery.dart

@@ -0,0 +1,161 @@
+// Copyright 2014 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'package:meta/meta.dart';
+import 'package:process/process.dart';
+
+import '../base/common.dart';
+import '../base/io.dart';
+import '../base/logger.dart';
+import '../base/process.dart';
+import '../device.dart';
+import 'adb.dart';
+import 'android_device.dart';
+import 'android_sdk.dart';
+import 'android_workflow.dart';
+
+/// Device discovery for Android physical devices and emulators.s
+class AndroidDevices extends PollingDeviceDiscovery {
+  AndroidDevices({
+    @required AndroidWorkflow androidWorkflow,
+    @required ProcessManager processManager,
+    @required Logger logger,
+    @required AndroidSdk androidSdk,
+  }) : _androidWorkflow = androidWorkflow,
+       _androidSdk = androidSdk,
+       _processUtils = ProcessUtils(logger: logger, processManager: processManager),
+       super('Android devices');
+
+  final AndroidWorkflow _androidWorkflow;
+  final ProcessUtils _processUtils;
+  final AndroidSdk _androidSdk;
+
+  @override
+  bool get supportsPlatform => true;
+
+  @override
+  bool get canListAnything => _androidWorkflow.canListDevices;
+
+  @override
+  Future<List<Device>> pollingGetDevices() async {
+    final String adbPath = getAdbPath(_androidSdk);
+    if (adbPath == null) {
+      return <AndroidDevice>[];
+    }
+    String text;
+    try {
+      text = (await _processUtils.run(
+        <String>[adbPath, 'devices', '-l'],
+        throwOnError: true,
+      )).stdout.trim();
+    } on ArgumentError catch (exception) {
+      throwToolExit('Unable to find "adb", check your Android SDK installation and '
+        'ANDROID_HOME environment variable: ${exception.message}');
+    } on ProcessException catch (exception) {
+      throwToolExit('Unable to run "adb", check your Android SDK installation and '
+        'ANDROID_HOME environment variable: ${exception.executable}');
+    }
+    final List<AndroidDevice> devices = <AndroidDevice>[];
+    parseADBDeviceOutput(text, devices: devices);
+    return devices;
+  }
+
+  @override
+  Future<List<String>> getDiagnostics() async {
+    final String adbPath = getAdbPath(_androidSdk);
+    if (adbPath == null) {
+      return <String>[];
+    }
+
+    final RunResult result = await _processUtils.run(<String>[adbPath, 'devices', '-l']);
+    if (result.exitCode != 0) {
+      return <String>[];
+    } else {
+      final String text = result.stdout;
+      final List<String> diagnostics = <String>[];
+      parseADBDeviceOutput(text, diagnostics: diagnostics);
+      return diagnostics;
+    }
+  }
+
+  // 015d172c98400a03       device usb:340787200X product:nakasi model:Nexus_7 device:grouper
+  static final RegExp _kDeviceRegex = RegExp(r'^(\S+)\s+(\S+)(.*)');
+
+  /// Parse the given `adb devices` output in [text], and fill out the given list
+  /// of devices and possible device issue diagnostics. Either argument can be null,
+  /// in which case information for that parameter won't be populated.
+  @visibleForTesting
+  static void parseADBDeviceOutput(
+    String text, {
+    List<AndroidDevice> devices,
+    List<String> diagnostics,
+  }) {
+    // Check for error messages from adb
+    if (!text.contains('List of devices')) {
+      diagnostics?.add(text);
+      return;
+    }
+
+    for (final String line in text.trim().split('\n')) {
+      // Skip lines like: * daemon started successfully *
+      if (line.startsWith('* daemon ')) {
+        continue;
+      }
+
+      // Skip lines about adb server and client version not matching
+      if (line.startsWith(RegExp(r'adb server (version|is out of date)'))) {
+        diagnostics?.add(line);
+        continue;
+      }
+
+      if (line.startsWith('List of devices')) {
+        continue;
+      }
+
+      if (_kDeviceRegex.hasMatch(line)) {
+        final Match match = _kDeviceRegex.firstMatch(line);
+
+        final String deviceID = match[1];
+        final String deviceState = match[2];
+        String rest = match[3];
+
+        final Map<String, String> info = <String, String>{};
+        if (rest != null && rest.isNotEmpty) {
+          rest = rest.trim();
+          for (final String data in rest.split(' ')) {
+            if (data.contains(':')) {
+              final List<String> fields = data.split(':');
+              info[fields[0]] = fields[1];
+            }
+          }
+        }
+
+        if (info['model'] != null) {
+          info['model'] = cleanAdbDeviceName(info['model']);
+        }
+
+        if (deviceState == 'unauthorized') {
+          diagnostics?.add(
+            'Device $deviceID is not authorized.\n'
+            'You might need to check your device for an authorization dialog.'
+          );
+        } else if (deviceState == 'offline') {
+          diagnostics?.add('Device $deviceID is offline.');
+        } else {
+          devices?.add(AndroidDevice(
+            deviceID,
+            productID: info['product'],
+            modelID: info['model'] ?? deviceID,
+            deviceCodeName: info['device'],
+          ));
+        }
+      } else {
+        diagnostics?.add(
+          'Unexpected failure parsing device information from adb output:\n'
+          '$line\n'
+          'Please report a bug at https://github.com/flutter/flutter/issues/new/choose');
+      }
+    }
+  }
+}

packages/flutter_tools/lib/src/device.dart

@@ -7,7 +7,8 @@ import 'dart:math' as math;
 
 import 'package:meta/meta.dart';
 
-import 'android/android_device.dart';
+import 'android/android_device_discovery.dart';
+import 'android/android_workflow.dart';
 import 'application_package.dart';
 import 'artifacts.dart';
 import 'base/context.dart';
@@ -69,7 +70,12 @@ class DeviceManager {
   /// of their methods are called.
   List<DeviceDiscovery> get deviceDiscoverers => _deviceDiscoverers;
   final List<DeviceDiscovery> _deviceDiscoverers = List<DeviceDiscovery>.unmodifiable(<DeviceDiscovery>[
-    AndroidDevices(),
+    AndroidDevices(
+      logger: globals.logger,
+      androidSdk: globals.androidSdk,
+      androidWorkflow: androidWorkflow,
+      processManager: globals.processManager,
+    ),
     IOSDevices(),
     IOSSimulators(iosSimulatorUtils: globals.iosSimulatorUtils),
     FuchsiaDevices(),

packages/flutter_tools/test/general.shard/android/android_device_discovery_test.dart

@@ -0,0 +1,124 @@
+// Copyright 2014 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'package:flutter_tools/src/android/android_device.dart';
+import 'package:flutter_tools/src/android/android_device_discovery.dart';
+import 'package:flutter_tools/src/android/android_sdk.dart';
+import 'package:flutter_tools/src/android/android_workflow.dart';
+import 'package:flutter_tools/src/base/logger.dart';
+import 'package:flutter_tools/src/device.dart';
+import 'package:mockito/mockito.dart';
+
+import '../../src/common.dart';
+import '../../src/fake_process_manager.dart';
+
+void main() {
+  testWithoutContext('AndroidDevices returns empty device list on null adb', () async {
+    final AndroidDevices androidDevices = AndroidDevices(
+      androidSdk: MockAndroidSdk(null),
+      logger: BufferLogger.test(),
+      androidWorkflow: AndroidWorkflow(),
+      processManager: FakeProcessManager.list(<FakeCommand>[]),
+    );
+
+    expect(await androidDevices.pollingGetDevices(), isEmpty);
+  }, skip: true); // a null adb unconditionally calls a static method in AndroidSDK that hits the context.
+
+  testWithoutContext('AndroidDevices throwsToolExit on missing adb path', () {
+    final ProcessManager processManager = FakeProcessManager.list(<FakeCommand>[
+      FakeCommand(
+        command: const <String>['adb', 'devices', '-l'],
+        onRun: () {
+          throw ArgumentError('adb');
+        }
+      )
+    ]);
+    final AndroidDevices androidDevices = AndroidDevices(
+      androidSdk: MockAndroidSdk(),
+      logger: BufferLogger.test(),
+      androidWorkflow: AndroidWorkflow(),
+      processManager: processManager,
+    );
+
+    expect(androidDevices.pollingGetDevices(),
+      throwsToolExit(message: RegExp('Unable to find "adb"')));
+  });
+
+  testWithoutContext('AndroidDevices throwsToolExit on failing adb', () {
+    final ProcessManager processManager = FakeProcessManager.list(<FakeCommand>[
+      const FakeCommand(
+        command: <String>['adb', 'devices', '-l'],
+        exitCode: 1,
+      )
+    ]);
+    final AndroidDevices androidDevices = AndroidDevices(
+      androidSdk: MockAndroidSdk(),
+      logger: BufferLogger.test(),
+      androidWorkflow: AndroidWorkflow(),
+      processManager: processManager,
+    );
+
+    expect(androidDevices.pollingGetDevices(),
+      throwsToolExit(message: RegExp('Unable to run "adb"')));
+  });
+
+  testWithoutContext('physical devices', () {
+    final List<AndroidDevice> devices = <AndroidDevice>[];
+    AndroidDevices.parseADBDeviceOutput('''
+List of devices attached
+05a02bac               device usb:336592896X product:razor model:Nexus_7 device:flo
+
+''', devices: devices);
+
+    expect(devices, hasLength(1));
+    expect(devices.first.name, 'Nexus 7');
+    expect(devices.first.category, Category.mobile);
+  });
+
+  testWithoutContext('emulators and short listings', () {
+    final List<AndroidDevice> devices = <AndroidDevice>[];
+    AndroidDevices.parseADBDeviceOutput('''
+List of devices attached
+localhost:36790        device
+0149947A0D01500C       device usb:340787200X
+emulator-5612          host features:shell_2
+
+''', devices: devices);
+
+    expect(devices, hasLength(3));
+    expect(devices.first.name, 'localhost:36790');
+  });
+
+  testWithoutContext('android n', () {
+    final List<AndroidDevice> devices = <AndroidDevice>[];
+    AndroidDevices.parseADBDeviceOutput('''
+List of devices attached
+ZX1G22JJWR             device usb:3-3 product:shamu model:Nexus_6 device:shamu features:cmd,shell_v2
+''', devices: devices);
+
+    expect(devices, hasLength(1));
+    expect(devices.first.name, 'Nexus 6');
+  });
+
+  testWithoutContext('adb error message', () {
+    final List<AndroidDevice> devices = <AndroidDevice>[];
+    final List<String> diagnostics = <String>[];
+    AndroidDevices.parseADBDeviceOutput('''
+It appears you do not have 'Android SDK Platform-tools' installed.
+Use the 'android' tool to install them:
+  android update sdk --no-ui --filter 'platform-tools'
+''', devices: devices, diagnostics: diagnostics);
+
+    expect(devices, isEmpty);
+    expect(diagnostics, hasLength(1));
+    expect(diagnostics.first, contains('you do not have'));
+  });
+}
+
+class MockAndroidSdk extends Mock implements AndroidSdk {
+  MockAndroidSdk([this.adbPath = 'adb']);
+
+  @override
+  final String adbPath;
+}

packages/flutter_tools/test/general.shard/android/android_device_test.dart

@@ -166,92 +166,6 @@ void main() {
     });
   });
 
-  group('getAdbDevices', () {
-    MockProcessManager mockProcessManager;
-
-    setUp(() {
-      mockProcessManager = MockProcessManager();
-    });
-
-    testUsingContext('throws on missing adb path', () {
-      final Directory sdkDir = MockAndroidSdk.createSdkDirectory();
-      globals.config.setValue('android-sdk', sdkDir.path);
-
-      final File adbExe = globals.fs.file(getAdbPath(androidSdk));
-      when(mockProcessManager.runSync(
-        <String>[adbExe.path, 'devices', '-l'],
-      )).thenThrow(ArgumentError(adbExe.path));
-      expect(() => getAdbDevices(), throwsToolExit(message: RegExp('Unable to find "adb".*${adbExe.path}')));
-    }, overrides: <Type, Generator>{
-      AndroidSdk: () => MockAndroidSdk(),
-      FileSystem: () => MemoryFileSystem(),
-      ProcessManager: () => mockProcessManager,
-    });
-
-    testUsingContext('throws on failing adb', () {
-      final Directory sdkDir = MockAndroidSdk.createSdkDirectory();
-      globals.config.setValue('android-sdk', sdkDir.path);
-
-      final File adbExe = globals.fs.file(getAdbPath(androidSdk));
-      when(mockProcessManager.runSync(
-        <String>[adbExe.path, 'devices', '-l'],
-      )).thenThrow(ProcessException(adbExe.path, <String>['devices', '-l']));
-      expect(() => getAdbDevices(), throwsToolExit(message: RegExp('Unable to run "adb".*${adbExe.path}')));
-    }, overrides: <Type, Generator>{
-      AndroidSdk: () => MockAndroidSdk(),
-      FileSystem: () => MemoryFileSystem(),
-      ProcessManager: () => mockProcessManager,
-    });
-
-    testUsingContext('physical devices', () {
-      final List<AndroidDevice> devices = <AndroidDevice>[];
-      parseADBDeviceOutput('''
-List of devices attached
-05a02bac               device usb:336592896X product:razor model:Nexus_7 device:flo
-
-''', devices: devices);
-      expect(devices, hasLength(1));
-      expect(devices.first.name, 'Nexus 7');
-      expect(devices.first.category, Category.mobile);
-    });
-
-    testUsingContext('emulators and short listings', () {
-      final List<AndroidDevice> devices = <AndroidDevice>[];
-      parseADBDeviceOutput('''
-List of devices attached
-localhost:36790        device
-0149947A0D01500C       device usb:340787200X
-emulator-5612          host features:shell_2
-
-''', devices: devices);
-      expect(devices, hasLength(3));
-      expect(devices.first.name, 'localhost:36790');
-    });
-
-    testUsingContext('android n', () {
-      final List<AndroidDevice> devices = <AndroidDevice>[];
-      parseADBDeviceOutput('''
-List of devices attached
-ZX1G22JJWR             device usb:3-3 product:shamu model:Nexus_6 device:shamu features:cmd,shell_v2
-''', devices: devices);
-      expect(devices, hasLength(1));
-      expect(devices.first.name, 'Nexus 6');
-    });
-
-    testUsingContext('adb error message', () {
-      final List<AndroidDevice> devices = <AndroidDevice>[];
-      final List<String> diagnostics = <String>[];
-      parseADBDeviceOutput('''
-It appears you do not have 'Android SDK Platform-tools' installed.
-Use the 'android' tool to install them:
-    android update sdk --no-ui --filter 'platform-tools'
-''', devices: devices, diagnostics: diagnostics);
-      expect(devices, hasLength(0));
-      expect(diagnostics, hasLength(1));
-      expect(diagnostics.first, contains('you do not have'));
-    });
-  });
-
   group('parseAdbDeviceProperties', () {
     test('parse adb shell output', () {
       final Map<String, String> properties = parseAdbDeviceProperties(kAdbShellGetprop);