Skip to content

Commit

Permalink
Improve coverage speed by using new caching option for package:covera…
Browse files Browse the repository at this point in the history 
…ge (#107395)

Speedup coverage test runs by using (new) coverage handle with caching.

On a `flutter test --coverage` run on `packages/flutter` the runtime goes from ~828 seconds to ~617 seconds, or a ~25% reduction in time spent (testing without coverage takes ~236 seconds so the overhead of `--coverage` in this case goes from ~592 seconds to ~381 seconds, or a ~35% reduction).
  • Loading branch information
@jensjoha
jensjoha authored Jul 13, 2022
1 parent c251856 commit 803ef6a
Show file tree
Hide file tree
Showing 4 changed files with 216 additions and 101 deletions.
6 changes: 4 additions & 2 deletions packages/flutter_tools/bin/fuchsia_tester.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,11 @@ Future<void> run(List<String> args) async {
// setting libraryNames to null.
final Set<String> libraryNames = coverageDirectory != null ? null :
<String>{FlutterProject.current().manifest.appName};
final String packagesPath = globals.fs.path.normalize(globals.fs.path.absolute(argResults[_kOptionPackages] as String));
collector = CoverageCollector(
packagesPath: globals.fs.path.normalize(globals.fs.path.absolute(argResults[_kOptionPackages] as String)),
libraryNames: libraryNames);
packagesPath: packagesPath,
libraryNames: libraryNames,
resolver: await CoverageCollector.getResolver(packagesPath));
if (!argResults.options.contains(_kOptionTestDirectory)) {
throwToolExit('Use of --coverage requires setting --test-directory');
}
Expand Down
3 changes: 2 additions & 1 deletion packages/flutter_tools/lib/src/commands/test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,8 @@ class TestCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts {
collector = CoverageCollector(
verbose: !machine,
libraryNames: <String>{projectName},
packagesPath: buildInfo.packagesPath
packagesPath: buildInfo.packagesPath,
resolver: await CoverageCollector.getResolver(buildInfo.packagesPath)
);
}

Expand Down
34 changes: 24 additions & 10 deletions packages/flutter_tools/lib/src/test/coverage_collector.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import 'watcher.dart';

/// A class that collects code coverage data during test runs.
class CoverageCollector extends TestWatcher {
CoverageCollector({this.libraryNames, this.verbose = true, required this.packagesPath});
CoverageCollector({this.libraryNames, this.verbose = true, required this.packagesPath, this.resolver});

/// True when log messages should be emitted.
final bool verbose;
Expand All @@ -35,6 +35,19 @@ class CoverageCollector extends TestWatcher {
/// will be accepted.
Set<String>? libraryNames;

final coverage.Resolver? resolver;
final Map<String, List<List<int>>> _ignoredLinesInFilesCache = <String, List<List<int>>>{};

static Future<coverage.Resolver> getResolver(String? packagesPath) async {
try {
return await coverage.Resolver.create(packagesPath: packagesPath);
} on FileSystemException {
// When given a bad packages path (as for instance done in some tests)
// just ignore it and return one without a packages path.
return coverage.Resolver.create();
}
}

@override
Future<void> handleFinishedTest(TestDevice testDevice) async {
_logMessage('Starting coverage collection');
Expand Down Expand Up @@ -104,7 +117,7 @@ class CoverageCollector extends TestWatcher {
/// has been run to completion so that all coverage data has been recorded.
///
/// The returned [Future] completes when the coverage is collected.
Future<void> collectCoverage(TestDevice testDevice) async {
Future<void> collectCoverage(TestDevice testDevice, {@visibleForTesting Future<FlutterVmService> Function(Uri?)? connector}) async {
assert(testDevice != null);

Map<String, dynamic>? data;
Expand All @@ -119,7 +132,7 @@ class CoverageCollector extends TestWatcher {
final Future<void> collectionComplete = testDevice.observatoryUri
.then((Uri? observatoryUri) {
_logMessage('collecting coverage data from $testDevice at $observatoryUri...');
return collect(observatoryUri, libraryNames)
return collect(observatoryUri, libraryNames, connector: connector ?? _defaultConnect)
.then<void>((Map<String, dynamic> result) {
if (result == null) {
throw Exception('Failed to collect coverage.');
Expand All @@ -133,11 +146,12 @@ class CoverageCollector extends TestWatcher {
assert(data != null);

_logMessage('Merging coverage data...');
_addHitmap(await coverage.HitMap.parseJson(
data!['coverage'] as List<Map<String, dynamic>>,
packagePath: packageDirectory,
checkIgnoredLines: true,
));
final Map<String, coverage.HitMap> hitmap = coverage.HitMap.parseJsonSync(
data!['coverage'] as List<Map<String, dynamic>>,
checkIgnoredLines: true,
resolver: resolver ?? await CoverageCollector.getResolver(packageDirectory),
ignoredLinesInFilesCache: _ignoredLinesInFilesCache);
_addHitmap(hitmap);
_logMessage('Done merging coverage data into global coverage map.');
}

Expand All @@ -154,13 +168,13 @@ class CoverageCollector extends TestWatcher {
return null;
}
if (formatter == null) {
resolver ??= await coverage.Resolver.create(packagesPath: packagesPath);
final coverage.Resolver usedResolver = resolver ?? this.resolver ?? await CoverageCollector.getResolver(packagesPath);
final String packagePath = globals.fs.currentDirectory.path;
final List<String> reportOn = coverageDirectory == null
? <String>[globals.fs.path.join(packagePath, 'lib')]
: <String>[coverageDirectory.path];
formatter = (Map<String, coverage.HitMap> hitmap) => hitmap
.formatLcov(resolver!, reportOn: reportOn, basePath: packagePath);
.formatLcov(usedResolver, reportOn: reportOn, basePath: packagePath);
}
final String result = formatter(_globalHitmap!);
_globalHitmap = null;
Expand Down
274 changes: 186 additions & 88 deletions packages/flutter_tools/test/general.shard/coverage_collector_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,13 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'dart:convert' show jsonEncode;
import 'dart:io' show Directory, File;

import 'package:coverage/src/hitmap.dart';
import 'package:flutter_tools/src/test/coverage_collector.dart';
import 'package:flutter_tools/src/test/test_device.dart' show TestDevice;
import 'package:stream_channel/stream_channel.dart' show StreamChannel;
import 'package:vm_service/vm_service.dart';

import '../src/common.dart';
Expand Down Expand Up @@ -138,94 +144,7 @@ void main() {
});

testWithoutContext('Coverage collector with null libraryNames accepts all libraries', () async {
final FakeVmServiceHost fakeVmServiceHost = FakeVmServiceHost(
requests: <VmServiceExpectation>[
FakeVmServiceRequest(
method: 'getVersion',
jsonResponse: Version(major: 3, minor: 51).toJson(),
),
FakeVmServiceRequest(
method: 'getVM',
jsonResponse: (VM.parse(<String, Object>{})!
..isolates = <IsolateRef>[
IsolateRef.parse(<String, Object>{
'id': '1',
})!,
]
).toJson(),
),
FakeVmServiceRequest(
method: 'getScripts',
args: <String, Object>{
'isolateId': '1',
},
jsonResponse: ScriptList(scripts: <ScriptRef>[
ScriptRef(uri: 'package:foo/foo.dart', id: '1'),
ScriptRef(uri: 'package:bar/bar.dart', id: '2'),
]).toJson(),
),
FakeVmServiceRequest(
method: 'getSourceReport',
args: <String, Object>{
'isolateId': '1',
'reports': <Object>['Coverage'],
'scriptId': '1',
'forceCompile': true,
'reportLines': true,
},
jsonResponse: SourceReport(
ranges: <SourceReportRange>[
SourceReportRange(
scriptIndex: 0,
startPos: 0,
endPos: 0,
compiled: true,
coverage: SourceReportCoverage(
hits: <int>[1, 3],
misses: <int>[2],
),
),
],
scripts: <ScriptRef>[
ScriptRef(
uri: 'package:foo/foo.dart',
id: '1',
),
],
).toJson(),
),
FakeVmServiceRequest(
method: 'getSourceReport',
args: <String, Object>{
'isolateId': '1',
'reports': <Object>['Coverage'],
'scriptId': '2',
'forceCompile': true,
'reportLines': true,
},
jsonResponse: SourceReport(
ranges: <SourceReportRange>[
SourceReportRange(
scriptIndex: 0,
startPos: 0,
endPos: 0,
compiled: true,
coverage: SourceReportCoverage(
hits: <int>[47, 21],
misses: <int>[32, 86],
),
),
],
scripts: <ScriptRef>[
ScriptRef(
uri: 'package:bar/bar.dart',
id: '2',
),
],
).toJson(),
),
],
);
final FakeVmServiceHost fakeVmServiceHost = createFakeVmServiceHostWithFooAndBar();

final Map<String, Object?> result = await collect(
null,
Expand Down Expand Up @@ -417,4 +336,183 @@ void main() {
});
expect(fakeVmServiceHost.hasRemainingExpectations, false);
});

testWithoutContext('Coverage collector caches read files', () async {
Directory? tempDir;
try {
tempDir = Directory.systemTemp.createTempSync('flutter_coverage_collector_test.');
final File file = File('${tempDir.path}/packages.json');
file.writeAsStringSync(jsonEncode(<String, dynamic>{
'configVersion': 2,
'packages': <Map<String, String>>[
<String, String>{
'name': 'foo',
'rootUri': 'foo',
},
<String, String>{
'name': 'bar',
'rootUri': 'bar',
},
],
}));
final Directory fooDir = Directory('${tempDir.path}/foo/');
fooDir.createSync();
final File fooFile = File('${fooDir.path}/foo.dart');
fooFile.writeAsStringSync('hit\nnohit but ignored // coverage:ignore-line\nhit\n');

final String packagesPath = file.path;
final CoverageCollector collector = CoverageCollector(
libraryNames: <String>{'foo', 'bar'},
verbose: false,
packagesPath: packagesPath,
resolver: await CoverageCollector.getResolver(packagesPath)
);
await collector.collectCoverage(TestTestDevice(), connector: (Uri? uri) async {
return createFakeVmServiceHostWithFooAndBar().vmService;
});

Future<void> getHitMapAndVerify() async {
final Map<String, HitMap> gottenHitmap = <String, HitMap>{};
await collector.finalizeCoverage(formatter: (Map<String, HitMap> hitmap) {
gottenHitmap.addAll(hitmap);
return '';
});
expect(gottenHitmap.keys.toList()..sort(), <String>['package:bar/bar.dart', 'package:foo/foo.dart']);
expect(gottenHitmap['package:foo/foo.dart']!.lineHits, <int, int>{1: 1, /* 2: 0, is ignored in file */ 3: 1});
expect(gottenHitmap['package:bar/bar.dart']!.lineHits, <int, int>{21: 1, 32: 0, 47: 1, 86: 0});
}

Future<void> verifyHitmapEmpty() async {
final Map<String, HitMap> gottenHitmap = <String, HitMap>{};
await collector.finalizeCoverage(formatter: (Map<String, HitMap> hitmap) {
gottenHitmap.addAll(hitmap);
return '';
});
expect(gottenHitmap.isEmpty, isTrue);
}

// Get hit map the first time.
await getHitMapAndVerify();

// Getting the hitmap clears it so we now doesn't get any data.
await verifyHitmapEmpty();

// Collecting again gets us the same data even though the foo file has been deleted.
// This means that the fact that line 2 was ignored has been cached.
fooFile.deleteSync();
await collector.collectCoverage(TestTestDevice(), connector: (Uri? uri) async {
return createFakeVmServiceHostWithFooAndBar().vmService;
});
await getHitMapAndVerify();
} finally {
tempDir?.deleteSync(recursive: true);
}
});
}

FakeVmServiceHost createFakeVmServiceHostWithFooAndBar() {
return FakeVmServiceHost(
requests: <VmServiceExpectation>[
FakeVmServiceRequest(
method: 'getVersion',
jsonResponse: Version(major: 3, minor: 51).toJson(),
),
FakeVmServiceRequest(
method: 'getVM',
jsonResponse: (VM.parse(<String, Object>{})!
..isolates = <IsolateRef>[
IsolateRef.parse(<String, Object>{
'id': '1',
})!,
]
).toJson(),
),
FakeVmServiceRequest(
method: 'getScripts',
args: <String, Object>{
'isolateId': '1',
},
jsonResponse: ScriptList(scripts: <ScriptRef>[
ScriptRef(uri: 'package:foo/foo.dart', id: '1'),
ScriptRef(uri: 'package:bar/bar.dart', id: '2'),
]).toJson(),
),
FakeVmServiceRequest(
method: 'getSourceReport',
args: <String, Object>{
'isolateId': '1',
'reports': <Object>['Coverage'],
'scriptId': '1',
'forceCompile': true,
'reportLines': true,
},
jsonResponse: SourceReport(
ranges: <SourceReportRange>[
SourceReportRange(
scriptIndex: 0,
startPos: 0,
endPos: 0,
compiled: true,
coverage: SourceReportCoverage(
hits: <int>[1, 3],
misses: <int>[2],
),
),
],
scripts: <ScriptRef>[
ScriptRef(
uri: 'package:foo/foo.dart',
id: '1',
),
],
).toJson(),
),
FakeVmServiceRequest(
method: 'getSourceReport',
args: <String, Object>{
'isolateId': '1',
'reports': <Object>['Coverage'],
'scriptId': '2',
'forceCompile': true,
'reportLines': true,
},
jsonResponse: SourceReport(
ranges: <SourceReportRange>[
SourceReportRange(
scriptIndex: 0,
startPos: 0,
endPos: 0,
compiled: true,
coverage: SourceReportCoverage(
hits: <int>[47, 21],
misses: <int>[32, 86],
),
),
],
scripts: <ScriptRef>[
ScriptRef(
uri: 'package:bar/bar.dart',
id: '2',
),
],
).toJson(),
),
],
);
}

class TestTestDevice extends TestDevice {
@override
Future<void> get finished => Future<void>.delayed(const Duration(seconds: 1));

@override
Future<void> kill() => Future<void>.value();

@override
Future<Uri?> get observatoryUri => Future<Uri?>.value();

@override
Future<StreamChannel<String>> start(String entrypointPath) {
throw UnimplementedError();
}
}

0 comments on commit 803ef6a

Please sign in to comment.