Attempt to enable tool coverage redux (#35074)

.cirrus.yml

@@ -120,6 +120,19 @@ task:
       container:
         cpu: 4
         memory: 12G
+    - name: tool_coverage-linux
+      # Only re-run tool coverage if the package itself changed.
+      skip: "!changesInclude('./packages/flutter_tools/**/*', '.cirrus.yml', './dev/bots/test.dart')"
+      env:
+        GCLOUD_SERVICE_ACCOUNT_KEY: ENCRYPTED[f12abe60f5045d619ef4c79b83dd1e0722a0b0b13dbea95fbe334e2db7fffbcd841a5a92da8824848b539a19afe0c9fb]
+        CODECOV_TOKEN: ENCRYPTED[7c76a7f8c9264f3b7f3fd63fcf186f93c62c4dfe43ec288861c2f506d456681032b89efe7b7a139c82156350ca2c752c]
+        SHARD: tool_coverage
+      test_script:
+        - dart --enable-asserts ./dev/bots/test.dart
+        - bash <(curl -s https://codecov.io/bash) -c -s ./packages/flutter_tools/coverage/ -f '*.lcov.info' -F flutter_tool
+      container:
+        cpu: 8
+        memory: 24G
     - name: web_tests-linux
       # TODO(jonahwilliams): re-enabled once we've determined causes for flakiness
       allow_failures: true

dev/bots/test.dart

@@ -20,6 +20,7 @@ final String flutter = path.join(flutterRoot, 'bin', Platform.isWindows ? 'flutt
 final String dart = path.join(flutterRoot, 'bin', 'cache', 'dart-sdk', 'bin', Platform.isWindows ? 'dart.exe' : 'dart');
 final String pub = path.join(flutterRoot, 'bin', 'cache', 'dart-sdk', 'bin', Platform.isWindows ? 'pub.bat' : 'pub');
 final String pubCache = path.join(flutterRoot, '.pub-cache');
+final String toolRoot = path.join(flutterRoot, 'packages', 'flutter_tools');
 final List<String> flutterTestArgs = <String>[];
 
 final bool useFlutterTestFormatter = Platform.environment['FLUTTER_TEST_FORMATTER'] == 'true';
@@ -30,6 +31,7 @@ const Map<String, ShardRunner> _kShards = <String, ShardRunner>{
   'tests': _runTests,
   'web_tests': _runWebTests,
   'tool_tests': _runToolTests,
+  'tool_coverage': _runToolCoverage,
   'build_tests': _runBuildTests,
   'coverage': _runCoverage,
   'integration_tests': _runIntegrationTests,
@@ -176,6 +178,58 @@ Future<bq.BigqueryApi> _getBigqueryApi() async {
   }
 }
 
+// Partition tool tests into two groups, see explanation on `_runToolCoverage`.
+List<List<String>> _partitionToolTests() {
+  final List<String> pending = <String>[];
+  final String toolTestDir = path.join(toolRoot, 'test');
+  for (FileSystemEntity entity in Directory(toolTestDir).listSync(recursive: true)) {
+    if (entity is File && entity.path.endsWith('_test.dart')) {
+      final String relativePath = path.relative(entity.path, from: toolRoot);
+      pending.add(relativePath);
+    }
+  }
+  // Shuffle the tests to avoid giving an expensive test directory like
+  // integration to a single run of tests.
+  pending..shuffle();
+  final int aboutHalf = pending.length ~/ 2;
+  final List<String> groupA = pending.take(aboutHalf).toList();
+  final List<String> groupB = pending.skip(aboutHalf).toList();
+  return <List<String>>[groupA, groupB];
+}
+
+// Tools tests run with coverage enabled have much higher memory usage than
+// our current CI infrastructure can support. We partition the tests into
+// two sets and run them in separate invocations of dart to reduce peak memory
+// usage. codecov.io automatically handles merging different coverage files
+// together, so producing separate files is OK.
+//
+// See: https://github.com/flutter/flutter/issues/35025
+Future<void> _runToolCoverage() async {
+  final List<List<String>> tests = _partitionToolTests();
+  // Precompile tests to speed up subsequent runs.
+  await runCommand(
+    pub,
+    <String>['run', 'build_runner', 'build'],
+    workingDirectory: toolRoot,
+  );
+
+  // The name of this subshard has to match the --file path provided at
+  // the end of this test script in `.cirrus.yml`.
+  const List<String> subshards = <String>['A', 'B'];
+  for (int i = 0; i < tests.length; i++) {
+    final List<String> testGroup = tests[i];
+    await runCommand(
+      dart,
+      <String>[path.join('tool', 'tool_coverage.dart'), '--']..addAll(testGroup),
+      workingDirectory: toolRoot,
+      environment: <String, String>{
+        'FLUTTER_ROOT': flutterRoot,
+        'SUBSHARD': subshards[i],
+      }
+    );
+  }
+}
+
 Future<void> _runToolTests() async {
   final bq.BigqueryApi bigqueryApi = await _getBigqueryApi();
   await _runSmokeTests();

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

@@ -41,6 +41,32 @@ class CoverageCollector extends TestWatcher {
     }
   }
 
+  /// Collects coverage for an isolate using the given `port`.
+  ///
+  /// This should be called when the code whose coverage data is being collected
+  /// has been run to completion so that all coverage data has been recorded.
+  ///
+  /// The returned [Future] completes when the coverage is collected.
+  Future<void> collectCoverageIsolate(Uri observatoryUri) async {
+    assert(observatoryUri != null);
+    printTrace('collecting coverage data from $observatoryUri...');
+    final Map<String, dynamic> data = await collect(observatoryUri, (String libraryName) {
+      // If we have a specified coverage directory or could not find the package name, then
+      // accept all libraries.
+      return (coverageDirectory != null)
+          || (flutterProject == null)
+          || libraryName.contains(flutterProject.manifest.appName);
+    });
+    if (data == null) {
+      throw Exception('Failed to collect coverage.');
+    }
+    assert(data != null);
+
+    print('($observatoryUri): collected coverage data; merging...');
+    _addHitmap(coverage.createHitmap(data['coverage']));
+    print('($observatoryUri): done merging coverage data into global coverage map.');
+  }
+
   /// Collects coverage for the given [Process] using the given `port`.
   ///
   /// This should be called when the code whose coverage data is being collected
@@ -91,7 +117,6 @@ class CoverageCollector extends TestWatcher {
     coverage.Formatter formatter,
     Directory coverageDirectory,
   }) async {
-    printTrace('formating coverage data');
     if (_globalHitmap == null) {
       return null;
     }

packages/flutter_tools/test/commands/create_test.dart

@@ -2,7 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-@Tags(<String>['create'])
+// This test performs too poorly to run with coverage enabled.
+@Tags(<String>['create', 'no_coverage'])
 import 'dart:async';
 import 'dart:convert';
 import 'dart:typed_data';

packages/flutter_tools/test/integration/daemon_mode_test.dart

@@ -2,6 +2,11 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+// Integration tests which invoke flutter instead of unit testing the code
+// will not produce meaningful coverage information - we can measure coverage
+// from the isolate running the test, but not from the isolate started via
+// the command line process.
+@Tags(<String>['no_coverage'])
 import 'dart:async';
 import 'dart:convert';
 

packages/flutter_tools/test/integration/debugger_stepping_test.dart

@@ -2,6 +2,11 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+// Integration tests which invoke flutter instead of unit testing the code
+// will not produce meaningful coverage information - we can measure coverage
+// from the isolate running the test, but not from the isolate started via
+// the command line process.
+@Tags(<String>['no_coverage'])
 import 'package:file/file.dart';
 import 'package:flutter_tools/src/base/file_system.dart';
 

packages/flutter_tools/test/integration/expression_evaluation_test.dart

@@ -2,6 +2,11 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+// Integration tests which invoke flutter instead of unit testing the code
+// will not produce meaningful coverage information - we can measure coverage
+// from the isolate running the test, but not from the isolate started via
+// the command line process.
+@Tags(<String>['no_coverage'])
 import 'dart:async';
 
 import 'package:file/file.dart';

packages/flutter_tools/test/integration/flutter_attach_test.dart

@@ -2,6 +2,11 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+// Integration tests which invoke flutter instead of unit testing the code
+// will not produce meaningful coverage information - we can measure coverage
+// from the isolate running the test, but not from the isolate started via
+// the command line process.
+@Tags(<String>['no_coverage'])
 import 'package:file/file.dart';
 import 'package:flutter_tools/src/base/file_system.dart';
 

packages/flutter_tools/test/integration/flutter_run_test.dart

@@ -2,6 +2,11 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+// Integration tests which invoke flutter instead of unit testing the code
+// will not produce meaningful coverage information - we can measure coverage
+// from the isolate running the test, but not from the isolate started via
+// the command line process.
+@Tags(<String>['no_coverage'])
 import 'package:file/file.dart';
 import 'package:flutter_tools/src/base/file_system.dart';
 import 'package:flutter_tools/src/base/io.dart';

packages/flutter_tools/test/integration/hot_reload_test.dart

@@ -2,6 +2,11 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+// Integration tests which invoke flutter instead of unit testing the code
+// will not produce meaningful coverage information - we can measure coverage
+// from the isolate running the test, but not from the isolate started via
+// the command line process.
+@Tags(<String>['no_coverage'])
 import 'dart:async';
 
 import 'package:file/file.dart';

packages/flutter_tools/test/integration/lifetime_test.dart

@@ -2,6 +2,11 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+// Integration tests which invoke flutter instead of unit testing the code
+// will not produce meaningful coverage information - we can measure coverage
+// from the isolate running the test, but not from the isolate started via
+// the command line process.
+@Tags(<String>['no_coverage'])
 import 'dart:async';
 
 import 'package:file/file.dart';

packages/flutter_tools/test/src/common.dart

@@ -18,7 +18,7 @@ import 'package:flutter_tools/src/commands/create.dart';
 import 'package:flutter_tools/src/runner/flutter_command.dart';
 import 'package:flutter_tools/src/runner/flutter_command_runner.dart';
 
-export 'package:test_api/test_api.dart' hide TypeMatcher, isInstanceOf; // Defines a 'package:test' shim.
+export 'package:test_core/test_core.dart' hide TypeMatcher, isInstanceOf; // Defines a 'package:test' shim.
 
 /// Disable both web and desktop to make testing easier. For example, prevent
 /// them from showing up in the devices list if the host happens to be setup

packages/flutter_tools/tool/tool_coverage.dart

@@ -3,165 +3,174 @@
 // found in the LICENSE file.
 
 import 'dart:async';
-import 'dart:convert';
+import 'dart:developer';
 import 'dart:io';
+import 'dart:isolate';
 
-import 'package:args/args.dart';
+import 'package:async/async.dart';
+import 'package:coverage/coverage.dart';
 import 'package:flutter_tools/src/context_runner.dart';
+import 'package:path/path.dart' as p;
+import 'package:pedantic/pedantic.dart';
+import 'package:stream_channel/isolate_channel.dart';
+import 'package:stream_channel/stream_channel.dart';
+import 'package:test_core/src/runner/hack_register_platform.dart' as hack; // ignore: implementation_imports
+import 'package:test_core/src/executable.dart' as test; // ignore: implementation_imports
+import 'package:vm_service_client/vm_service_client.dart';
+import 'package:test_api/src/backend/runtime.dart'; // ignore: implementation_imports
+import 'package:test_api/src/backend/suite_platform.dart'; // ignore: implementation_imports
+import 'package:test_core/src/runner/platform.dart'; // ignore: implementation_imports
+import 'package:test_core/src/runner/runner_suite.dart'; // ignore: implementation_imports
+import 'package:test_core/src/runner/suite.dart'; // ignore: implementation_imports
+import 'package:test_core/src/runner/plugin/platform_helpers.dart'; // ignore: implementation_imports
+import 'package:test_core/src/runner/environment.dart'; // ignore: implementation_imports
 import 'package:flutter_tools/src/project.dart';
 import 'package:flutter_tools/src/test/coverage_collector.dart';
-import 'package:pool/pool.dart';
-import 'package:path/path.dart' as path;
-
-final ArgParser argParser = ArgParser()
-  ..addOption('output-html',
-    defaultsTo: 'coverage/report.html',
-    help: 'The output path for the genhtml report.'
-  )
-  ..addOption('output-lcov',
-    defaultsTo: 'coverage/lcov.info',
-    help: 'The output path for the lcov data.'
-  )
-  ..addOption('test-directory',
-    defaultsTo: 'test/',
-    help: 'The path to the test directory.'
-  )
-  ..addOption('packages',
-    defaultsTo: '.packages',
-    help: 'The path to the .packages file.'
-  )
-  ..addOption('genhtml',
-    defaultsTo: 'genhtml',
-    help: 'The genhtml executable.');
-
-
-/// Generates an html coverage report for the flutter_tool.
+
+/// Generates an lcov report for the flutter tool unit tests.
 ///
 /// Example invocation:
 ///
-///    dart tool/tool_coverage.dart --packages=.packages --test-directory=test
+///    dart tool/tool_coverage.dart.
 Future<void> main(List<String> arguments) async {
-  final ArgResults argResults = argParser.parse(arguments);
-  await runInContext(() async {
-    final CoverageCollector coverageCollector = CoverageCollector(
-      flutterProject: FlutterProject.current(),
+  return runInContext(() async {
+    final VMPlatform vmPlatform = VMPlatform();
+    hack.registerPlatformPlugin(
+      <Runtime>[Runtime.vm],
+      () => vmPlatform,
     );
-    /// A temp directory to create synthetic test files in.
-    final Directory tempDirectory = Directory.systemTemp.createTempSync('_flutter_coverage')
-      ..createSync();
-    final String flutterRoot = File(Platform.script.toFilePath()).parent.parent.parent.parent.path;
-    await ToolCoverageRunner(tempDirectory, coverageCollector, flutterRoot, argResults).collectCoverage();
+    await test.main(<String>['-x', 'no_coverage', '--no-color', '-r', 'compact', '-j', '1', ...arguments]);
+    exit(exitCode);
   });
 }
 
-class ToolCoverageRunner {
-  ToolCoverageRunner(
-    this.tempDirectory,
-    this.coverageCollector,
-    this.flutterRoot,
-    this.argResults,
+/// A platform that loads tests in isolates spawned within this Dart process.
+class VMPlatform extends PlatformPlugin {
+  final CoverageCollector coverageCollector = CoverageCollector(
+    flutterProject: FlutterProject.current(),
   );
+  final Map<String, Future<void>> _pending = <String, Future<void>>{};
+  final String precompiledPath = p.join('.dart_tool', 'build', 'generated', 'flutter_tools');
 
-  final ArgResults argResults;
-  final Pool pool = Pool(Platform.numberOfProcessors);
-  final Directory tempDirectory;
-  final CoverageCollector coverageCollector;
-  final String flutterRoot;
-
-  Future<void> collectCoverage() async {
-    final List<Future<void>> pending = <Future<void>>[];
+  @override
+  StreamChannel<void> loadChannel(String path, SuitePlatform platform) =>
+      throw UnimplementedError();
 
-    final Directory testDirectory = Directory(argResults['test-directory']);
-    final List<FileSystemEntity> fileSystemEntities = testDirectory.listSync(recursive: true);
-    for (FileSystemEntity fileSystemEntity in fileSystemEntities) {
-      if (!fileSystemEntity.path.endsWith('_test.dart')) {
-        continue;
-      }
-      pending.add(_runTest(fileSystemEntity));
+  @override
+  Future<RunnerSuite> load(String path, SuitePlatform platform,
+      SuiteConfiguration suiteConfig, Object message) async {
+    final ReceivePort receivePort = ReceivePort();
+    Isolate isolate;
+    try {
+      isolate = await _spawnIsolate(path, receivePort.sendPort);
+    } catch (error) {
+      receivePort.close();
+      rethrow;
     }
-    await Future.wait(pending);
+    final Completer<void> completer = Completer<void>();
+    // When this is completed we remove it from the map of pending so we can
+    // log the futures that get "stuck".
+    unawaited(completer.future.whenComplete(() {
+      _pending.remove(path);
+    }));
+    final ServiceProtocolInfo info = await Service.controlWebServer(enable: true);
+    final dynamic channel = IsolateChannel<Object>.connectReceive(receivePort)
+        .transformStream(StreamTransformer<Object, Object>.fromHandlers(handleDone: (EventSink<Object> sink) async {
+      try {
+        // this will throw if collection fails.
+        await coverageCollector.collectCoverageIsolate(info.serverUri);
+      } finally {
+        isolate.kill(priority: Isolate.immediate);
+        isolate = null;
+        sink.close();
+        completer.complete();
+      }
+    }, handleError: (dynamic error, StackTrace stackTrace, EventSink<Object> sink) {
+      isolate.kill(priority: Isolate.immediate);
+      isolate = null;
+      sink.close();
+      completer.complete();
+    }));
 
-    final String lcovData = await coverageCollector.finalizeCoverage();
-    final String outputLcovPath = argResults['output-lcov'];
-    final String outputHtmlPath = argResults['output-html'];
-    final String genHtmlExecutable = argResults['genhtml'];
-    File(outputLcovPath)
-      ..createSync(recursive: true)
-      ..writeAsStringSync(lcovData);
-    await Process.run(genHtmlExecutable, <String>[outputLcovPath, '-o', outputHtmlPath], runInShell: true);
+    VMEnvironment environment;
+    final RunnerSuiteController controller = deserializeSuite(
+      path,
+      platform,
+      suiteConfig,
+      environment,
+      channel,
+      message,
+    );
+    _pending[path] = completer.future;
+    return await controller.suite;
   }
 
-  // Creates a synthetic test file to wrap the test main in a group invocation.
-  // This will set up several fields used by the test methods on the context. Normally
-  // this would be handled automatically by the test runner, but since we're executing
-  // the files directly with dart we need to handle it manually.
-  String _createTest(File testFile) {
-    final File fakeTest = File(path.join(tempDirectory.path, testFile.path))
-      ..createSync(recursive: true)
-      ..writeAsStringSync('''
-import "package:test/test.dart";
-import "${path.absolute(testFile.path)}" as entrypoint;
-
-void main() {
-  group('', entrypoint.main);
-}
-''');
-    return fakeTest.path;
+  /// Spawns an isolate and passes it [message].
+  ///
+  /// This isolate connects an [IsolateChannel] to [message] and sends the
+  /// serialized tests over that channel.
+  Future<Isolate> _spawnIsolate(String path, SendPort message) async {
+    String testPath = p.absolute(p.join(precompiledPath, path) + '.vm_test.dart');
+    testPath = testPath.substring(0, testPath.length - '.dart'.length) + '.vm.app.dill';
+    return await Isolate.spawnUri(p.toUri(testPath), <String>[], message,
+      packageConfig: p.toUri('.packages'),
+      checked: true,
+    );
   }
 
-  Future<void> _runTest(File testFile) async {
-    final PoolResource resource = await pool.request();
-    final String testPath = _createTest(testFile);
-    final int port = await _findPort();
-    final Uri coverageUri = Uri.parse('http://127.0.0.1:$port');
-    final Completer<void> completer = Completer<void>();
-    final String packagesPath = argResults['packages'];
-    final Process testProcess = await Process.start(
-      Platform.resolvedExecutable,
-      <String>[
-        '--packages=$packagesPath',
-        '--pause-isolates-on-exit',
-        '--enable-asserts',
-        '--enable-vm-service=${coverageUri.port}',
-        testPath,
-      ],
-      runInShell: true,
-      environment: <String, String>{
-        'FLUTTER_ROOT': flutterRoot,
-      }).timeout(const Duration(seconds: 30));
-    testProcess.stdout
-      .transform(utf8.decoder)
-      .transform(const LineSplitter())
-      .listen((String line) {
-        print(line);
-        if (line.contains('All tests passed') || line.contains('Some tests failed')) {
-          completer.complete(null);
-        }
-      });
+  @override
+  Future<void> close() async {
     try {
-      await completer.future;
-      await coverageCollector.collectCoverage(testProcess, coverageUri).timeout(const Duration(seconds: 30));
-      testProcess?.kill();
+      await Future.wait(_pending.values).timeout(const Duration(seconds: 10));
     } on TimeoutException {
-      print('Failed to collect coverage for ${testFile.path} after 30 seconds');
-    } finally {
-      resource.release();
+      // TODO(jonahwilliams): resolve whether there are any specific tests that
+      // get stuck or if it is a general infra issue with how we are collecting
+      // coverage.
+      // Log tests that are "Stuck" waiuting for coverage.
+      print('The folllowing tests timed out waiting for coverage:');
+      print(_pending.keys.join(', '));
     }
+    final String packagePath = Directory.current.path;
+    final Resolver resolver = Resolver(packagesPath: '.packages');
+    final Formatter formatter = LcovFormatter(resolver, reportOn: <String>[
+      'lib',
+    ], basePath: packagePath);
+    final String result = await coverageCollector.finalizeCoverage(
+      formatter: formatter,
+    );
+    final String prefix = Platform.environment['SUBSHARD'] ?? '';
+    final String outputLcovPath = p.join('coverage', '$prefix.lcov.info');
+    File(outputLcovPath)
+      ..createSync(recursive: true)
+      ..writeAsStringSync(result);
   }
+}
 
-  Future<int> _findPort() async {
-    int port = 0;
-    ServerSocket serverSocket;
-    try {
-      serverSocket = await ServerSocket.bind(InternetAddress.loopbackIPv4.address, 0);
-      port = serverSocket.port;
-    } catch (e) {
-      // Failures are signaled by a return value of 0 from this function.
-      print('_findPort failed: $e');
-    }
-    if (serverSocket != null) {
-      await serverSocket.close();
-    }
-    return port;
+class VMEnvironment implements Environment {
+  VMEnvironment(this.observatoryUrl, this._isolate);
+
+  @override
+  final bool supportsDebugging = false;
+
+  @override
+  final Uri observatoryUrl;
+
+  /// The VM service isolate object used to control this isolate.
+  final VMIsolateRef _isolate;
+
+  @override
+  Uri get remoteDebuggerUrl => null;
+
+  @override
+  Stream<void> get onRestart => StreamController<dynamic>.broadcast().stream;
+
+  @override
+  CancelableOperation<void> displayPause() {
+    final CancelableCompleter<dynamic> completer = CancelableCompleter<dynamic>(onCancel: () => _isolate.resume());
+
+    completer.complete(_isolate.pause().then((dynamic _) => _isolate.onPauseOrResume
+        .firstWhere((VMPauseEvent event) => event is VMResumeEvent)));
+
+    return completer.operation;
   }
 }