From 03ae2cf366fb7f26b5cfe8c0583a396f812d8d80 Mon Sep 17 00:00:00 2001 From: Ian Hickson Date: Wed, 15 Feb 2017 18:50:19 -0800 Subject: [PATCH] Catch crashes during or before coverage collection. (#8207) This assumes a fix to https://github.com/dart-lang/test/issues/542 The timeout added in this patch is a workaround for https://github.com/dart-lang/coverage/issues/165 --- .../flutter_tools/lib/src/commands/test.dart | 6 +++++- .../lib/src/test/coverage_collector.dart | 21 ++++++++++++------- .../lib/src/test/flutter_platform.dart | 20 ++++++++++++++---- 3 files changed, 34 insertions(+), 13 deletions(-) diff --git a/packages/flutter_tools/lib/src/commands/test.dart b/packages/flutter_tools/lib/src/commands/test.dart index a86ad2eb752..cdca01ac254 100644 --- a/packages/flutter_tools/lib/src/commands/test.dart +++ b/packages/flutter_tools/lib/src/commands/test.dart @@ -193,7 +193,11 @@ class TestCommand extends FlutterCommand { final String shellPath = tools.getHostToolPath(HostTool.SkyShell) ?? platform.environment['SKY_SHELL']; if (!fs.isFileSync(shellPath)) throwToolExit('Cannot find Flutter shell at $shellPath'); - loader.installHook(shellPath: shellPath, collector: collector, debuggerMode: argResults['start-paused']); + loader.installHook( + shellPath: shellPath, + collector: collector, + debuggerMode: argResults['start-paused'], + ); Cache.releaseLockEarly(); diff --git a/packages/flutter_tools/lib/src/test/coverage_collector.dart b/packages/flutter_tools/lib/src/test/coverage_collector.dart index 1c638854a1f..44fa13361bf 100644 --- a/packages/flutter_tools/lib/src/test/coverage_collector.dart +++ b/packages/flutter_tools/lib/src/test/coverage_collector.dart @@ -4,7 +4,7 @@ import 'dart:async'; -import 'package:coverage/coverage.dart'; +import 'package:coverage/coverage.dart' as coverage; import '../base/file_system.dart'; import '../base/io.dart'; @@ -19,7 +19,7 @@ class CoverageCollector { if (_globalHitmap == null) _globalHitmap = hitmap; else - mergeHitmaps(hitmap, _globalHitmap); + coverage.mergeHitmaps(hitmap, _globalHitmap); } /// Collects coverage for the given [Process] using the given `port`. @@ -37,13 +37,18 @@ class CoverageCollector { process.exitCode.then((int code) { exitCode = code; }); + if (exitCode != null) + throw new Exception('Failed to collect coverage, process terminated before coverage could be collected.'); printTrace('pid $pid (port $port): collecting coverage data...'); - final Map data = await collect(host.address, port, false, false); + final Map data = await coverage.collect(host.address, port, false, false).timeout( + const Duration(seconds: 30), + onTimeout: () { throw new Exception('Failed to collect coverage, it took more than thirty seconds.'); }, + ); printTrace('pid $pid (port $port): ${ exitCode != null ? "process terminated prematurely with exit code $exitCode; aborting" : "collected coverage data; merging..." }'); if (exitCode != null) - throw new Exception('Failed to collect coverage, process terminated prematurely.'); - _addHitmap(createHitmap(data['coverage'])); + throw new Exception('Failed to collect coverage, process terminated while coverage was being collected.'); + _addHitmap(coverage.createHitmap(data['coverage'])); printTrace('pid $pid (port $port): done merging coverage data into global coverage map.'); } @@ -56,17 +61,17 @@ class CoverageCollector { /// If [timeout] is specified, the future will timeout (with a /// [TimeoutException]) after the specified duration. Future finalizeCoverage({ - Formatter formatter, + coverage.Formatter formatter, Duration timeout, }) async { printTrace('formating coverage data'); if (_globalHitmap == null) return null; if (formatter == null) { - Resolver resolver = new Resolver(packagesPath: PackageMap.globalPackagesPath); + coverage.Resolver resolver = new coverage.Resolver(packagesPath: PackageMap.globalPackagesPath); String packagePath = fs.currentDirectory.path; List reportOn = [fs.path.join(packagePath, 'lib')]; - formatter = new LcovFormatter(resolver, reportOn: reportOn, basePath: packagePath); + formatter = new coverage.LcovFormatter(resolver, reportOn: reportOn, basePath: packagePath); } String result = await formatter.format(_globalHitmap); _globalHitmap = null; diff --git a/packages/flutter_tools/lib/src/test/flutter_platform.dart b/packages/flutter_tools/lib/src/test/flutter_platform.dart index c812ee4090f..5fafa712363 100644 --- a/packages/flutter_tools/lib/src/test/flutter_platform.dart +++ b/packages/flutter_tools/lib/src/test/flutter_platform.dart @@ -71,7 +71,13 @@ enum _TestResult { crashed, harnessBailed, testBailed } typedef Future _Finalizer(); class _FlutterPlatform extends PlatformPlugin { - _FlutterPlatform({ this.shellPath, this.collector, this.debuggerMode, this.explicitObservatoryPort, this.explicitDiagnosticPort }) { + _FlutterPlatform({ + this.shellPath, + this.collector, + this.debuggerMode, + this.explicitObservatoryPort, + this.explicitDiagnosticPort, + }) { assert(shellPath != null); } @@ -113,15 +119,15 @@ class _FlutterPlatform extends PlatformPlugin { localController.stream, remoteSink, ); - _startTest(testPath, localChannel, ourTestCount).whenComplete(() { - testCompleteCompleter.complete(); - }); + testCompleteCompleter.complete(_startTest(testPath, localChannel, ourTestCount)); return remoteChannel; } Future _startTest(String testPath, StreamChannel controller, int ourTestCount) async { printTrace('test $ourTestCount: starting test $testPath'); + dynamic outOfBandError; // error that we couldn't send to the harness that we need to send via our future + final List<_Finalizer> finalizers = <_Finalizer>[]; bool subprocessActive = false; bool controllerSinkClosed = false; @@ -334,6 +340,7 @@ class _FlutterPlatform extends PlatformPlugin { controller.sink.addError(error, stack); } else { printError('unhandled error during test:\n$testPath\n$error'); + outOfBandError ??= error; } } finally { printTrace('test $ourTestCount: cleaning up...'); @@ -346,6 +353,7 @@ class _FlutterPlatform extends PlatformPlugin { controller.sink.addError(error, stack); } else { printError('unhandled error during finalization of test:\n$testPath\n$error'); + outOfBandError ??= error; } } } @@ -357,6 +365,10 @@ class _FlutterPlatform extends PlatformPlugin { } assert(!subprocessActive); assert(controllerSinkClosed); + if (outOfBandError != null) { + printTrace('test $ourTestCount: finished with out-of-band failure'); + throw outOfBandError; + } printTrace('test $ourTestCount: finished'); return null; }