Skip to content

Commit e12d1a7

Browse files
liamappelbeliamappelbe
authored
Reland coverableLineCache optimisation (#136851) (#137385)
Relands #136851, which was rolled back in #137121 package:coverage has been rolled, so the breakages should be fixed. Also, in this reland I've changed the `coverableLineCache` parameter to be optional, which is safer.
1 parent f05bb9a commit e12d1a7

File tree

2 files changed

+197
-7
lines changed

2 files changed

+197
-7
lines changed

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

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ 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>>{};
4041

4142
final TestTimeRecorder? testTimeRecorder;
4243

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

108113
_logMessage('($vmServiceUri): collected coverage data; merging...');
109114
_addHitmap(await coverage.HitMap.parseJson(
@@ -145,9 +150,12 @@ class CoverageCollector extends TestWatcher {
145150
.then((Uri? vmServiceUri) {
146151
_logMessage('collecting coverage data from $testDevice at $vmServiceUri...');
147152
return collect(
148-
vmServiceUri!, libraryNames, serviceOverride: serviceOverride,
149-
branchCoverage: branchCoverage)
150-
.then<void>((Map<String, dynamic> result) {
153+
vmServiceUri!,
154+
libraryNames,
155+
serviceOverride: serviceOverride,
156+
branchCoverage: branchCoverage,
157+
coverableLineCache: _coverableLineCache,
158+
).then<void>((Map<String, dynamic> result) {
151159
_logMessage('Collected coverage data.');
152160
data = result;
153161
});
@@ -267,9 +275,12 @@ Future<Map<String, dynamic>> collect(Uri serviceUri, Set<String>? libraryNames,
267275
@visibleForTesting bool forceSequential = false,
268276
@visibleForTesting FlutterVmService? serviceOverride,
269277
bool branchCoverage = false,
278+
Map<String, Set<int>>? coverableLineCache,
270279
}) {
271280
return coverage.collect(
272-
serviceUri, false, false, false, libraryNames,
273-
serviceOverrideForTesting: serviceOverride?.service,
274-
branchCoverage: branchCoverage);
281+
serviceUri, false, false, false, libraryNames,
282+
serviceOverrideForTesting: serviceOverride?.service,
283+
branchCoverage: branchCoverage,
284+
coverableLineCache: coverableLineCache,
285+
);
275286
}

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

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

5859
expect(result, <String, Object>{'type': 'CodeCoverage', 'coverage': <Object>[]});
@@ -123,6 +124,7 @@ void main() {
123124
Uri(),
124125
<String>{'foo'},
125126
serviceOverride: fakeVmServiceHost.vmService,
127+
coverableLineCache: <String, Set<int>>{},
126128
);
127129

128130
expect(result, <String, Object>{
@@ -151,6 +153,7 @@ void main() {
151153
Uri(),
152154
null,
153155
serviceOverride: fakeVmServiceHost.vmService,
156+
coverableLineCache: <String, Set<int>>{},
154157
);
155158

156159
expect(result, <String, Object>{
@@ -237,6 +240,7 @@ void main() {
237240
Uri(),
238241
<String>{'foo'},
239242
serviceOverride: fakeVmServiceHost.vmService,
243+
coverableLineCache: <String, Set<int>>{},
240244
);
241245

242246
expect(result, <String, Object>{
@@ -311,6 +315,7 @@ void main() {
311315
Uri(),
312316
null,
313317
serviceOverride: fakeVmServiceHost.vmService,
318+
coverableLineCache: <String, Set<int>>{},
314319
);
315320

316321
expect(result, <String, Object>{
@@ -401,6 +406,7 @@ void main() {
401406
<String>{'foo'},
402407
serviceOverride: fakeVmServiceHost.vmService,
403408
branchCoverage: true,
409+
coverableLineCache: <String, Set<int>>{},
404410
);
405411

406412
expect(result, <String, Object>{
@@ -601,6 +607,179 @@ void main() {
601607
tempDir?.deleteSync(recursive: true);
602608
}
603609
});
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+
});
604783
}
605784

606785
File writeFooBarPackagesJson(Directory tempDir) {

0 commit comments

Comments
 (0)