From 861d8d27615655521f4c5056e3800605a042e1b3 Mon Sep 17 00:00:00 2001
From: nturgut <nurhan@google.com>
Date: Fri, 13 Mar 2020 16:33:11 -0700
Subject: [PATCH] addressing the reviewer comments. part 2

---
 packages/e2e/README.md       |  4 ++--
 packages/e2e/lib/common.dart | 23 +++++++++++++++--------
 packages/e2e/lib/e2e.dart    | 10 ++++++----
 3 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/packages/e2e/README.md b/packages/e2e/README.md
index f4540928082b..2c44d1f3c7d5 100644
--- a/packages/e2e/README.md
+++ b/packages/e2e/README.md
@@ -51,7 +51,7 @@ Future<void> main() async {
   final FlutterDriver driver = await FlutterDriver.connect();
   final String jsonResult =
       await driver.requestData(null, timeout: const Duration(minutes: 1));
-  common.Response response = common.Response.fromJson(jsonResult);
+  final common.Response response = common.Response.fromJson(jsonResult);
   await driver.close();
   exit(response.result == 'pass' ? 0 : 1);
 }
@@ -71,7 +71,7 @@ cd example
 flutter drive --driver=test_driver/<package_name>_test.dart test/<package_name>_e2e.dart
 ```
 
-You can run tests on web on release or profile mode.
+You can run tests on web in release or profile mode.
 
 First you need to make sure you have downloaded the driver for the browser.
 
diff --git a/packages/e2e/lib/common.dart b/packages/e2e/lib/common.dart
index d04eb5b51e61..3326cb391e53 100644
--- a/packages/e2e/lib/common.dart
+++ b/packages/e2e/lib/common.dart
@@ -7,23 +7,26 @@ import 'dart:convert';
 /// An object sent from e2e back to the Flutter Driver in response to
 /// `request_data` command.
 class Response {
-  String _failureDetails;
+  final String _failureDetails;
 
   final bool _allTestsPassed;
 
-  /// Constructor which receives tests results' flag and failure details.
-  Response(this._allTestsPassed, String failureDetails) {
-    this._failureDetails = failureDetails;
-  }
+  /// Constructor to use for positive response.
+  Response.allTestsPassed()
+      : this._allTestsPassed = true,
+        this._failureDetails = '';
+
+  /// Constructor for failure response.
+  Response.someTestsFailed(this._failureDetails) : this._allTestsPassed = false;
 
-  /// Whether the test run successfully or not.
+  /// Whether the test ran successfully or not.
   String get result => _allTestsPassed ? 'pass' : 'fail';
 
   /// If the result are failures get the formatted details.
   String get failureDetails => _allTestsPassed ? '' : _failureDetails;
 
   /// Convert a string pass/fail result to a boolean.
-  static bool allTestsPassed(String result) => (result == 'pass')
+  static bool testsPassed(String result) => (result == 'pass')
       ? true
       : (result == 'fail') ? false : throw 'Invalid State for result.';
 
@@ -36,7 +39,11 @@ class Response {
   /// Deserializes the result from JSON.
   static Response fromJson(String source) {
     Map<String, dynamic> result = json.decode(source);
-    return Response(allTestsPassed(result['result']), result['failureDetails']);
+    if (testsPassed(result['result'])) {
+      return Response.allTestsPassed();
+    } else {
+      return Response.someTestsFailed(result['failureDetails']);
+    }
   }
 }
 
diff --git a/packages/e2e/lib/e2e.dart b/packages/e2e/lib/e2e.dart
index 14ad246307b3..e09653f19895 100644
--- a/packages/e2e/lib/e2e.dart
+++ b/packages/e2e/lib/e2e.dart
@@ -37,9 +37,9 @@ class E2EWidgetsFlutterBinding extends LiveTestWidgetsFlutterBinding {
 
   final Completer<bool> _allTestsPassed = Completer<bool>();
 
-  /// Map which stores failure details.
+  /// Stores failure details.
   ///
-  /// Method names are used as key.
+  /// Failed test method's names used as key.
   final Map<String, String> _failureMethodsDetails = Map<String, String>();
 
   /// Similar to [WidgetsFlutterBinding.ensureInitialized].
@@ -69,8 +69,10 @@ class E2EWidgetsFlutterBinding extends LiveTestWidgetsFlutterBinding {
         case 'request_data':
           final bool allTestsPassed = await _allTestsPassed.future;
           response = <String, String>{
-            'message':
-                Response(allTestsPassed, formatFailures(_failureMethodsDetails))
+            'message': allTestsPassed
+                ? Response.allTestsPassed()
+                : Response.someTestsFailed(
+                        formatFailures(_failureMethodsDetails))
                     .toJson(),
           };
           break;