[flutter_tools] provide --timeout option to flutter drive (#114458)

Author: christopherfujino

dev/devicelab/bin/tasks/new_gallery__transition_perf.dart

@@ -17,7 +17,14 @@ Future<void> main() async {
   final Directory galleryDir = Directory(path.join(galleryParentDir.path, 'gallery'));
 
   try {
-    await task(NewGalleryPerfTest(galleryDir).run);
+    await task(
+      NewGalleryPerfTest(
+        galleryDir,
+        // time out after 20 minutes allowing the tool to take a screenshot to debug
+        // https://github.com/flutter/flutter/issues/114025.
+        timeoutSeconds: 20 * 60,
+      ).run,
+    );
   } finally {
     rmTree(galleryParentDir);
   }

dev/devicelab/lib/tasks/new_gallery.dart

@@ -15,6 +15,7 @@ class NewGalleryPerfTest extends PerfTest {
     String timelineFileName = 'transitions',
     String dartDefine = '',
     bool enableImpeller = false,
+    super.timeoutSeconds,
   }) : super(
     galleryDir.path,
     'test_driver/transitions_perf.dart',

dev/devicelab/lib/tasks/perf_tests.dart

@@ -914,6 +914,7 @@ class PerfTest {
     this.device,
     this.flutterDriveCallback,
     this.enableImpeller = false,
+    this.timeoutSeconds,
   }): _resultFilename = resultFilename;
 
   const PerfTest.e2e(
@@ -929,6 +930,7 @@ class PerfTest {
     this.device,
     this.flutterDriveCallback,
     this.enableImpeller = false,
+    this.timeoutSeconds,
   }) : saveTraceFile = false, timelineFileName = null, _resultFilename = resultFilename;
 
   /// The directory where the app under test is defined.
@@ -963,6 +965,9 @@ class PerfTest {
   /// Whether the perf test should enable Impeller.
   final bool enableImpeller;
 
+  /// Number of seconds to time out the test after, allowing debug callbacks to run.
+  final int? timeoutSeconds;
+
   /// The keys of the values that need to be reported.
   ///
   /// If it's `null`, then report:
@@ -1020,6 +1025,11 @@ class PerfTest {
         '-v',
         '--verbose-system-logs',
         '--profile',
+        if (timeoutSeconds != null)
+          ...<String>[
+            '--timeout',
+            timeoutSeconds.toString(),
+          ],
         if (needsFullTimeline)
           '--trace-startup', // Enables "endless" timeline event buffering.
         '-t', testTarget,
@@ -1039,7 +1049,7 @@ class PerfTest {
       if (flutterDriveCallback != null) {
         flutterDriveCallback!(options);
       } else {
-        await flutter('drive', options:options);
+        await flutter('drive', options: options);
       }
       final Map<String, dynamic> data = json.decode(
         file('${_testOutputDirectory(testDirectory)}/$resultFilename.json').readAsStringSync(),
@@ -1140,7 +1150,7 @@ class PerfTestWithSkSL extends PerfTest {
   );
 
   @override
-  Future<TaskResult> run() async {
+  Future<TaskResult> run({int? timeoutSeconds}) async {
     return inDirectory<TaskResult>(testDirectory, () async {
       // Some initializations
       _device = await devices.workingDevice;

packages/flutter_tools/lib/src/commands/drive.dart

@@ -150,7 +150,12 @@ class DriveCommand extends RunCommandBase {
           'Dart VM running The test script.')
       ..addOption('profile-memory', help: 'Launch devtools and profile application memory, writing '
           'The output data to the file path provided to this argument as JSON.',
-          valueHelp: 'profile_memory.json');
+          valueHelp: 'profile_memory.json')
+      ..addOption('timeout',
+        help: 'Timeout the test after the given number of seconds. If the '
+              '"--screenshot" option is provided, a screenshot will be taken '
+              'before exiting. Defaults to no timeout.',
+        valueHelp: '360');
   }
 
   final Signals signals;
@@ -173,6 +178,8 @@ class DriveCommand extends RunCommandBase {
   final FileSystem _fileSystem;
   final Logger _logger;
   final FileSystemUtils _fsUtils;
+  Timer? timeoutTimer;
+  Map<ProcessSignal, Object>? screenshotTokens;
 
   @override
   final String name = 'drive';
@@ -295,13 +302,17 @@ class DriveCommand extends RunCommandBase {
         androidEmulator: boolArgDeprecated('android-emulator'),
         profileMemory: stringArgDeprecated('profile-memory'),
       );
-      // If the test is sent a signal, take a screenshot before exiting
-      final Map<ProcessSignal, Object> screenshotTokens = _registerScreenshotCallbacks((ProcessSignal signal) async {
-        _logger.printError('Caught $signal');
-        await _takeScreenshot(device);
-      });
+
+      // If the test is sent a signal or times out, take a screenshot
+      _registerScreenshotCallbacks(device);
+
       final int testResult = await testResultFuture;
-      _unregisterScreenshotCallbacks(screenshotTokens);
+
+      if (timeoutTimer != null) {
+        timeoutTimer!.cancel();
+      }
+      _unregisterScreenshotCallbacks();
+
       if (testResult != 0 && screenshot != null) {
         // Take a screenshot while the app is still running.
         await _takeScreenshot(device);
@@ -331,21 +342,59 @@ class DriveCommand extends RunCommandBase {
     return FlutterCommandResult.success();
   }
 
-  Map<ProcessSignal, Object> _registerScreenshotCallbacks(Function(ProcessSignal) callback) {
+  int? get _timeoutSeconds {
+    final String? timeoutString = stringArg('timeout');
+    if (timeoutString == null) {
+      return null;
+    }
+    final int? timeoutSeconds = int.tryParse(timeoutString);
+    if (timeoutSeconds == null || timeoutSeconds <= 0) {
+      throwToolExit(
+        'Invalid value "$timeoutString" provided to the option --timeout: '
+        'expected a positive integer representing seconds.',
+      );
+    }
+    return timeoutSeconds;
+  }
+
+  void _registerScreenshotCallbacks(Device device) {
     _logger.printTrace('Registering signal handlers...');
     final Map<ProcessSignal, Object> tokens = <ProcessSignal, Object>{};
     for (final ProcessSignal signal in signalsToHandle) {
-      tokens[signal] = signals.addHandler(signal, callback);
+      tokens[signal] = signals.addHandler(
+        signal,
+        (ProcessSignal signal) {
+          _unregisterScreenshotCallbacks();
+          _logger.printError('Caught $signal');
+          return _takeScreenshot(device);
+        },
+      );
+    }
+    screenshotTokens = tokens;
+
+    final int? timeoutSeconds = _timeoutSeconds;
+    if (timeoutSeconds != null) {
+      timeoutTimer = Timer(
+        Duration(seconds: timeoutSeconds),
+        () {
+          _unregisterScreenshotCallbacks();
+          _takeScreenshot(device);
+          throwToolExit('Timed out after $timeoutSeconds seconds');
+        }
+      );
     }
-    return tokens;
   }
 
-  void _unregisterScreenshotCallbacks(Map<ProcessSignal, Object> tokens) {
-    _logger.printTrace('Unregistering signal handlers...');
-    for (final MapEntry<ProcessSignal, Object> entry in tokens.entries) {
-      signals.removeHandler(entry.key, entry.value);
+  void _unregisterScreenshotCallbacks() {
+    if (screenshotTokens != null) {
+      _logger.printTrace('Unregistering signal handlers...');
+      for (final MapEntry<ProcessSignal, Object> entry in screenshotTokens!.entries) {
+        signals.removeHandler(entry.key, entry.value);
+      }
     }
+    timeoutTimer?.cancel();
   }
+
   String? _getTestFile() {
     if (argResults!['driver'] != null) {
       return stringArgDeprecated('driver');

packages/flutter_tools/test/commands.shard/hermetic/drive_test.dart

@@ -5,8 +5,10 @@
 import 'dart:async';
 import 'dart:io' as io;
 
+import 'package:fake_async/fake_async.dart';
 import 'package:file/memory.dart';
 import 'package:flutter_tools/src/application_package.dart';
+import 'package:flutter_tools/src/base/async_guard.dart';
 import 'package:flutter_tools/src/base/common.dart';
 import 'package:flutter_tools/src/base/file_system.dart';
 import 'package:flutter_tools/src/base/io.dart';
@@ -203,6 +205,72 @@ void main() {
     DeviceManager: () => fakeDeviceManager,
   });
 
+  testUsingContext('drive --timeout takes screenshot and tool exits after timeout', () async {
+    final DriveCommand command = DriveCommand(
+      fileSystem: fileSystem,
+      logger: logger,
+      platform: platform,
+      signals: Signals.test(),
+      flutterDriverFactory: NeverEndingFlutterDriverFactory(() {}),
+    );
+
+    fileSystem.file('lib/main.dart').createSync(recursive: true);
+    fileSystem.file('test_driver/main_test.dart').createSync(recursive: true);
+    fileSystem.file('pubspec.yaml').createSync();
+    fileSystem.directory('drive_screenshots').createSync();
+
+    final ScreenshotDevice screenshotDevice = ScreenshotDevice();
+    fakeDeviceManager.devices = <Device>[screenshotDevice];
+
+    expect(screenshotDevice.screenshots, isEmpty);
+    bool caughtToolExit = false;
+    FakeAsync().run<void>((FakeAsync time) {
+      // Because the tool exit will be thrown asynchronously by a [Timer],
+      // use [asyncGuard] to catch it
+      asyncGuard<void>(
+        () => createTestCommandRunner(command).run(
+          <String>[
+            'drive',
+            '--no-pub',
+            '-d',
+            screenshotDevice.id,
+            '--use-existing-app',
+            'http://localhost:8181',
+            '--screenshot',
+            'drive_screenshots',
+            '--timeout',
+            '300', // 5 minutes
+          ],
+        ),
+        onError: (Object error) {
+          expect(error, isA<ToolExit>());
+          expect(
+            (error as ToolExit).message,
+            contains('Timed out after 300 seconds'),
+          );
+          caughtToolExit = true;
+        }
+      );
+      time.elapse(const Duration(seconds: 299));
+      expect(screenshotDevice.screenshots, isEmpty);
+      time.elapse(const Duration(seconds: 2));
+      expect(
+        screenshotDevice.screenshots,
+        contains(isA<File>().having(
+          (File file) => file.path,
+          'path',
+          'drive_screenshots/drive_01.png',
+        )),
+      );
+    });
+    expect(caughtToolExit, isTrue);
+  }, overrides: <Type, Generator>{
+    FileSystem: () => fileSystem,
+    ProcessManager: () => FakeProcessManager.any(),
+    Pub: () => FakePub(),
+    DeviceManager: () => fakeDeviceManager,
+  });
+
   testUsingContext('drive --screenshot takes screenshot if sent a registered signal', () async {
     final FakeProcessSignal signal = FakeProcessSignal();
     final ProcessSignal signalUnderTest = ProcessSignal(signal);