Issue 1376 - Add tests for SetFilterChangeCallback

test/FilterEngine.cpp

Index: test/FilterEngine.cpp
===================================================================
--- a/test/FilterEngine.cpp
+++ b/test/FilterEngine.cpp
@@ -49,6 +49,19 @@
   typedef FilterEngineTestGeneric<LazyFileSystem, AdblockPlus::DefaultLogSystem> FilterEngineTest;
   typedef FilterEngineTestGeneric<VeryLazyFileSystem, LazyLogSystem> FilterEngineTestNoData;
 
+  struct MockFilterChangeCallback
+  {
+    MockFilterChangeCallback(int& timesCalled) : timesCalled(timesCalled) {}
+
+    void operator()(const std::string&, const AdblockPlus::JsValuePtr)
+    {
+      timesCalled++;
+    }
+
+  private:
+    int& timesCalled;
+  };
+
   class UpdaterTest : public ::testing::Test
   {
   protected:
@@ -361,6 +374,20 @@
   ASSERT_TRUE(filterEngine->IsFirstRun());
 }
 
+TEST_F(FilterEngineTest, SetRemoveFilterChangeCallback)
+{
+  int timesCalled = 0;
+  MockFilterChangeCallback mockFilterChangeCallback(timesCalled);
+
+  filterEngine->SetFilterChangeCallback(mockFilterChangeCallback);
+  filterEngine->GetFilter("foo")->AddToList();
+  ASSERT_EQ(2, timesCalled);

[sergei, 2015/08/06 08:09:37]

Nit: it could be a good practice to use EXPECT_* when the test can continue the
execution.

[sergei, 2015/08/06 08:09:37]

It would be also good to test when timesCalled is equal to 1 to better
understand what is happening, although it may be look a little bit
implementation specific.

[Felix Dahlke, 2015/08/13 14:17:50]

Hm, you're right, I think I should use it more often. In this case, one could
not only see whether timesCalled was invoked as often as required here, but also
whether it was called after the callback has been removed below. Done.

I guess we should at some point go over all the libadblockplus tests and use
EXPECT where it makes sense? We use it rarely so far.

[Felix Dahlke, 2015/08/13 14:17:50]

That's not really possible - it's called two times as part of `AddToList()`.

+
+  filterEngine->RemoveFilterChangeCallback();
+  filterEngine->GetFilter("foo")->RemoveFromList();
+  ASSERT_EQ(2, timesCalled);
+}
+
 TEST_F(UpdaterTest, SetRemoveUpdateAvailableCallback)
 {
   mockWebRequest->response.status = 0;