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

Unified Diff: test/Notification.cpp

Issue 29539858: Issue 5506 - Make the notification test work. (Closed) Base URL: https://hg.adblockplus.org/libadblockplus/
Patch Set: This version work using a fake "ManualTimer" Created Sept. 11, 2017, 10:01 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 | « test/BaseJsTest.h ('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
@@ -10,17 +10,20 @@
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with Adblock Plus. If not, see <http://www.gnu.org/licenses/>.
*/
+#include <iostream>
+#include <AdblockPlus/DefaultLogSystem.h>
#include "BaseJsTest.h"
+#include "../src/DefaultTimer.h"
sergei 2017/09/12 09:22:48 It seems these headers are not required.
hub 2017/09/12 12:59:09 Done.
using namespace AdblockPlus;
namespace
{
class NotificationTest : public BaseJsTest
{
protected:
@@ -75,64 +78,66 @@
ServerResponse serverResponse;
serverResponse.status = IWebRequest::NS_OK;
serverResponse.responseStatus = 200;
serverResponse.responseText = responseText;
getCallback(serverResponse);
}
};
-
// To run this test one needs to set INITIAL_DELAY to about 2000 msec
// in notification.js.
class NotificationMockWebRequestTest : public BaseJsTest
{
protected:
- bool isNotificationCallbackCalled;
+ AdblockPlus::Sync sync;
sergei 2017/09/12 09:22:48 It seems sync is not needed.
hub 2017/09/12 12:59:09 yes it is. Line 112 and line 135
sergei 2017/09/12 13:34:09 There are no other threads, so sync is not needed,
hub 2017/09/12 15:51:10 Done.
void SetUp()
{
- isNotificationCallbackCalled = false;
const char* responseJsonText = "{"
"\"notifications\": [{"
"\"id\": \"some id\","
"\"type\": \"information\","
"\"message\": {"
"\"en-US\": \"message\""
"},"
"\"title\": \"Title\""
"}]"
"}";
- ThrowingPlatformCreationParameters platformParams;
+ AdblockPlus::Platform::CreationParameters platformParams;
+ platformParams.logSystem.reset(new AdblockPlus::DefaultLogSystem());
sergei 2017/09/12 09:22:48 What about using of ThrowingLogSystem as earlier?
hub 2017/09/12 12:59:09 Done.
+ platformParams.timer.reset(new ManualTimer());
sergei 2017/09/12 09:22:48 I would recommend to use DelayedTimer and not crea
hub 2017/09/12 12:59:09 DelayedTimer doesn't do what I want. The idea of M
sergei 2017/09/12 13:34:09 I still think that DelayedTimer is better here bec
hub 2017/09/12 15:51:10 I see. Done.
platformParams.fileSystem.reset(new LazyFileSystem());
platformParams.webRequest.reset(new MockWebRequest(responseJsonText));
platform.reset(new Platform(std::move(platformParams)));
auto& filterEngine = platform->GetFilterEngine();
sergei 2017/09/12 09:22:48 I think here is a dead lock, it should be as above
hub 2017/09/12 12:59:08 Done.
filterEngine.SetShowNotificationCallback(
[this](Notification&& notification) {
- isNotificationCallbackCalled = true;
+ sync.Set();
EXPECT_EQ(NotificationType::NOTIFICATION_TYPE_INFORMATION, notification.GetType());
EXPECT_EQ("Title", notification.GetTexts().title);
EXPECT_EQ("message", notification.GetTexts().message);
notification.MarkAsShown();
});
}
};
}
TEST_F(NotificationTest, NoNotifications)
{
EXPECT_FALSE(PeekNotification());
}
-TEST_F(NotificationMockWebRequestTest, DISABLED_SingleNotification)
+TEST_F(NotificationMockWebRequestTest, SingleNotification)
{
- AdblockPlus::Sleep(5000/*msec*/); // it's a hack
- EXPECT_TRUE(isNotificationCallbackCalled);
+ auto& filterEngine = platform->GetFilterEngine();
+ static_cast<ManualTimer&>(platform->GetTimer()).runOne();
sergei 2017/09/12 09:22:48 I still think that it would be more reliable to fi
hub 2017/09/12 12:59:08 I changed it to a runPending() which will run all
+ filterEngine.ShowNextNotification();
+ EXPECT_TRUE(sync.WaitFor(std::chrono::seconds(4)));
}
TEST_F(NotificationTest, AddNotification)
{
AddNotification("{"
"type: 'critical',"
"title: 'testTitle',"
"message: 'testMessage',"
« no previous file with comments | « test/BaseJsTest.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld