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

ISSUE_TEMPLATE.md

@@ -13,6 +13,6 @@ Run `flutter analyze` and attach any output of that command also.
 
 ## 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/).

packages/flutter_tools/lib/executable.dart

@@ -37,8 +37,12 @@ import 'src/runner/flutter_command.dart';
 /// 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 @@ Future<Null> main(List<String> args) async {
     new CreateCommand(),
     new DaemonCommand(hidden: !verboseHelp),
     new DevicesCommand(),
-    new DoctorCommand(),
+    new DoctorCommand(verbose: verbose),
     new DriveCommand(),
     new FormatCommand(),
     new FuchsiaReloadCommand(),
@@ -67,5 +71,7 @@ Future<Null> main(List<String> args) async {
     new TraceCommand(),
     new UpdatePackagesCommand(hidden: !verboseHelp),
     new UpgradeCommand(),
-  ], verbose: verbose, verboseHelp: verboseHelp);
+  ], verbose: verbose,
+     muteCommandLogging: muteCommandLogging,
+     verboseHelp: verboseHelp);
 }

packages/flutter_tools/lib/runner.dart

@@ -36,6 +36,7 @@ import 'src/version.dart';
 Future<int> run(
   List<String> args,
   List<FlutterCommand> commands, {
+  bool muteCommandLogging: false,
   bool verbose: false,
   bool verboseHelp: false,
   bool reportCrashes,
@@ -43,8 +44,9 @@ Future<int> run(
 }) async {
   reportCrashes ??= !isRunningOnBot;
 
-  if (verboseHelp) {
+  if (muteCommandLogging) {
-    // Remove the verbose option; for help, users don't need to see verbose logs.
+    // 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');
   }

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

@@ -8,7 +8,7 @@ import '../doctor.dart';
 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 @@ class DoctorCommand extends FlutterCommand {
     );
   }
 
+  final bool verbose;
+
   @override
   final String name = 'doctor';
 
@@ -24,7 +26,7 @@ class DoctorCommand extends FlutterCommand {
 
   @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);
   }
 }

packages/flutter_tools/lib/src/doctor.dart

@@ -95,18 +95,26 @@ class Doctor {
     return buffer.toString();
   }
 
-  /// Print verbose information about the state of installed tooling.
+  /// Print information about the state of installed tooling.
-  Future<bool> diagnose({ bool androidLicenses: false }) async {
+  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 @@ class Doctor {
         printStatus('${result.leadingBox} ${validator.title}');
 
       for (ValidationMessage message in result.messages) {
-        final String text = message.message.replaceAll('\n', '\n      ');
+        if (message.isError || message.isHint || verbose == true) {
-        if (message.isError) {
+          final String text = message.message.replaceAll('\n', '\n      ');
-          printStatus('    ✗ $text', emphasis: true);
+          if (message.isError) {
-        } else {
+            printStatus('    ✗ $text', emphasis: true);
-          printStatus('    • $text');
+          } 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 @@ abstract class DoctorValidator {
   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 @@ class ValidationResult {
   final List<ValidationMessage> messages;
 
   String get leadingBox {
-    if (type == ValidationType.missing)
+    assert(type != null);
-      return '[✗]';
+    switch (type) {
-    else if (type == ValidationType.installed)
+      case ValidationType.missing:
-      return '[✓]';
+        return '[✗]';
-    else
+      case ValidationType.installed:
-      return '[-]';
+        return '[✓]';
+      case ValidationType.partial:
+        return '[!]';
+    }
+    return null;
   }
 }
 
 class ValidationMessage {
-  ValidationMessage(this.message) : isError = false;
+  ValidationMessage(this.message) : isError = false, isHint = false;
-  ValidationMessage.error(this.message) : isError = true;
+  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 @@ class DeviceValidator extends DoctorValidator {
       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)

packages/flutter_tools/test/analytics_test.dart

@@ -96,7 +96,7 @@ void main() {
 
     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 @@ void main() {
 
     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']);

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 @@ void main() {
       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 @@ class IntelliJValidatorTestTarget extends IntelliJValidator {
   @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;
+  }
+}