Fix libadblockplus-android tests

adblock-android-tests/src/org/adblockplus/libadblockplus/tests/FilterEngineTest.java

Index: adblock-android-tests/src/org/adblockplus/libadblockplus/tests/FilterEngineTest.java
diff --git a/adblock-android-tests/src/org/adblockplus/libadblockplus/tests/FilterEngineTest.java b/adblock-android-tests/src/org/adblockplus/libadblockplus/tests/FilterEngineTest.java
index c5523b02a8dfd46c5321865688f09b1c71fe9a13..17efed16c9de6d8e073188a0ff161188d226b680 100644
--- a/adblock-android-tests/src/org/adblockplus/libadblockplus/tests/FilterEngineTest.java
+++ b/adblock-android-tests/src/org/adblockplus/libadblockplus/tests/FilterEngineTest.java
@@ -17,6 +17,9 @@
 
 package org.adblockplus.libadblockplus.tests;
 
+import android.util.Log;
+
+import org.adblockplus.libadblockplus.BaseFilterEngineTest;
 import org.adblockplus.libadblockplus.Filter;
 import org.adblockplus.libadblockplus.FilterEngine;
 import org.adblockplus.libadblockplus.MockFilterChangeCallback;
@@ -28,6 +31,8 @@ import java.util.List;
 
 public class FilterEngineTest extends BaseFilterEngineTest
 {
+  private static final String TAG = FilterEngineTest.class.getSimpleName();
+
   @Test
   public void testFilterCreation()
   {
@@ -48,7 +53,12 @@ public class FilterEngineTest extends BaseFilterEngineTest
   {
     while (filterEngine.getListedFilters().size() > 0)
     {
+      int prev = filterEngine.getListedFilters().size();

[anton, 2018/08/24 12:25:04]

Can you please explain what's happening here?
Is it to make sure `removeFromList()` really deletes a filter?

Originally it was just removing all the filters until we have it.

[René Jeschke, 2018/08/27 20:02:15]

There was a change in the way subscriptions work in libapb. The AA subscription
can not be removed from listed subscriptions as soon as it has been enabled.
Disabling is possible, but it will stay listed. This caused many tests to fail
or run in endless loops. The 'prev' thing now checks if we really removed a
subscription and breaks the loop otherwise. So we only end up with failing test
in the worst case and not endless loops.

       filterEngine.getListedFilters().get(0).removeFromList();
+      if (prev == filterEngine.getListedFilters().size())
+      {
+        Log.e(TAG, "Failed to clear listed filters.");
+      }
     }
 
     assertEquals(0, filterEngine.getListedFilters().size());
@@ -76,11 +86,53 @@ public class FilterEngineTest extends BaseFilterEngineTest
   }
 
   @Test
+  public void testAddedSubscriptionIsEnabled()
+  {
+    Subscription subscription = filterEngine.getSubscription("foo");
+    assertFalse(subscription.isDisabled());
+  }
+
+  @Test
+  public void testDisablingSubscriptionDisablesItAndFiresEvent()
+  {
+    Subscription subscription = filterEngine.getSubscription("foo");
+    MockFilterChangeCallback callback = new MockFilterChangeCallback("subscription.disabled", "url","foo");
+    filterEngine.setFilterChangeCallback(callback);
+    assertFalse(subscription.isDisabled());
+    subscription.setDisabled(true);
+    assertEquals(1, callback.getTimesCalled());
+    assertTrue(subscription.isDisabled());
+    filterEngine.removeFilterChangeCallback();
+  }
+
+  @Test
+  public void testEnablingSubscriptionEnablesItAndFiresEvent()
+  {
+    Subscription subscription = filterEngine.getSubscription("foo");
+    assertFalse(subscription.isDisabled());
+    subscription.setDisabled(true);
+    assertTrue(subscription.isDisabled());
+
+    MockFilterChangeCallback callback = new MockFilterChangeCallback("subscription.disabled", "url", "foo");
+    filterEngine.setFilterChangeCallback(callback);
+    subscription.setDisabled(false);
+    assertEquals(1, callback.getTimesCalled());
+    assertFalse(subscription.isDisabled());
+    filterEngine.removeFilterChangeCallback();
+  }
+
+  @Test
   public void testAddRemoveSubscriptions()
   {
     while (filterEngine.getListedSubscriptions().size() > 0)

[anton, 2018/08/24 12:25:04]

copy-paste from above. Can you create separate function like `clearFilters()`
and call it from there and here?
I agree it was also copy-paster before but it was 2 lines and now it's much
more.

[René Jeschke, 2018/08/27 20:02:15]

Done.

     {
+      int prev = filterEngine.getListedSubscriptions().size();
       filterEngine.getListedSubscriptions().get(0).removeFromList();
+      if (prev == filterEngine.getListedSubscriptions().size())
+      {
+        Log.e(TAG, "Failed to clear listed subscriptions.");
+        break;
+      }
     }
 
     assertEquals(0, filterEngine.getListedSubscriptions().size());
@@ -324,23 +376,21 @@ public class FilterEngineTest extends BaseFilterEngineTest
   }
 
   @Test
-  public void testFirstRunFlag()
-  {
-    assertFalse(filterEngine.isFirstRun());
-  }
-
-  @Test
   public void testSetRemoveFilterChangeCallback()
   {
-    MockFilterChangeCallback mockFilterChangeCallback = new MockFilterChangeCallback(0);
+    MockFilterChangeCallback callback = new MockFilterChangeCallback("subscription.added", "defaults", "blocking");
 
-    filterEngine.setFilterChangeCallback(mockFilterChangeCallback);
-    filterEngine.getFilter("foo").addToList();
-    assertEquals(1, mockFilterChangeCallback.getTimesCalled());
+    filterEngine.setFilterChangeCallback(callback);
+    Filter filter = filterEngine.getFilter("foo");
+    assertFalse(filter.isListed());
+    filter.addToList();
+    assertEquals(1, callback.getTimesCalled());
 
     filterEngine.removeFilterChangeCallback();
-    filterEngine.getFilter("foo").removeFromList();
-    assertEquals(1, mockFilterChangeCallback.getTimesCalled());
+    callback.clearCheckValues();
+    filter.removeFromList();
+    assertFalse(filter.isListed());
+    assertEquals(1, callback.getTimesCalled());
   }
 
   @Test