Take screenshot when drive fails to start app or test (#96828)

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

@@ -237,68 +237,74 @@ class DriveCommand extends RunCommandBase {
       ? null
       : _fileSystem.file(stringArg('use-application-binary'));
 
-    if (stringArg('use-existing-app') == null) {
+    try {
-      await driverService.start(
+      if (stringArg('use-existing-app') == null) {
-        buildInfo,
+        await driverService.start(
-        device,
+          buildInfo,
-        debuggingOptions,
+          device,
-        ipv6,
+          debuggingOptions,
-        applicationBinary: applicationBinary,
+          ipv6,
-        route: route,
+          applicationBinary: applicationBinary,
-        userIdentifier: userIdentifier,
+          route: route,
-        mainPath: targetFile,
+          userIdentifier: userIdentifier,
-        platformArgs: <String, Object>{
+          mainPath: targetFile,
-          if (traceStartup)
+          platformArgs: <String, Object>{
-            'trace-startup': traceStartup,
+            if (traceStartup)
-          if (web)
+              'trace-startup': traceStartup,
-            '--no-launch-chrome': true,
+            if (web)
-          if (boolArg('multidex'))
+              '--no-launch-chrome': true,
-            'multidex': true,
+            if (boolArg('multidex'))
+              'multidex': true,
+          }
+        );
+      } else {
+        final Uri uri = Uri.tryParse(stringArg('use-existing-app'));
+        if (uri == null) {
+          throwToolExit('Invalid VM Service URI: ${stringArg('use-existing-app')}');
         }
-      );
+        await driverService.reuseApplication(
-    } else {
+          uri,
-      final Uri uri = Uri.tryParse(stringArg('use-existing-app'));
+          device,
-      if (uri == null) {
+          debuggingOptions,
-        throwToolExit('Invalid VM Service URI: ${stringArg('use-existing-app')}');
+          ipv6,
+        );
       }
-      await driverService.reuseApplication(
+
-        uri,
+      final int testResult = await driverService.startTest(
-        device,
+        testFile,
-        debuggingOptions,
+        stringsArg('test-arguments'),
-        ipv6,
+        <String, String>{},
+        packageConfig,
+        chromeBinary: stringArg('chrome-binary'),
+        headless: boolArg('headless'),
+        browserDimension: stringArg('browser-dimension').split(','),
+        browserName: stringArg('browser-name'),
+        driverPort: stringArg('driver-port') != null
+          ? int.tryParse(stringArg('driver-port'))
+          : null,
+        androidEmulator: boolArg('android-emulator'),
+        profileMemory: stringArg('profile-memory'),
       );
+
+      if (boolArg('keep-app-running') ?? (argResults['use-existing-app'] != null)) {
+        _logger.printStatus('Leaving the application running.');
+      } else {
+        final File skslFile = stringArg('write-sksl-on-exit') != null
+          ? _fileSystem.file(stringArg('write-sksl-on-exit'))
+          : null;
+        await driverService.stop(userIdentifier: userIdentifier, writeSkslOnExit: skslFile);
+      }
+      if (testResult != 0) {
+        throwToolExit(null);
+      }
+    } on Exception catch(_) {
+      // On exceptions, including ToolExit, take a screenshot on the device.
+      if (screenshot != null) {
+        await _takeScreenshot(device);
+      }
+      rethrow;
     }
 
-    final int testResult = await driverService.startTest(
-      testFile,
-      stringsArg('test-arguments'),
-      <String, String>{},
-      packageConfig,
-      chromeBinary: stringArg('chrome-binary'),
-      headless: boolArg('headless'),
-      browserDimension: stringArg('browser-dimension').split(','),
-      browserName: stringArg('browser-name'),
-      driverPort: stringArg('driver-port') != null
-        ? int.tryParse(stringArg('driver-port'))
-        : null,
-      androidEmulator: boolArg('android-emulator'),
-      profileMemory: stringArg('profile-memory'),
-    );
-    if (testResult != 0 && screenshot != null) {
-      await takeScreenshot(device, screenshot, _fileSystem, _logger, _fsUtils);
-    }
-
-    if (boolArg('keep-app-running') ?? (argResults['use-existing-app'] != null)) {
-      _logger.printStatus('Leaving the application running.');
-    } else {
-      final File skslFile = stringArg('write-sksl-on-exit') != null
-        ? _fileSystem.file(stringArg('write-sksl-on-exit'))
-        : null;
-      await driverService.stop(userIdentifier: userIdentifier, writeSkslOnExit: skslFile);
-    }
-    if (testResult != 0) {
-      throwToolExit(null);
-    }
     return FlutterCommandResult.success();
   }
 
@@ -344,27 +350,20 @@ class DriveCommand extends RunCommandBase {
       <String>[packageDir, 'test_driver', ...parts.skip(1)]));
     return '${pathWithNoExtension}_test${_fileSystem.path.extension(appFile)}';
   }
-}
 
-@visibleForTesting
+  Future<void> _takeScreenshot(Device device) async {
-Future<void> takeScreenshot(
+    try {
-  Device device,
+      final Directory outputDirectory = _fileSystem.directory(screenshot)
-  String screenshotPath,
+        ..createSync(recursive: true);
-  FileSystem fileSystem,
+      final File outputFile = _fsUtils.getUniqueFile(
-  Logger logger,
+        outputDirectory,
-  FileSystemUtils fileSystemUtils,
+        'drive',
-) async {
+        'png',
-  try {
+      );
-    final Directory outputDirectory = fileSystem.directory(screenshotPath);
+      await device.takeScreenshot(outputFile);
-    outputDirectory.createSync(recursive: true);
+      _logger.printStatus('Screenshot written to ${outputFile.path}');
-    final File outputFile = fileSystemUtils.getUniqueFile(
+    } on Exception catch (error) {
-      outputDirectory,
+      _logger.printError('Error taking screenshot: $error');
-      'drive',
+    }
-      'png',
-    );
-    await device.takeScreenshot(outputFile);
-    logger.printStatus('Screenshot written to ${outputFile.path}');
-  } on Exception catch (error) {
-    logger.printError('Error taking screenshot: $error');
   }
 }

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

@@ -5,13 +5,19 @@
 // @dart = 2.8
 
 import 'package:file/memory.dart';
+import 'package:flutter_tools/src/application_package.dart';
+import 'package:flutter_tools/src/base/common.dart';
 import 'package:flutter_tools/src/base/file_system.dart';
 import 'package:flutter_tools/src/base/logger.dart';
 import 'package:flutter_tools/src/base/platform.dart';
+import 'package:flutter_tools/src/build_info.dart';
 import 'package:flutter_tools/src/cache.dart';
 import 'package:flutter_tools/src/commands/drive.dart';
 import 'package:flutter_tools/src/dart/pub.dart';
 import 'package:flutter_tools/src/device.dart';
+import 'package:flutter_tools/src/drive/drive_service.dart';
+import 'package:flutter_tools/src/project.dart';
+import 'package:package_config/package_config.dart';
 import 'package:test/fake.dart';
 
 import '../../src/common.dart';
@@ -22,11 +28,13 @@ void main() {
   FileSystem fileSystem;
   BufferLogger logger;
   Platform platform;
+  FakeDeviceManager fakeDeviceManager;
 
   setUp(() {
     fileSystem = MemoryFileSystem.test();
     logger = BufferLogger.test();
     platform = FakePlatform();
+    fakeDeviceManager = FakeDeviceManager();
   });
 
   setUpAll(() {
@@ -37,41 +45,106 @@ void main() {
     Cache.enableLocking();
   });
 
+  testUsingContext('takes screenshot and rethrows on drive exception', () async {
+    final DriveCommand command = DriveCommand(fileSystem: fileSystem, logger: logger, platform: platform);
+    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();
 
-  testWithoutContext('drive --screenshot writes to expected output', () async {
+    final Device screenshotDevice = ThrowingScreenshotDevice();
-    final Device screenshotDevice = ScreenshotDevice();
+    fakeDeviceManager.devices = <Device>[screenshotDevice];
 
-    await takeScreenshot(
+    await expectLater(() => createTestCommandRunner(command).run(
-      screenshotDevice,
+      <String>[
-      'drive_screenshots',
+        'drive',
-      fileSystem,
+        '--no-pub',
-      logger,
+        '-d',
-      FileSystemUtils(
+        screenshotDevice.id,
-        fileSystem: fileSystem,
+        '--screenshot',
-        platform: platform,
+        'drive_screenshots',
-      ),
+      ]),
+      throwsToolExit(message: 'cannot start app'),
     );
 
     expect(logger.statusText, contains('Screenshot written to drive_screenshots/drive_01.png'));
+    expect(logger.statusText, isNot(contains('drive_02.png')));
+  }, overrides: <Type, Generator>{
+    FileSystem: () => fileSystem,
+    ProcessManager: () => FakeProcessManager.any(),
+    Pub: () => FakePub(),
+    DeviceManager: () => fakeDeviceManager,
   });
 
-  testWithoutContext('drive --screenshot errors but does not fail if screenshot fails', () async {
+  testUsingContext('takes screenshot on drive test failure', () async {
+    final DriveCommand command = DriveCommand(
+      fileSystem: fileSystem,
+      logger: logger,
+      platform: platform,
+      flutterDriverFactory: FailingFakeFlutterDriverFactory(),
+    );
+
+    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 Device screenshotDevice = ScreenshotDevice();
+    fakeDeviceManager.devices = <Device>[screenshotDevice];
+
+    await expectLater(() => createTestCommandRunner(command).run(
+      <String>[
+        'drive',
+        '--no-pub',
+        '-d',
+        screenshotDevice.id,
+        '--use-existing-app',
+        'http://localhost:8181',
+        '--keep-app-running',
+        '--screenshot',
+        'drive_screenshots',
+      ]),
+      throwsToolExit(),
+    );
+
+    expect(logger.statusText, contains('Screenshot written to drive_screenshots/drive_01.png'));
+    expect(logger.statusText, isNot(contains('drive_02.png')));
+  }, overrides: <Type, Generator>{
+    FileSystem: () => fileSystem,
+    ProcessManager: () => FakeProcessManager.any(),
+    Pub: () => FakePub(),
+    DeviceManager: () => fakeDeviceManager,
+  });
+
+  testUsingContext('drive --screenshot errors but does not fail if screenshot fails', () async {
+    final DriveCommand command = DriveCommand(fileSystem: fileSystem, logger: logger, platform: platform);
+    fileSystem.file('lib/main.dart').createSync(recursive: true);
+    fileSystem.file('test_driver/main_test.dart').createSync(recursive: true);
+    fileSystem.file('pubspec.yaml').createSync();
     fileSystem.file('drive_screenshots').createSync();
 
-    await takeScreenshot(
+    final Device screenshotDevice = ThrowingScreenshotDevice();
-      screenshotDevice,
+    fakeDeviceManager.devices = <Device>[screenshotDevice];
-      'drive_screenshots',
+
-      fileSystem,
+    await expectLater(() => createTestCommandRunner(command).run(
-      logger,
+      <String>[
-      FileSystemUtils(
+        'drive',
-        fileSystem: fileSystem,
+        '--no-pub',
-        platform: platform,
+        '-d',
-      ),
+        screenshotDevice.id,
+        '--screenshot',
+        'drive_screenshots',
+      ]),
+      throwsToolExit(message: 'cannot start app'),
     );
 
     expect(logger.statusText, isEmpty);
     expect(logger.errorText, contains('Error taking screenshot: FileSystemException: Not a directory'));
+  }, overrides: <Type, Generator>{
+    FileSystem: () => fileSystem,
+    ProcessManager: () => FakeProcessManager.any(),
+    Pub: () => FakePub(),
+    DeviceManager: () => fakeDeviceManager,
   });
 
   testUsingContext('shouldRunPub is true unless user specifies --no-pub', () async {
@@ -102,10 +175,58 @@ void main() {
   });
 }
 
+// Unfortunately Device, despite not being immutable, has an `operator ==`.
+// Until we fix that, we have to also ignore related lints here.
+// ignore: avoid_implementing_value_types
+class ThrowingScreenshotDevice extends ScreenshotDevice {
+  @override
+  Future<LaunchResult> startApp(
+    ApplicationPackage package, {
+      String mainPath,
+      String route,
+      DebuggingOptions debuggingOptions,
+      Map<String, dynamic> platformArgs,
+      bool prebuiltApplication = false,
+      bool usesTerminalUi = true,
+      bool ipv6 = false,
+      String userIdentifier,
+    }) async {
+    throwToolExit('cannot start app');
+  }
+}
+
 // Unfortunately Device, despite not being immutable, has an `operator ==`.
 // Until we fix that, we have to also ignore related lints here.
 // ignore: avoid_implementing_value_types
 class ScreenshotDevice extends Fake implements Device {
+  @override
+  final String name = 'FakeDevice';
+
+  @override
+  final Category category = Category.mobile;
+
+  @override
+  final String id = 'fake_device';
+
+  @override
+  Future<TargetPlatform> get targetPlatform async => TargetPlatform.android;
+
+  @override
+  final bool supportsScreenshot = true;
+
+  @override
+  Future<LaunchResult> startApp(
+    ApplicationPackage package, {
+      String mainPath,
+      String route,
+      DebuggingOptions debuggingOptions,
+      Map<String, dynamic> platformArgs,
+      bool prebuiltApplication = false,
+      bool usesTerminalUi = true,
+      bool ipv6 = false,
+      String userIdentifier,
+    }) async => LaunchResult.succeeded();
+
   @override
   Future<void> takeScreenshot(File outputFile) async {}
 }
@@ -125,3 +246,41 @@ class FakePub extends Fake implements Pub {
     bool printProgress = true,
   }) async { }
 }
+
+class FakeDeviceManager extends Fake implements DeviceManager {
+  List<Device> devices = <Device>[];
+
+  @override
+  String specifiedDeviceId;
+
+  @override
+  Future<List<Device>> getDevices() async => devices;
+
+  @override
+  Future<List<Device>> findTargetDevices(FlutterProject flutterProject, {Duration timeout}) async => devices;
+}
+
+class FailingFakeFlutterDriverFactory extends Fake implements FlutterDriverFactory {
+  @override
+  DriverService createDriverService(bool web) => FailingFakeDriverService();
+}
+
+class FailingFakeDriverService extends Fake implements DriverService {
+  @override
+  Future<void> reuseApplication(Uri vmServiceUri, Device device, DebuggingOptions debuggingOptions, bool ipv6) async { }
+
+  @override
+  Future<int> startTest(
+    String testFile,
+    List<String> arguments,
+    Map<String, String> environment,
+    PackageConfig packageConfig, {
+      bool headless,
+      String chromeBinary,
+      String browserName,
+      bool androidEmulator,
+      int driverPort,
+      List<String> browserDimension,
+      String profileMemory,
+    }) async => 1;
+}