Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Unified Diff: test/Notification.cpp

Issue 29419629: Issue 5164 - Remove NotificationPtr (Closed) Base URL: https://hg.adblockplus.org/libadblockplus/
Patch Set: Updated following review feedback Created April 21, 2017, 4:29 p.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « src/FilterEngine.cpp ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: test/Notification.cpp
===================================================================
--- a/test/Notification.cpp
+++ b/test/Notification.cpp
@@ -43,31 +43,32 @@
void AddNotification(const std::string& notification)
{
jsEngine->Evaluate("(function()"
"{"
"require('notification').Notification.addNotification(" + notification + ");"
"})();");
}
- NotificationPtr PeekNotification(const std::string& url = std::string())
+ std::unique_ptr<Notification> PeekNotification(const std::string& url = std::string())
{
- NotificationPtr retValue;
- filterEngine->SetShowNotificationCallback(std::bind(
- &NotificationTest::NotificationAvailableCallback,
- std::placeholders::_1, std::ref(retValue)));
+ std::unique_ptr<Notification> retValue;
+ filterEngine->SetShowNotificationCallback(
+ [&retValue](const Notification& notification) {
+ NotificationAvailableCallback(notification, retValue);
+ });
filterEngine->ShowNextNotification(url);
filterEngine->RemoveShowNotificationCallback();
return retValue;
}
- static void NotificationAvailableCallback(const NotificationPtr& src, NotificationPtr& dst)
+ static void NotificationAvailableCallback(const Notification& src, std::unique_ptr<Notification>& dst)
sergei 2017/04/21 17:57:38 Could you please also remove this method and move
hub 2017/04/21 18:37:28 Done
{
- EXPECT_TRUE(src);
- dst = src;
+ EXPECT_FALSE(src.IsUndefined());
sergei 2017/04/21 17:57:38 Not sure whether we need that check.
hub 2017/04/21 18:37:28 Removed.
+ dst.reset(new Notification(src));
}
};
class MockWebRequest : public WebRequest
{
public:
std::string responseText;
explicit MockWebRequest(const std::string& notification)
@@ -109,30 +110,30 @@
"\"en-US\": \"message\""
"},"
"\"title\": \"Title\""
"}]"
"}";
jsEngine->SetWebRequest(std::shared_ptr<MockWebRequest>(
new MockWebRequest(responseJsonText)));
jsEngine->SetLogSystem(LogSystemPtr(new DefaultLogSystem()));
- filterEngine.reset(new FilterEngine(jsEngine));
- filterEngine->SetShowNotificationCallback(
- std::bind(&NotificationMockWebRequestTest::OnNotification,
- this, std::placeholders::_1));
+ filterEngine = FilterEngine::Create(jsEngine);
+ filterEngine->SetShowNotificationCallback([this](Notification& notification) {
+ OnNotification(notification);
+ });
}
- void OnNotification(const NotificationPtr& notification)
+ void OnNotification(Notification& notification)
{
isNotificationCallbackCalled = true;
- ASSERT_TRUE(notification);
- EXPECT_EQ(NotificationType::NOTIFICATION_TYPE_INFORMATION, notification->GetType());
- EXPECT_EQ("Title", notification->GetTexts().title);
- EXPECT_EQ("message", notification->GetTexts().message);
- notification->MarkAsShown();
+ ASSERT_FALSE(notification.IsUndefined());
+ EXPECT_EQ(NotificationType::NOTIFICATION_TYPE_INFORMATION, notification.GetType());
+ EXPECT_EQ("Title", notification.GetTexts().title);
+ EXPECT_EQ("message", notification.GetTexts().message);
+ notification.MarkAsShown();
}
};
#endif
}
TEST_F(NotificationTest, NoNotifications)
{
EXPECT_FALSE(PeekNotification());
@@ -148,66 +149,65 @@
TEST_F(NotificationTest, AddNotification)
{
AddNotification("{"
"type: 'critical',"
"title: 'testTitle',"
"message: 'testMessage',"
"}");
- NotificationPtr notification = PeekNotification();
- ASSERT_TRUE(notification);
+ auto notification = PeekNotification();
+ EXPECT_TRUE(notification);
sergei 2017/04/21 17:57:38 here it should stay ASSERT because if the pointer
hub 2017/04/21 18:37:28 Changing this and elsewhere it applies.
EXPECT_EQ(NotificationType::NOTIFICATION_TYPE_CRITICAL, notification->GetType());
EXPECT_EQ("testTitle", notification->GetTexts().title);
EXPECT_EQ("testMessage", notification->GetTexts().message);
}
TEST_F(NotificationTest, FilterByUrl)
{
AddNotification("{ id: 'no-filter', type: 'critical' }");
AddNotification("{ id: 'www.com', type: 'information',"
"urlFilters:['||www.com$document']"
"}");
AddNotification("{ id: 'www.de', type: 'question',"
"urlFilters:['||www.de$document']"
"}");
- NotificationPtr notification = PeekNotification();
- ASSERT_TRUE(notification);
+ auto notification = PeekNotification();
+ EXPECT_TRUE(notification);
EXPECT_EQ(NotificationType::NOTIFICATION_TYPE_CRITICAL, notification->GetType());
notification = PeekNotification("http://www.de");
- ASSERT_TRUE(notification);
+ EXPECT_TRUE(notification);
EXPECT_EQ(NotificationType::NOTIFICATION_TYPE_QUESTION, notification->GetType());
notification = PeekNotification("http://www.com");
- ASSERT_TRUE(notification);
+ EXPECT_TRUE(notification);
EXPECT_EQ(NotificationType::NOTIFICATION_TYPE_INFORMATION, notification->GetType());
}
TEST_F(NotificationTest, MarkAsShown)
{
AddNotification("{ id: 'id', type: 'question' }");
EXPECT_TRUE(PeekNotification());
- NotificationPtr notification = PeekNotification();
- ASSERT_TRUE(notification);
+ auto notification = PeekNotification();
notification->MarkAsShown();
EXPECT_FALSE(PeekNotification());
}
TEST_F(NotificationTest, NoLinks)
{
AddNotification("{ id: 'id'}");
- NotificationPtr notification = PeekNotification();
- ASSERT_TRUE(notification);
+ auto notification = PeekNotification();
+ EXPECT_TRUE(notification);
EXPECT_EQ(0u, notification->GetLinks().size());
}
TEST_F(NotificationTest, Links)
{
AddNotification("{ id: 'id', links: ['link1', 'link2'] }");
- NotificationPtr notification = PeekNotification();
- ASSERT_TRUE(notification);
+ auto notification = PeekNotification();
+ EXPECT_TRUE(notification);
std::vector<std::string> notificationLinks = notification->GetLinks();
ASSERT_EQ(2u, notificationLinks.size());
EXPECT_EQ("link1", notificationLinks[0]);
EXPECT_EQ("link2", notificationLinks[1]);
}
« no previous file with comments | « src/FilterEngine.cpp ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld