Take screenshot when drive fails (#78822)

dev/devicelab/bin/tasks/complex_layout_semantics_perf.dart

@@ -4,6 +4,7 @@
 
 import 'package:flutter_devicelab/framework/adb.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;
@@ -29,6 +30,8 @@ void main() {
         p.join(complexLayoutPath, 'test_driver', 'semantics_perf.dart'),
         '-d',
         deviceId,
+        '--screenshot',
+        hostAgent.dumpDirectory.path,
       ]);
     });
 

dev/devicelab/bin/tasks/drive_perf_debug_warning.dart

@@ -4,6 +4,7 @@
 
 import 'package:flutter_devicelab/framework/adb.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';
 
@@ -15,6 +16,8 @@ Future<String> _runWithMode(String mode, String deviceId) async {
     'test_driver/scroll_perf.dart',
     '-d',
     deviceId,
+    '--screenshot',
+    hostAgent.dumpDirectory.path,
   ]);
   return stderr.toString();
 }

dev/devicelab/bin/tasks/routing_test.dart

@@ -10,6 +10,7 @@ import 'package:path/path.dart' as path;
 
 import 'package:flutter_devicelab/framework/adb.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';
 
@@ -24,7 +25,16 @@ void main() {
     await inDirectory(appDir, () async {
       return flutter(
         'drive',
-        options: <String>['--verbose', '-d', device.deviceId, '--route', '/smuggle-it', 'lib/route.dart'],
+        options: <String>[
+          '--verbose',
+          '-d',
+          device.deviceId,
+          '--screenshot',
+          hostAgent.dumpDirectory.path,
+          '--route',
+          '/smuggle-it',
+          'lib/route.dart',
+        ],
         canFail: false,
       );
     });

dev/devicelab/lib/tasks/gallery.dart

@@ -6,9 +6,9 @@ import 'dart:convert';
 import 'dart:io';
 import 'dart:math' as math;
 
-
 import '../framework/adb.dart';
 import '../framework/framework.dart';
+import '../framework/host_agent.dart';
 import '../framework/task_result.dart';
 import '../framework/utils.dart';
 
@@ -101,6 +101,8 @@ class GalleryTransitionTest {
         'test_driver/$testDriver.dart',
         '-d',
         deviceId,
+        '--screenshot',
+        hostAgent.dumpDirectory.path,
       ]);
     });
 

dev/devicelab/lib/tasks/integration_tests.dart

@@ -4,6 +4,7 @@
 
 import '../framework/adb.dart';
 import '../framework/framework.dart';
+import '../framework/host_agent.dart';
 import '../framework/task_result.dart';
 import '../framework/utils.dart';
 
@@ -151,6 +152,8 @@ class DriverTest {
         testTarget,
         '-d',
         deviceId,
+        '--screenshot',
+        hostAgent.dumpDirectory.path,
         ...extraOptions,
       ];
       await flutter('drive', options: options, environment: Map<String, String>.from(environment));

dev/devicelab/lib/tasks/perf_tests.dart

@@ -12,6 +12,7 @@ import 'package:path/path.dart' as path;
 
 import 'package:flutter_devicelab/framework/adb.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';
 
@@ -292,6 +293,8 @@ TaskFunction createStackSizeTest() {
         '--driver', testDriver,
         '-d',
         deviceId,
+        '--screenshot',
+        hostAgent.dumpDirectory.path,
       ]);
       final Map<String, dynamic> data = json.decode(
         file('$testDirectory/build/stack_size.json').readAsStringSync(),
@@ -387,6 +390,8 @@ TaskFunction createsScrollSmoothnessPerfTest() {
         '-t', testTarget,
         '-d',
         deviceId,
+        '--screenshot',
+        hostAgent.dumpDirectory.path,
       ]);
       final Map<String, dynamic> data = json.decode(
         file('$testDirectory/build/scroll_smoothness_test.json').readAsStringSync(),
@@ -436,6 +441,8 @@ TaskFunction createFramePolicyIntegrationTest() {
         '-t', testTarget,
         '-d',
         deviceId,
+        '--screenshot',
+        hostAgent.dumpDirectory.path,
       ]);
       final Map<String, dynamic> data = json.decode(
         file('$testDirectory/build/frame_policy_event_delay.json').readAsStringSync(),
@@ -689,6 +696,8 @@ class PerfTest {
           ...<String>['--dart-define', dartDefine],
         '-d',
         deviceId,
+        '--screenshot',
+        hostAgent.dumpDirectory.path,
       ]);
       final Map<String, dynamic> data = json.decode(
         file('$testDirectory/build/$resultFilename.json').readAsStringSync(),
@@ -1334,6 +1343,8 @@ class DevToolsMemoryTest {
         options: <String>[
           '--use-existing-app', _observatoryUri,
           '-d', _device.deviceId,
+          '--screenshot',
+          hostAgent.dumpDirectory.path,
           '--profile',
           driverTest,
         ],

packages/flutter_tools/lib/executable.dart

@@ -157,6 +157,7 @@ List<FlutterCommand> generateCommands({
   DriveCommand(verboseHelp: verboseHelp,
     fileSystem: globals.fs,
     logger: globals.logger,
+    platform: globals.platform,
   ),
   EmulatorsCommand(),
   FormatCommand(),

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

@@ -15,6 +15,7 @@ import '../artifacts.dart';
 import '../base/common.dart';
 import '../base/file_system.dart';
 import '../base/logger.dart';
+import '../base/platform.dart';
 import '../build_info.dart';
 import '../dart/package_map.dart';
 import '../device.dart';
@@ -50,9 +51,11 @@ class DriveCommand extends RunCommandBase {
     @visibleForTesting FlutterDriverFactory flutterDriverFactory,
     @required FileSystem fileSystem,
     @required Logger logger,
+    @required Platform platform,
   }) : _flutterDriverFactory = flutterDriverFactory,
        _fileSystem = fileSystem,
        _logger = logger,
+       _fsUtils = FileSystemUtils(fileSystem: fileSystem, platform: platform),
        super(verboseHelp: verboseHelp) {
     requiresPubspecYaml();
     addEnableExperimentation(hide: !verboseHelp);
@@ -90,6 +93,10 @@ class DriveCommand extends RunCommandBase {
         help: '(deprecated) Build the app before running. To use an existing app, pass the "--use-application-binary" '
               'flag with an existing APK.',
       )
+      ..addOption('screenshot',
+        valueHelp: 'path/to/directory',
+        help: 'Directory location to write screenshots on test failure.',
+      )
       ..addOption('driver-port',
         defaultsTo: '4444',
         help: 'The port where Webdriver server is launched at.',
@@ -147,6 +154,7 @@ class DriveCommand extends RunCommandBase {
   FlutterDriverFactory _flutterDriverFactory;
   final FileSystem _fileSystem;
   final Logger _logger;
+  final FileSystemUtils _fsUtils;
 
   @override
   final String name = 'drive';
@@ -159,6 +167,8 @@ class DriveCommand extends RunCommandBase {
 
   String get userIdentifier => stringArg(FlutterOptions.kDeviceUser);
 
+  String get screenshot => stringArg('screenshot');
+
   @override
   bool get startPausedDefault => true;
 
@@ -189,6 +199,9 @@ class DriveCommand extends RunCommandBase {
     if (device == null) {
       throwToolExit(null);
     }
+    if (screenshot != null && !device.supportsScreenshot) {
+      throwToolExit('Screenshot not supported for ${device.name}.');
+    }
 
     final bool web = device is WebServerDevice || device is ChromiumDevice;
     _flutterDriverFactory ??= FlutterDriverFactory(
@@ -198,7 +211,7 @@ class DriveCommand extends RunCommandBase {
       dartSdkPath: globals.artifacts.getArtifactPath(Artifact.engineDartBinary),
     );
     final PackageConfig packageConfig = await loadPackageConfigWithLogging(
-      globals.fs.file('.packages'),
+      _fileSystem.file('.packages'),
       logger: _logger,
       throwOnError: false,
     ) ?? PackageConfig.empty;
@@ -253,6 +266,9 @@ class DriveCommand extends RunCommandBase {
         : null,
       androidEmulator: boolArg('android-emulator'),
     );
+    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.');
@@ -311,3 +327,26 @@ class DriveCommand extends RunCommandBase {
     return '${pathWithNoExtension}_test${_fileSystem.path.extension(appFile)}';
   }
 }
+
+@visibleForTesting
+Future<void> takeScreenshot(
+  Device device,
+  String screenshotPath,
+  FileSystem fileSystem,
+  Logger logger,
+  FileSystemUtils fileSystemUtils,
+) async {
+  try {
+    final Directory outputDirectory = fileSystem.directory(screenshotPath);
+    outputDirectory.createSync(recursive: true);
+    final File outputFile = fileSystemUtils.getUniqueFile(
+      outputDirectory,
+      '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

@@ -0,0 +1,68 @@
+// Copyright 2014 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// @dart = 2.8
+
+import 'package:file/memory.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/commands/drive.dart';
+import 'package:flutter_tools/src/device.dart';
+import 'package:test/fake.dart';
+
+import '../../src/common.dart';
+
+void main() {
+  FileSystem fileSystem;
+  BufferLogger logger;
+  Platform platform;
+
+  setUp(() {
+    fileSystem = MemoryFileSystem.test();
+    logger = BufferLogger.test();
+    platform = FakePlatform(operatingSystem: 'linux');
+  });
+
+  testWithoutContext('drive --screenshot writes to expected output', () async {
+    final Device screenshotDevice = ScreenshotDevice();
+
+    await takeScreenshot(
+      screenshotDevice,
+      'drive_screenshots',
+      fileSystem,
+      logger,
+      FileSystemUtils(
+        fileSystem: fileSystem,
+        platform: platform,
+      ),
+    );
+
+    expect(logger.statusText, contains('Screenshot written to drive_screenshots/drive_01.png'));
+  });
+
+  testWithoutContext('drive --screenshot errors but does not fail if screenshot fails', () async {
+    final Device screenshotDevice = ScreenshotDevice();
+    fileSystem.file('drive_screenshots').createSync();
+
+    await takeScreenshot(
+      screenshotDevice,
+      'drive_screenshots',
+      fileSystem,
+      logger,
+      FileSystemUtils(
+        fileSystem: fileSystem,
+        platform: platform,
+      ),
+    );
+
+    expect(logger.statusText, isEmpty);
+    expect(logger.errorText, contains('Error taking screenshot: FileSystemException: Not a directory'));
+  });
+}
+
+class ScreenshotDevice extends Fake implements Device {
+  @override
+  Future<void> takeScreenshot(File outputFile) async {}
+}