Fix libadblockplus-android tests

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

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present 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 60 matching lines @@
     public void updateBasePath(Context context)
     {
       if (basePath == null)
       {
         basePath = context.getFilesDir().getAbsolutePath();
       }
     }
   }
 
   protected void setAppInfo(AppInfo appInfo)
-    {

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

wrong indentation

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

Done.

-        setupInfo.appInfo = appInfo;
-    }
+  {
+    setupInfo.appInfo = appInfo;
+  }
 
   protected void setLogSystem(LogSystem logSystem)
-    {

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

wrong indentation

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

Done.

-        setupInfo.logSystem = logSystem;
-    }
+  {
+    setupInfo.logSystem = logSystem;
+  }
 
   protected void setWebRequest(WebRequest webRequest)
-    {

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

wrong indentation

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

Done.

-        setupInfo.webRequest = webRequest;
-    }
+  {
+    setupInfo.webRequest = webRequest;
+  }
 
   protected void setBasePath(String basePath)
-    {

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

wrong indentation

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

Done.

-        setupInfo.basePath = basePath;
-    }
+  {
+    setupInfo.basePath = basePath;
+  }
 
   protected void setIsAllowedConnectionCallback(IsAllowedConnectionCallback callback)
   {
     setupInfo.isAllowedConnectionCallback = callback;
   }
 
   protected void deleteFiles()

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

let's move it to `Util` or `Helper` class

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

It is only used here and in the context of tests. Do we gain anything if we move
this method to a helper class?

   {
     for (File f : getContext().getFilesDir().listFiles())
     {
       f.delete();
     }
   }
 
   protected void setupPlatform()
   {
     if (platform == null)
@@ skipping 21 matching lines @@
       if (setupInfo.isAllowedConnectionCallback != null)
       {
         platform.setUpFilterEngine(setupInfo.isAllowedConnectionCallback);
       }
 
       filterEngine = platform.getFilterEngine();
 
       if (filterEngine.isFirstRun())
       {
         // Wait until stuff got persisted ...
         File patterns = new File(getContext().getFilesDir(), "patterns.ini");

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

It sounds like we know too much of `libadblockplus` implementation details.
Let's say filename will be changed one day and the test fails, though it should
not as everything still works.

How do you think?

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

You got a point, but libadblockplus does not implement any kind of event
callback system for getting any information about it's state. There are tests
that depend on a clean state. The solution you used before was using 'sleep(n)',
but this had two issues: any timeout you specify can be too short, it makes the
test use an unbearable amount of time to complete. Going back to the previous
state would mean getting back to unreliable tests which may fail without any
(real) reason.

If you know any other reliable way to query libadblockplus' persistence state,
then I will gladly remove this. Until then, this method provides a reliable way
to make the tests deterministic. Also, a file name change is more than unlikely,
as the file name is in valid 8.3 format and so supported by any file system that
I am aware of.

[René Jeschke, 2018/08/27 20:16:36]

As you said that everything 'still works', did you test it? On an emulator or a
real device? Here's what I get when I remove this check and run the tests on an
emulator:

org.adblockplus.libadblockplus.tests.AndroidWebRequestResourceWrapperTest >
testNotIntercepted_Easylist[Nexus_5X_API_27_x86(AVD) - 8.1.0] FAILED 
        junit.framework.AssertionFailedError: expected:<1> but was:<2>
        at
org.adblockplus.libadblockplus.tests.AndroidWebRequestResourceWrapperTest.testNotIntercepted(AndroidWebRequestResourceWrapperTest.java:241)

org.adblockplus.libadblockplus.tests.FilterEngineFirstRunTest >
testFirstRunClearWithData[Nexus_5X_API_27_x86(AVD) - 8.1.0] FAILED 
        junit.framework.AssertionFailedError
        at
org.adblockplus.libadblockplus.tests.FilterEngineFirstRunTest.testFirstRunClearWithData(FilterEngineFirstRunTest.java:48)

FAILURE: Build failed with an exception.

         while (!patterns.exists())
         {
           SystemClock.sleep(50);
         }
       }
     }
   }
 
   protected void disposeEngines()
   {
@@ skipping 11 matching lines @@
     return getInstrumentation().getTargetContext();
   }
 
   @Override
   protected void tearDown() throws Exception
   {
     super.tearDown();
     disposeEngines();
   }
 }