From 253900e927389bbdfc2a960ed2f78962db445dc5 Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Wed, 8 Jun 2016 07:39:28 +1000 Subject: [PATCH] Multi-repo updater ported to robolectric. The tests pass, but there is a lingering message that gets logged: ``` Jun 08, 2016 7:31:13 AM com.almworks.sqlite4java.Internal log WARNING: [sqlite] [DETACH DATABASE temp_update_db]DB[1][C]: exception when clearing com.almworks.sqlite4java.SQLiteException: [1] DB[1] reset [no such database: temp_update_db] at com.almworks.sqlite4java.SQLiteConnection.throwResult(SQLiteConnection.java:1309) at com.almworks.sqlite4java.SQLiteConnection.throwResult(SQLiteConnection.java:1282) at com.almworks.sqlite4java.SQLiteConnection.cacheStatementHandle(SQLiteConnection.java:1211) at com.almworks.sqlite4java.SQLiteConnection.access$900(SQLiteConnection.java:54) at com.almworks.sqlite4java.SQLiteConnection$CachedController.dispose(SQLiteConnection.java:1606) at com.almworks.sqlite4java.SQLiteStatement.dispose(SQLiteStatement.java:187) at org.robolectric.shadows.ShadowSQLiteConnection$Connections$4.call(ShadowSQLiteConnection.java:421) at org.robolectric.shadows.ShadowSQLiteConnection$Connections$6.call(ShadowSQLiteConnection.java:449) at org.robolectric.shadows.ShadowSQLiteConnection$Connections$6.call(ShadowSQLiteConnection.java:443) at java.util.concurrent.FutureTask.run(FutureTask.java:266) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617) at java.lang.Thread.run(Thread.java:745) ``` The `temp_update_db` is the one used for repo updates, but I thought that it correctly gets dropped/detached by the `TempAppProvider` when required. In fact, given the nature of the error message (no such database: temp_update_db), that hints at the fact that it is indeed dropped. I'm struggling to figure out what causes this, but it should not be harmful to the running of the tests. If a test actually fails, then it is picked up correctly by JUnit. --- .../org/fdroid/fdroid/FileCompatTest.java | 4 +- .../{TestUtils.java => TestUtilsOld.java} | 4 +- .../java/org/fdroid/fdroid/UtilsTest.java | 2 +- .../java/org/fdroid/fdroid/Preferences.java | 11 ++ .../java/org/fdroid/fdroid/RepoUpdater.java | 4 +- .../fdroid/fdroid/MultiRepoUpdaterTest.java | 126 +++++------------- .../java/org/fdroid/fdroid/TestUtils.java | 38 ++++++ .../resources}/multiRepo.archive.jar | Bin .../resources}/multiRepo.conflicting.jar | Bin .../resources}/multiRepo.normal.jar | Bin 10 files changed, 94 insertions(+), 95 deletions(-) rename app/src/androidTest/java/org/fdroid/fdroid/{TestUtils.java => TestUtilsOld.java} (96%) rename app/src/{androidTest => test}/java/org/fdroid/fdroid/MultiRepoUpdaterTest.java (82%) create mode 100644 app/src/test/java/org/fdroid/fdroid/TestUtils.java rename app/src/{androidTest/assets => test/resources}/multiRepo.archive.jar (100%) rename app/src/{androidTest/assets => test/resources}/multiRepo.conflicting.jar (100%) rename app/src/{androidTest/assets => test/resources}/multiRepo.normal.jar (100%) diff --git a/app/src/androidTest/java/org/fdroid/fdroid/FileCompatTest.java b/app/src/androidTest/java/org/fdroid/fdroid/FileCompatTest.java index 819d8539b..ff7e86378 100644 --- a/app/src/androidTest/java/org/fdroid/fdroid/FileCompatTest.java +++ b/app/src/androidTest/java/org/fdroid/fdroid/FileCompatTest.java @@ -36,8 +36,8 @@ public class FileCompatTest { @Before public void setUp() { Instrumentation instrumentation = InstrumentationRegistry.getInstrumentation(); - File dir = TestUtils.getWriteableDir(instrumentation); - sourceFile = SanitizedFile.knownSanitized(TestUtils.copyAssetToDir(instrumentation.getContext(), "simpleIndex.jar", dir)); + File dir = TestUtilsOld.getWriteableDir(instrumentation); + sourceFile = SanitizedFile.knownSanitized(TestUtilsOld.copyAssetToDir(instrumentation.getContext(), "simpleIndex.jar", dir)); destFile = new SanitizedFile(dir, "dest-" + UUID.randomUUID() + ".testproduct"); assertFalse(destFile.exists()); assertTrue(sourceFile.getAbsolutePath() + " should exist.", sourceFile.exists()); diff --git a/app/src/androidTest/java/org/fdroid/fdroid/TestUtils.java b/app/src/androidTest/java/org/fdroid/fdroid/TestUtilsOld.java similarity index 96% rename from app/src/androidTest/java/org/fdroid/fdroid/TestUtils.java rename to app/src/androidTest/java/org/fdroid/fdroid/TestUtilsOld.java index e30070124..df8144457 100644 --- a/app/src/androidTest/java/org/fdroid/fdroid/TestUtils.java +++ b/app/src/androidTest/java/org/fdroid/fdroid/TestUtilsOld.java @@ -12,9 +12,9 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -public class TestUtils { +public class TestUtilsOld { - private static final String TAG = "TestUtils"; + private static final String TAG = "TestUtilsOld"; @Nullable public static File copyAssetToDir(Context context, String assetName, File directory) { diff --git a/app/src/androidTest/java/org/fdroid/fdroid/UtilsTest.java b/app/src/androidTest/java/org/fdroid/fdroid/UtilsTest.java index 96f701328..3aadf8849 100644 --- a/app/src/androidTest/java/org/fdroid/fdroid/UtilsTest.java +++ b/app/src/androidTest/java/org/fdroid/fdroid/UtilsTest.java @@ -146,7 +146,7 @@ public class UtilsTest { @Test public void testClearOldFiles() throws IOException, InterruptedException { Instrumentation instrumentation = InstrumentationRegistry.getInstrumentation(); - File dir = new File(TestUtils.getWriteableDir(instrumentation), "clearOldFiles"); + File dir = new File(TestUtilsOld.getWriteableDir(instrumentation), "clearOldFiles"); FileUtils.deleteQuietly(dir); dir.mkdirs(); assertTrue(dir.isDirectory()); diff --git a/app/src/main/java/org/fdroid/fdroid/Preferences.java b/app/src/main/java/org/fdroid/fdroid/Preferences.java index 20144be5f..ff521293e 100644 --- a/app/src/main/java/org/fdroid/fdroid/Preferences.java +++ b/app/src/main/java/org/fdroid/fdroid/Preferences.java @@ -372,6 +372,17 @@ public final class Preferences implements SharedPreferences.OnSharedPreferenceCh private static Preferences instance; + /** + * Should only be used for unit testing, whereby separate tests are required to invoke `setup()`. + * The reason we don't instead ask for the singleton to be lazily loaded in the {@link Preferences#get()} + * method is because that would require each call to that method to require a {@link Context}. + * While it is likely that most places asking for preferences have access to a {@link Context}, + * it is a minor convenience to be able to ask for preferences without. + */ + public static void clearSingletonForTesting() { + instance = null; + } + /** * Needs to be setup before anything else tries to access it. */ diff --git a/app/src/main/java/org/fdroid/fdroid/RepoUpdater.java b/app/src/main/java/org/fdroid/fdroid/RepoUpdater.java index 683f39235..d72f83210 100644 --- a/app/src/main/java/org/fdroid/fdroid/RepoUpdater.java +++ b/app/src/main/java/org/fdroid/fdroid/RepoUpdater.java @@ -190,7 +190,9 @@ public class RepoUpdater { processXmlProgressListener, new URL(repo.address), (int) indexEntry.getSize()); // Process the index... - final SAXParser parser = SAXParserFactory.newInstance().newSAXParser(); + SAXParserFactory factory = SAXParserFactory.newInstance(); + factory.setNamespaceAware(true); + final SAXParser parser = factory.newSAXParser(); final XMLReader reader = parser.getXMLReader(); final RepoXMLHandler repoXMLHandler = new RepoXMLHandler(repo, createIndexReceiver()); reader.setContentHandler(repoXMLHandler); diff --git a/app/src/androidTest/java/org/fdroid/fdroid/MultiRepoUpdaterTest.java b/app/src/test/java/org/fdroid/fdroid/MultiRepoUpdaterTest.java similarity index 82% rename from app/src/androidTest/java/org/fdroid/fdroid/MultiRepoUpdaterTest.java rename to app/src/test/java/org/fdroid/fdroid/MultiRepoUpdaterTest.java index 9fc9c9443..ac77a566d 100644 --- a/app/src/androidTest/java/org/fdroid/fdroid/MultiRepoUpdaterTest.java +++ b/app/src/test/java/org/fdroid/fdroid/MultiRepoUpdaterTest.java @@ -1,16 +1,9 @@ package org.fdroid.fdroid; -import android.content.ContentProvider; -import android.content.ContentResolver; import android.content.ContentValues; import android.content.Context; -import android.content.res.AssetManager; -import android.content.res.Resources; import android.support.annotation.NonNull; -import android.test.InstrumentationTestCase; -import android.test.RenamingDelegatingContext; -import android.test.mock.MockContentResolver; import android.text.TextUtils; import android.util.Log; @@ -18,29 +11,37 @@ import org.fdroid.fdroid.RepoUpdater.UpdateException; import org.fdroid.fdroid.data.Apk; import org.fdroid.fdroid.data.ApkProvider; import org.fdroid.fdroid.data.AppProvider; -import org.fdroid.fdroid.data.FDroidProvider; +import org.fdroid.fdroid.data.FDroidProviderTest; import org.fdroid.fdroid.data.Repo; import org.fdroid.fdroid.data.RepoProvider; -import org.fdroid.fdroid.data.TempApkProvider; -import org.fdroid.fdroid.data.TempAppProvider; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.RobolectricGradleTestRunner; +import org.robolectric.annotation.Config; import java.io.File; import java.util.List; import java.util.UUID; -@SuppressWarnings("PMD") // TODO port this to JUnit 4 semantics -public class MultiRepoUpdaterTest extends InstrumentationTestCase { +import static org.fdroid.fdroid.TestUtils.copyResourceToTempFile; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + +@Config(constants = BuildConfig.class) +@RunWith(RobolectricGradleTestRunner.class) +public class MultiRepoUpdaterTest extends FDroidProviderTest { private static final String TAG = "MultiRepoUpdaterTest"; private static final String REPO_MAIN = "Test F-Droid repo"; private static final String REPO_ARCHIVE = "Test F-Droid repo (Archive)"; private static final String REPO_CONFLICTING = "Test F-Droid repo with different apps"; - private Context context; private RepoUpdater conflictingRepoUpdater; private RepoUpdater mainRepoUpdater; private RepoUpdater archiveRepoUpdater; - private File testFilesDir; private static final String PUB_KEY = "3082050b308202f3a003020102020420d8f212300d06092a864886f70d01010b050030363110300e0603" + @@ -75,73 +76,8 @@ public class MultiRepoUpdaterTest extends InstrumentationTestCase { "98f848e0dbfce5a0f2da0198c47e6935a47fda12c518ef45adfb66ddf5aebaab13948a66c004b8592d22" + "e8af60597c4ae2977977cf61dc715a572e241ae717cafdb4f71781943945ac52e0f50b"; - public class TestContext extends RenamingDelegatingContext { - - private MockContentResolver resolver; - - public TestContext() { - super(getInstrumentation().getTargetContext(), "test."); - - resolver = new MockContentResolver(); - resolver.addProvider(AppProvider.getAuthority(), prepareProvider(new AppProvider())); - resolver.addProvider(ApkProvider.getAuthority(), prepareProvider(new ApkProvider())); - resolver.addProvider(RepoProvider.getAuthority(), prepareProvider(new RepoProvider())); - resolver.addProvider(TempAppProvider.getAuthority(), prepareProvider(new TempAppProvider())); - resolver.addProvider(TempApkProvider.getAuthority(), prepareProvider(new TempApkProvider())); - } - - private ContentProvider prepareProvider(ContentProvider provider) { - provider.attachInfo(this, null); - provider.onCreate(); - return provider; - } - - @Override - public File getFilesDir() { - return getInstrumentation().getTargetContext().getFilesDir(); - } - - /** - * String resources used during testing (e.g. when bootstraping the database) are from - * the real org.fdroid.fdroid app, not the test org.fdroid.fdroid.test app. - */ - @Override - public Resources getResources() { - return getInstrumentation().getTargetContext().getResources(); - } - - @Override - public ContentResolver getContentResolver() { - return resolver; - } - - @Override - public AssetManager getAssets() { - return getInstrumentation().getContext().getAssets(); - } - - @Override - public File getDatabasePath(String name) { - return new File(getInstrumentation().getContext().getFilesDir(), "fdroid_test.db"); - } - - @Override - public Context getApplicationContext() { - // Used by the DBHelper singleton instance. - return this; - } - } - - @Override - public void setUp() throws Exception { - super.setUp(); - - FDroidProvider.clearDbHelperSingleton(); - - context = new TestContext(); - - testFilesDir = TestUtils.getWriteableDir(getInstrumentation()); - + @Before + public void setup() throws Exception { // On a fresh database install, there will be F-Droid + GP repos, including their Archive // repos that we are not interested in. RepoProvider.Helper.remove(context, 1); @@ -152,6 +88,13 @@ public class MultiRepoUpdaterTest extends InstrumentationTestCase { conflictingRepoUpdater = createUpdater(REPO_CONFLICTING, context); mainRepoUpdater = createUpdater(REPO_MAIN, context); archiveRepoUpdater = createUpdater(REPO_ARCHIVE, context); + + Preferences.setup(context); + } + + @After + public void tearDown() { + Preferences.clearSingletonForTesting(); } /** @@ -169,9 +112,6 @@ public class MultiRepoUpdaterTest extends InstrumentationTestCase { assertConflictingRepo(repos); } - /** - * - */ private void assertSomewhatAcceptable() { Log.i(TAG, "Asserting at least one versions of each .apk is in index."); List repos = RepoProvider.Helper.all(context); @@ -361,6 +301,7 @@ public class MultiRepoUpdaterTest extends InstrumentationTestCase { */ + @Test public void testAcceptableConflictingThenMainThenArchive() throws UpdateException { assertEmpty(); if (updateConflicting() && updateMain() && updateArchive()) { @@ -368,6 +309,7 @@ public class MultiRepoUpdaterTest extends InstrumentationTestCase { } } + @Test public void testAcceptableConflictingThenArchiveThenMain() throws UpdateException { assertEmpty(); if (updateConflicting() && updateArchive() && updateMain()) { @@ -375,6 +317,7 @@ public class MultiRepoUpdaterTest extends InstrumentationTestCase { } } + @Test public void testAcceptableArchiveThenMainThenConflicting() throws UpdateException { assertEmpty(); if (updateArchive() && updateMain() && updateConflicting()) { @@ -382,6 +325,7 @@ public class MultiRepoUpdaterTest extends InstrumentationTestCase { } } + @Test public void testAcceptableArchiveThenConflictingThenMain() throws UpdateException { assertEmpty(); if (updateArchive() && updateConflicting() && updateMain()) { @@ -389,6 +333,7 @@ public class MultiRepoUpdaterTest extends InstrumentationTestCase { } } + @Test public void testAcceptableMainThenArchiveThenConflicting() throws UpdateException { assertEmpty(); if (updateMain() && updateArchive() && updateConflicting()) { @@ -396,6 +341,7 @@ public class MultiRepoUpdaterTest extends InstrumentationTestCase { } } + @Test public void testAcceptableMainThenConflictingThenArchive() throws UpdateException { assertEmpty(); if (updateMain() && updateConflicting() && updateArchive()) { @@ -434,12 +380,14 @@ public class MultiRepoUpdaterTest extends InstrumentationTestCase { } private boolean updateRepo(RepoUpdater updater, String indexJarPath) throws UpdateException { - if (!testFilesDir.canWrite()) { - return false; + File indexJar = copyResourceToTempFile(indexJarPath); + try { + updater.processDownloadedFile(indexJar); + } finally { + if (indexJar != null && indexJar.exists()) { + indexJar.delete(); + } } - - File indexJar = TestUtils.copyAssetToDir(context, indexJarPath, testFilesDir); - updater.processDownloadedFile(indexJar); return true; } diff --git a/app/src/test/java/org/fdroid/fdroid/TestUtils.java b/app/src/test/java/org/fdroid/fdroid/TestUtils.java new file mode 100644 index 000000000..598c60a3b --- /dev/null +++ b/app/src/test/java/org/fdroid/fdroid/TestUtils.java @@ -0,0 +1,38 @@ +package org.fdroid.fdroid; + +import java.io.File; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; + +import static org.junit.Assert.fail; + +public class TestUtils { + + private static final String TAG = "TestUtils"; + + public static File copyResourceToTempFile(String resourceName) { + File tempFile = null; + InputStream input = null; + OutputStream output = null; + try { + tempFile = File.createTempFile(resourceName + "-", ".testasset"); + input = TestUtils.class.getClassLoader().getResourceAsStream(resourceName); + output = new FileOutputStream(tempFile); + Utils.copy(input, output); + } catch (IOException e) { + e.printStackTrace(); + if (tempFile != null && tempFile.exists()) { + tempFile.delete(); + } + fail(); + return null; + } finally { + Utils.closeQuietly(output); + Utils.closeQuietly(input); + } + return tempFile; + } + +} diff --git a/app/src/androidTest/assets/multiRepo.archive.jar b/app/src/test/resources/multiRepo.archive.jar similarity index 100% rename from app/src/androidTest/assets/multiRepo.archive.jar rename to app/src/test/resources/multiRepo.archive.jar diff --git a/app/src/androidTest/assets/multiRepo.conflicting.jar b/app/src/test/resources/multiRepo.conflicting.jar similarity index 100% rename from app/src/androidTest/assets/multiRepo.conflicting.jar rename to app/src/test/resources/multiRepo.conflicting.jar diff --git a/app/src/androidTest/assets/multiRepo.normal.jar b/app/src/test/resources/multiRepo.normal.jar similarity index 100% rename from app/src/androidTest/assets/multiRepo.normal.jar rename to app/src/test/resources/multiRepo.normal.jar