Issue 1376 - Add tests for SetFilterChangeCallback

test/FilterEngine.cpp

 /*
- * This file is part of Adblock Plus <http://adblockplus.org/>,
- * Copyright (C) 2006-2014 Eyeo GmbH
+ * 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
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
@@ skipping 30 matching lines @@
       jsEngine->SetLogSystem(AdblockPlus::LogSystemPtr(new LogSystem));
       filterEngine = FilterEnginePtr(new AdblockPlus::FilterEngine(jsEngine));
     }
   };
 
   typedef FilterEngineTestGeneric<LazyFileSystem, AdblockPlus::DefaultLogSystem> FilterEngineTest;
   typedef FilterEngineTestGeneric<VeryLazyFileSystem, LazyLogSystem> FilterEngineTestNoData;
 
   struct MockFilterChangeCallback
   {
     MockFilterChangeCallback(int& timesCalled) : timesCalled(timesCalled) {}

[Felix Dahlke, 2014/09/16 09:37:19]

It's somewhat unintuitive that the functor doesn't store timesCalled itself, but
that wouldn't work - we pass callbacks by value. See:
https://issues.adblockplus.org/ticket/1378

[Wladimir Palant, 2014/12/09 21:44:41]

Yes, that's a footgun. Should we at least have a comment that the parameter of
this class should always outlive it? Still, how about a global variable instead?

[Felix Dahlke, 2015/05/29 23:06:40]

It just occurred to me that a shared_ptr should be the safest alternative.
However, considering that this is just a mock, and considering that
UpdateAvailableCallback uses the same approach, I'd rather leave this as it is
for now, and tackle both together with #1378.

 
     void operator()(const std::string&, const AdblockPlus::JsValuePtr)
     {
       timesCalled++;
     }
 
   private:
     int& timesCalled;
   };
+
+  class UpdaterTest : public ::testing::Test
+  {
+  protected:
+    class MockWebRequest : public AdblockPlus::WebRequest
+    {
+    public:
+      AdblockPlus::ServerResponse response;
+
+      AdblockPlus::ServerResponse GET(const std::string& url,
+          const AdblockPlus::HeaderList& requestHeaders) const
+      {
+        return response;
+      }
+    };
+
+    MockWebRequest* mockWebRequest;
+    FilterEnginePtr filterEngine;
+
+    void SetUp()
+    {
+      AdblockPlus::AppInfo appInfo;
+      appInfo.name = "test";
+      appInfo.version = "1.0.1";
+      AdblockPlus::JsEnginePtr jsEngine = AdblockPlus::JsEngine::New(appInfo);
+      jsEngine->SetFileSystem(AdblockPlus::FileSystemPtr(new LazyFileSystem));
+      mockWebRequest = new MockWebRequest;
+      jsEngine->SetWebRequest(AdblockPlus::WebRequestPtr(mockWebRequest));
+      filterEngine = FilterEnginePtr(new AdblockPlus::FilterEngine(jsEngine));
+    }
+  };
+
+  struct MockUpdateAvailableCallback
+  {
+    MockUpdateAvailableCallback(int& timesCalled) : timesCalled(timesCalled) {}
+
+    void operator()(const std::string&)
+    {
+      timesCalled++;
+    }
+
+  private:
+    // We currently cannot store timesCalled in the functor, see:
+    // https://issues.adblockplus.org/ticket/1378.
+    int& timesCalled;
+  };
+
+  // Workaround for https://issues.adblockplus.org/ticket/1397.
+  void NoOpUpdaterCallback(const std::string&) {}
 }
 
 TEST_F(FilterEngineTest, FilterCreation)
 {
   AdblockPlus::FilterPtr filter1 = filterEngine->GetFilter("foo");
   ASSERT_EQ(AdblockPlus::Filter::TYPE_BLOCKING, filter1->GetType());
   AdblockPlus::FilterPtr filter2 = filterEngine->GetFilter("@@foo");
   ASSERT_EQ(AdblockPlus::Filter::TYPE_EXCEPTION, filter2->GetType());
   AdblockPlus::FilterPtr filter3 = filterEngine->GetFilter("example.com##foo");
   ASSERT_EQ(AdblockPlus::Filter::TYPE_ELEMHIDE, filter3->GetType());
@@ skipping 250 matching lines @@
 {
   ASSERT_TRUE(filterEngine->IsFirstRun());
 }
 
 TEST_F(FilterEngineTest, SetRemoveFilterChangeCallback)
 {
   int timesCalled = 0;
   MockFilterChangeCallback mockFilterChangeCallback(timesCalled);
 
   filterEngine->SetFilterChangeCallback(mockFilterChangeCallback);
-  filterEngine->GetSubscription("foo")->AddToList();

[Felix Dahlke, 2014/09/16 09:37:19]

I tried this with fitlerEngine->GetFilter("foo")->AddToList(), but it did not
trigger "filter.added" as I would have expected. Any idea why?

[Wladimir Palant, 2014/12/09 21:44:41]

Not really, no. If the filter was added it should trigger a notification.

[Felix Dahlke, 2015/05/29 23:06:40]

Debugged this a bit. filter.added is actually not called for the first custom
filter that's being added, which is the case here. A new subscription is added,
which triggers subscription.title and subscription.added, but not filter.added.
Went for GetFilter here anyway, since I think that's a more reasonable use case
than this bogus subscription.

+  filterEngine->GetFilter("foo")->AddToList();
+  EXPECT_EQ(2, timesCalled);
+
+  filterEngine->RemoveFilterChangeCallback();
+  filterEngine->GetFilter("foo")->RemoveFromList();
+  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->RemoveFilterChangeCallback();
-  filterEngine->GetSubscription("foo")->RemoveFromList();
+  filterEngine->RemoveUpdateAvailableCallback();
+  filterEngine->ForceUpdateCheck(&NoOpUpdaterCallback);
+  AdblockPlus::Sleep(100);
   ASSERT_EQ(1, timesCalled);
 }