Refactor Assert[No]CookieExists to return a boolean

ios/chrome/browser/ui/settings/settings_egtest.mm

Index: ios/chrome/browser/ui/settings/settings_egtest.mm
diff --git a/ios/chrome/browser/ui/settings/settings_egtest.mm b/ios/chrome/browser/ui/settings/settings_egtest.mm
index 2c18971ac806df1614f158ad74af625b82deeb4c..3a7daeef857b62f5849e177f1d26f24c4a55fadf 100644
--- a/ios/chrome/browser/ui/settings/settings_egtest.mm
+++ b/ios/chrome/browser/ui/settings/settings_egtest.mm
@@ -9,6 +9,7 @@
 
 #include "base/bind.h"
 #import "base/mac/bind_objc_block.h"
+#include "base/mac/foundation_util.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/sys_string_conversions.h"
 #include "components/browsing_data/core/pref_names.h"
@@ -61,7 +62,7 @@
 const char kUrlWithSetCookie[] = "http://foo/set_cookie";
 const char kResponse[] = "bar";
 const char kResponseWithSetCookie[] = "bar with set cookie";
-const char kNoCookieText[] = "No cookies";
+const char kNoCookieText[] = "No cookies!";
 const char kCookie[] = "a=b";
 const char kJavaScriptGetCookies[] =
     "var c = document.cookie ? document.cookie.split(/;\\s*/) : [];"
@@ -164,27 +165,35 @@
   return ButtonWithAccessibilityLabelId(IDS_IOS_PASSWORD_MANAGER_SAVE_BUTTON);
 }
 
-// Asserts that there is no cookie in current web state.
-void AssertNoCookieExists() {
-  NSError* error = nil;
-  chrome_test_util::ExecuteJavaScript(
+// Returns true if there is no cookie in current web state.

[baxley, 2017/03/10 21:58:17]

nit: s/current/the current

[liaoyuke, 2017/03/13 16:12:50]

Done.

+bool IsCookieEmpty() {

[baxley, 2017/03/10 21:58:17]

Would IsCookiePresent() be more clear? We're not checking for an empty "cookie",
are we checking for there being no cookies?

[liaoyuke, 2017/03/13 16:12:50]

Acknowledged.

+  NSError* error;
+  id result = chrome_test_util::ExecuteJavaScript(
       base::SysUTF8ToNSString(kJavaScriptGetCookies), &error);
-  [[EarlGrey selectElementWithMatcher:chrome_test_util::WebViewContainingText(
-                                          kNoCookieText)]
-      assertWithMatcher:grey_notNil()];
+
+  if (error)
+    return false;
+
+  NSString* const resultCookie = base::mac::ObjCCastStrict<NSString>(result);
+  NSString* const expectedCookie = base::SysUTF8ToNSString(kNoCookieText);
+
+  return [resultCookie isEqualToString:expectedCookie];
 }
 
-// Asserts |cookie| exists in current web state.
-void AssertCookieExists(const char cookie[]) {
-  NSError* error = nil;
-  chrome_test_util::ExecuteJavaScript(
+// Returns true if |cookie| is the only cookie exists in current web state.

[baxley, 2017/03/10 21:58:17]

Returns true if |cookie| is the only cookie that exists in the current web
state.

i.e. s/cookie exists/cookie that exists/ and s/current/the current

[liaoyuke, 2017/03/13 16:12:50]

Acknowledged.

+bool IsCookieEquals(const char cookie[]) {

[baxley, 2017/03/10 21:58:17]

Sorry, I don't have great suggestions, but IsCookieEquals doesn't make it clear
what two cookies we're comparing.

IsCookiePresent(cookie)? IsOneCookiePresent(cookie)?

[liaoyuke, 2017/03/13 16:12:50]

Acknowledged.

+  NSError* error;
+  id result = chrome_test_util::ExecuteJavaScript(
       base::SysUTF8ToNSString(kJavaScriptGetCookies), &error);
-  NSString* const expectedCookieText =
+
+  if (error)
+    return false;
+
+  NSString* const resultCookie = base::mac::ObjCCastStrict<NSString>(result);
+  NSString* const expectedCookie =
       [NSString stringWithFormat:@"OK: %@", base::SysUTF8ToNSString(cookie)];
-  [[EarlGrey
-      selectElementWithMatcher:chrome_test_util::WebViewContainingText(
-                                   base::SysNSStringToUTF8(expectedCookieText))]
-      assertWithMatcher:grey_notNil()];
+
+  return [resultCookie isEqualToString:expectedCookie];

[baxley, 2017/03/10 21:58:17]

Do you see any value in having a helper method to do the comparison, since it
looks like the above 11 lines of code are the same (with the exception of
building the expectedCookie here). Just a thought, I leave it up to you, I don't
want to over engineer this or drag it out longer than necessary.

[liaoyuke, 2017/03/13 16:12:50]

Done.

 }
 
 // Run as a task to check if a certificate has been added to the ChannelIDStore.
@@ -687,7 +696,8 @@ - (void)testClearCookies {
   [[EarlGrey selectElementWithMatcher:chrome_test_util::WebViewContainingText(
                                           kResponse)]
       assertWithMatcher:grey_notNil()];
-  AssertNoCookieExists();
+
+  GREYAssertTrue(IsCookieEmpty(), @"Web state is expected to have no cookie.");
 
   // Visit |kUrlWithSetCookie| to set a cookie and then load |kUrl| to check it
   // is still set.
@@ -700,7 +710,10 @@ - (void)testClearCookies {
                                           kResponse)]
       assertWithMatcher:grey_notNil()];
 
-  AssertCookieExists(kCookie);
+  NSString* const errorMessage = [NSString
+      stringWithFormat:@"Web state is expected to have following cookie: %@.",
+                       base::SysUTF8ToNSString(kCookie)];
+  GREYAssertTrue(IsCookieEquals(kCookie), errorMessage);
 
   // Restore the Clear Browsing Data checkmarks prefs to their default state in
   // Teardown.
@@ -716,7 +729,7 @@ - (void)testClearCookies {
   [[EarlGrey selectElementWithMatcher:chrome_test_util::WebViewContainingText(
                                           kResponse)]
       assertWithMatcher:grey_notNil()];
-  AssertNoCookieExists();
+  GREYAssertTrue(IsCookieEmpty(), @"Web state is expected to have no cookie.");
   chrome_test_util::CloseAllTabs();
 }