From 8e8add524eecc3a47bb7964bda809c1303f83d9f Mon Sep 17 00:00:00 2001
From: Kaushik Iska <iska.kaushik@gmail.com>
Date: Wed, 6 Nov 2019 10:59:29 -0800
Subject: [PATCH] [flutter_tool] Screenshot command must require device only
 for _kDeviceType (#44227)

There are cases where we have access to the observatory without having a device connection accessible.
---
 .../lib/src/commands/screenshot.dart          | 27 ++++++++++++-------
 .../screenshot_command_test.dart              | 26 ++++++++++++++++++
 2 files changed, 44 insertions(+), 9 deletions(-)
 create 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 b175456d288..c3f2fccaf70 100644
--- a/packages/flutter_tools/lib/src/commands/screenshot.dart
+++ b/packages/flutter_tools/lib/src/commands/screenshot.dart
@@ -63,18 +63,27 @@ class ScreenshotCommand extends FlutterCommand {
 
   Device device;
 
+  static void validateOptions(String screenshotType, Device device, String observatoryUri) {
+    switch (screenshotType) {
+      case _kDeviceType:
+        if (device == null) {
+          throwToolExit('Must have a connected device for screenshot type $screenshotType');
+        }
+        if (!device.supportsScreenshot) {
+          throwToolExit('Screenshot not supported for ${device.name}.');
+        }
+        break;
+      default:
+        if (observatoryUri == null) {
+          throwToolExit('Observatory URI must be specified for screenshot type $screenshotType');
+        }
+    }
+  }
+
   @override
   Future<FlutterCommandResult> verifyThenRunCommand(String commandPath) async {
     device = await findTargetDevice();
-    if (device == null) {
-      throwToolExit('Must have a connected device');
-    }
-    if (argResults[_kType] == _kDeviceType && !device.supportsScreenshot) {
-      throwToolExit('Screenshot not supported for ${device.name}.');
-    }
-    if (argResults[_kType] != _kDeviceType && argResults[_kObservatoryUri] == null) {
-      throwToolExit('Observatory URI must be specified for screenshot type ${argResults[_kType]}');
-    }
+    validateOptions(argResults[_kType], device, argResults[_kObservatoryUri]);
     return super.verifyThenRunCommand(commandPath);
   }
 
diff --git a/packages/flutter_tools/test/general.shard/screenshot_command_test.dart b/packages/flutter_tools/test/general.shard/screenshot_command_test.dart
new file mode 100644
index 00000000000..686d6194300
--- /dev/null
+++ b/packages/flutter_tools/test/general.shard/screenshot_command_test.dart
@@ -0,0 +1,26 @@
+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+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());
+      expect(() => ScreenshotCommand.validateOptions('skia', null, null), throwsToolExit());
+    });
+
+    testUsingContext('device screenshots require device', () async {
+      expect(() => ScreenshotCommand.validateOptions('device', null, null), throwsToolExit());
+    });
+  });
+}