diff --git a/packages/flutter_tools/lib/src/commands/drive.dart b/packages/flutter_tools/lib/src/commands/drive.dart index 4371f246c9..f60956669a 100644 --- a/packages/flutter_tools/lib/src/commands/drive.dart +++ b/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) { - await driverService.start( - buildInfo, - device, - debuggingOptions, - ipv6, - applicationBinary: applicationBinary, - route: route, - userIdentifier: userIdentifier, - mainPath: targetFile, - platformArgs: { - if (traceStartup) - 'trace-startup': traceStartup, - if (web) - '--no-launch-chrome': true, - if (boolArg('multidex')) - 'multidex': true, + try { + if (stringArg('use-existing-app') == null) { + await driverService.start( + buildInfo, + device, + debuggingOptions, + ipv6, + applicationBinary: applicationBinary, + route: route, + userIdentifier: userIdentifier, + mainPath: targetFile, + platformArgs: { + if (traceStartup) + 'trace-startup': traceStartup, + if (web) + '--no-launch-chrome': 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')}'); } - ); - } 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( + uri, + device, + debuggingOptions, + ipv6, + ); } - await driverService.reuseApplication( - uri, - device, - debuggingOptions, - ipv6, + + final int testResult = await driverService.startTest( + testFile, + stringsArg('test-arguments'), + {}, + 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'), - {}, - 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 { [packageDir, 'test_driver', ...parts.skip(1)])); return '${pathWithNoExtension}_test${_fileSystem.path.extension(appFile)}'; } -} -@visibleForTesting -Future 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'); + Future _takeScreenshot(Device device) async { + try { + final Directory outputDirectory = _fileSystem.directory(screenshot) + ..createSync(recursive: true); + final File outputFile = _fsUtils.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'); + } } } diff --git a/packages/flutter_tools/test/commands.shard/hermetic/drive_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/drive_test.dart index 7688ca80d5..d55b56ca88 100644 --- a/packages/flutter_tools/test/commands.shard/hermetic/drive_test.dart +++ b/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 = ScreenshotDevice(); + final Device screenshotDevice = ThrowingScreenshotDevice(); + fakeDeviceManager.devices = [screenshotDevice]; - await takeScreenshot( - screenshotDevice, - 'drive_screenshots', - fileSystem, - logger, - FileSystemUtils( - fileSystem: fileSystem, - platform: platform, - ), + await expectLater(() => createTestCommandRunner(command).run( + [ + 'drive', + '--no-pub', + '-d', + screenshotDevice.id, + '--screenshot', + '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: { + 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 = [screenshotDevice]; + + await expectLater(() => createTestCommandRunner(command).run( + [ + '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: { + 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( - screenshotDevice, - 'drive_screenshots', - fileSystem, - logger, - FileSystemUtils( - fileSystem: fileSystem, - platform: platform, - ), + final Device screenshotDevice = ThrowingScreenshotDevice(); + fakeDeviceManager.devices = [screenshotDevice]; + + await expectLater(() => createTestCommandRunner(command).run( + [ + 'drive', + '--no-pub', + '-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: { + 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 startApp( + ApplicationPackage package, { + String mainPath, + String route, + DebuggingOptions debuggingOptions, + Map 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 get targetPlatform async => TargetPlatform.android; + + @override + final bool supportsScreenshot = true; + + @override + Future startApp( + ApplicationPackage package, { + String mainPath, + String route, + DebuggingOptions debuggingOptions, + Map platformArgs, + bool prebuiltApplication = false, + bool usesTerminalUi = true, + bool ipv6 = false, + String userIdentifier, + }) async => LaunchResult.succeeded(); + @override Future 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 devices = []; + + @override + String specifiedDeviceId; + + @override + Future> getDevices() async => devices; + + @override + Future> 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 reuseApplication(Uri vmServiceUri, Device device, DebuggingOptions debuggingOptions, bool ipv6) async { } + + @override + Future startTest( + String testFile, + List arguments, + Map environment, + PackageConfig packageConfig, { + bool headless, + String chromeBinary, + String browserName, + bool androidEmulator, + int driverPort, + List browserDimension, + String profileMemory, + }) async => 1; +}