From 03034e9e4aabe4ae99cc21a7010f623c69842fd3 Mon Sep 17 00:00:00 2001 From: Jenn Magder Date: Fri, 18 Jun 2021 00:14:03 -0700 Subject: [PATCH] Audit devicelab log streaming for nullability (#84820) --- .../tasks/complex_layout_semantics_perf.dart | 3 -- .../bin/tasks/drive_perf_debug_warning.dart | 3 -- dev/devicelab/bin/tasks/module_test_ios.dart | 33 ++++++++++-------- .../bin/tasks/native_ui_tests_ios32.dart | 31 +++++++++-------- dev/devicelab/bin/tasks/routing_test.dart | 3 -- dev/devicelab/lib/framework/devices.dart | 18 +++++----- dev/devicelab/lib/framework/framework.dart | 3 +- dev/devicelab/lib/framework/utils.dart | 6 ++++ dev/devicelab/lib/tasks/gallery.dart | 3 -- .../lib/tasks/integration_tests.dart | 3 -- dev/devicelab/lib/tasks/perf_tests.dart | 34 ++++++++----------- 11 files changed, 66 insertions(+), 74 deletions(-) diff --git a/dev/devicelab/bin/tasks/complex_layout_semantics_perf.dart b/dev/devicelab/bin/tasks/complex_layout_semantics_perf.dart index 1dc64b5363..0a40d263f9 100644 --- a/dev/devicelab/bin/tasks/complex_layout_semantics_perf.dart +++ b/dev/devicelab/bin/tasks/complex_layout_semantics_perf.dart @@ -8,7 +8,6 @@ import 'dart:io'; import 'package:flutter_devicelab/framework/devices.dart'; import 'package:flutter_devicelab/framework/framework.dart'; -import 'package:flutter_devicelab/framework/host_agent.dart'; import 'package:flutter_devicelab/framework/task_result.dart'; import 'package:flutter_devicelab/framework/utils.dart'; import 'package:path/path.dart' as p; @@ -34,8 +33,6 @@ void main() { p.join(complexLayoutPath, 'test_driver', 'semantics_perf.dart'), '-d', deviceId, - '--screenshot', - hostAgent.dumpDirectory.path, ]); }); diff --git a/dev/devicelab/bin/tasks/drive_perf_debug_warning.dart b/dev/devicelab/bin/tasks/drive_perf_debug_warning.dart index 981e413c6f..08be167624 100644 --- a/dev/devicelab/bin/tasks/drive_perf_debug_warning.dart +++ b/dev/devicelab/bin/tasks/drive_perf_debug_warning.dart @@ -6,7 +6,6 @@ import 'package:flutter_devicelab/framework/devices.dart'; import 'package:flutter_devicelab/framework/framework.dart'; -import 'package:flutter_devicelab/framework/host_agent.dart'; import 'package:flutter_devicelab/framework/task_result.dart'; import 'package:flutter_devicelab/framework/utils.dart'; @@ -18,8 +17,6 @@ Future _runWithMode(String mode, String deviceId) async { 'test_driver/scroll_perf.dart', '-d', deviceId, - '--screenshot', - hostAgent.dumpDirectory.path, ]); return stderr.toString(); } diff --git a/dev/devicelab/bin/tasks/module_test_ios.dart b/dev/devicelab/bin/tasks/module_test_ios.dart index 98f7d61ec6..76fd41dc92 100644 --- a/dev/devicelab/bin/tasks/module_test_ios.dart +++ b/dev/devicelab/bin/tasks/module_test_ios.dart @@ -360,21 +360,24 @@ Future main() async { ); if (testResultExit != 0) { - // Zip the test results to the artifacts directory for upload. - await inDirectory(resultBundleTemp, () { - final String zipPath = path.join(hostAgent.dumpDirectory.path, - 'module_test_ios-objc-${DateTime.now().toLocal().toIso8601String()}.zip'); - return exec( - 'zip', - [ - '-r', - '-9', - zipPath, - 'result.xcresult', - ], - canFail: true, // Best effort to get the logs. - ); - }); + final Directory dumpDirectory = hostAgent.dumpDirectory; + if (dumpDirectory != null) { + // Zip the test results to the artifacts directory for upload. + await inDirectory(resultBundleTemp, () { + final String zipPath = path.join(dumpDirectory.path, + 'module_test_ios-objc-${DateTime.now().toLocal().toIso8601String()}.zip'); + return exec( + 'zip', + [ + '-r', + '-9', + zipPath, + 'result.xcresult', + ], + canFail: true, // Best effort to get the logs. + ); + }); + } throw TaskResult.failure('Platform unit tests failed'); } diff --git a/dev/devicelab/bin/tasks/native_ui_tests_ios32.dart b/dev/devicelab/bin/tasks/native_ui_tests_ios32.dart index abfb566a0a..800632d9bc 100644 --- a/dev/devicelab/bin/tasks/native_ui_tests_ios32.dart +++ b/dev/devicelab/bin/tasks/native_ui_tests_ios32.dart @@ -74,20 +74,23 @@ Future main() async { ); if (testResultExit != 0) { - // Zip the test results to the artifacts directory for upload. - final String zipPath = path.join(hostAgent.dumpDirectory.path, - 'native_ui_tests_ios32-${DateTime.now().toLocal().toIso8601String()}.zip'); - await exec( - 'zip', - [ - '-r', - '-9', - zipPath, - 'result.xcresult', - ], - workingDirectory: resultBundleTemp, - canFail: true, // Best effort to get the logs. - ); + final Directory dumpDirectory = hostAgent.dumpDirectory; + if (dumpDirectory != null) { + // Zip the test results to the artifacts directory for upload. + final String zipPath = path.join(dumpDirectory.path, + 'native_ui_tests_ios32-${DateTime.now().toLocal().toIso8601String()}.zip'); + await exec( + 'zip', + [ + '-r', + '-9', + zipPath, + 'result.xcresult', + ], + workingDirectory: resultBundleTemp, + canFail: true, // Best effort to get the logs. + ); + } return TaskResult.failure('Platform unit tests failed'); } diff --git a/dev/devicelab/bin/tasks/routing_test.dart b/dev/devicelab/bin/tasks/routing_test.dart index d417e496d0..2b981a50e9 100644 --- a/dev/devicelab/bin/tasks/routing_test.dart +++ b/dev/devicelab/bin/tasks/routing_test.dart @@ -11,7 +11,6 @@ import 'dart:io'; import 'package:flutter_devicelab/common.dart'; import 'package:flutter_devicelab/framework/devices.dart'; import 'package:flutter_devicelab/framework/framework.dart'; -import 'package:flutter_devicelab/framework/host_agent.dart'; import 'package:flutter_devicelab/framework/task_result.dart'; import 'package:flutter_devicelab/framework/utils.dart'; import 'package:path/path.dart' as path; @@ -31,8 +30,6 @@ void main() { '--verbose', '-d', device.deviceId, - '--screenshot', - hostAgent.dumpDirectory.path, '--route', '/smuggle-it', 'lib/route.dart', diff --git a/dev/devicelab/lib/framework/devices.dart b/dev/devicelab/lib/framework/devices.dart index b83880b24a..4800311014 100644 --- a/dev/devicelab/lib/framework/devices.dart +++ b/dev/devicelab/lib/framework/devices.dart @@ -615,10 +615,11 @@ class AndroidDevice extends Device { @override Future stopLoggingToSink() async { - assert(_loggingProcess != null); - _abortedLogging = true; - _loggingProcess.kill(); - await _loggingProcess.exitCode; + if (_loggingProcess != null) { + _abortedLogging = true; + _loggingProcess.kill(); + await _loggingProcess.exitCode; + } } @override @@ -877,10 +878,11 @@ class IosDevice extends Device { @override Future stopLoggingToSink() async { - assert(_loggingProcess != null); - _abortedLogging = true; - _loggingProcess.kill(); - await _loggingProcess.exitCode; + if (_loggingProcess != null) { + _abortedLogging = true; + _loggingProcess.kill(); + await _loggingProcess.exitCode; + } } // The methods below are stubs for now. They will need to be expanded. diff --git a/dev/devicelab/lib/framework/framework.dart b/dev/devicelab/lib/framework/framework.dart index 2056f48aed..e73e373a5b 100644 --- a/dev/devicelab/lib/framework/framework.dart +++ b/dev/devicelab/lib/framework/framework.dart @@ -161,9 +161,8 @@ class _TaskRunner { result = await futureResult; } finally { if (device != null && device.canStreamLogs) { - assert(sink != null); await device.stopLoggingToSink(); - await sink.close(); + await sink?.close(); } } diff --git a/dev/devicelab/lib/framework/utils.dart b/dev/devicelab/lib/framework/utils.dart index 3354142bd3..aeb501a69d 100644 --- a/dev/devicelab/lib/framework/utils.dart +++ b/dev/devicelab/lib/framework/utils.dart @@ -16,6 +16,7 @@ import 'package:path/path.dart' as path; import 'package:process/process.dart'; import 'package:stack_trace/stack_trace.dart'; +import 'host_agent.dart'; import 'task_result.dart'; /// Virtual current working directory, which affect functions, such as [exec]. @@ -453,6 +454,11 @@ List flutterCommandArgs(String command, List options) { '--device-timeout', '5', ], + + if (command == 'drive' && hostAgent.dumpDirectory != null) ...[ + '--screenshot', + hostAgent.dumpDirectory.path, + ], if (localEngine != null) ...['--local-engine', localEngine], if (localEngineSrcPath != null) ...['--local-engine-src-path', localEngineSrcPath], ...options, diff --git a/dev/devicelab/lib/tasks/gallery.dart b/dev/devicelab/lib/tasks/gallery.dart index 5a64f07812..0be7bd45f3 100644 --- a/dev/devicelab/lib/tasks/gallery.dart +++ b/dev/devicelab/lib/tasks/gallery.dart @@ -10,7 +10,6 @@ import 'dart:math' as math; import '../framework/devices.dart'; import '../framework/framework.dart'; -import '../framework/host_agent.dart'; import '../framework/task_result.dart'; import '../framework/utils.dart'; @@ -104,8 +103,6 @@ class GalleryTransitionTest { 'test_driver/$testDriver.dart', '-d', deviceId, - '--screenshot', - hostAgent.dumpDirectory.path, ]); }); diff --git a/dev/devicelab/lib/tasks/integration_tests.dart b/dev/devicelab/lib/tasks/integration_tests.dart index c917524d1e..7408538923 100644 --- a/dev/devicelab/lib/tasks/integration_tests.dart +++ b/dev/devicelab/lib/tasks/integration_tests.dart @@ -6,7 +6,6 @@ import '../framework/devices.dart'; import '../framework/framework.dart'; -import '../framework/host_agent.dart'; import '../framework/task_result.dart'; import '../framework/utils.dart'; @@ -161,8 +160,6 @@ class DriverTest { testTarget, '-d', deviceId, - '--screenshot', - hostAgent.dumpDirectory.path, ...extraOptions, ]; await flutter('drive', options: options, environment: Map.from(environment)); diff --git a/dev/devicelab/lib/tasks/perf_tests.dart b/dev/devicelab/lib/tasks/perf_tests.dart index d1f4585ca0..e7235edbb1 100644 --- a/dev/devicelab/lib/tasks/perf_tests.dart +++ b/dev/devicelab/lib/tasks/perf_tests.dart @@ -314,8 +314,6 @@ TaskFunction createStackSizeTest() { '--driver', testDriver, '-d', deviceId, - '--screenshot', - hostAgent.dumpDirectory.path, ]); final Map data = json.decode( file('${_testOutputDirectory(testDirectory)}/stack_size.json').readAsStringSync(), @@ -411,8 +409,6 @@ TaskFunction createsScrollSmoothnessPerfTest() { '-t', testTarget, '-d', deviceId, - '--screenshot', - hostAgent.dumpDirectory.path, ]); final Map data = json.decode( file('${_testOutputDirectory(testDirectory)}/scroll_smoothness_test.json').readAsStringSync(), @@ -464,8 +460,6 @@ TaskFunction createFramePolicyIntegrationTest() { '-t', testTarget, '-d', deviceId, - '--screenshot', - hostAgent.dumpDirectory.path, ]); final Map data = json.decode( file('${_testOutputDirectory(testDirectory)}/frame_policy_event_delay.json').readAsStringSync(), @@ -585,16 +579,20 @@ class StartupTest { results.add(data); } else { currentFailures += 1; - await flutter( - 'screenshot', - options: [ - '-d', - device.deviceId, - '--out', - hostAgent.dumpDirectory.childFile('screenshot_startup_failure_$currentFailures.png').path, - ], - canFail: true, - ); + if (hostAgent.dumpDirectory != null) { + await flutter( + 'screenshot', + options: [ + '-d', + device.deviceId, + '--out', + hostAgent.dumpDirectory + .childFile('screenshot_startup_failure_$currentFailures.png') + .path, + ], + canFail: true, + ); + } i -= 1; if (currentFailures == maxFailures) { return TaskResult.failure('Application failed to start $maxFailures times'); @@ -830,8 +828,6 @@ class PerfTest { ...['--dart-define', dartDefine], '-d', deviceId, - '--screenshot', - hostAgent.dumpDirectory.path, ]); final Map data = json.decode( file('${_testOutputDirectory(testDirectory)}/$resultFilename.json').readAsStringSync(), @@ -1461,8 +1457,6 @@ class DevToolsMemoryTest { 'drive', options: [ '-d', _device.deviceId, - '--screenshot', - hostAgent.dumpDirectory.path, '--profile', '--profile-memory', _kJsonFileName, '--no-publish-port',