From 33ad5bac3492be62b7939fced093336e205360fa Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Wed, 26 Jun 2019 16:02:49 -0700 Subject: [PATCH] Attempt to enable tool coverage redux (#35074) --- .cirrus.yml | 13 + dev/bots/test.dart | 54 ++++ .../lib/src/test/coverage_collector.dart | 27 +- .../test/commands/create_test.dart | 3 +- .../test/integration/daemon_mode_test.dart | 5 + .../integration/debugger_stepping_test.dart | 5 + .../expression_evaluation_test.dart | 5 + .../test/integration/flutter_attach_test.dart | 5 + .../test/integration/flutter_run_test.dart | 5 + .../test/integration/hot_reload_test.dart | 5 + .../test/integration/lifetime_test.dart | 5 + packages/flutter_tools/test/src/common.dart | 2 +- .../flutter_tools/tool/tool_coverage.dart | 287 +++++++++--------- 13 files changed, 279 insertions(+), 142 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index ad595e8a9cf..53693ebfc70 100644 --- a/.cirrus.yml +++ b/.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 diff --git a/dev/bots/test.dart b/dev/bots/test.dart index e62b8946767..8efc6ca2ac7 100644 --- a/dev/bots/test.dart +++ b/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 flutterTestArgs = []; final bool useFlutterTestFormatter = Platform.environment['FLUTTER_TEST_FORMATTER'] == 'true'; @@ -30,6 +31,7 @@ const Map _kShards = { 'tests': _runTests, 'web_tests': _runWebTests, 'tool_tests': _runToolTests, + 'tool_coverage': _runToolCoverage, 'build_tests': _runBuildTests, 'coverage': _runCoverage, 'integration_tests': _runIntegrationTests, @@ -176,6 +178,58 @@ Future _getBigqueryApi() async { } } +// Partition tool tests into two groups, see explanation on `_runToolCoverage`. +List> _partitionToolTests() { + final List pending = []; + 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 groupA = pending.take(aboutHalf).toList(); + final List groupB = pending.skip(aboutHalf).toList(); + return >[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 _runToolCoverage() async { + final List> tests = _partitionToolTests(); + // Precompile tests to speed up subsequent runs. + await runCommand( + pub, + ['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 subshards = ['A', 'B']; + for (int i = 0; i < tests.length; i++) { + final List testGroup = tests[i]; + await runCommand( + dart, + [path.join('tool', 'tool_coverage.dart'), '--']..addAll(testGroup), + workingDirectory: toolRoot, + environment: { + 'FLUTTER_ROOT': flutterRoot, + 'SUBSHARD': subshards[i], + } + ); + } +} + Future _runToolTests() async { final bq.BigqueryApi bigqueryApi = await _getBigqueryApi(); await _runSmokeTests(); diff --git a/packages/flutter_tools/lib/src/test/coverage_collector.dart b/packages/flutter_tools/lib/src/test/coverage_collector.dart index c5e6982c41b..c166d07e62c 100644 --- a/packages/flutter_tools/lib/src/test/coverage_collector.dart +++ b/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 collectCoverageIsolate(Uri observatoryUri) async { + assert(observatoryUri != null); + printTrace('collecting coverage data from $observatoryUri...'); + final Map 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; } diff --git a/packages/flutter_tools/test/commands/create_test.dart b/packages/flutter_tools/test/commands/create_test.dart index 5008d104860..68c60c8dd78 100644 --- a/packages/flutter_tools/test/commands/create_test.dart +++ b/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(['create']) +// This test performs too poorly to run with coverage enabled. +@Tags(['create', 'no_coverage']) import 'dart:async'; import 'dart:convert'; import 'dart:typed_data'; diff --git a/packages/flutter_tools/test/integration/daemon_mode_test.dart b/packages/flutter_tools/test/integration/daemon_mode_test.dart index c5950cc8f3e..f1ce5bab5f2 100644 --- a/packages/flutter_tools/test/integration/daemon_mode_test.dart +++ b/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(['no_coverage']) import 'dart:async'; import 'dart:convert'; diff --git a/packages/flutter_tools/test/integration/debugger_stepping_test.dart b/packages/flutter_tools/test/integration/debugger_stepping_test.dart index 00d70de3754..a53cff9e412 100644 --- a/packages/flutter_tools/test/integration/debugger_stepping_test.dart +++ b/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(['no_coverage']) import 'package:file/file.dart'; import 'package:flutter_tools/src/base/file_system.dart'; diff --git a/packages/flutter_tools/test/integration/expression_evaluation_test.dart b/packages/flutter_tools/test/integration/expression_evaluation_test.dart index bd131bdfe4d..b5d61a6b007 100644 --- a/packages/flutter_tools/test/integration/expression_evaluation_test.dart +++ b/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(['no_coverage']) import 'dart:async'; import 'package:file/file.dart'; diff --git a/packages/flutter_tools/test/integration/flutter_attach_test.dart b/packages/flutter_tools/test/integration/flutter_attach_test.dart index 572f9e4a445..b5592c1a2d3 100644 --- a/packages/flutter_tools/test/integration/flutter_attach_test.dart +++ b/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(['no_coverage']) import 'package:file/file.dart'; import 'package:flutter_tools/src/base/file_system.dart'; diff --git a/packages/flutter_tools/test/integration/flutter_run_test.dart b/packages/flutter_tools/test/integration/flutter_run_test.dart index 784faca92be..cc833231eb0 100644 --- a/packages/flutter_tools/test/integration/flutter_run_test.dart +++ b/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(['no_coverage']) import 'package:file/file.dart'; import 'package:flutter_tools/src/base/file_system.dart'; import 'package:flutter_tools/src/base/io.dart'; diff --git a/packages/flutter_tools/test/integration/hot_reload_test.dart b/packages/flutter_tools/test/integration/hot_reload_test.dart index 47c4e99f661..41e64abe891 100644 --- a/packages/flutter_tools/test/integration/hot_reload_test.dart +++ b/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(['no_coverage']) import 'dart:async'; import 'package:file/file.dart'; diff --git a/packages/flutter_tools/test/integration/lifetime_test.dart b/packages/flutter_tools/test/integration/lifetime_test.dart index 8bad8cb5bd9..9e5284acc0f 100644 --- a/packages/flutter_tools/test/integration/lifetime_test.dart +++ b/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(['no_coverage']) import 'dart:async'; import 'package:file/file.dart'; diff --git a/packages/flutter_tools/test/src/common.dart b/packages/flutter_tools/test/src/common.dart index dea43fde5f7..27fe71c415e 100644 --- a/packages/flutter_tools/test/src/common.dart +++ b/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 diff --git a/packages/flutter_tools/tool/tool_coverage.dart b/packages/flutter_tools/tool/tool_coverage.dart index f1680904e6b..0de3a1b05d1 100644 --- a/packages/flutter_tools/tool/tool_coverage.dart +++ b/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 main(List 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.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(['-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> _pending = >{}; + 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; + @override + StreamChannel loadChannel(String path, SuitePlatform platform) => + throw UnimplementedError(); - Future collectCoverage() async { - final List> pending = >[]; - - final Directory testDirectory = Directory(argResults['test-directory']); - final List fileSystemEntities = testDirectory.listSync(recursive: true); - for (FileSystemEntity fileSystemEntity in fileSystemEntities) { - if (!fileSystemEntity.path.endsWith('_test.dart')) { - continue; - } - pending.add(_runTest(fileSystemEntity)); + @override + Future 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 completer = Completer(); + // 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.connectReceive(receivePort) + .transformStream(StreamTransformer.fromHandlers(handleDone: (EventSink 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 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']; + VMEnvironment environment; + final RunnerSuiteController controller = deserializeSuite( + path, + platform, + suiteConfig, + environment, + channel, + message, + ); + _pending[path] = completer.future; + return await controller.suite; + } + + /// Spawns an isolate and passes it [message]. + /// + /// This isolate connects an [IsolateChannel] to [message] and sends the + /// serialized tests over that channel. + Future _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), [], message, + packageConfig: p.toUri('.packages'), + checked: true, + ); + } + + @override + Future close() async { + try { + await Future.wait(_pending.values).timeout(const Duration(seconds: 10)); + } on TimeoutException { + // 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: [ + '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(lcovData); - await Process.run(genHtmlExecutable, [outputLcovPath, '-o', outputHtmlPath], runInShell: true); - } - - // 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; - } - - Future _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 completer = Completer(); - final String packagesPath = argResults['packages']; - final Process testProcess = await Process.start( - Platform.resolvedExecutable, - [ - '--packages=$packagesPath', - '--pause-isolates-on-exit', - '--enable-asserts', - '--enable-vm-service=${coverageUri.port}', - testPath, - ], - runInShell: true, - environment: { - '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); - } - }); - try { - await completer.future; - await coverageCollector.collectCoverage(testProcess, coverageUri).timeout(const Duration(seconds: 30)); - testProcess?.kill(); - } on TimeoutException { - print('Failed to collect coverage for ${testFile.path} after 30 seconds'); - } finally { - resource.release(); - } - } - - Future _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; + ..writeAsStringSync(result); + } +} + +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 get onRestart => StreamController.broadcast().stream; + + @override + CancelableOperation displayPause() { + final CancelableCompleter completer = CancelableCompleter(onCancel: () => _isolate.resume()); + + completer.complete(_isolate.pause().then((dynamic _) => _isolate.onPauseOrResume + .firstWhere((VMPauseEvent event) => event is VMResumeEvent))); + + return completer.operation; } }