From 7533c47465817ab7c6de2178342c65761e21c2f2 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Fri, 9 Apr 2021 16:59:02 -0700 Subject: [PATCH] [flutter_tools] dont require a device for screenshot type skia/rasterizer (#80153) --- .../lib/src/commands/screenshot.dart | 9 ++- .../hermetic/screenshot_command_test.dart | 69 +++++++++++++++++++ .../screenshot_command_test.dart | 38 ---------- 3 files changed, 75 insertions(+), 41 deletions(-) create mode 100644 packages/flutter_tools/test/commands.shard/hermetic/screenshot_command_test.dart delete mode 100644 packages/flutter_tools/test/general.shard/screenshot_command_test.dart diff --git a/packages/flutter_tools/lib/src/commands/screenshot.dart b/packages/flutter_tools/lib/src/commands/screenshot.dart index 853fc888546..698b28827ae 100644 --- a/packages/flutter_tools/lib/src/commands/screenshot.dart +++ b/packages/flutter_tools/lib/src/commands/screenshot.dart @@ -65,9 +65,13 @@ class ScreenshotCommand extends FlutterCommand { Device device; - static void validateOptions(String screenshotType, Device device, String observatoryUri) { + Future _validateOptions(String screenshotType, String observatoryUri) async { switch (screenshotType) { case _kDeviceType: + if (observatoryUri != null) { + throwToolExit('Observatory URI cannot be provided for screenshot type $screenshotType'); + } + device = await findTargetDevice(); if (device == null) { throwToolExit('Must have a connected device for screenshot type $screenshotType'); } @@ -87,8 +91,7 @@ class ScreenshotCommand extends FlutterCommand { @override Future verifyThenRunCommand(String commandPath) async { - device = await findTargetDevice(); - validateOptions(stringArg(_kType), device, stringArg(_kObservatoryUri)); + await _validateOptions(stringArg(_kType), stringArg(_kObservatoryUri)); return super.verifyThenRunCommand(commandPath); } 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 new file mode 100644 index 00000000000..f0d8eca1d80 --- /dev/null +++ b/packages/flutter_tools/test/commands.shard/hermetic/screenshot_command_test.dart @@ -0,0 +1,69 @@ +// 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:flutter_tools/src/base/io.dart'; +import 'package:flutter_tools/src/base/logger.dart'; +import 'package:flutter_tools/src/cache.dart'; +import 'package:flutter_tools/src/commands/screenshot.dart'; +import 'package:flutter_tools/src/vmservice.dart'; + +import '../../src/common.dart'; +import '../../src/context.dart'; +import '../../src/test_flutter_command_runner.dart'; + +void main() { + setUpAll(() { + Cache.disableLocking(); + }); + + group('Validate screenshot options', () { + testUsingContext('rasterizer and skia screenshots do not require a device', () async { + // Throw a specific exception when attempting to make a VM Service connection to + // verify that we've made it past the initial validation. + openChannelForTesting = (String url, {CompressionOptions compression, Logger logger}) async { + expect(url, 'ws://localhost:8181/ws'); + throw Exception('dummy'); + }; + + await expectLater(() => createTestCommandRunner(ScreenshotCommand()) + .run(['screenshot', '--type=skia', '--observatory-uri=http://localhost:8181']), + throwsA(isA().having((dynamic exception) => exception.toString(), 'message', contains('dummy'))), + ); + + await expectLater(() => createTestCommandRunner(ScreenshotCommand()) + .run(['screenshot', '--type=rasterizer', '--observatory-uri=http://localhost:8181']), + throwsA(isA().having((dynamic exception) => exception.toString(), 'message', contains('dummy'))), + ); + }); + + + testUsingContext('rasterizer and skia screenshots require observatory uri', () async { + await expectLater(() => createTestCommandRunner(ScreenshotCommand()) + .run(['screenshot', '--type=skia']), + throwsToolExit(message: 'Observatory URI must be specified for screenshot type skia') + ); + + await expectLater(() => createTestCommandRunner(ScreenshotCommand()) + .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()) + .run(['screenshot']), + throwsToolExit(message: 'Must have a connected device for screenshot type device'), + ); + }); + + testUsingContext('device screenshots cannot provided Observatory', () async { + await expectLater(() => createTestCommandRunner(ScreenshotCommand()) + .run(['screenshot', '--observatory-uri=http://localhost:8181']), + throwsToolExit(message: 'Observatory URI cannot be provided for screenshot type device'), + ); + }); + }); +} diff --git a/packages/flutter_tools/test/general.shard/screenshot_command_test.dart b/packages/flutter_tools/test/general.shard/screenshot_command_test.dart deleted file mode 100644 index cbd1edacc58..00000000000 --- a/packages/flutter_tools/test/general.shard/screenshot_command_test.dart +++ /dev/null @@ -1,38 +0,0 @@ -// 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:flutter_tools/src/commands/screenshot.dart'; - -import '../src/common.dart'; -import '../src/context.dart'; - -void main() { - group('Validate screenshot options', () { - testUsingContext('rasterizer and skia screenshots do not require a device', () async { - ScreenshotCommand.validateOptions('rasterizer', null, 'dummy_observatory_uri'); - ScreenshotCommand.validateOptions('skia', null, 'dummy_observatory_uri'); - }); - - testUsingContext('rasterizer and skia screenshots require observatory uri', () async { - expect( - () => ScreenshotCommand.validateOptions('rasterizer', null, null), - throwsToolExit( - message: - 'Observatory URI must be specified for screenshot type rasterizer')); - expect( - () => ScreenshotCommand.validateOptions('skia', null, null), - throwsToolExit( - message: - 'Observatory URI must be specified for screenshot type skia')); - expect(() => ScreenshotCommand.validateOptions('skia', null, ''), - throwsToolExit(message: 'Observatory URI "" is invalid')); - }); - - testUsingContext('device screenshots require device', () async { - expect(() => ScreenshotCommand.validateOptions('device', null, null), throwsToolExit()); - }); - }); -}