From 55176c4d6f514c0260a0dc19a131ff5528013c6c Mon Sep 17 00:00:00 2001 From: Jenn Magder Date: Fri, 17 Jul 2020 09:41:03 -0700 Subject: [PATCH] Show device diagnostic messages in doctor (#61585) --- .../lib/src/commands/daemon.dart | 2 +- .../lib/src/commands/devices.dart | 4 +- .../flutter_tools/lib/src/commands/drive.dart | 1 + .../flutter_tools/lib/src/commands/run.dart | 2 +- packages/flutter_tools/lib/src/device.dart | 2 +- packages/flutter_tools/lib/src/doctor.dart | 56 +++++++++---- packages/flutter_tools/lib/src/globals.dart | 2 + .../lib/src/runner/flutter_command.dart | 6 +- .../src/runner/flutter_command_runner.dart | 3 +- .../commands.shard/hermetic/devices_test.dart | 3 +- .../commands.shard/hermetic/doctor_test.dart | 81 +++++++++++++++++++ .../commands.shard/hermetic/run_test.dart | 14 ++-- .../permeable/devices_test.dart | 7 +- 13 files changed, 146 insertions(+), 37 deletions(-) diff --git a/packages/flutter_tools/lib/src/commands/daemon.dart b/packages/flutter_tools/lib/src/commands/daemon.dart index 4daf9428e41..9ab0733e48e 100644 --- a/packages/flutter_tools/lib/src/commands/daemon.dart +++ b/packages/flutter_tools/lib/src/commands/daemon.dart @@ -770,7 +770,7 @@ class DeviceDomain extends Domain { // Use the device manager discovery so that client provided device types // are usable via the daemon protocol. - deviceManager.deviceDiscoverers.forEach(addDeviceDiscoverer); + globals.deviceManager.deviceDiscoverers.forEach(addDeviceDiscoverer); } void addDeviceDiscoverer(DeviceDiscovery discoverer) { diff --git a/packages/flutter_tools/lib/src/commands/devices.dart b/packages/flutter_tools/lib/src/commands/devices.dart index dafe88406d5..53c9eea4494 100644 --- a/packages/flutter_tools/lib/src/commands/devices.dart +++ b/packages/flutter_tools/lib/src/commands/devices.dart @@ -56,7 +56,7 @@ class DevicesCommand extends FlutterCommand { exitCode: 1); } - final List devices = await deviceManager.refreshAllConnectedDevices(timeout: timeout); + final List devices = await globals.deviceManager.refreshAllConnectedDevices(timeout: timeout); if (boolArg('machine')) { await printDevicesAsJson(devices); @@ -84,7 +84,7 @@ class DevicesCommand extends FlutterCommand { } Future _printDiagnostics() async { - final List diagnostics = await deviceManager.getDeviceDiagnostics(); + final List diagnostics = await globals.deviceManager.getDeviceDiagnostics(); if (diagnostics.isNotEmpty) { globals.printStatus(''); for (final String diagnostic in diagnostics) { diff --git a/packages/flutter_tools/lib/src/commands/drive.dart b/packages/flutter_tools/lib/src/commands/drive.dart index abdf34d8eec..148e0e2ca3e 100644 --- a/packages/flutter_tools/lib/src/commands/drive.dart +++ b/packages/flutter_tools/lib/src/commands/drive.dart @@ -388,6 +388,7 @@ $ex } Future findTargetDevice() async { + final DeviceManager deviceManager = globals.deviceManager; final List devices = await deviceManager.findTargetDevices(FlutterProject.current()); if (deviceManager.hasSpecifiedDeviceId) { diff --git a/packages/flutter_tools/lib/src/commands/run.dart b/packages/flutter_tools/lib/src/commands/run.dart index 862bd18fda4..387d4e816d3 100644 --- a/packages/flutter_tools/lib/src/commands/run.dart +++ b/packages/flutter_tools/lib/src/commands/run.dart @@ -342,7 +342,7 @@ class RunCommand extends RunCommandBase { if (devices == null) { throwToolExit(null); } - if (deviceManager.hasSpecifiedAllDevices && runningWithPrebuiltApplication) { + if (globals.deviceManager.hasSpecifiedAllDevices && runningWithPrebuiltApplication) { throwToolExit('Using -d all with --use-application-binary is not supported'); } diff --git a/packages/flutter_tools/lib/src/device.dart b/packages/flutter_tools/lib/src/device.dart index 0e9454b3336..2c7259910d6 100644 --- a/packages/flutter_tools/lib/src/device.dart +++ b/packages/flutter_tools/lib/src/device.dart @@ -238,7 +238,7 @@ abstract class DeviceManager { globals.printStatus(globals.userMessages.flutterMultipleDevicesFound); await Device.printDevices(devices); final Device chosenDevice = await _chooseOneOfAvailableDevices(devices); - deviceManager.specifiedDeviceId = chosenDevice.id; + globals.deviceManager.specifiedDeviceId = chosenDevice.id; devices = [chosenDevice]; } } diff --git a/packages/flutter_tools/lib/src/doctor.dart b/packages/flutter_tools/lib/src/doctor.dart index 069bb7757c7..389068e1eb9 100644 --- a/packages/flutter_tools/lib/src/doctor.dart +++ b/packages/flutter_tools/lib/src/doctor.dart @@ -108,8 +108,11 @@ class _DefaultDoctorValidatorsProvider implements DoctorValidatorsProvider { NoIdeValidator(), if (proxyValidator.shouldShow) proxyValidator, - if (deviceManager.canListAnything) - DeviceValidator(), + if (globals.deviceManager.canListAnything) + DeviceValidator( + deviceManager: globals.deviceManager, + userMessages: globals.userMessages, + ), ]; return _validators; } @@ -948,31 +951,52 @@ class IntelliJValidatorOnMac extends IntelliJValidator { } class DeviceValidator extends DoctorValidator { - DeviceValidator() : super('Connected device'); + // TODO(jmagman): Make required once g3 rolls and is updated. + DeviceValidator({ + DeviceManager deviceManager, + UserMessages userMessages, + }) : _deviceManager = deviceManager ?? globals.deviceManager, + _userMessages = userMessages ?? globals.userMessages, + super('Connected device'); + + final DeviceManager _deviceManager; + final UserMessages _userMessages; @override String get slowWarning => 'Scanning for devices is taking a long time...'; @override Future validate() async { - final List devices = await deviceManager.getAllConnectedDevices(); - List messages; - if (devices.isEmpty) { - final List diagnostics = await deviceManager.getDeviceDiagnostics(); - if (diagnostics.isNotEmpty) { - messages = diagnostics.map((String message) => ValidationMessage(message)).toList(); - } else { - messages = [ValidationMessage.hint(userMessages.devicesMissing)]; - } - } else { - messages = await Device.descriptions(devices) + final List devices = await _deviceManager.getAllConnectedDevices(); + List installedMessages = []; + if (devices.isNotEmpty) { + installedMessages = await Device.descriptions(devices) .map((String msg) => ValidationMessage(msg)).toList(); } + List diagnosticMessages = []; + final List diagnostics = await _deviceManager.getDeviceDiagnostics(); + if (diagnostics.isNotEmpty) { + diagnosticMessages = diagnostics.map((String message) => ValidationMessage.hint(message)).toList(); + } else if (devices.isEmpty) { + diagnosticMessages = [ValidationMessage.hint(_userMessages.devicesMissing)]; + } + if (devices.isEmpty) { - return ValidationResult(ValidationType.notAvailable, messages); + return ValidationResult(ValidationType.notAvailable, diagnosticMessages); + } else if (diagnostics.isNotEmpty) { + installedMessages.addAll(diagnosticMessages); + return ValidationResult( + ValidationType.installed, + installedMessages, + statusInfo: _userMessages.devicesAvailable(devices.length) + ); } else { - return ValidationResult(ValidationType.installed, messages, statusInfo: userMessages.devicesAvailable(devices.length)); + return ValidationResult( + ValidationType.installed, + installedMessages, + statusInfo: _userMessages.devicesAvailable(devices.length) + ); } } } diff --git a/packages/flutter_tools/lib/src/globals.dart b/packages/flutter_tools/lib/src/globals.dart index 874874dee21..48071f3162c 100644 --- a/packages/flutter_tools/lib/src/globals.dart +++ b/packages/flutter_tools/lib/src/globals.dart @@ -24,6 +24,7 @@ import 'base/time.dart'; import 'base/user_messages.dart'; import 'build_system/build_system.dart'; import 'cache.dart'; +import 'device.dart'; import 'doctor.dart'; import 'fuchsia/fuchsia_sdk.dart'; import 'ios/ios_workflow.dart'; @@ -49,6 +50,7 @@ OperatingSystemUtils get os => context.get(); PersistentToolState get persistentToolState => PersistentToolState.instance; Signals get signals => context.get() ?? LocalSignals.instance; Usage get flutterUsage => context.get(); +DeviceManager get deviceManager => context.get(); FlutterProjectFactory get projectFactory { return context.get() ?? FlutterProjectFactory( diff --git a/packages/flutter_tools/lib/src/runner/flutter_command.dart b/packages/flutter_tools/lib/src/runner/flutter_command.dart index 98188c95af3..9650da2d315 100644 --- a/packages/flutter_tools/lib/src/runner/flutter_command.dart +++ b/packages/flutter_tools/lib/src/runner/flutter_command.dart @@ -892,7 +892,7 @@ abstract class FlutterCommand extends Command { globals.printError(userMessages.flutterNoDevelopmentDevice); return null; } - + final DeviceManager deviceManager = globals.deviceManager; List devices = await deviceManager.findTargetDevices(FlutterProject.current()); if (devices.isEmpty && deviceManager.hasSpecifiedDeviceId) { @@ -946,7 +946,7 @@ abstract class FlutterCommand extends Command { } if (deviceList.length > 1) { globals.printStatus(userMessages.flutterSpecifyDevice); - deviceList = await deviceManager.getAllConnectedDevices(); + deviceList = await globals.deviceManager.getAllConnectedDevices(); globals.printStatus(''); await Device.printDevices(deviceList); return null; @@ -1023,7 +1023,7 @@ mixin DeviceBasedDevelopmentArtifacts on FlutterCommand { // If there are no attached devices, use the default configuration. // Otherwise, only add development artifacts which correspond to a // connected device. - final List devices = await deviceManager.getDevices(); + final List devices = await globals.deviceManager.getDevices(); if (devices.isEmpty) { return super.requiredArtifacts; } diff --git a/packages/flutter_tools/lib/src/runner/flutter_command_runner.dart b/packages/flutter_tools/lib/src/runner/flutter_command_runner.dart index 21be933e103..8a284113405 100644 --- a/packages/flutter_tools/lib/src/runner/flutter_command_runner.dart +++ b/packages/flutter_tools/lib/src/runner/flutter_command_runner.dart @@ -21,7 +21,6 @@ import '../base/utils.dart'; import '../cache.dart'; import '../convert.dart'; import '../dart/package_map.dart'; -import '../device.dart'; import '../globals.dart' as globals; import '../tester/flutter_tester.dart'; @@ -311,7 +310,7 @@ class FlutterCommandRunner extends CommandRunner { } // See if the user specified a specific device. - deviceManager.specifiedDeviceId = topLevelResults['device-id'] as String; + globals.deviceManager.specifiedDeviceId = topLevelResults['device-id'] as String; if (topLevelResults['version'] as bool) { globals.flutterUsage.sendCommand('version'); diff --git a/packages/flutter_tools/test/commands.shard/hermetic/devices_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/devices_test.dart index 21d09e2faaf..75d55b9ad12 100644 --- a/packages/flutter_tools/test/commands.shard/hermetic/devices_test.dart +++ b/packages/flutter_tools/test/commands.shard/hermetic/devices_test.dart @@ -11,6 +11,7 @@ import 'package:flutter_tools/src/artifacts.dart'; import 'package:flutter_tools/src/cache.dart'; import 'package:flutter_tools/src/commands/devices.dart'; import 'package:flutter_tools/src/device.dart'; +import 'package:flutter_tools/src/globals.dart' as globals; import 'package:mockito/mockito.dart'; import 'package:process/process.dart'; @@ -53,7 +54,7 @@ void main() { testUsingContext('get devices\' platform types', () async { final List platformTypes = Device.devicesPlatformTypes( - await deviceManager.getAllConnectedDevices(), + await globals.deviceManager.getAllConnectedDevices(), ); expect(platformTypes, ['android', 'web']); }, overrides: { diff --git a/packages/flutter_tools/test/commands.shard/hermetic/doctor_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/doctor_test.dart index 4a82adc0345..e9560533140 100644 --- a/packages/flutter_tools/test/commands.shard/hermetic/doctor_test.dart +++ b/packages/flutter_tools/test/commands.shard/hermetic/doctor_test.dart @@ -14,8 +14,10 @@ import 'package:flutter_tools/src/base/logger.dart'; import 'package:flutter_tools/src/base/platform.dart'; import 'package:flutter_tools/src/base/terminal.dart'; import 'package:flutter_tools/src/base/user_messages.dart'; +import 'package:flutter_tools/src/build_info.dart'; import 'package:flutter_tools/src/cache.dart'; import 'package:flutter_tools/src/commands/doctor.dart'; +import 'package:flutter_tools/src/device.dart'; import 'package:flutter_tools/src/doctor.dart'; import 'package:flutter_tools/src/features.dart'; import 'package:flutter_tools/src/globals.dart' as globals; @@ -187,6 +189,76 @@ void main() { expect(message.message, startsWith('Flutter extension not installed')); expect(message.isError, isTrue); }, overrides: noColorTerminalOverride); + + group('device validator', () { + testWithoutContext('no devices', () async { + final MockDeviceManager mockDeviceManager = MockDeviceManager(); + + when(mockDeviceManager.getAllConnectedDevices()).thenAnswer( + (Invocation invocation) => Future>.value([]) + ); + when(mockDeviceManager.getDeviceDiagnostics()).thenAnswer( + (Invocation invocation) => Future>.value([]) + ); + + final DeviceValidator deviceValidator = DeviceValidator( + deviceManager: mockDeviceManager, + userMessages: UserMessages(), + ); + final ValidationResult result = await deviceValidator.validate(); + expect(result.type, ValidationType.notAvailable); + expect(result.messages, const [ + ValidationMessage.hint('No devices available'), + ]); + expect(result.statusInfo, isNull); + }); + + testWithoutContext('diagnostic message', () async { + final MockDeviceManager mockDeviceManager = MockDeviceManager(); + + when(mockDeviceManager.getAllConnectedDevices()).thenAnswer( + (Invocation invocation) => Future>.value([]) + ); + when(mockDeviceManager.getDeviceDiagnostics()).thenAnswer( + (Invocation invocation) => Future>.value(['Device locked']) + ); + + final DeviceValidator deviceValidator = DeviceValidator( + deviceManager: mockDeviceManager, + userMessages: UserMessages(), + ); + final ValidationResult result = await deviceValidator.validate(); + expect(result.type, ValidationType.notAvailable); + expect(result.messages, const [ + ValidationMessage.hint('Device locked'), + ]); + expect(result.statusInfo, isNull); + }); + + testWithoutContext('diagnostic message and devices', () async { + final MockDeviceManager mockDeviceManager = MockDeviceManager(); + final MockDevice mockDevice = MockDevice(); + + when(mockDeviceManager.getAllConnectedDevices()).thenAnswer( + (_) => Future>.value([mockDevice]) + ); + when(mockDeviceManager.getDeviceDiagnostics()).thenAnswer( + (_) => Future>.value(['Device locked']) + ); + + final DeviceValidator deviceValidator = DeviceValidator( + deviceManager: mockDeviceManager, + userMessages: UserMessages(), + ); + final ValidationResult result = await deviceValidator.validate(); + expect(result.type, ValidationType.installed); + expect(result.messages, const [ + ValidationMessage('null (null) • device-id • android • null'), + ValidationMessage.hint('Device locked'), + ]); + expect(result.statusInfo, '1 available'); + }); + }); }); group('doctor with overridden validators', () { @@ -1154,3 +1226,12 @@ class VsCodeValidatorTestTargets extends VsCodeValidator { class MockProcessManager extends Mock implements ProcessManager {} class MockArtifacts extends Mock implements Artifacts {} class MockPlistParser extends Mock implements PlistParser {} +class MockDeviceManager extends Mock implements DeviceManager {} +class MockDevice extends Mock implements Device { + MockDevice() { + when(isSupported()).thenReturn(true); + when(id).thenReturn('device-id'); + when(isLocalEmulator).thenAnswer((_) => Future.value(false)); + when(targetPlatform).thenAnswer((_) => Future.value(TargetPlatform.android)); + } +} diff --git a/packages/flutter_tools/test/commands.shard/hermetic/run_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/run_test.dart index 65c8e4b4cb2..52f3324b825 100644 --- a/packages/flutter_tools/test/commands.shard/hermetic/run_test.dart +++ b/packages/flutter_tools/test/commands.shard/hermetic/run_test.dart @@ -208,17 +208,17 @@ void main() { globals.fs.file('.packages').writeAsStringSync('\n'); globals.fs.file('lib/main.dart').createSync(recursive: true); final FakeDevice device = FakeDevice(isLocalEmulator: true); - when(deviceManager.getAllConnectedDevices()).thenAnswer((Invocation invocation) async { + when(mockDeviceManager.getAllConnectedDevices()).thenAnswer((Invocation invocation) async { return [device]; }); - when(deviceManager.getDevices()).thenAnswer((Invocation invocation) async { + when(mockDeviceManager.getDevices()).thenAnswer((Invocation invocation) async { return [device]; }); - when(deviceManager.findTargetDevices(any)).thenAnswer((Invocation invocation) async { + when(mockDeviceManager.findTargetDevices(any)).thenAnswer((Invocation invocation) async { return [device]; }); - when(deviceManager.hasSpecifiedAllDevices).thenReturn(false); - when(deviceManager.deviceDiscoverers).thenReturn([]); + when(mockDeviceManager.hasSpecifiedAllDevices).thenReturn(false); + when(mockDeviceManager.deviceDiscoverers).thenReturn([]); final RunCommand command = RunCommand(); applyMocksToCommand(command); @@ -231,7 +231,7 @@ void main() { }, overrides: { FileSystem: () => MemoryFileSystem.test(), ProcessManager: () => FakeProcessManager.any(), - DeviceManager: () => MockDeviceManager(), + DeviceManager: () => mockDeviceManager, Stdio: () => MockStdio(), }); @@ -557,7 +557,7 @@ class TestRunCommand extends RunCommand { @override // ignore: must_call_super Future validateCommand() async { - devices = await deviceManager.getDevices(); + devices = await globals.deviceManager.getDevices(); } } diff --git a/packages/flutter_tools/test/commands.shard/permeable/devices_test.dart b/packages/flutter_tools/test/commands.shard/permeable/devices_test.dart index 9c0bccdf50f..af6b98232ad 100644 --- a/packages/flutter_tools/test/commands.shard/permeable/devices_test.dart +++ b/packages/flutter_tools/test/commands.shard/permeable/devices_test.dart @@ -8,10 +8,11 @@ import 'package:args/command_runner.dart'; import 'package:flutter_tools/src/cache.dart'; import 'package:flutter_tools/src/commands/devices.dart'; +import 'package:flutter_tools/src/base/context.dart'; import 'package:flutter_tools/src/base/logger.dart'; import 'package:flutter_tools/src/device.dart'; import 'package:flutter_tools/src/features.dart'; -import 'package:flutter_tools/src/base/context.dart'; +import 'package:flutter_tools/src/globals.dart' as globals; import 'package:flutter_tools/src/web/web_device.dart'; import 'package:mockito/mockito.dart'; @@ -39,7 +40,7 @@ void main() { }); testUsingContext('devices can display via the --machine flag', () async { - when(deviceManager.refreshAllConnectedDevices()).thenAnswer((Invocation invocation) async { + when(globals.deviceManager.refreshAllConnectedDevices()).thenAnswer((Invocation invocation) async { return [ WebServerDevice(logger: BufferLogger.test()), ]; @@ -80,4 +81,4 @@ void main() { class MockDeviceManager extends Mock implements DeviceManager { -} \ No newline at end of file +}