Skip to content
This repository was archived by the owner on Aug 28, 2024. It is now read-only.

Branch coverage #361

Merged
merged 13 commits into from
Feb 25, 2022
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a breaking change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so. I'm adding significant functionality, but not changing existing functionality, so I think that makes it a minor version bump.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does any of our internal testing infrastructure consume lcov reports? Will they be broken with the change in format? If not, then it's probably fine to model as a non breaking releases.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The lcov output was also modified when I added function coverage, and that didn't seem to be an issue, so I guess not.

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
Expand Down
27 changes: 27 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
```
23 changes: 20 additions & 3 deletions bin/collect_coverage.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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;
Expand All @@ -44,6 +54,7 @@ class Options {
final bool resume;
final bool includeDart;
final bool functionCoverage;
final bool branchCoverage;
final Set<String> scopedOutput;
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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(),
);
}
23 changes: 18 additions & 5 deletions bin/format_coverage.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -37,6 +38,7 @@ class Environment {
String? packagesPath;
bool prettyPrint;
bool prettyPrintFunc;
bool prettyPrintBranch;
List<String>? reportOn;
String? sdkRoot;
bool verbose;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand All @@ -244,6 +256,7 @@ Environment parseArgs(List<String> arguments) {
packagesPath: packagesPath,
prettyPrint: prettyPrint,
prettyPrintFunc: prettyPrintFunc,
prettyPrintBranch: prettyPrintBranch,
reportOn: reportOn,
sdkRoot: sdkRoot,
verbose: verbose,
Expand Down
78 changes: 56 additions & 22 deletions lib/src/collect.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand All @@ -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.
Expand Down Expand Up @@ -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);
Expand All @@ -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
Expand Down Expand Up @@ -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);
Expand All @@ -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,
);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
18 changes: 17 additions & 1 deletion lib/src/formatter.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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');
}

Expand All @@ -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();
Expand All @@ -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;
Expand Down
Loading