Condense and summarize doctor output without --verbose (#14173) * Flatten change out from lots of merges and temporary hacks for #7224 * Expand fakes to cover cases where doctor passes * -v != --verbose * Cosmetic touchups * delint * Clean up summary line, add a switch for review comments * Review comments * nit * review comment * review comments
diff --git a/ISSUE_TEMPLATE.md b/ISSUE_TEMPLATE.md index 6fe7f35..7f88cd3 100644 --- a/ISSUE_TEMPLATE.md +++ b/ISSUE_TEMPLATE.md
@@ -13,6 +13,6 @@ ## Flutter Doctor -Paste the output of running `flutter doctor` here. +Paste the output of running `flutter doctor -v` here. > For more information about diagnosing and reporting Flutter bugs, please see [https://flutter.io/bug-reports/](https://flutter.io/bug-reports/).
diff --git a/packages/flutter_tools/lib/executable.dart b/packages/flutter_tools/lib/executable.dart index c3e3835..c26443b 100644 --- a/packages/flutter_tools/lib/executable.dart +++ b/packages/flutter_tools/lib/executable.dart
@@ -37,8 +37,12 @@ /// This function is intended to be used from the `flutter` command line tool. Future<Null> main(List<String> args) async { final bool verbose = args.contains('-v') || args.contains('--verbose'); + + final bool doctor = (args.isNotEmpty && args.first == 'doctor') || + (args.length == 2 && verbose && args.last == 'doctor'); final bool help = args.contains('-h') || args.contains('--help') || (args.isNotEmpty && args.first == 'help') || (args.length == 1 && verbose); + final bool muteCommandLogging = (help || doctor); final bool verboseHelp = help && verbose; await runner.run(args, <FlutterCommand>[ @@ -51,7 +55,7 @@ new CreateCommand(), new DaemonCommand(hidden: !verboseHelp), new DevicesCommand(), - new DoctorCommand(), + new DoctorCommand(verbose: verbose), new DriveCommand(), new FormatCommand(), new FuchsiaReloadCommand(), @@ -67,5 +71,7 @@ new TraceCommand(), new UpdatePackagesCommand(hidden: !verboseHelp), new UpgradeCommand(), - ], verbose: verbose, verboseHelp: verboseHelp); + ], verbose: verbose, + muteCommandLogging: muteCommandLogging, + verboseHelp: verboseHelp); }
diff --git a/packages/flutter_tools/lib/runner.dart b/packages/flutter_tools/lib/runner.dart index e204578..a49caeb 100644 --- a/packages/flutter_tools/lib/runner.dart +++ b/packages/flutter_tools/lib/runner.dart
@@ -36,6 +36,7 @@ Future<int> run( List<String> args, List<FlutterCommand> commands, { + bool muteCommandLogging: false, bool verbose: false, bool verboseHelp: false, bool reportCrashes, @@ -43,8 +44,9 @@ }) async { reportCrashes ??= !isRunningOnBot; - if (verboseHelp) { - // Remove the verbose option; for help, users don't need to see verbose logs. + if (muteCommandLogging) { + // Remove the verbose option; for help and doctor, users don't need to see + // verbose logs. args = new List<String>.from(args); args.removeWhere((String option) => option == '-v' || option == '--verbose'); }
diff --git a/packages/flutter_tools/lib/src/commands/doctor.dart b/packages/flutter_tools/lib/src/commands/doctor.dart index 9845653..b441c20 100644 --- a/packages/flutter_tools/lib/src/commands/doctor.dart +++ b/packages/flutter_tools/lib/src/commands/doctor.dart
@@ -8,7 +8,7 @@ import '../runner/flutter_command.dart'; class DoctorCommand extends FlutterCommand { - DoctorCommand() { + DoctorCommand({this.verbose: false}) { argParser.addFlag('android-licenses', defaultsTo: false, negatable: false, @@ -16,6 +16,8 @@ ); } + final bool verbose; + @override final String name = 'doctor'; @@ -24,7 +26,7 @@ @override Future<FlutterCommandResult> runCommand() async { - final bool success = await doctor.diagnose(androidLicenses: argResults['android-licenses']); + final bool success = await doctor.diagnose(androidLicenses: argResults['android-licenses'], verbose: verbose); return new FlutterCommandResult(success ? ExitStatus.success : ExitStatus.warning); } }
diff --git a/packages/flutter_tools/lib/src/doctor.dart b/packages/flutter_tools/lib/src/doctor.dart index 040d12c..7b8cb07 100644 --- a/packages/flutter_tools/lib/src/doctor.dart +++ b/packages/flutter_tools/lib/src/doctor.dart
@@ -95,18 +95,26 @@ return buffer.toString(); } - /// Print verbose information about the state of installed tooling. - Future<bool> diagnose({ bool androidLicenses: false }) async { + /// Print information about the state of installed tooling. + Future<bool> diagnose({ bool androidLicenses: false, bool verbose: true }) async { if (androidLicenses) return AndroidWorkflow.runLicenseManager(); + if (!verbose) { + printStatus('Doctor summary (to see all details, run flutter doctor -v):'); + } bool doctorResult = true; + int issues = 0; for (DoctorValidator validator in validators) { final ValidationResult result = await validator.validate(); - if (result.type == ValidationType.missing) + if (result.type == ValidationType.missing) { doctorResult = false; + } + if (result.type != ValidationType.installed) { + issues += 1; + } if (result.statusInfo != null) printStatus('${result.leadingBox} ${validator.title} (${result.statusInfo})'); @@ -114,15 +122,28 @@ printStatus('${result.leadingBox} ${validator.title}'); for (ValidationMessage message in result.messages) { - final String text = message.message.replaceAll('\n', '\n '); - if (message.isError) { - printStatus(' ✗ $text', emphasis: true); - } else { - printStatus(' • $text'); + if (message.isError || message.isHint || verbose == true) { + final String text = message.message.replaceAll('\n', '\n '); + if (message.isError) { + printStatus(' ✗ $text', emphasis: true); + } else if (message.isHint) { + printStatus(' ! $text'); + } else { + printStatus(' • $text'); + } } } + if (verbose) + printStatus(''); + } + // Make sure there's always one line before the summary even when not verbose. + if (!verbose) printStatus(''); + if (issues > 0) { + printStatus('! Doctor found issues in $issues categor${issues > 1 ? "ies" : "y"}.'); + } else { + printStatus('• No issues found!'); } return doctorResult; @@ -159,7 +180,10 @@ Future<ValidationResult> validate(); } + class ValidationResult { + /// [ValidationResult.type] should only equal [ValidationResult.installed] + /// if no [messages] are hints or errors. ValidationResult(this.type, this.messages, { this.statusInfo }); final ValidationType type; @@ -168,20 +192,26 @@ final List<ValidationMessage> messages; String get leadingBox { - if (type == ValidationType.missing) - return '[✗]'; - else if (type == ValidationType.installed) - return '[✓]'; - else - return '[-]'; + assert(type != null); + switch (type) { + case ValidationType.missing: + return '[✗]'; + case ValidationType.installed: + return '[✓]'; + case ValidationType.partial: + return '[!]'; + } + return null; } } class ValidationMessage { - ValidationMessage(this.message) : isError = false; - ValidationMessage.error(this.message) : isError = true; + ValidationMessage(this.message) : isError = false, isHint = false; + ValidationMessage.error(this.message) : isError = true, isHint = false; + ValidationMessage.hint(this.message) : isError = false, isHint = true; final bool isError; + final bool isHint; final String message; @override @@ -512,7 +542,7 @@ if (diagnostics.isNotEmpty) { messages = diagnostics.map((String message) => new ValidationMessage(message)).toList(); } else { - messages = <ValidationMessage>[new ValidationMessage('None')]; + messages = <ValidationMessage>[new ValidationMessage.hint('No devices available')]; } } else { messages = await Device.descriptions(devices)
diff --git a/packages/flutter_tools/test/analytics_test.dart b/packages/flutter_tools/test/analytics_test.dart index 21b4d3d..f9fb7b2 100644 --- a/packages/flutter_tools/test/analytics_test.dart +++ b/packages/flutter_tools/test/analytics_test.dart
@@ -96,7 +96,7 @@ testUsingContext('flutter commands send timing events', () async { mockTimes = <int>[1000, 2000]; - when(mockDoctor.diagnose(androidLicenses: false)).thenReturn(true); + when(mockDoctor.diagnose(androidLicenses: false, verbose: false)).thenReturn(true); final DoctorCommand command = new DoctorCommand(); final CommandRunner<Null> runner = createTestCommandRunner(command); await runner.run(<String>['doctor']); @@ -115,7 +115,7 @@ testUsingContext('doctor fail sends warning', () async { mockTimes = <int>[1000, 2000]; - when(mockDoctor.diagnose(androidLicenses: false)).thenReturn(false); + when(mockDoctor.diagnose(androidLicenses: false, verbose: false)).thenReturn(false); final DoctorCommand command = new DoctorCommand(); final CommandRunner<Null> runner = createTestCommandRunner(command); await runner.run(<String>['doctor']);
diff --git a/packages/flutter_tools/test/commands/doctor_test.dart b/packages/flutter_tools/test/commands/doctor_test.dart index 8140c65..9427ac9 100644 --- a/packages/flutter_tools/test/commands/doctor_test.dart +++ b/packages/flutter_tools/test/commands/doctor_test.dart
@@ -2,6 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'dart:async'; + import 'package:flutter_tools/src/base/file_system.dart'; import 'package:flutter_tools/src/doctor.dart'; import 'package:test/test.dart'; @@ -26,6 +28,91 @@ expect(message.message, contains('recommended minimum version')); }); }); + + group('doctor with fake validators', () { + testUsingContext('validate non-verbose output format for run without issues', () async { + expect(await new FakeQuietDoctor().diagnose(verbose: false), isTrue); + expect(testLogger.statusText, equals( + 'Doctor summary (to see all details, run flutter doctor -v):\n' + '[✓] Passing Validator (with statusInfo)\n' + '[✓] Another Passing Validator (with statusInfo)\n' + '[✓] Validators are fun (with statusInfo)\n' + '[✓] Four score and seven validators ago (with statusInfo)\n' + '\n' + '• No issues found!\n' + )); + }); + + testUsingContext('validate non-verbose output format when only one category fails', () async { + expect(await new FakeSinglePassingDoctor().diagnose(verbose: false), isTrue); + expect(testLogger.statusText, equals( + 'Doctor summary (to see all details, run flutter doctor -v):\n' + '[!] Partial Validator with only a Hint\n' + ' ! There is a hint here\n' + '\n' + '! Doctor found issues in 1 category.\n' + )); + }); + + testUsingContext('validate non-verbose output format for a passing run', () async { + expect(await new FakePassingDoctor().diagnose(verbose: false), isTrue); + expect(testLogger.statusText, equals( + 'Doctor summary (to see all details, run flutter doctor -v):\n' + '[✓] Passing Validator (with statusInfo)\n' + '[!] Partial Validator with only a Hint\n' + ' ! There is a hint here\n' + '[!] Partial Validator with Errors\n' + ' ✗ A error message indicating partial installation\n' + ' ! Maybe a hint will help the user\n' + '[✓] Another Passing Validator (with statusInfo)\n' + '\n' + '! Doctor found issues in 2 categories.\n' + )); + }); + + testUsingContext('validate non-verbose output format', () async { + expect(await new FakeDoctor().diagnose(verbose: false), isFalse); + expect(testLogger.statusText, equals( + 'Doctor summary (to see all details, run flutter doctor -v):\n' + '[✓] Passing Validator (with statusInfo)\n' + '[✗] Missing Validator\n' + ' ✗ A useful error message\n' + ' ! A hint message\n' + '[!] Partial Validator with only a Hint\n' + ' ! There is a hint here\n' + '[!] Partial Validator with Errors\n' + ' ✗ A error message indicating partial installation\n' + ' ! Maybe a hint will help the user\n' + '\n' + '! Doctor found issues in 3 categories.\n' + )); + }); + + testUsingContext('validate verbose output format', () async { + expect(await new FakeDoctor().diagnose(verbose: true), isFalse); + expect(testLogger.statusText, equals( + '[✓] Passing Validator (with statusInfo)\n' + ' • A helpful message\n' + ' • A second, somewhat longer helpful message\n' + '\n' + '[✗] Missing Validator\n' + ' ✗ A useful error message\n' + ' • A message that is not an error\n' + ' ! A hint message\n' + '\n' + '[!] Partial Validator with only a Hint\n' + ' ! There is a hint here\n' + ' • But there is no error\n' + '\n' + '[!] Partial Validator with Errors\n' + ' ✗ A error message indicating partial installation\n' + ' ! Maybe a hint will help the user\n' + ' • An extra message with some verbose details\n' + '\n' + '! Doctor found issues in 3 categories.\n' + )); + }); + }); } class IntelliJValidatorTestTarget extends IntelliJValidator { @@ -37,3 +124,116 @@ @override String get version => 'test.test.test'; } + +class PassingValidator extends DoctorValidator { + PassingValidator(String name) : super(name); + + @override + Future<ValidationResult> validate() async { + final List<ValidationMessage> messages = <ValidationMessage>[]; + messages.add(new ValidationMessage('A helpful message')); + messages.add(new ValidationMessage('A second, somewhat longer helpful message')); + return new ValidationResult(ValidationType.installed, messages, statusInfo: 'with statusInfo'); + } +} + +class MissingValidator extends DoctorValidator { + MissingValidator(): super('Missing Validator'); + + @override + Future<ValidationResult> validate() async { + final List<ValidationMessage> messages = <ValidationMessage>[]; + messages.add(new ValidationMessage.error('A useful error message')); + messages.add(new ValidationMessage('A message that is not an error')); + messages.add(new ValidationMessage.hint('A hint message')); + return new ValidationResult(ValidationType.missing, messages); + } +} + +class PartialValidatorWithErrors extends DoctorValidator { + PartialValidatorWithErrors() : super('Partial Validator with Errors'); + + @override + Future<ValidationResult> validate() async { + final List<ValidationMessage> messages = <ValidationMessage>[]; + messages.add(new ValidationMessage.error('A error message indicating partial installation')); + messages.add(new ValidationMessage.hint('Maybe a hint will help the user')); + messages.add(new ValidationMessage('An extra message with some verbose details')); + return new ValidationResult(ValidationType.partial, messages); + } +} + +class PartialValidatorWithHintsOnly extends DoctorValidator { + PartialValidatorWithHintsOnly() : super('Partial Validator with only a Hint'); + + @override + Future<ValidationResult> validate() async { + final List<ValidationMessage> messages = <ValidationMessage>[]; + messages.add(new ValidationMessage.hint('There is a hint here')); + messages.add(new ValidationMessage('But there is no error')); + return new ValidationResult(ValidationType.partial, messages); + } +} + +/// A doctor that fails with a missing [ValidationResult]. +class FakeDoctor extends Doctor { + List<DoctorValidator> _validators; + + @override + List<DoctorValidator> get validators { + if (_validators == null) { + _validators = <DoctorValidator>[]; + _validators.add(new PassingValidator('Passing Validator')); + _validators.add(new MissingValidator()); + _validators.add(new PartialValidatorWithHintsOnly()); + _validators.add(new PartialValidatorWithErrors()); + } + return _validators; + } +} + +/// A doctor that should pass, but still has issues in some categories. +class FakePassingDoctor extends Doctor { + List<DoctorValidator> _validators; + @override + List<DoctorValidator> get validators { + if (_validators == null) { + _validators = <DoctorValidator>[]; + _validators.add(new PassingValidator('Passing Validator')); + _validators.add(new PartialValidatorWithHintsOnly()); + _validators.add(new PartialValidatorWithErrors()); + _validators.add(new PassingValidator('Another Passing Validator')); + } + return _validators; + } +} + +/// A doctor that should pass, but still has 1 issue to test the singular of +/// categories. +class FakeSinglePassingDoctor extends Doctor { + List<DoctorValidator> _validators; + @override + List<DoctorValidator> get validators { + if (_validators == null) { + _validators = <DoctorValidator>[]; + _validators.add(new PartialValidatorWithHintsOnly()); + } + return _validators; + } +} + +/// A doctor that passes and has no issues anywhere. +class FakeQuietDoctor extends Doctor { + List<DoctorValidator> _validators; + @override + List<DoctorValidator> get validators { + if (_validators == null) { + _validators = <DoctorValidator>[]; + _validators.add(new PassingValidator('Passing Validator')); + _validators.add(new PassingValidator('Another Passing Validator')); + _validators.add(new PassingValidator('Validators are fun')); + _validators.add(new PassingValidator('Four score and seven validators ago')); + } + return _validators; + } +} \ No newline at end of file