Branch coverage (#361)

* WIP: branch coverage

* Pretty printing of branch coverage

* WIP: testing

* Finish testing

* Changelog and pubspec

* Warn user if their VM is too old to support branch coverage

* Update readme to mention function and branch coverage

* Fix some of the failing tests on older VM versions

* Fix run_and_collect_test

* Actually fix run_and_collect_test

* Use skips instead of ifs in tests

Author: liamappelbe

CHANGELOG.md

@@ -1,5 +1,18 @@
-## 1.1.1-dev
-
+## 1.2.0-dev
+
+* Support branch level coverage information, when running tests in the Dart VM.
+  This is not supported for web tests yet.
+* Add flag `--branch-coverage` (abbr `-b`) to collect_coverage that collects
+  branch coverage information. The VM must also be run with the
+  `--branch-coverage` flag.
+* Add flag `--pretty-print-branch` to format_coverage that works
+  similarly to pretty print, but outputs branch level coverage, rather than
+  line level.
+* Update `--lcov` (abbr `-l`) in format_coverage to output branch level
+  coverage, in addition to line level.
+* Add an optional bool flag to `collect` that controls whether branch coverage
+  is collected.
+* Add a `branchHits` field to `HitMap`.
 * Add support for scraping the service URI from the new Dart VM service message.
 
 ## 1.1.0 - 2022-1-18

README.md

@@ -61,3 +61,30 @@ collected. If `--sdk-root` is set, Dart SDK coverage will also be output.
 - `// coverage:ignore-line` to ignore one line.
 - `// coverage:ignore-start` and `// coverage:ignore-end` to ignore range of lines inclusive.
 - `// coverage:ignore-file` to ignore the whole file.
+
+#### Function and branch coverage
+
+To gather function level coverage information, pass `--function-coverage` to
+collect_coverage:
+
+```
+dart --pause-isolates-on-exit --disable-service-auth-codes --enable-vm-service=NNNN script.dart
+pub global run coverage:collect_coverage --uri=http://... -o coverage.json --resume-isolates --function-coverage
+```
+
+To gather branch level coverage information, pass `--branch-coverage` to *both*
+collect_coverage and the Dart command you're gathering coverage from:
+
+```
+dart --pause-isolates-on-exit --disable-service-auth-codes --enable-vm-service=NNNN --branch-coverage script.dart
+pub global run coverage:collect_coverage --uri=http://... -o coverage.json --resume-isolates --branch-coverage
+```
+
+Branch coverage requires Dart VM 2.17.0, with service API v3.56. Function,
+branch, and line coverage can all be gathered at the same time, by combining
+those flags:
+
+```
+dart --pause-isolates-on-exit --disable-service-auth-codes --enable-vm-service=NNNN --branch-coverage script.dart
+pub global run coverage:collect_coverage --uri=http://... -o coverage.json --resume-isolates --function-coverage --branch-coverage
+```

bin/collect_coverage.dart

@@ -21,7 +21,9 @@ Future<void> main(List<String> arguments) async {
   await Chain.capture(() async {
     final coverage = await collect(options.serviceUri, options.resume,
         options.waitPaused, options.includeDart, options.scopedOutput,
-        timeout: options.timeout, functionCoverage: options.functionCoverage);
+        timeout: options.timeout,
+        functionCoverage: options.functionCoverage,
+        branchCoverage: options.branchCoverage);
     options.out.write(json.encode(coverage));
     await options.out.close();
   }, onError: (dynamic error, Chain chain) {
@@ -34,8 +36,16 @@ Future<void> main(List<String> arguments) async {
 }
 
 class Options {
-  Options(this.serviceUri, this.out, this.timeout, this.waitPaused, this.resume,
-      this.includeDart, this.functionCoverage, this.scopedOutput);
+  Options(
+      this.serviceUri,
+      this.out,
+      this.timeout,
+      this.waitPaused,
+      this.resume,
+      this.includeDart,
+      this.functionCoverage,
+      this.branchCoverage,
+      this.scopedOutput);
 
   final Uri serviceUri;
   final IOSink out;
@@ -44,6 +54,7 @@ class Options {
   final bool resume;
   final bool includeDart;
   final bool functionCoverage;
+  final bool branchCoverage;
   final Set<String> scopedOutput;
 }
 
@@ -75,6 +86,11 @@ Options _parseArgs(List<String> arguments) {
         abbr: 'd', defaultsTo: false, help: 'include "dart:" libraries')
     ..addFlag('function-coverage',
         abbr: 'f', defaultsTo: false, help: 'Collect function coverage info')
+    ..addFlag('branch-coverage',
+        abbr: 'b',
+        defaultsTo: false,
+        help: 'Collect branch coverage info (Dart VM must also be run with '
+            '--branch-coverage for this to work)')
     ..addFlag('help', abbr: 'h', negatable: false, help: 'show this help');
 
   final args = parser.parse(arguments);
@@ -127,6 +143,7 @@ Options _parseArgs(List<String> arguments) {
     args['resume-isolates'] as bool,
     args['include-dart'] as bool,
     args['function-coverage'] as bool,
+    args['branch-coverage'] as bool,
     scopedOutput.toSet(),
   );
 }

bin/format_coverage.dart

@@ -21,6 +21,7 @@ class Environment {
     required this.packagesPath,
     required this.prettyPrint,
     required this.prettyPrintFunc,
+    required this.prettyPrintBranch,
     required this.reportOn,
     required this.sdkRoot,
     required this.verbose,
@@ -37,6 +38,7 @@ class Environment {
   String? packagesPath;
   bool prettyPrint;
   bool prettyPrintFunc;
+  bool prettyPrintBranch;
   List<String>? reportOn;
   String? sdkRoot;
   bool verbose;
@@ -74,9 +76,11 @@ Future<void> main(List<String> arguments) async {
       ? BazelResolver(workspacePath: env.bazelWorkspace)
       : Resolver(packagesPath: env.packagesPath, sdkRoot: env.sdkRoot);
   final loader = Loader();
-  if (env.prettyPrint || env.prettyPrintFunc) {
+  if (env.prettyPrint) {
     output = await hitmap.prettyPrint(resolver, loader,
-        reportOn: env.reportOn, reportFuncs: env.prettyPrintFunc);
+        reportOn: env.reportOn,
+        reportFuncs: env.prettyPrintFunc,
+        reportBranches: env.prettyPrintBranch);
   } else {
     assert(env.lcov);
     output = hitmap.formatLcov(resolver,
@@ -135,6 +139,9 @@ Environment parseArgs(List<String> arguments) {
       abbr: 'f',
       negatable: false,
       help: 'convert function coverage data to pretty print format');
+  parser.addFlag('pretty-print-branch',
+      negatable: false,
+      help: 'convert branch coverage data to pretty print format');
   parser.addFlag('lcov',
       abbr: 'l',
       negatable: false,
@@ -217,12 +224,17 @@ Environment parseArgs(List<String> arguments) {
   final lcov = args['lcov'] as bool;
   var prettyPrint = args['pretty-print'] as bool;
   final prettyPrintFunc = args['pretty-print-func'] as bool;
-  if ((prettyPrint ? 1 : 0) + (prettyPrintFunc ? 1 : 0) + (lcov ? 1 : 0) > 1) {
+  final prettyPrintBranch = args['pretty-print-branch'] as bool;
+  final numModesChosen = (prettyPrint ? 1 : 0) +
+      (prettyPrintFunc ? 1 : 0) +
+      (prettyPrintBranch ? 1 : 0) +
+      (lcov ? 1 : 0);
+  if (numModesChosen > 1) {
     fail('Choose one of the pretty-print modes or lcov output');
   }
 
-  // Use pretty-print either explicitly or by default.
-  if (!lcov && !prettyPrintFunc) prettyPrint = true;
+  // The pretty printer is used by all modes other than lcov.
+  if (!lcov) prettyPrint = true;
 
   int workers;
   try {
@@ -244,6 +256,7 @@ Environment parseArgs(List<String> arguments) {
       packagesPath: packagesPath,
       prettyPrint: prettyPrint,
       prettyPrintFunc: prettyPrintFunc,
+      prettyPrintBranch: prettyPrintBranch,
       reportOn: reportOn,
       sdkRoot: sdkRoot,
       verbose: verbose,

lib/src/collect.dart

@@ -33,6 +33,10 @@ const _retryInterval = Duration(milliseconds: 200);
 /// If [functionCoverage] is true, function coverage information will be
 /// collected.
 ///
+/// If [branchCoverage] is true, branch coverage information will be collected.
+/// This will only work correctly if the target VM was run with the
+/// --branch-coverage flag.
+///
 /// If [scopedOutput] is non-empty, coverage will be restricted so that only
 /// scripts that start with any of the provided paths are considered.
 ///
@@ -42,7 +46,8 @@ Future<Map<String, dynamic>> collect(Uri serviceUri, bool resume,
     bool waitPaused, bool includeDart, Set<String>? scopedOutput,
     {Set<String>? isolateIds,
     Duration? timeout,
-    bool functionCoverage = false}) async {
+    bool functionCoverage = false,
+    bool branchCoverage = false}) async {
   scopedOutput ??= <String>{};
 
   // Create websocket URI. Handle any trailing slashes.
@@ -76,8 +81,8 @@ Future<Map<String, dynamic>> collect(Uri serviceUri, bool resume,
       await _waitIsolatesPaused(service, timeout: timeout);
     }
 
-    return await _getAllCoverage(
-        service, includeDart, functionCoverage, scopedOutput, isolateIds);
+    return await _getAllCoverage(service, includeDart, functionCoverage,
+        branchCoverage, scopedOutput, isolateIds);
   } finally {
     if (resume) {
       await _resumeIsolates(service);
@@ -88,19 +93,34 @@ Future<Map<String, dynamic>> collect(Uri serviceUri, bool resume,
   }
 }
 
+bool _versionCheck(Version version, int minMajor, int minMinor) {
+  final major = version.major ?? 0;
+  final minor = version.minor ?? 0;
+  return major > minMajor || (major == minMajor && minor >= minMinor);
+}
+
 Future<Map<String, dynamic>> _getAllCoverage(
     VmService service,
     bool includeDart,
     bool functionCoverage,
+    bool branchCoverage,
     Set<String>? scopedOutput,
     Set<String>? isolateIds) async {
   scopedOutput ??= <String>{};
   final vm = await service.getVM();
   final allCoverage = <Map<String, dynamic>>[];
   final version = await service.getVersion();
-  final reportLines =
-      (version.major == 3 && version.minor != null && version.minor! >= 51) ||
-          (version.major != null && version.major! > 3);
+  final reportLines = _versionCheck(version, 3, 51);
+  final branchCoverageSupported = _versionCheck(version, 3, 56);
+  if (branchCoverage && !branchCoverageSupported) {
+    branchCoverage = false;
+    stderr.write('Branch coverage was requested, but is not supported'
+        ' by the VM version. Try updating to a newer version of Dart');
+  }
+  final sourceReportKinds = [
+    SourceReportKind.kCoverage,
+    if (branchCoverage) SourceReportKind.kBranchCoverage,
+  ];
 
   // Program counters are shared between isolates in the same group. So we need
   // to make sure we're only gathering coverage data for one isolate in each
@@ -130,7 +150,7 @@ Future<Map<String, dynamic>> _getAllCoverage(
         // Skip scripts which should not be included in the report.
         if (!scopedOutput.contains(scope)) continue;
         final scriptReport = await service.getSourceReport(
-            isolateRef.id!, <String>[SourceReportKind.kCoverage],
+            isolateRef.id!, sourceReportKinds,
             forceCompile: true,
             scriptId: script.id,
             reportLines: reportLines ? true : null);
@@ -141,7 +161,7 @@ Future<Map<String, dynamic>> _getAllCoverage(
     } else {
       final isolateReport = await service.getSourceReport(
         isolateRef.id!,
-        <String>[SourceReportKind.kCoverage],
+        sourceReportKinds,
         forceCompile: true,
         reportLines: reportLines ? true : null,
       );
@@ -231,7 +251,8 @@ Future<void> _processFunction(VmService service, IsolateRef isolateRef,
     final line = _getLineFromTokenPos(script, tokenPos);
 
     if (line == null) {
-      print('tokenPos $tokenPos has no line mapping for script ${script.uri!}');
+      stderr.write(
+          'tokenPos $tokenPos has no line mapping for script ${script.uri!}');
       return;
     }
     hits.funcNames![line] = funcName;
@@ -306,28 +327,41 @@ Future<List<Map<String, dynamic>>> _getCoverageJson(
 
     if (coverage == null) continue;
 
-    for (final pos in coverage.hits!) {
-      final line = reportLines ? pos : _getLineFromTokenPos(script!, pos);
-      if (line == null) {
-        print('tokenPos $pos has no line mapping for script $scriptUri');
-        continue;
+    void forEachLine(List<int> tokenPositions, void Function(int line) body) {
+      for (final pos in tokenPositions) {
+        final line = reportLines ? pos : _getLineFromTokenPos(script!, pos);
+        if (line == null) {
+          stderr
+              .write('tokenPos $pos has no line mapping for script $scriptUri');
+          continue;
+        }
+        body(line);
       }
+    }
+
+    forEachLine(coverage.hits!, (line) {
       _incrementCountForKey(hits.lineHits, line);
       if (hits.funcNames != null && hits.funcNames!.containsKey(line)) {
         _incrementCountForKey(hits.funcHits!, line);
       }
-    }
-    for (final pos in coverage.misses!) {
-      final line = reportLines ? pos : _getLineFromTokenPos(script!, pos);
-      if (line == null) {
-        print('tokenPos $pos has no line mapping for script $scriptUri');
-        continue;
-      }
+    });
+    forEachLine(coverage.misses!, (line) {
       hits.lineHits.putIfAbsent(line, () => 0);
-    }
+    });
     hits.funcNames?.forEach((line, funcName) {
       hits.funcHits?.putIfAbsent(line, () => 0);
     });
+
+    final branchCoverage = range.branchCoverage;
+    if (branchCoverage != null) {
+      hits.branchHits ??= <int, int>{};
+      forEachLine(branchCoverage.hits!, (line) {
+        _incrementCountForKey(hits.branchHits!, line);
+      });
+      forEachLine(branchCoverage.misses!, (line) {
+        hits.branchHits!.putIfAbsent(line, () => 0);
+      });
+    }
   }
 
   // Output JSON

lib/src/formatter.dart

@@ -84,6 +84,7 @@ extension FileHitMapsFormatter on Map<String, HitMap> {
       final lineHits = v.lineHits;
       final funcHits = v.funcHits;
       final funcNames = v.funcNames;
+      final branchHits = v.branchHits;
       var source = resolver.resolve(entry.key);
       if (source == null) {
         continue;
@@ -115,6 +116,11 @@ extension FileHitMapsFormatter on Map<String, HitMap> {
       }
       buf.write('LF:${lineHits.length}\n');
       buf.write('LH:${lineHits.values.where((v) => v > 0).length}\n');
+      if (branchHits != null) {
+        for (final k in branchHits.keys.toList()..sort()) {
+          buf.write('BRDA:$k,0,0,${branchHits[k]}\n');
+        }
+      }
       buf.write('end_of_record\n');
     }
 
@@ -131,6 +137,7 @@ extension FileHitMapsFormatter on Map<String, HitMap> {
     Loader loader, {
     List<String>? reportOn,
     bool reportFuncs = false,
+    bool reportBranches = false,
   }) async {
     final pathFilter = _getPathFilter(reportOn);
     final buf = StringBuffer();
@@ -141,7 +148,16 @@ extension FileHitMapsFormatter on Map<String, HitMap> {
             'missing function coverage information. Did you run '
             'collect_coverage with the --function-coverage flag?';
       }
-      final hits = reportFuncs ? v.funcHits! : v.lineHits;
+      if (reportBranches && v.branchHits == null) {
+        throw 'Branch coverage formatting was requested, but the hit map is '
+            'missing branch coverage information. Did you run '
+            'collect_coverage with the --branch-coverage flag?';
+      }
+      final hits = reportFuncs
+          ? v.funcHits!
+          : reportBranches
+              ? v.branchHits!
+              : v.lineHits;
       final source = resolver.resolve(entry.key);
       if (source == null) {
         continue;