CP-907 Coverage: check for lcov dependency, fail helpfully

.gitignore

@@ -2,4 +2,6 @@
 packages
 pubspec.lock
 
-coverage/
+/coverage/
+/test/fixtures/coverage/browser/coverage/
+/test/fixtures/coverage/vm/coverage/

README.md

@@ -194,6 +194,13 @@ Name       | Type           | Default     | Description
 `output`   | `String`       | `coverage/` | Output directory for coverage artifacts.
 `reportOn` | `List<String>` | `['lib/']`  | List of paths to include in the generated coverage report (LCOV and HTML).
 
+> Note: "lcov" must be installed in order to generate the HTML report.
+>
+> If you're using brew, you can install it with:
+>    `brew update && brew install lcov`
+>
+> Otherwise, visit http://ltp.sourceforge.net/coverage/lcov.php
+
 ### `examples` Config
 All configuration options for the `examples` task are found on the `config.examples` object.
 

lib/src/platform_util/api.dart

@@ -0,0 +1,15 @@
+library dart_dev.src.platform_util.api;
+
+import 'dart:async';
+
+import 'package:dart_dev/src/platform_util/platform_util.dart';
+
+/// Determines whether or not the project in the current working directory has
+/// defined [packageName] as an immediate dependency. In other words, this
+/// checks if [packageName] is in the project's pubspec.yaml.
+bool hasImmediateDependency(String packageName) =>
+    PlatformUtil.retrieve().hasImmediateDependency(packageName);
+
+/// Determines whether or not [executable] is installed on this platform.
+Future<bool> isExecutableInstalled(String executable) =>
+    PlatformUtil.retrieve().isExecutableInstalled(executable);

lib/src/platform_util/mock_platform_util.dart

@@ -0,0 +1,44 @@
+library dart_dev.src.platform_util.mock_platform_util;
+
+import 'dart:async';
+
+import 'package:dart_dev/src/platform_util/platform_util.dart';
+import 'package:dart_dev/src/platform_util/standard_platform_util.dart';
+
+const List<String> _defaultInstalledExecutables = const ['lcov'];
+
+const Map<String, dynamic> _defaultProjectDependencies = const {
+  'coverage': '^0.7.2',
+  'dart_style': '^0.2.0',
+  'test': '^0.12.0'
+};
+
+class MockPlatformUtil implements PlatformUtil {
+  /// List of executables installed on this platform. Does not actually need
+  /// to be exhaustive, only needs to cover the executables that may be checked
+  /// by a dart_dev task.
+  static List<String> installedExecutables =
+      _defaultInstalledExecutables.toList();
+
+  /// Map of dependencies that are defined by the current project. This
+  /// effectively mocks out any platform util that checks the pubspec.yaml
+  /// for dependencies.
+  static Map<String, dynamic> projectDependencies =
+      new Map.from(_defaultProjectDependencies);
+
+  static void install() {
+    platformUtil = new MockPlatformUtil();
+  }
+
+  static void uninstall() {
+    platformUtil = new StandardPlatformUtil();
+    installedExecutables = _defaultInstalledExecutables.toList();
+    projectDependencies = new Map.from(_defaultProjectDependencies);
+  }
+
+  bool hasImmediateDependency(String packageName) =>
+      projectDependencies.containsKey(packageName);
+
+  Future<bool> isExecutableInstalled(String executable) async =>
+      installedExecutables.contains(executable);
+}

lib/src/platform_util/platform_util.dart

@@ -0,0 +1,26 @@
+library dart_dev.src.platform_util.platform_util;
+
+import 'dart:async';
+
+import 'package:dart_dev/src/platform_util/standard_platform_util.dart';
+
+PlatformUtil platformUtil = new StandardPlatformUtil();
+
+abstract class PlatformUtil {
+  static PlatformUtil retrieve() {
+    if (platformUtil == null) throw new StateError(
+        'dart_dev\'s PlatformUtil instance must not be null.');
+    return platformUtil;
+  }
+
+  /// Generates an HTML report for an LCOV formatted coverage file.
+  // TODO: Future<bool> generateLcovHtml(String lcovPath, String outputPath);
+
+  /// Determines whether or not the project in the current working directory has
+  /// defined [packageName] as an immediate dependency. In other words, this
+  /// checks if [packageName] is in the project's pubspec.yaml.
+  bool hasImmediateDependency(String packageName);
+
+  /// Determines whether or not [executable] is installed on this platform.
+  Future<bool> isExecutableInstalled(String executable);
+}

lib/src/platform_util/standard_platform_util.dart

@@ -0,0 +1,28 @@
+library dart_dev.src.platform_util.standard_platform_util;
+
+import 'dart:async';
+import 'dart:io';
+
+import 'package:yaml/yaml.dart';
+
+import 'package:dart_dev/src/platform_util/platform_util.dart';
+
+class StandardPlatformUtil implements PlatformUtil {
+  bool hasImmediateDependency(String packageName) {
+    File pubspec = new File('pubspec.yaml');
+    Map pubspecYaml = loadYaml(pubspec.readAsStringSync());
+    List deps = [];
+    if (pubspecYaml.containsKey('dependencies')) {
+      deps.addAll((pubspecYaml['dependencies'] as Map).keys);
+    }
+    if (pubspecYaml.containsKey('dev_dependencies')) {
+      deps.addAll((pubspecYaml['dev_dependencies'] as Map).keys);
+    }
+    return deps.contains(packageName);
+  }
+
+  Future<bool> isExecutableInstalled(String executable) async {
+    ProcessResult result = await Process.run('which', [executable]);
+    return result.exitCode == 0;
+  }
+}

lib/src/tasks/coverage/api.dart

@@ -21,7 +21,9 @@ import 'dart:io';
 import 'package:dart_dev/util.dart' show Reporter, TaskProcess, getOpenPort;
 import 'package:path/path.dart' as path;
 
+import 'package:dart_dev/src/platform_util/api.dart' as platform_util;
 import 'package:dart_dev/src/tasks/coverage/config.dart';
+import 'package:dart_dev/src/tasks/coverage/exceptions.dart';
 import 'package:dart_dev/src/tasks/task.dart';
 
 const String _testFilePattern = '_test.dart';
@@ -67,16 +69,9 @@ class CoverageTask extends Task {
       String output: defaultOutput,
       List<String> reportOn: defaultReportOn}) async {
     CoverageTask coverage =
-        new CoverageTask._(tests, html: html, output: output);
-    await coverage._collect();
-    await coverage._format(reportOn);
-
-    if (html) {
-      await coverage._generateReport();
-    }
-    return new CoverageResult.success(
-        coverage.tests, coverage.collection, coverage.lcov,
-        report: coverage.report);
+        new CoverageTask._(tests, reportOn, html: html, output: output);
+    await coverage._run();
+    return coverage.done;
   }
 
   /// Collect and format coverage for the given suite of [tests]. The
@@ -96,22 +91,8 @@ class CoverageTask extends Task {
       String output: defaultOutput,
       List<String> reportOn: defaultReportOn}) {
     CoverageTask coverage =
-        new CoverageTask._(tests, html: html, output: output);
-
-    // Execute the coverage collection and formatting, but don't wait on it.
-    () async {
-      await coverage._collect();
-      await coverage._format(reportOn);
-
-      if (html) {
-        await coverage._generateReport();
-      }
-      CoverageResult result = new CoverageResult.success(
-          coverage.tests, coverage.collection, coverage.lcov,
-          report: coverage.report);
-      coverage._done.complete(result);
-    }();
-
+        new CoverageTask._(tests, reportOn, html: html, output: output);
+    coverage._run();
     return coverage;
   }
 
@@ -135,6 +116,9 @@ class CoverageTask extends Task {
   /// searching all directories for valid test files.
   List<File> _files = [];
 
+  /// Whether or not to generate the HTML report.
+  bool _html = defaultHtml;
+
   /// File created to run the test in a browser. Need to store it so it can be
   /// cleaned up after the test finishes.
   File _lastHtmlFile;
@@ -146,9 +130,14 @@ class CoverageTask extends Task {
   /// Directory to output all coverage related artifacts.
   Directory _outputDirectory;
 
-  CoverageTask._(List<String> tests,
+  /// List of directories on which coverage should be reported.
+  List<String> _reportOn;
+
+  CoverageTask._(List<String> tests, List<String> reportOn,
       {bool html: defaultHtml, String output: defaultOutput})
-      : _outputDirectory = new Directory(output) {
+      : _html = html,
+        _outputDirectory = new Directory(output),
+        _reportOn = reportOn {
     // Build the list of test files.
     tests.forEach((path) {
       if (path.endsWith(_testFilePattern) &&
@@ -212,7 +201,7 @@ class CoverageTask extends Task {
       // Run the test and obtain the observatory port for coverage collection.
       try {
         observatoryPort = await _test(_files[i]);
-      } on TestException {
+      } on CoverageTestSuiteException {
         _coverageErrorOutput.add('Tests failed: ${_files[i].path}');
         continue;
       }
@@ -244,7 +233,7 @@ class CoverageTask extends Task {
     _collection = _merge(collections);
   }
 
-  Future _format(List<String> reportOn) async {
+  Future _format() async {
     _lcov = new File(path.join(_outputDirectory.path, 'coverage.lcov'));
 
     String executable = 'pub';
@@ -259,7 +248,7 @@ class CoverageTask extends Task {
       lcov.path,
       '--verbose'
     ];
-    args.addAll(reportOn.map((p) => '--report-on=$p'));
+    args.addAll(_reportOn.map((p) => '--report-on=$p'));
 
     _coverageOutput.add('');
     _coverageOutput.add('Formatting coverage');
@@ -323,6 +312,23 @@ class CoverageTask extends Task {
     return coverage;
   }
 
+  Future _run() async {
+    if (_html && !(await platform_util.isExecutableInstalled('lcov'))) {
+      _done.completeError(new MissingLcovException());
+      return;
+    }
+
+    await _collect();
+    await _format();
+
+    if (_html) {
+      await _generateReport();
+    }
+
+    _done.complete(
+        new CoverageResult.success(tests, collection, lcov, report: report));
+  }
+
   Future<int> _test(File file) async {
     // Look for a correlating HTML file.
     String htmlPath = file.absolute.path;
@@ -419,14 +425,14 @@ class CoverageTask extends Task {
       await for (String line in process.stderr) {
         _coverageOutput.add('    $line');
         if (line.contains(_observatoryFailPattern)) {
-          throw new TestException();
+          throw new CoverageTestSuiteException(file.path);
         }
         if (line.contains(_observatoryPortPattern)) {
           Match m = _observatoryPortPattern.firstMatch(line);
           observatoryPort = int.parse(m.group(2));
         }
         if (line.contains(_testsFailedPattern)) {
-          throw new TestException();
+          throw new CoverageTestSuiteException(file.path);
         }
         if (line.contains(_testsPassedPattern)) {
           break;
@@ -451,10 +457,10 @@ class CoverageTask extends Task {
       await for (String line in process.stdout) {
         _coverageOutput.add('    $line');
         if (line.contains(_observatoryFailPattern)) {
-          throw new TestException();
+          throw new CoverageTestSuiteException(file.path);
         }
         if (line.contains(_testsFailedPattern)) {
-          throw new TestException();
+          throw new CoverageTestSuiteException(file.path);
         }
         if (line.contains(_testsPassedPattern)) {
           break;
@@ -465,5 +471,3 @@ class CoverageTask extends Task {
     }
   }
 }
-
-class TestException implements Exception {}

lib/src/tasks/coverage/cli.dart

@@ -19,12 +19,14 @@ import 'dart:io';
 
 import 'package:args/args.dart';
 
-import 'package:dart_dev/util.dart' show hasImmediateDependency, reporter;
+import 'package:dart_dev/util.dart' show reporter;
 
+import 'package:dart_dev/src/platform_util/api.dart' as platform_util;
 import 'package:dart_dev/src/tasks/cli.dart';
 import 'package:dart_dev/src/tasks/config.dart';
 import 'package:dart_dev/src/tasks/coverage/api.dart';
 import 'package:dart_dev/src/tasks/coverage/config.dart';
+import 'package:dart_dev/src/tasks/coverage/exceptions.dart';
 import 'package:dart_dev/src/tasks/test/config.dart';
 
 class CoverageCli extends TaskCli {
@@ -46,7 +48,8 @@ class CoverageCli extends TaskCli {
   final String command = 'coverage';
 
   Future<CliResult> run(ArgResults parsedArgs) async {
-    if (!hasImmediateDependency('coverage')) return new CliResult.fail(
+    if (!platform_util
+        .hasImmediateDependency('coverage')) return new CliResult.fail(
         'Package "coverage" must be an immediate dependency in order to run its executables.');
 
     bool unit = parsedArgs['unit'];
@@ -78,13 +81,19 @@ class CoverageCli extends TaskCli {
       }
     }
 
-    CoverageTask task = CoverageTask.start(tests,
-        html: html,
-        output: config.coverage.output,
-        reportOn: config.coverage.reportOn);
-    reporter.logGroup('Collecting coverage',
-        outputStream: task.output, errorStream: task.errorOutput);
-    CoverageResult result = await task.done;
+    CoverageResult result;
+    try {
+      CoverageTask task = CoverageTask.start(tests,
+          html: html,
+          output: config.coverage.output,
+          reportOn: config.coverage.reportOn);
+      reporter.logGroup('Collecting coverage',
+          outputStream: task.output, errorStream: task.errorOutput);
+      result = await task.done;
+    } on MissingLcovException catch (e) {
+      return new CliResult.fail(e.message);
+    }
+
     if (result.successful && html && open) {
       Process.run('open', [result.reportIndex.path]);
     }

lib/src/tasks/coverage/exceptions.dart

@@ -0,0 +1,26 @@
+library dart_dev.src.tasks.coverage.exceptions;
+
+const String missingLcovMessage = '''
+The "lcov" dependency is missing. It's required for generating the HTML report.
+
+If using brew, you can install it with:
+    brew update
+    brew install lcov
+
+Otherwise, visit http://ltp.sourceforge.net/coverage/lcov.php
+''';
+
+/// Thrown when collecting coverage on a test suite that has failing tests.
+class CoverageTestSuiteException implements Exception {
+  final String message;
+  CoverageTestSuiteException(String testSuite)
+      : this.message = 'Test suite has failing tests: $testSuite';
+  String toString() => 'CoverageTestSuiteException: $message';
+}
+
+/// Thrown when attempting to generate the HTML coverage report without the
+/// required "lcov" dependency being installed.
+class MissingLcovException implements Exception {
+  final String message = missingLcovMessage;
+  String toString() => 'MissingLcovException: $message';
+}