Add tests for the Display operation of the mac notification bridge.

chrome/browser/notifications/notification_platform_bridge_mac_unittest.mm

Index: chrome/browser/notifications/notification_platform_bridge_mac_unittest.mm
diff --git a/chrome/browser/notifications/notification_platform_bridge_mac_unittest.mm b/chrome/browser/notifications/notification_platform_bridge_mac_unittest.mm
index 05d30e51886a6c91367bbdfc88a1c10f8a74948e..84ed1b17b6771aab4941db1d1ab88c1ff35c06d5 100644
--- a/chrome/browser/notifications/notification_platform_bridge_mac_unittest.mm
+++ b/chrome/browser/notifications/notification_platform_bridge_mac_unittest.mm
@@ -5,12 +5,16 @@
 #import <AppKit/AppKit.h>
 
 #include "base/mac/scoped_nsobject.h"
-#include "chrome/browser/notifications/notification_common.h"
+#include "chrome/browser/notifications/notification.h"
 #include "chrome/browser/notifications/notification_platform_bridge_mac.h"
+#include "chrome/browser/notifications/notification_test_util.h"
+#import "chrome/browser/ui/cocoa/cocoa_test_helper.h"
 #include "chrome/browser/ui/cocoa/notifications/notification_builder_mac.h"
 #include "chrome/browser/ui/cocoa/notifications/notification_constants_mac.h"
 #include "chrome/browser/ui/cocoa/notifications/notification_response_builder_mac.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#import "third_party/ocmock/OCMock/OCMock.h"
+#include "third_party/ocmock/gtest_support.h"
 
 namespace {
 
@@ -39,58 +43,67 @@ NSMutableDictionary* BuildDefaultNotificationResponse() {
 
 }  // namespace
 
-TEST(NotificationPlatformBridgeMacTest, TestNotificationValidResponse) {
+class NotificationPlatformBridgeMacTest : public CocoaTest {
+ public:
+  NotificationPlatformBridgeMacTest() {}
+
+  void SetUp() override { CocoaTest::SetUp(); }
+
+  void TearDown() override { CocoaTest::TearDown(); }

[Peter Beverloo, 2016/07/11 17:25:48]

Please define this as:

class NotificationPlatformBridgeMacTest : public CocoaTest {};

(You can drop the rest.)

[Miguel Garcia, 2016/07/12 16:16:24]

Really? Don't we have to call CocoaTest::SetUp and so?

[Peter Beverloo, 2016/07/12 17:44:19]

CocoaTest already defines them, which you inherit from. Since they're virtual
functions, the vtable will make sure that the calls reach the right place.

[Miguel Garcia, 2016/07/13 14:05:16]

Duh, I guess I deserve the lecture :) I misread it as doing some static
initialization of AppKit instead of just calling the superclass method.  

+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(NotificationPlatformBridgeMacTest);
+};
+
+TEST_F(NotificationPlatformBridgeMacTest, TestNotificationVerifyValidResponse) {
   NSDictionary* response = BuildDefaultNotificationResponse();
   EXPECT_TRUE(NotificationPlatformBridgeMac::VerifyNotificationData(response));
 }
 
-TEST(NotificationPlatformBridgeMacTest, TestNotificationUnknownType) {

[Peter Beverloo, 2016/07/11 17:25:48]

Why did this test go away?

[Miguel Garcia, 2016/07/12 16:16:24]

I think it was a merge issue. Brought it back

-  NSMutableDictionary* response = BuildDefaultNotificationResponse();
-  [response setValue:[NSNumber numberWithInt:210581]
-              forKey:notification_constants::kNotificationType];
-  EXPECT_FALSE(NotificationPlatformBridgeMac::VerifyNotificationData(response));
-}
-
-TEST(NotificationPlatformBridgeMacTest, TestNotificationUnknownOperation) {
+TEST_F(NotificationPlatformBridgeMacTest,
+       TestNotificationVerifyUnknownOperation) {
   NSMutableDictionary* response = BuildDefaultNotificationResponse();
   [response setValue:[NSNumber numberWithInt:40782]
               forKey:notification_constants::kNotificationOperation];
   EXPECT_FALSE(NotificationPlatformBridgeMac::VerifyNotificationData(response));
 }
 
-TEST(NotificationPlatformBridgeMacTest, TestNotificationMissingOperation) {
+TEST_F(NotificationPlatformBridgeMacTest,
+       TestNotificationVerifyMissingOperation) {
   NSMutableDictionary* response = BuildDefaultNotificationResponse();
   [response removeObjectForKey:notification_constants::kNotificationOperation];
   EXPECT_FALSE(NotificationPlatformBridgeMac::VerifyNotificationData(response));
 }
 
-TEST(NotificationPlatformBridgeMacTest, TestNotificationNoProfileId) {
+TEST_F(NotificationPlatformBridgeMacTest, TestNotificationVerifyNoProfileId) {
   NSMutableDictionary* response = BuildDefaultNotificationResponse();
   [response removeObjectForKey:notification_constants::kNotificationProfileId];
   EXPECT_FALSE(NotificationPlatformBridgeMac::VerifyNotificationData(response));
 }
 
-TEST(NotificationPlatformBridgeMacTest, TestNotificationNoNotificationId) {
+TEST_F(NotificationPlatformBridgeMacTest,
+       TestNotificationVerifyNoNotificationId) {
   NSMutableDictionary* response = BuildDefaultNotificationResponse();
   [response setValue:@"" forKey:notification_constants::kNotificationId];
   EXPECT_FALSE(NotificationPlatformBridgeMac::VerifyNotificationData(response));
 }
 
-TEST(NotificationPlatformBridgeMacTest, TestNotificationInvalidButton) {
+TEST_F(NotificationPlatformBridgeMacTest, TestNotificationVerifyInvalidButton) {
   NSMutableDictionary* response = BuildDefaultNotificationResponse();
   [response setValue:[NSNumber numberWithInt:-5]
               forKey:notification_constants::kNotificationButtonIndex];
   EXPECT_FALSE(NotificationPlatformBridgeMac::VerifyNotificationData(response));
 }
 
-TEST(NotificationPlatformBridgeMacTest, TestNotificationMissingButtonIndex) {
+TEST_F(NotificationPlatformBridgeMacTest,
+       TestNotificationVerifyMissingButtonIndex) {
   NSMutableDictionary* response = BuildDefaultNotificationResponse();
   [response
       removeObjectForKey:notification_constants::kNotificationButtonIndex];
   EXPECT_FALSE(NotificationPlatformBridgeMac::VerifyNotificationData(response));
 }
 
-TEST(NotificationPlatformBridgeMacTest, TestNotificationOrigin) {
+TEST_F(NotificationPlatformBridgeMacTest, TestNotificationVerifyOrigin) {
   NSMutableDictionary* response = BuildDefaultNotificationResponse();
   [response setValue:@"invalidorigin"
               forKey:notification_constants::kNotificationOrigin];
@@ -100,3 +113,49 @@ TEST(NotificationPlatformBridgeMacTest, TestNotificationOrigin) {
   [response removeObjectForKey:notification_constants::kNotificationOrigin];
   EXPECT_TRUE(NotificationPlatformBridgeMac::VerifyNotificationData(response));
 }
+
+TEST_F(NotificationPlatformBridgeMacTest, TestDisplayNoButtons) {
+  std::unique_ptr<Notification> notification =
+      NotificationGenerator::CreateNotification(
+          "Title", "Context", "https://gmail.com", "", "",

[Peter Beverloo, 2016/07/11 17:25:48]

I'm don't think I'm in favor of this generalization..

  (a) It exposes a very random subset of notification functionality. The
Notification object already has a very.. capable constructor for most these
settings, and setters for the rest.

  (b) The unnamed string arguments are very unclear at the call-site.

  (c) It creates yet another way to create notifications, which old code is
unlikely to ever get updated to.

  (d) We know we're going to switch to Mojo eventually, in which case a builder
of sorts for tests might be useful, but that's unlikely to look like this.

[Miguel Garcia, 2016/07/12 11:06:59]

What do you suggest? I can't really use the constructor directly since it needs
to compose all the optional args (which are not that optional). I can duplicate
code as well if you prefer but other than that I am open to suggestions on what
to do to pass notification objects to the tests.


On 2016/07/11 17:25:48, Peter Beverloo wrote:

[Miguel Garcia, 2016/07/12 16:16:24]

Discussed offline, added its own CreateNotification method in this test instead
of refactoring the one in notification conversions.

On 2016/07/12 11:06:59, Miguel Garcia wrote:

+          message_center::NOTIFICATION_TYPE_SIMPLE);
+
+  id notificationCenter =
+      [OCMockObject mockForClass:[NSUserNotificationCenter class]];
+  NotificationPlatformBridgeMac* bridge = new NotificationPlatformBridgeMac(
+      notificationCenter, false /* set_delegate */);
+  [[notificationCenter expect]
+      deliverNotification:[OCMArg checkWithBlock:^BOOL(id toast) {
+        return [[toast title] isEqualToString:@"Title"] &&
+               [[toast informativeText] isEqualToString:@"Context"] &&
+               [[toast subtitle] isEqualToString:@"https://gmail.com"] &&
+               [[toast actionButtonTitle] isEqualToString:@"Settings"] &&
+               [[toast otherButtonTitle] isEqualToString:@"Close"];
+      }]];
+  bridge->Display(NotificationCommon::PERSISTENT,
+      "notification_id", "profile_id", false, *notification);
+  [notificationCenter verify];
+}
+
+TEST_F(NotificationPlatformBridgeMacTest, TestDisplayOneButton) {
+  std::unique_ptr<Notification> notification =
+      NotificationGenerator::CreateNotification(
+          "Title", "Context", "https://gmail.com", "Button 1", "",
+          message_center::NOTIFICATION_TYPE_SIMPLE);
+
+  id notificationCenter =
+      [OCMockObject mockForClass:[NSUserNotificationCenter class]];
+  NotificationPlatformBridgeMac* bridge = new NotificationPlatformBridgeMac(
+      notificationCenter, false /* set_delegate */);
+  [[notificationCenter expect]
+      deliverNotification:[OCMArg checkWithBlock:^BOOL(id toast) {
+        return [[toast title] isEqualToString:@"Title"] &&
+               [[toast informativeText] isEqualToString:@"Context"] &&
+               [[toast subtitle] isEqualToString:@"https://gmail.com"] &&
+               [[toast actionButtonTitle] isEqualToString:@"Options"] &&
+               [[toast otherButtonTitle] isEqualToString:@"Close"];
+      }]];
+  bridge->Display(NotificationCommon::PERSISTENT,
+      "notification_id", "profile_id", false, *notification);
+  [notificationCenter verify];
+}