Skip to content

Commit 8df6218

Browse files
auto-submit[bot]auto-submit[bot]
authored
Reverts "Use coverage.collect's coverableLineCache param to speed up coverage" (flutter#137121)
Reverts flutter#136851 Initiated by: CaseyHillers This change reverts the following previous change: Original Description: One of the reasons gathering coverage information is expensive is that we have to force compile every function in the libraries we're interested in. Without this, functions that haven't been invoked (so haven't been compiled) won't have any line number information, so the coverage tool doesn't know which lines to add to the list of misses. In flutter's case, the test infra spawns many VMs, and each of these needs to recompile all those libraries. To fix this, we need a way of skipping force compilation for libraries we've already seen in previous tests, without losing the information about which lines in each library are coverable. So I [added](dart-archive/coverage#466) the `coverableLineCache` to `coverage.collect` in package:coverage v1.7.0. This cache starts out empty, but fills up with lists of all the lines that are coverable for every library as coverage is gathered. package:coverage can then tell the VM not to force compile any libraries in this cache (using `getSourceReport`'s `librariesAlreadyCompiled` param). So the first test suite will still have to compile everything, but subsequent test suites will be much faster. This speeds up coverage collection significantly, for large test suites: | Running flutter/packages/flutter tests... | Time | Overhead | | --- | --- | --- | | without coverage | 8:53 | - | | with coverage | 20:25 | 130% | | with `coverableLineCache` | 12:21 | 40% | Bug: flutter#100751
1 parent d1884e7 commit 8df6218

File tree

2 files changed

+7
-197
lines changed

2 files changed

+7
-197
lines changed

‎packages/flutter_tools/lib/src/test/coverage_collector.dart

Lines changed: 7 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ class CoverageCollector extends TestWatcher {
3737

3838
final coverage.Resolver? resolver;
3939
final Map<String, List<List<int>>?> _ignoredLinesInFilesCache = <String, List<List<int>>?>{};
40-
final Map<String, Set<int>> _coverableLineCache = <String, Set<int>>{};
4140

4241
final TestTimeRecorder? testTimeRecorder;
4342

@@ -104,11 +103,7 @@ class CoverageCollector extends TestWatcher {
104103
Future<void> collectCoverageIsolate(Uri vmServiceUri) async {
105104
_logMessage('collecting coverage data from $vmServiceUri...');
106105
final Map<String, dynamic> data = await collect(
107-
vmServiceUri,
108-
libraryNames,
109-
branchCoverage: branchCoverage,
110-
coverableLineCache: _coverableLineCache,
111-
);
106+
vmServiceUri, libraryNames, branchCoverage: branchCoverage);
112107

113108
_logMessage('($vmServiceUri): collected coverage data; merging...');
114109
_addHitmap(await coverage.HitMap.parseJson(
@@ -150,12 +145,9 @@ class CoverageCollector extends TestWatcher {
150145
.then((Uri? vmServiceUri) {
151146
_logMessage('collecting coverage data from $testDevice at $vmServiceUri...');
152147
return collect(
153-
vmServiceUri!,
154-
libraryNames,
155-
serviceOverride: serviceOverride,
156-
branchCoverage: branchCoverage,
157-
coverableLineCache: _coverableLineCache,
158-
).then<void>((Map<String, dynamic> result) {
148+
vmServiceUri!, libraryNames, serviceOverride: serviceOverride,
149+
branchCoverage: branchCoverage)
150+
.then<void>((Map<String, dynamic> result) {
159151
_logMessage('Collected coverage data.');
160152
data = result;
161153
});
@@ -275,12 +267,9 @@ Future<Map<String, dynamic>> collect(Uri serviceUri, Set<String>? libraryNames,
275267
@visibleForTesting bool forceSequential = false,
276268
@visibleForTesting FlutterVmService? serviceOverride,
277269
bool branchCoverage = false,
278-
required Map<String, Set<int>> coverableLineCache,
279270
}) {
280271
return coverage.collect(
281-
serviceUri, false, false, false, libraryNames,
282-
serviceOverrideForTesting: serviceOverride?.service,
283-
branchCoverage: branchCoverage,
284-
coverableLineCache: coverableLineCache,
285-
);
272+
serviceUri, false, false, false, libraryNames,
273+
serviceOverrideForTesting: serviceOverride?.service,
274+
branchCoverage: branchCoverage);
286275
}

‎packages/flutter_tools/test/general.shard/coverage_collector_test.dart

Lines changed: 0 additions & 179 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@ void main() {
5353
Uri(),
5454
<String>{'foo'},
5555
serviceOverride: fakeVmServiceHost.vmService,
56-
coverableLineCache: <String, Set<int>>{},
5756
);
5857

5958
expect(result, <String, Object>{'type': 'CodeCoverage', 'coverage': <Object>[]});
@@ -124,7 +123,6 @@ void main() {
124123
Uri(),
125124
<String>{'foo'},
126125
serviceOverride: fakeVmServiceHost.vmService,
127-
coverableLineCache: <String, Set<int>>{},
128126
);
129127

130128
expect(result, <String, Object>{
@@ -153,7 +151,6 @@ void main() {
153151
Uri(),
154152
null,
155153
serviceOverride: fakeVmServiceHost.vmService,
156-
coverableLineCache: <String, Set<int>>{},
157154
);
158155

159156
expect(result, <String, Object>{
@@ -240,7 +237,6 @@ void main() {
240237
Uri(),
241238
<String>{'foo'},
242239
serviceOverride: fakeVmServiceHost.vmService,
243-
coverableLineCache: <String, Set<int>>{},
244240
);
245241

246242
expect(result, <String, Object>{
@@ -315,7 +311,6 @@ void main() {
315311
Uri(),
316312
null,
317313
serviceOverride: fakeVmServiceHost.vmService,
318-
coverableLineCache: <String, Set<int>>{},
319314
);
320315

321316
expect(result, <String, Object>{
@@ -406,7 +401,6 @@ void main() {
406401
<String>{'foo'},
407402
serviceOverride: fakeVmServiceHost.vmService,
408403
branchCoverage: true,
409-
coverableLineCache: <String, Set<int>>{},
410404
);
411405

412406
expect(result, <String, Object>{
@@ -607,179 +601,6 @@ void main() {
607601
tempDir?.deleteSync(recursive: true);
608602
}
609603
});
610-
611-
testWithoutContext('Coverage collector fills coverableLineCache', () async {
612-
final FakeVmServiceHost fakeVmServiceHost = FakeVmServiceHost(
613-
requests: <VmServiceExpectation>[
614-
FakeVmServiceRequest(
615-
method: 'getVM',
616-
jsonResponse: (VM.parse(<String, Object>{})!
617-
..isolates = <IsolateRef>[
618-
IsolateRef.parse(<String, Object>{
619-
'id': '1',
620-
})!,
621-
]
622-
).toJson(),
623-
),
624-
FakeVmServiceRequest(
625-
method: 'getVersion',
626-
jsonResponse: Version(major: 4, minor: 13).toJson(),
627-
),
628-
FakeVmServiceRequest(
629-
method: 'getSourceReport',
630-
args: <String, Object>{
631-
'isolateId': '1',
632-
'reports': <Object>['Coverage'],
633-
'forceCompile': true,
634-
'reportLines': true,
635-
'libraryFilters': <String>['package:foo/'],
636-
'librariesAlreadyCompiled': <String>[],
637-
},
638-
jsonResponse: SourceReport(
639-
ranges: <SourceReportRange>[
640-
SourceReportRange(
641-
scriptIndex: 0,
642-
startPos: 0,
643-
endPos: 0,
644-
compiled: true,
645-
coverage: SourceReportCoverage(
646-
hits: <int>[1, 3],
647-
misses: <int>[2],
648-
),
649-
),
650-
],
651-
scripts: <ScriptRef>[
652-
ScriptRef(
653-
uri: 'package:foo/foo.dart',
654-
id: '1',
655-
),
656-
],
657-
).toJson(),
658-
),
659-
],
660-
);
661-
662-
final Map<String, Set<int>> coverableLineCache = <String, Set<int>>{};
663-
final Map<String, Object?> result = await collect(
664-
Uri(),
665-
<String>{'foo'},
666-
serviceOverride: fakeVmServiceHost.vmService,
667-
coverableLineCache: coverableLineCache,
668-
);
669-
670-
expect(result, <String, Object>{
671-
'type': 'CodeCoverage',
672-
'coverage': <Object>[
673-
<String, Object>{
674-
'source': 'package:foo/foo.dart',
675-
'script': <String, Object>{
676-
'type': '@Script',
677-
'fixedId': true,
678-
'id': 'libraries/1/scripts/package%3Afoo%2Ffoo.dart',
679-
'uri': 'package:foo/foo.dart',
680-
'_kind': 'library',
681-
},
682-
'hits': <Object>[1, 1, 3, 1, 2, 0],
683-
},
684-
],
685-
});
686-
687-
// coverableLineCache should contain every line mentioned in the report.
688-
expect(coverableLineCache, <String, Set<int>>{
689-
'package:foo/foo.dart': <int>{1, 2, 3},
690-
});
691-
692-
expect(fakeVmServiceHost.hasRemainingExpectations, false);
693-
});
694-
695-
testWithoutContext('Coverage collector avoids recompiling libraries in coverableLineCache', () async {
696-
final FakeVmServiceHost fakeVmServiceHost = FakeVmServiceHost(
697-
requests: <VmServiceExpectation>[
698-
FakeVmServiceRequest(
699-
method: 'getVM',
700-
jsonResponse: (VM.parse(<String, Object>{})!
701-
..isolates = <IsolateRef>[
702-
IsolateRef.parse(<String, Object>{
703-
'id': '1',
704-
})!,
705-
]
706-
).toJson(),
707-
),
708-
FakeVmServiceRequest(
709-
method: 'getVersion',
710-
jsonResponse: Version(major: 4, minor: 13).toJson(),
711-
),
712-
713-
// This collection sets librariesAlreadyCompiled. The response doesn't
714-
// include any misses.
715-
FakeVmServiceRequest(
716-
method: 'getSourceReport',
717-
args: <String, Object>{
718-
'isolateId': '1',
719-
'reports': <Object>['Coverage'],
720-
'forceCompile': true,
721-
'reportLines': true,
722-
'libraryFilters': <String>['package:foo/'],
723-
'librariesAlreadyCompiled': <String>['package:foo/foo.dart'],
724-
},
725-
jsonResponse: SourceReport(
726-
ranges: <SourceReportRange>[
727-
SourceReportRange(
728-
scriptIndex: 0,
729-
startPos: 0,
730-
endPos: 0,
731-
compiled: true,
732-
coverage: SourceReportCoverage(
733-
hits: <int>[1, 3],
734-
misses: <int>[],
735-
),
736-
),
737-
],
738-
scripts: <ScriptRef>[
739-
ScriptRef(
740-
uri: 'package:foo/foo.dart',
741-
id: '1',
742-
),
743-
],
744-
).toJson(),
745-
),
746-
],
747-
);
748-
749-
final Map<String, Set<int>> coverableLineCache = <String, Set<int>>{
750-
'package:foo/foo.dart': <int>{1, 2, 3},
751-
};
752-
final Map<String, Object?> result2 = await collect(
753-
Uri(),
754-
<String>{'foo'},
755-
serviceOverride: fakeVmServiceHost.vmService,
756-
coverableLineCache: coverableLineCache,
757-
);
758-
759-
// Expect that line 2 is marked as missed, even though it wasn't mentioned
760-
// in the getSourceReport response.
761-
expect(result2, <String, Object>{
762-
'type': 'CodeCoverage',
763-
'coverage': <Object>[
764-
<String, Object>{
765-
'source': 'package:foo/foo.dart',
766-
'script': <String, Object>{
767-
'type': '@Script',
768-
'fixedId': true,
769-
'id': 'libraries/1/scripts/package%3Afoo%2Ffoo.dart',
770-
'uri': 'package:foo/foo.dart',
771-
'_kind': 'library',
772-
},
773-
'hits': <Object>[1, 1, 2, 0, 3, 1],
774-
},
775-
],
776-
});
777-
expect(coverableLineCache, <String, Set<int>>{
778-
'package:foo/foo.dart': <int>{1, 2, 3},
779-
});
780-
781-
expect(fakeVmServiceHost.hasRemainingExpectations, false);
782-
});
783604
}
784605

785606
File writeFooBarPackagesJson(Directory tempDir) {

0 commit comments

Comments
 (0)