Unverified Commit 0c829caa authored by Elias Yishak's avatar Elias Yishak Committed by GitHub

error handling when path to dir provided instead of file (#109796)

parent 435c000b
......@@ -194,7 +194,7 @@ List<FlutterCommand> generateCommands({
featureFlags: featureFlags,
),
RunCommand(verboseHelp: verboseHelp),
ScreenshotCommand(),
ScreenshotCommand(fs: globals.fs),
ShellCompletionCommand(),
TestCommand(verboseHelp: verboseHelp, verbose: verbose),
UpgradeCommand(verboseHelp: verboseHelp),
......
......@@ -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<FlutterCommandResult> 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<void> 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');
}
checkOutput(outputFile, fs);
try {
_showOutputFileInfo(outputFile);
} on Exception catch (error) {
throwToolExit(
'Error with provided file path: "${outputFile.path}"\n'
'Error: $error'
);
}
}
Future<bool> 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).');
}
}
......@@ -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(<String>['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(<String>['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(<String>['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(<String>['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(<String>['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(<String>['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'));
});
});
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment