[flutter_tools] Support coverage collection for dependencies (#129513)

PR provides a new option to the `test` command to include coverage info of specified packages. It helps collecting coverage info in test setups where test code lives in separate packages or for multi-package projects. At present, only current package is included to the final report. Usage: Consider an app with two packages: `app`, `common`. Some of the tests in `app` use (indirectly) code that is located in `common`. When running with `--coverage` flag, that code is not included in the coverage report by default. To include `common` package in report, we can run: ```sh flutter test --coverage --coverage-package app --coverage-package common ``` Note that `--coverage-package` accepts regular expression. Fixes https://github.com/flutter/flutter/issues/79661 Fixes https://github.com/flutter/flutter/issues/101486 Fixes https://github.com/flutter/flutter/issues/93619
2023-07-17 11:42:13 +03:00 · 2023-07-17 11:42:13 +03:00 · 3a1190a5a8
commit 3a1190a5a8
parent a6187d9a92
4 changed files with 257 additions and 5 deletions
--- a/packages/flutter_tools/lib/src/commands/test.dart
+++ b/packages/flutter_tools/lib/src/commands/test.dart
@ -3,6 +3,7 @@
 // found in the LICENSE file.

 import 'package:meta/meta.dart';
+import 'package:package_config/package_config_types.dart';

 import '../asset.dart';
 import '../base/common.dart';
@ -131,6 +132,13 @@ class TestCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts {
        defaultsTo: 'coverage/lcov.info',
        help: 'Where to store coverage information (if coverage is enabled).',
      )
+      ..addMultiOption('coverage-package',
+        help: 'A regular expression matching packages names '
+              'to include in the coverage report (if coverage is enabled). '
+              'If unset, matches the current package name.',
+        valueHelp: 'package-name-regexp',
+        splitCommas: false,
+      )
      ..addFlag('machine',
        hide: !verboseHelp,
        negatable: false,
@ -395,10 +403,14 @@ class TestCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts {
    CoverageCollector? collector;
    if (boolArg('coverage') || boolArg('merge-coverage') ||
        boolArg('branch-coverage')) {
-      final String projectName = flutterProject.manifest.appName;
+      final Set<String> packagesToInclude = _getCoveragePackages(
+        stringsArg('coverage-package'),
+        flutterProject,
+        buildInfo.packageConfig,
+      );
      collector = CoverageCollector(
        verbose: !machine,
-        libraryNames: <String>{projectName},
+        libraryNames: packagesToInclude,
        packagesPath: buildInfo.packagesPath,
        resolver: await CoverageCollector.getResolver(buildInfo.packagesPath),
        testTimeRecorder: testTimeRecorder,
@ -508,6 +520,30 @@ class TestCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts {
    return FlutterCommandResult.success();
  }

+  Set<String> _getCoveragePackages(
+    List<String> packagesRegExps,
+    FlutterProject flutterProject,
+    PackageConfig packageConfig,
+  ) {
+    final String projectName = flutterProject.manifest.appName;
+    final Set<String> packagesToInclude = <String>{
+      if (packagesRegExps.isEmpty) projectName,
+    };
+    try {
+      for (final String regExpStr in packagesRegExps) {
+        final RegExp regExp = RegExp(regExpStr);
+        packagesToInclude.addAll(
+          packageConfig.packages
+              .map((Package e) => e.name)
+              .where((String e) => regExp.hasMatch(e)),
+        );
+      }
+    } on FormatException catch (e) {
+      throwToolExit('Regular expression syntax is invalid. $e');
+    }
+    return packagesToInclude;
+  }
+
  /// Parses a test file/directory target passed as an argument and returns it
  /// as an absolute file:/// [URI] with optional querystring for name/line/col.
  Uri _parseTestArgument(String arg) {
--- a/packages/flutter_tools/lib/src/test/coverage_collector.dart
+++ b/packages/flutter_tools/lib/src/test/coverage_collector.dart
@ -187,8 +187,13 @@ class CoverageCollector extends TestWatcher {
    if (formatter == null) {
      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')]
+      // find paths for libraryNames so we can include them to report
+      final List<String>? libraryPaths = libraryNames
+          ?.map((String e) => usedResolver.resolve('package:$e'))
+          .whereType<String>()
+          .toList();
+      final List<String>? reportOn = coverageDirectory == null
+          ? libraryPaths
          : <String>[coverageDirectory.path];
      formatter = (Map<String, coverage.HitMap> hitmap) => hitmap
          .formatLcov(usedResolver, reportOn: reportOn, basePath: packagePath);
--- a/packages/flutter_tools/test/commands.shard/hermetic/test_test.dart
+++ b/packages/flutter_tools/test/commands.shard/hermetic/test_test.dart
@ -16,14 +16,19 @@ import 'package:flutter_tools/src/device.dart';
 import 'package:flutter_tools/src/globals.dart' as globals;
 import 'package:flutter_tools/src/project.dart';
 import 'package:flutter_tools/src/runner/flutter_command.dart';
+import 'package:flutter_tools/src/test/coverage_collector.dart';
 import 'package:flutter_tools/src/test/runner.dart';
+import 'package:flutter_tools/src/test/test_device.dart';
 import 'package:flutter_tools/src/test/test_time_recorder.dart';
 import 'package:flutter_tools/src/test/test_wrapper.dart';
 import 'package:flutter_tools/src/test/watcher.dart';
+import 'package:stream_channel/stream_channel.dart';
+import 'package:vm_service/vm_service.dart';

 import '../../src/common.dart';
 import '../../src/context.dart';
 import '../../src/fake_devices.dart';
+import '../../src/fake_vm_services.dart';
 import '../../src/logging_logger.dart';
 import '../../src/test_flutter_command_runner.dart';

@ -249,6 +254,137 @@ dev_dependencies:
    Cache: () => Cache.test(processManager: FakeProcessManager.any()),
  });

+  testUsingContext('Coverage provides current library name to Coverage Collector by default', () async {
+    const String currentPackageName = '';
+    final FakeVmServiceHost fakeVmServiceHost = FakeVmServiceHost(
+      requests: <VmServiceExpectation>[
+        FakeVmServiceRequest(
+          method: 'getVM',
+          jsonResponse: (VM.parse(<String, Object>{})!
+            ..isolates = <IsolateRef>[
+              IsolateRef.parse(<String, Object>{
+                'id': '1',
+              })!,
+            ]
+          ).toJson(),
+        ),
+        FakeVmServiceRequest(
+          method: 'getVersion',
+          jsonResponse: Version(major: 3, minor: 57).toJson(),
+        ),
+        FakeVmServiceRequest(
+          method: 'getSourceReport',
+          args: <String, Object>{
+            'isolateId': '1',
+            'reports': <Object>['Coverage'],
+            'forceCompile': true,
+            'reportLines': true,
+            'libraryFilters': <String>['package:$currentPackageName/'],
+          },
+          jsonResponse: SourceReport(
+            ranges: <SourceReportRange>[],
+          ).toJson(),
+        ),
+      ],
+    );
+    final FakeFlutterTestRunner testRunner = FakeFlutterTestRunner(0, null, fakeVmServiceHost);
+
+    final TestCommand testCommand = TestCommand(testRunner: testRunner);
+    final CommandRunner<void> commandRunner =
+        createTestCommandRunner(testCommand);
+    await commandRunner.run(const <String>[
+      'test',
+      '--no-pub',
+      '--coverage',
+      '--',
+      'test/some_test.dart',
+    ]);
+    expect(fakeVmServiceHost.hasRemainingExpectations, false);
+    expect(
+      (testRunner.lastTestWatcher! as CoverageCollector).libraryNames,
+      <String>{currentPackageName},
+    );
+  }, overrides: <Type, Generator>{
+    FileSystem: () => fs,
+    ProcessManager: () => FakeProcessManager.any(),
+    Cache: () => Cache.test(processManager: FakeProcessManager.any()),
+  });
+
+  testUsingContext('Coverage provides library names matching regexps to Coverage Collector', () async {
+    final FakeVmServiceHost fakeVmServiceHost = FakeVmServiceHost(
+      requests: <VmServiceExpectation>[
+        FakeVmServiceRequest(
+          method: 'getVM',
+          jsonResponse: (VM.parse(<String, Object>{})!
+            ..isolates = <IsolateRef>[
+              IsolateRef.parse(<String, Object>{
+                'id': '1',
+              })!,
+            ]
+          ).toJson(),
+        ),
+        FakeVmServiceRequest(
+          method: 'getVersion',
+          jsonResponse: Version(major: 3, minor: 57).toJson(),
+        ),
+        FakeVmServiceRequest(
+          method: 'getSourceReport',
+          args: <String, Object>{
+            'isolateId': '1',
+            'reports': <Object>['Coverage'],
+            'forceCompile': true,
+            'reportLines': true,
+            'libraryFilters': <String>['package:test_api/'],
+          },
+          jsonResponse: SourceReport(
+            ranges: <SourceReportRange>[],
+          ).toJson(),
+        ),
+      ],
+    );
+    final FakeFlutterTestRunner testRunner = FakeFlutterTestRunner(0, null, fakeVmServiceHost);
+
+    final TestCommand testCommand = TestCommand(testRunner: testRunner);
+    final CommandRunner<void> commandRunner =
+        createTestCommandRunner(testCommand);
+    await commandRunner.run(const <String>[
+      'test',
+      '--no-pub',
+      '--coverage',
+      '--coverage-package=^test',
+      '--',
+      'test/some_test.dart',
+    ]);
+    expect(fakeVmServiceHost.hasRemainingExpectations, false);
+    expect(
+      (testRunner.lastTestWatcher! as CoverageCollector).libraryNames,
+      <String>{'test_api'},
+    );
+  }, overrides: <Type, Generator>{
+    FileSystem: () => fs,
+    ProcessManager: () => FakeProcessManager.any(),
+    Cache: () => Cache.test(processManager: FakeProcessManager.any()),
+  });
+
+  testUsingContext('Coverage provides error message if regular expression syntax is invalid', () async {
+    final FakeFlutterTestRunner testRunner = FakeFlutterTestRunner(0);
+
+    final TestCommand testCommand = TestCommand(testRunner: testRunner);
+    final CommandRunner<void> commandRunner = createTestCommandRunner(testCommand);
+
+    expect(() => commandRunner.run(const <String>[
+      'test',
+      '--no-pub',
+      '--coverage',
+      r'--coverage-package="$+"',
+      '--',
+      'test/some_test.dart',
+    ]), throwsToolExit(message: RegExp(r'Regular expression syntax is invalid. FormatException: Nothing to repeat[ \t]*"\$\+"')));
+  }, overrides: <Type, Generator>{
+    FileSystem: () => fs,
+    ProcessManager: () => FakeProcessManager.any(),
+  });
+
  testUsingContext('Pipes start-paused to package:test',
      () async {
    final FakePackageTest fakePackageTest = FakePackageTest();
@ -864,7 +1000,7 @@ dev_dependencies:
 }

 class FakeFlutterTestRunner implements FlutterTestRunner {
-  FakeFlutterTestRunner(this.exitCode, [this.leastRunTime]);
+  FakeFlutterTestRunner(this.exitCode, [this.leastRunTime, this.fakeVmServiceHost]);

  int exitCode;
  Duration? leastRunTime;
@ -873,6 +1009,8 @@ class FakeFlutterTestRunner implements FlutterTestRunner {
  String? lastFileReporterValue;
  String? lastReporterOption;
  int? lastConcurrency;
+  TestWatcher? lastTestWatcher;
+  FakeVmServiceHost? fakeVmServiceHost;

  @override
  Future<int> runTests(
@ -912,15 +1050,39 @@ class FakeFlutterTestRunner implements FlutterTestRunner {
    lastFileReporterValue = fileReporter;
    lastReporterOption = reporter;
    lastConcurrency = concurrency;
+    lastTestWatcher = watcher;

    if (leastRunTime != null) {
      await Future<void>.delayed(leastRunTime!);
    }

+    if (watcher is CoverageCollector) {
+      await watcher.collectCoverage(
+        TestTestDevice(),
+        serviceOverride: fakeVmServiceHost?.vmService,
+      );
+    }
+
    return exitCode;
  }
 }

+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 vmServiceUri => Future<Uri?>.value(Uri());
+
+  @override
+  Future<StreamChannel<String>> start(String entrypointPath) {
+    throw UnimplementedError();
+  }
+}
+
 class FakePackageTest implements TestWrapper {
  List<String>? lastArgs;

--- a/packages/flutter_tools/test/general.shard/coverage_collector_test.dart
+++ b/packages/flutter_tools/test/general.shard/coverage_collector_test.dart
@ -6,6 +6,8 @@ import 'dart:convert' show jsonEncode;
 import 'dart:io' show Directory, File;

 import 'package:coverage/src/hitmap.dart';
+import 'package:file/memory.dart';
+import 'package:flutter_tools/src/base/file_system.dart' show FileSystem;
 import 'package:flutter_tools/src/test/coverage_collector.dart';
 import 'package:flutter_tools/src/test/test_device.dart' show TestDevice;
 import 'package:flutter_tools/src/test/test_time_recorder.dart';
@ -13,6 +15,7 @@ import 'package:stream_channel/stream_channel.dart' show StreamChannel;
 import 'package:vm_service/vm_service.dart';

 import '../src/common.dart';
+import '../src/context.dart';
 import '../src/fake_vm_services.dart';
 import '../src/logging_logger.dart';

@ -515,6 +518,52 @@ void main() {
    }
  });

+  testUsingContext('Coverage collector respects libraryNames in finalized report', () async {
+    Directory? tempDir;
+    try {
+      tempDir = Directory.systemTemp.createTempSync('flutter_coverage_collector_test.');
+      final File packagesFile = writeFooBarPackagesJson(tempDir);
+      File('${tempDir.path}/foo/foo.dart').createSync(recursive: true);
+      File('${tempDir.path}/bar/bar.dart').createSync(recursive: true);
+
+      final String packagesPath = packagesFile.path;
+      CoverageCollector collector = CoverageCollector(
+          libraryNames: <String>{'foo', 'bar'},
+          verbose: false,
+          packagesPath: packagesPath,
+          resolver: await CoverageCollector.getResolver(packagesPath)
+      );
+      await collector.collectCoverage(
+        TestTestDevice(),
+        serviceOverride: createFakeVmServiceHostWithFooAndBar(libraryFilters: <String>['package:foo/', 'package:bar/']).vmService,
+      );
+
+      String? report = await collector.finalizeCoverage();
+      expect(report, contains('foo.dart'));
+      expect(report, contains('bar.dart'));
+
+      collector = CoverageCollector(
+          libraryNames: <String>{'foo'},
+          verbose: false,
+          packagesPath: packagesPath,
+          resolver: await CoverageCollector.getResolver(packagesPath)
+      );
+      await collector.collectCoverage(
+        TestTestDevice(),
+        serviceOverride: createFakeVmServiceHostWithFooAndBar(libraryFilters: <String>['package:foo/']).vmService,
+      );
+
+      report = await collector.finalizeCoverage();
+      expect(report, contains('foo.dart'));
+      expect(report, isNot(contains('bar.dart')));
+    } finally {
+      tempDir?.deleteSync(recursive: true);
+    }
+  }, overrides: <Type, Generator>{
+    FileSystem: () => MemoryFileSystem.test(),
+    ProcessManager: () => FakeProcessManager.any(),
+  });
+
  testWithoutContext('Coverage collector records test timings when provided TestTimeRecorder', () async {
    Directory? tempDir;
    try {