diff --git a/packages/flutter_tools/lib/executable.dart b/packages/flutter_tools/lib/executable.dart index a26aeb52317..9b852ef3a87 100644 --- a/packages/flutter_tools/lib/executable.dart +++ b/packages/flutter_tools/lib/executable.dart @@ -194,7 +194,7 @@ List generateCommands({ featureFlags: featureFlags, ), RunCommand(verboseHelp: verboseHelp), - ScreenshotCommand(), + ScreenshotCommand(fs: globals.fs), ShellCompletionCommand(), TestCommand(verboseHelp: verboseHelp, verbose: verbose), UpgradeCommand(verboseHelp: verboseHelp), diff --git a/packages/flutter_tools/lib/src/commands/screenshot.dart b/packages/flutter_tools/lib/src/commands/screenshot.dart index 9f7caae2176..165345319c2 100644 --- a/packages/flutter_tools/lib/src/commands/screenshot.dart +++ b/packages/flutter_tools/lib/src/commands/screenshot.dart @@ -20,7 +20,7 @@ const String _kSkiaType = 'skia'; const String _kRasterizerType = 'rasterizer'; class ScreenshotCommand extends FlutterCommand { - ScreenshotCommand() { + ScreenshotCommand({required this.fs}) { argParser.addOption( _kOut, abbr: 'o', @@ -53,6 +53,8 @@ class ScreenshotCommand extends FlutterCommand { usesDeviceTimeoutOption(); } + final FileSystem fs; + @override String get name => 'screenshot'; @@ -101,7 +103,7 @@ class ScreenshotCommand extends FlutterCommand { Future runCommand() async { File? outputFile; if (argResults?.wasParsed(_kOut) ?? false) { - outputFile = globals.fs.file(stringArgDeprecated(_kOut)); + outputFile = fs.file(stringArgDeprecated(_kOut)); } bool success = true; @@ -123,16 +125,27 @@ class ScreenshotCommand extends FlutterCommand { Future runScreenshot(File? outputFile) async { outputFile ??= globals.fsUtils.getUniqueFile( - globals.fs.currentDirectory, + fs.currentDirectory, 'flutter', 'png', ); + try { await device!.takeScreenshot(outputFile); } on Exception catch (error) { throwToolExit('Error taking screenshot: $error'); } - _showOutputFileInfo(outputFile); + + checkOutput(outputFile, fs); + + try { + _showOutputFileInfo(outputFile); + } on Exception catch (error) { + throwToolExit( + 'Error with provided file path: "${outputFile.path}"\n' + 'Error: $error' + ); + } } Future runSkia(File? outputFile) async { @@ -147,7 +160,7 @@ class ScreenshotCommand extends FlutterCommand { return false; } outputFile ??= globals.fsUtils.getUniqueFile( - globals.fs.currentDirectory, + fs.currentDirectory, 'flutter', 'skp', ); @@ -171,7 +184,7 @@ class ScreenshotCommand extends FlutterCommand { return false; } outputFile ??= globals.fsUtils.getUniqueFile( - globals.fs.currentDirectory, + fs.currentDirectory, 'flutter', 'png', ); @@ -183,6 +196,15 @@ class ScreenshotCommand extends FlutterCommand { return true; } + static void checkOutput(File outputFile, FileSystem fs) { + if (!fs.file(outputFile.path).existsSync()) { + throwToolExit( + 'File was not created, ensure path is valid\n' + 'Path provided: "${outputFile.path}"' + ); + } + } + void _ensureOutputIsNotJsonRpcError(File outputFile) { if (outputFile.lengthSync() >= 1000) { return; @@ -197,6 +219,6 @@ class ScreenshotCommand extends FlutterCommand { void _showOutputFileInfo(File outputFile) { final int sizeKB = (outputFile.lengthSync()) ~/ 1024; - globals.printStatus('Screenshot written to ${globals.fs.path.relative(outputFile.path)} (${sizeKB}kB).'); + globals.printStatus('Screenshot written to ${fs.path.relative(outputFile.path)} (${sizeKB}kB).'); } } diff --git a/packages/flutter_tools/test/commands.shard/hermetic/screenshot_command_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/screenshot_command_test.dart index 8c005b69dec..55fd6585a43 100644 --- a/packages/flutter_tools/test/commands.shard/hermetic/screenshot_command_test.dart +++ b/packages/flutter_tools/test/commands.shard/hermetic/screenshot_command_test.dart @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'package:file/memory.dart'; import 'package:flutter_tools/src/base/io.dart'; import 'package:flutter_tools/src/base/logger.dart'; import 'package:flutter_tools/src/cache.dart'; @@ -26,12 +27,12 @@ void main() { throw Exception('dummy'); }; - await expectLater(() => createTestCommandRunner(ScreenshotCommand()) + await expectLater(() => createTestCommandRunner(ScreenshotCommand(fs: MemoryFileSystem.test())) .run(['screenshot', '--type=skia', '--observatory-url=http://localhost:8181']), throwsA(isException.having((Exception exception) => exception.toString(), 'message', contains('dummy'))), ); - await expectLater(() => createTestCommandRunner(ScreenshotCommand()) + await expectLater(() => createTestCommandRunner(ScreenshotCommand(fs: MemoryFileSystem.test())) .run(['screenshot', '--type=rasterizer', '--observatory-url=http://localhost:8181']), throwsA(isException.having((Exception exception) => exception.toString(), 'message', contains('dummy'))), ); @@ -39,29 +40,70 @@ void main() { testUsingContext('rasterizer and skia screenshots require observatory uri', () async { - await expectLater(() => createTestCommandRunner(ScreenshotCommand()) + await expectLater(() => createTestCommandRunner(ScreenshotCommand(fs: MemoryFileSystem.test())) .run(['screenshot', '--type=skia']), throwsToolExit(message: 'Observatory URI must be specified for screenshot type skia') ); - await expectLater(() => createTestCommandRunner(ScreenshotCommand()) + await expectLater(() => createTestCommandRunner(ScreenshotCommand(fs: MemoryFileSystem.test())) .run(['screenshot', '--type=rasterizer',]), throwsToolExit(message: 'Observatory URI must be specified for screenshot type rasterizer'), ); }); testUsingContext('device screenshots require device', () async { - await expectLater(() => createTestCommandRunner(ScreenshotCommand()) + await expectLater(() => createTestCommandRunner(ScreenshotCommand(fs: MemoryFileSystem.test())) .run(['screenshot']), throwsToolExit(message: 'Must have a connected device for screenshot type device'), ); }); testUsingContext('device screenshots cannot provided Observatory', () async { - await expectLater(() => createTestCommandRunner(ScreenshotCommand()) + await expectLater(() => createTestCommandRunner(ScreenshotCommand(fs: MemoryFileSystem.test())) .run(['screenshot', '--observatory-url=http://localhost:8181']), throwsToolExit(message: 'Observatory URI cannot be provided for screenshot type device'), ); }); }); + + group('Screenshot file validation', () { + testWithoutContext('successful in pwd', () async { + final MemoryFileSystem fs = MemoryFileSystem.test(); + fs.file('test.png').createSync(); + fs.directory('sub_dir').createSync(); + fs.file('sub_dir/test.png').createSync(); + + expect(() => ScreenshotCommand.checkOutput(fs.file('test.png'), fs), + returnsNormally); + expect( + () => ScreenshotCommand.checkOutput(fs.file('sub_dir/test.png'), fs), + returnsNormally); + }); + + testWithoutContext('failed in pwd', () async { + final MemoryFileSystem fs = MemoryFileSystem.test(); + fs.directory('sub_dir').createSync(); + + expect( + () => ScreenshotCommand.checkOutput(fs.file('test.png'), fs), + throwsToolExit( + message: 'File was not created, ensure path is valid')); + expect( + () => ScreenshotCommand.checkOutput(fs.file('../'), fs), + throwsToolExit( + message: 'File was not created, ensure path is valid')); + expect( + () => ScreenshotCommand.checkOutput(fs.file('.'), fs), + throwsToolExit( + message: 'File was not created, ensure path is valid')); + expect( + () => ScreenshotCommand.checkOutput(fs.file('/'), fs), + throwsToolExit( + message: 'File was not created, ensure path is valid')); + expect( + () => ScreenshotCommand.checkOutput(fs.file('sub_dir/test.png'), fs), + throwsToolExit( + message: 'File was not created, ensure path is valid')); + }); + }); }