Issue 1376 - Add tests for SetFilterChangeCallback

test/FilterEngine.cpp

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2015 Eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 363 matching lines @@
   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()`.

+  EXPECT_EQ(2, timesCalled);
 
   filterEngine->RemoveFilterChangeCallback();
   filterEngine->GetFilter("foo")->RemoveFromList();
-  ASSERT_EQ(2, timesCalled);
+  EXPECT_EQ(2, timesCalled);
 }
 
 TEST_F(UpdaterTest, SetRemoveUpdateAvailableCallback)
 {
   mockWebRequest->response.status = 0;
   mockWebRequest->response.responseStatus = 200;
   mockWebRequest->response.responseText = "\
 {\
   \"test\": {\
     \"version\": \"1.0.2\",\
     \"url\": \"https://downloads.adblockplus.org/test-1.0.2.tar.gz?update\"\
   }\
 }";
 
   int timesCalled = 0;
   MockUpdateAvailableCallback mockUpdateAvailableCallback(timesCalled);
 
   filterEngine->SetUpdateAvailableCallback(mockUpdateAvailableCallback);
   filterEngine->ForceUpdateCheck(&NoOpUpdaterCallback);
   AdblockPlus::Sleep(100);
   ASSERT_EQ(1, timesCalled);
 
   filterEngine->RemoveUpdateAvailableCallback();
   filterEngine->ForceUpdateCheck(&NoOpUpdaterCallback);
   AdblockPlus::Sleep(100);
   ASSERT_EQ(1, timesCalled);
 }