From 295530dcaf88045c8e60cf4b44d8f183ec0eb91b Mon Sep 17 00:00:00 2001 From: Ian Hickson Date: Mon, 22 Jul 2019 14:13:33 -0700 Subject: [PATCH] Some minor cleanup in devicelab (#36571) --- dev/devicelab/bin/run.dart | 2 +- dev/devicelab/bin/tasks/dartdocs.dart | 5 +-- dev/devicelab/lib/framework/apk_utils.dart | 15 ++++++-- dev/devicelab/lib/framework/framework.dart | 12 +++---- dev/devicelab/lib/framework/runner.dart | 41 ++++------------------ 5 files changed, 27 insertions(+), 48 deletions(-) diff --git a/dev/devicelab/bin/run.dart b/dev/devicelab/bin/run.dart index 4cd54b5a6c1..2d74927bef3 100644 --- a/dev/devicelab/bin/run.dart +++ b/dev/devicelab/bin/run.dart @@ -107,7 +107,7 @@ final ArgParser _argParser = ArgParser() 'stage', abbr: 's', help: 'Name of the stage. Runs all tasks for that stage. ' - 'The tasks and their stages are read from manifest.yaml.', + 'The tasks and their stages are read from manifest.yaml.', ) ..addFlag( 'all', diff --git a/dev/devicelab/bin/tasks/dartdocs.dart b/dev/devicelab/bin/tasks/dartdocs.dart index 65ff5f6101c..1833b2d0ff8 100644 --- a/dev/devicelab/bin/tasks/dartdocs.dart +++ b/dev/devicelab/bin/tasks/dartdocs.dart @@ -11,6 +11,7 @@ import 'package:flutter_devicelab/framework/utils.dart'; import 'package:path/path.dart' as path; Future main() async { + final String dot = Platform.isWindows ? '-' : '•'; await task(() async { final Stopwatch clock = Stopwatch()..start(); final Process analysis = await startProcess( @@ -27,9 +28,9 @@ Future main() async { print('analyzer stdout: $entry'); if (entry == 'Building flutter tool...') { // ignore this line - } else if (entry.startsWith('info • Document all public members •')) { + } else if (entry.startsWith('info $dot Document all public members $dot')) { publicMembers += 1; - } else if (entry.startsWith('info •') || entry.startsWith('warning •') || entry.startsWith('error •')) { + } else if (entry.startsWith('info $dot') || entry.startsWith('warning $dot') || entry.startsWith('error $dot')) { otherErrors += 1; } else if (entry.contains(' (ran in ') && !sawFinalLine) { // ignore this line once diff --git a/dev/devicelab/lib/framework/apk_utils.dart b/dev/devicelab/lib/framework/apk_utils.dart index 62bee93d3d1..0dc28b92e2e 100644 --- a/dev/devicelab/lib/framework/apk_utils.dart +++ b/dev/devicelab/lib/framework/apk_utils.dart @@ -256,9 +256,18 @@ Future _resultOfGradleTask({String workingDirectory, String task, 'app:$task', ...?options, ]; - final String gradle = Platform.isWindows ? 'gradlew.bat' : './gradlew'; - print('Running Gradle: ${path.join(workingDirectory, gradle)} ${args.join(' ')}'); - print(File(path.join(workingDirectory, gradle)).readAsStringSync()); + final String gradle = path.join(workingDirectory, Platform.isWindows ? 'gradlew.bat' : './gradlew'); + print('┌── $gradle'); + print('│ ' + File(path.join(workingDirectory, gradle)).readAsLinesSync().join('\n│ ')); + print('└─────────────────────────────────────────────────────────────────────────────────────'); + print( + 'Running Gradle:\n' + ' Executable: $gradle\n' + ' Arguments: ${args.join(' ')}\n' + ' Working directory: $workingDirectory\n' + ' JAVA_HOME: $javaHome\n' + '' + ); return Process.run( gradle, args, diff --git a/dev/devicelab/lib/framework/framework.dart b/dev/devicelab/lib/framework/framework.dart index 34d2f6469ab..ec41d5b17c5 100644 --- a/dev/devicelab/lib/framework/framework.dart +++ b/dev/devicelab/lib/framework/framework.dart @@ -14,11 +14,6 @@ import 'package:stack_trace/stack_trace.dart'; import 'running_processes.dart'; import 'utils.dart'; -/// Maximum amount of time a single task is allowed to take to run. -/// -/// If exceeded the task is considered to have failed. -const Duration _kDefaultTaskTimeout = Duration(minutes: 15); - /// Represents a unit of work performed in the CI environment that can /// succeed, fail and be retried independently of others. typedef TaskFunction = Future Function(); @@ -55,7 +50,7 @@ class _TaskRunner { (String method, Map parameters) async { final Duration taskTimeout = parameters.containsKey('timeoutInMinutes') ? Duration(minutes: int.parse(parameters['timeoutInMinutes'])) - : _kDefaultTaskTimeout; + : null; final TaskResult result = await run(taskTimeout); return ServiceExtensionResponse.result(json.encode(result.toJson())); }); @@ -90,7 +85,10 @@ class _TaskRunner { ).toSet(); beforeRunningDartInstances.forEach(print); - TaskResult result = await _performTask().timeout(taskTimeout); + Future futureResult = _performTask(); + if (taskTimeout != null) + futureResult = futureResult.timeout(taskTimeout); + TaskResult result = await futureResult; section('Checking running Dart$exe processes after task...'); final List afterRunningDartInstances = await getRunningProcesses( diff --git a/dev/devicelab/lib/framework/runner.dart b/dev/devicelab/lib/framework/runner.dart index dfaeb7f0e5a..642afd476ec 100644 --- a/dev/devicelab/lib/framework/runner.dart +++ b/dev/devicelab/lib/framework/runner.dart @@ -11,10 +11,6 @@ import 'package:vm_service_client/vm_service_client.dart'; import 'package:flutter_devicelab/framework/utils.dart'; -/// Slightly longer than task timeout that gives the task runner a chance to -/// clean-up before forcefully quitting it. -const Duration taskTimeoutWithGracePeriod = Duration(minutes: 26); - /// Runs a task in a separate Dart VM and collects the result using the VM /// service protocol. /// @@ -71,21 +67,11 @@ Future> runTask( stderr.writeln('[$taskName] [STDERR] $line'); }); - String waitingFor = 'connection'; try { final VMIsolateRef isolate = await _connectToRunnerIsolate(await uri.future); - waitingFor = 'task completion'; - final Map taskResult = - await isolate.invokeExtension('ext.cocoonRunTask').timeout(taskTimeoutWithGracePeriod); - waitingFor = 'task process to exit'; - await runner.exitCode.timeout(const Duration(seconds: 60)); + final Map taskResult = await isolate.invokeExtension('ext.cocoonRunTask'); + await runner.exitCode; return taskResult; - } on TimeoutException catch (timeout) { - runner.kill(ProcessSignal.sigint); - return { - 'success': false, - 'reason': 'Timeout in runner.dart waiting for $waitingFor: ${timeout.message}', - }; } finally { if (!runnerFinished) runner.kill(ProcessSignal.sigkill); @@ -104,14 +90,7 @@ Future _connectToRunnerIsolate(Uri vmServiceUri) async { pathSegments.add('ws'); final String url = vmServiceUri.replace(scheme: 'ws', pathSegments: pathSegments).toString(); - final DateTime started = DateTime.now(); - - // TODO(yjbanov): due to lack of imagination at the moment the handshake with - // the task process is very rudimentary and requires this small - // delay to let the task process open up the VM service port. - // Otherwise we almost always hit the non-ready case first and - // wait a whole 1 second, which is annoying. - await Future.delayed(const Duration(milliseconds: 100)); + final Stopwatch stopwatch = Stopwatch()..start(); while (true) { try { @@ -127,17 +106,9 @@ Future _connectToRunnerIsolate(Uri vmServiceUri) async { throw 'not ready yet'; return isolate; } catch (error) { - const Duration connectionTimeout = Duration(seconds: 10); - if (DateTime.now().difference(started) > connectionTimeout) { - throw TimeoutException( - 'Failed to connect to the task runner process', - connectionTimeout, - ); - } - print('VM service not ready yet: $error'); - const Duration pauseBetweenRetries = Duration(milliseconds: 200); - print('Will retry in $pauseBetweenRetries.'); - await Future.delayed(pauseBetweenRetries); + if (stopwatch.elapsed > const Duration(seconds: 10)) + print('VM service still not ready after ${stopwatch.elapsed}: $error\nContinuing to retry...'); + await Future.delayed(const Duration(milliseconds: 50)); } } }