Improve coverage speed by using new caching option for package:coverage (#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).

packages/flutter_tools/bin/fuchsia_tester.dart

@@ -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');
       }

packages/flutter_tools/lib/src/commands/test.dart

@@ -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)
       );
     }
 

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

@@ -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;
@@ -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');
@@ -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;
@@ -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.');
@@ -133,11 +146,12 @@ class CoverageCollector extends TestWatcher {
     assert(data != null);
 
     _logMessage('Merging coverage data...');
-    _addHitmap(await coverage.HitMap.parseJson(
+    final Map<String, coverage.HitMap> hitmap = coverage.HitMap.parseJsonSync(
         data!['coverage'] as List<Map<String, dynamic>>,
-      packagePath: packageDirectory,
         checkIgnoredLines: true,
-    ));
+        resolver: resolver ?? await CoverageCollector.getResolver(packageDirectory),
+        ignoredLinesInFilesCache: _ignoredLinesInFilesCache);
+    _addHitmap(hitmap);
     _logMessage('Done merging coverage data into global coverage map.');
   }
 
@@ -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;

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

@@ -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';
@@ -138,11 +144,52 @@ void main() {
   });
 
   testWithoutContext('Coverage collector with null libraryNames accepts all libraries', () async {
+    final FakeVmServiceHost fakeVmServiceHost = createFakeVmServiceHostWithFooAndBar();
+
+    final Map<String, Object?> result = await collect(
+      null,
+      null,
+      connector: (Uri? uri) async {
+        return fakeVmServiceHost.vmService;
+      },
+    );
+
+    expect(result, <String, Object>{
+      'type': 'CodeCoverage',
+      'coverage': <Object>[
+        <String, Object>{
+          'source': 'package:foo/foo.dart',
+          'script': <String, Object>{
+            'type': '@Script',
+            'fixedId': true,
+            'id': 'libraries/1/scripts/package%3Afoo%2Ffoo.dart',
+            'uri': 'package:foo/foo.dart',
+            '_kind': 'library',
+          },
+          'hits': <Object>[1, 1, 3, 1, 2, 0],
+        },
+        <String, Object>{
+          'source': 'package:bar/bar.dart',
+          'script': <String, Object>{
+            'type': '@Script',
+            'fixedId': true,
+            'id': 'libraries/1/scripts/package%3Abar%2Fbar.dart',
+            'uri': 'package:bar/bar.dart',
+            '_kind': 'library',
+          },
+          'hits': <Object>[47, 1, 21, 1, 32, 0, 86, 0],
+        },
+      ],
+    });
+    expect(fakeVmServiceHost.hasRemainingExpectations, false);
+  });
+
+  testWithoutContext('Coverage collector with libraryFilters', () async {
     final FakeVmServiceHost fakeVmServiceHost = FakeVmServiceHost(
       requests: <VmServiceExpectation>[
         FakeVmServiceRequest(
           method: 'getVersion',
-          jsonResponse: Version(major: 3, minor: 51).toJson(),
+          jsonResponse: Version(major: 3, minor: 57).toJson(),
         ),
         FakeVmServiceRequest(
           method: 'getVM',
@@ -154,24 +201,14 @@ void main() {
             ]
           ).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,
+            'libraryFilters': <Object>['package:foo/'],
           },
           jsonResponse: SourceReport(
             ranges: <SourceReportRange>[
@@ -194,42 +231,12 @@ void main() {
             ],
           ).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 Map<String, Object?> result = await collect(
       null,
-      null,
+      <String>{'foo'},
       connector: (Uri? uri) async {
         return fakeVmServiceHost.vmService;
       },
@@ -249,23 +256,12 @@ void main() {
           },
           'hits': <Object>[1, 1, 3, 1, 2, 0],
         },
-        <String, Object>{
-          'source': 'package:bar/bar.dart',
-          'script': <String, Object>{
-            'type': '@Script',
-            'fixedId': true,
-            'id': 'libraries/1/scripts/package%3Abar%2Fbar.dart',
-            'uri': 'package:bar/bar.dart',
-            '_kind': 'library',
-          },
-          'hits': <Object>[47, 1, 21, 1, 32, 0, 86, 0],
-        },
       ],
     });
     expect(fakeVmServiceHost.hasRemainingExpectations, false);
   });
 
-  testWithoutContext('Coverage collector with libraryFilters', () async {
+  testWithoutContext('Coverage collector with libraryFilters and null libraryNames', () async {
     final FakeVmServiceHost fakeVmServiceHost = FakeVmServiceHost(
       requests: <VmServiceExpectation>[
         FakeVmServiceRequest(
@@ -289,7 +285,6 @@ void main() {
             'reports': <Object>['Coverage'],
             'forceCompile': true,
             'reportLines': true,
-            'libraryFilters': <Object>['package:foo/'],
           },
           jsonResponse: SourceReport(
             ranges: <SourceReportRange>[
@@ -317,7 +312,7 @@ void main() {
 
     final Map<String, Object?> result = await collect(
       null,
-      <String>{'foo'},
+      null,
       connector: (Uri? uri) async {
         return fakeVmServiceHost.vmService;
       },
@@ -342,12 +337,85 @@ void main() {
     expect(fakeVmServiceHost.hasRemainingExpectations, false);
   });
 
-  testWithoutContext('Coverage collector with libraryFilters and null libraryNames', () async {
-    final FakeVmServiceHost fakeVmServiceHost = FakeVmServiceHost(
+  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: 57).toJson(),
+        jsonResponse: Version(major: 3, minor: 51).toJson(),
       ),
       FakeVmServiceRequest(
         method: 'getVM',
@@ -359,11 +427,22 @@ void main() {
           ]
         ).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,
         },
@@ -388,33 +467,52 @@ void main() {
           ],
         ).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 Map<String, Object?> result = await collect(
-      null,
-      null,
-      connector: (Uri? uri) async {
-        return fakeVmServiceHost.vmService;
-      },
-    );
+class TestTestDevice extends TestDevice {
+  @override
+  Future<void> get finished => Future<void>.delayed(const Duration(seconds: 1));
 
-    expect(result, <String, Object>{
-      'type': 'CodeCoverage',
-      'coverage': <Object>[
-        <String, Object>{
-          'source': 'package:foo/foo.dart',
-          'script': <String, Object>{
-            'type': '@Script',
-            'fixedId': true,
-            'id': 'libraries/1/scripts/package%3Afoo%2Ffoo.dart',
-            'uri': 'package:foo/foo.dart',
-            '_kind': 'library',
-          },
-          'hits': <Object>[1, 1, 3, 1, 2, 0],
-        },
-      ],
-    });
-    expect(fakeVmServiceHost.hasRemainingExpectations, false);
-  });
+  @override
+  Future<void> kill() => Future<void>.value();
+
+  @override
+  Future<Uri?> get observatoryUri => Future<Uri?>.value();
+
+  @override
+  Future<StreamChannel<String>> start(String entrypointPath) {
+    throw UnimplementedError();
+  }
 }