Fix up tests for the RefreshManager

I'm noticing I write much better code when I have tests in place, it takes much longer to do though...

app-test/src/test/java/org/bspeice/minimalbible/test/activity/downloader/manager/RefreshManagerTest.java

@@ -11,15 +11,11 @@ import org.crosswire.jsword.book.install.Installer;
 import org.junit.Before;
 import org.junit.Test;
 import org.mockito.Mockito;
-import org.mockito.invocation.InvocationOnMock;
-import org.mockito.stubbing.Answer;
 
 import java.util.ArrayList;
 import java.util.Calendar;
 import java.util.Collection;
 import java.util.List;
-import java.util.concurrent.Callable;
-import java.util.concurrent.TimeUnit;
 
 import javax.inject.Inject;
 import javax.inject.Singleton;
@@ -29,7 +25,6 @@ import dagger.ObjectGraph;
 import dagger.Provides;
 import rx.functions.Action1;
 
-import static com.jayway.awaitility.Awaitility.await;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertSame;
@@ -100,40 +95,6 @@ public class RefreshManagerTest implements Injector {
         verify(mockInstaller).getBooks();
     }
 
-    @Test
-    public void testRefreshSeparateThread() {
-        mockInstaller = mock(Installer.class);
-        final List<Book> bookList = new ArrayList<Book>();
-        when(mockInstaller.getBooks()).thenAnswer(new Answer<List<Book>>() {
-            @Override
-            public List<Book> answer(InvocationOnMock invocationOnMock) throws Throwable {
-                Thread.sleep(1000); // Just long enough to give us a gap between
-                // refresh start and complete
-                return bookList;
-            }
-        });
-
-        Collection<Installer> mockInstallers = new ArrayList<Installer>();
-        mockInstallers.add(mockInstaller);
-
-        RMTModules modules = new RMTModules(mockInstallers);
-        mObjectGraph = ObjectGraph.create(modules);
-
-        // And the actual test
-        mObjectGraph.inject(this);
-
-        // So the refresh should be kicked off at the constructor, meaning that it's not "complete"
-        assertFalse(rM.getRefreshComplete().get());
-
-        // But, if it's on another thread, it should finish up eventually, right?
-        await().atMost(5, TimeUnit.SECONDS).until(new Callable<Boolean>() {
-            @Override
-            public Boolean call() throws Exception {
-                return rM.getRefreshComplete().get();
-            }
-        });
-    }
-
     /**
      * Test the conditions are right for downloading
      * I'd like to point out that I can test all of this without requiring mocking of

app-test/src/test/kotlin/org/bspeice/minimalbible/activity/downloader/manager/RefreshManagerSpek.kt

@@ -0,0 +1,108 @@
+package org.bspeice.minimalbible.activity.downloader.manager
+
+import org.jetbrains.spek.api.Spek
+import org.bspeice.minimalbible.activity.downloader.DownloadPrefs
+import java.util.Calendar
+import org.crosswire.jsword.book.install.Installer
+import org.mockito.Mockito.*
+import org.mockito.Matchers.*
+import rx.schedulers.Schedulers
+import java.util.concurrent.atomic.AtomicBoolean
+import com.jayway.awaitility.Awaitility
+import java.util.concurrent.TimeUnit
+import rx.Subscriber
+import org.crosswire.jsword.book.Book
+
+/**
+ * Created by bspeice on 1/3/15.
+ */
+
+class RefreshManagerSpek() : Spek() {{
+
+    fun buildRefreshmanager(installers: List<Installer>, prefs: DownloadPrefs) =
+            RefreshManager(installers, listOf(""), prefs, null)
+
+    fun buildMockPrefs(): DownloadPrefs {
+        val currentTime = Calendar.getInstance().getTime().getTime()
+        val eighteenDaysAgo = currentTime - 1555200
+        val mockPrefs = mock(javaClass<DownloadPrefs>())
+        `when`(mockPrefs.downloadRefreshedOn())
+                .thenReturn(eighteenDaysAgo)
+
+        return mockPrefs
+    }
+
+    given("a mock installer") {
+        val installer = mock(javaClass<Installer>())
+
+        on("creating a new RefreshManager and mock preferences") {
+            val mockPrefs = buildMockPrefs()
+            val rM = buildRefreshmanager(listOf(installer, installer), mockPrefs)
+
+            it("should not have updated the prefs as part of the constructor") {
+                verify(mockPrefs, never())
+                        .downloadRefreshedOn(anyLong())
+            }
+        }
+
+        on("creating a new RefreshManager and mock preferences") {
+            val mockPrefs = buildMockPrefs()
+            val rM = buildRefreshmanager(listOf(installer, installer), mockPrefs)
+            reset(mockPrefs)
+
+            it("should not update the prefs after the first installer") {
+                // The process to do actually validate this is tricky. We have to block
+                // the Observable from producing before we can validate the preferences -
+                // I don't want to race the Observable since it's possible it's on another thread.
+                // So, we use backpressure (request(1)) to force the observable to
+                // produce only one result.
+                val success = AtomicBoolean(false)
+                rM.availableModules
+                        .subscribe(object : Subscriber<Map<Installer, List<Book>>>() {
+                            override fun onCompleted() {
+                            }
+
+                            override fun onError(e: Throwable?) {
+                            }
+
+                            override fun onStart() {
+                                super.onStart()
+                                request(1)
+                            }
+
+                            override fun onNext(t: Map<Installer, List<Book>>?) {
+                                // Verify the mock - if verification doesn't pass, we won't reach
+                                // the end of this method and set our AtomicBoolean to true
+                                verify(mockPrefs, never())
+                                        .downloadRefreshedOn(anyLong())
+                                success.set(true)
+                            }
+                        })
+
+                Awaitility.waitAtMost(2, TimeUnit.SECONDS)
+                        .untilTrue(success)
+            }
+        }
+
+        on("creating a new RefreshManager and mock preferences") {
+            val mockPrefs = buildMockPrefs()
+            val rM = buildRefreshmanager(listOf(installer, installer), mockPrefs)
+            reset(mockPrefs)
+
+            it("should update the prefs after completed") {
+                val complete = AtomicBoolean(false)
+                rM.availableModules.observeOn(Schedulers.immediate())
+                        .subscribe({}, {}, {
+                            complete.set(true)
+                        })
+
+                Awaitility.waitAtMost(3, TimeUnit.SECONDS)
+                        .untilTrue(complete)
+
+                verify(mockPrefs, times(1))
+                        .downloadRefreshedOn(anyLong())
+            }
+        }
+    }
+}
+}

app/src/main/kotlin/org/bspeice/minimalbible/activity/downloader/manager/RefreshManager.kt

@@ -8,7 +8,6 @@ import java.util.Calendar
 import org.bspeice.minimalbible.activity.downloader.DownloadPrefs
 import android.net.ConnectivityManager
 import org.crosswire.jsword.book.BookComparators
-import java.util.Date
 
 /**
  * Created by bspeice on 10/22/14.
@@ -19,19 +18,24 @@ class RefreshManager(val installers: Collection<Installer>,
                      val prefs: DownloadPrefs,
                      val connManager: ConnectivityManager?) {
 
+    val currentTime = Calendar.getInstance().getTime().getTime()
+    val fifteenDaysAgo = currentTime - 1296000
+
     val availableModules: Observable<Map<Installer, List<Book>>> =
             Observable.from(installers)
                     .map {
-                        if (doReload()) {
+                        if (performReload())
                             it.reloadBookList()
-                            prefs.downloadRefreshedOn(Date().getTime())
+
-                        }
+                        // TODO: mapOf(it to booksFromInstaller)
-                        val validBooks = it.getBooks()
+                        mapOf(Pair(it,
-                                .filterNot { exclude contains it.getInitials() }
+                                booksFromInstaller(it, exclude)))
-                        mapOf(Pair(it, validBooks))
                     }
+                    // Don't update timestamps until done. Additionally, make this operation
+                    // part of the pipeline, so it remains a cold observable
+                    .doOnCompleted { prefs.downloadRefreshedOn(currentTime) }
                     .subscribeOn(Schedulers.io())
-                    .cache();
+                    .cache()
 
     val flatModules: Observable<Book> =
             availableModules
@@ -42,9 +46,7 @@ class RefreshManager(val installers: Collection<Installer>,
 
     val flatModulesSorted = flatModules.toSortedList {(book1, book2) ->
         BookComparators.getInitialComparator().compare(book1, book2)
-    };
+    }
-
-    val fifteenDaysAgo = Calendar.getInstance().getTime().getTime() - 1296000
 
     fun doReload(downloadEnabled: Boolean, lastUpdated: Long,
                  networkState: Int? = ConnectivityManager.TYPE_DUMMY): Boolean =
@@ -55,10 +57,14 @@ class RefreshManager(val installers: Collection<Installer>,
             else
                 false
 
-    fun doReload(): Boolean = doReload(prefs.hasEnabledDownload(),
+    fun performReload() =
+            doReload(prefs.hasEnabledDownload(),
                     prefs.downloadRefreshedOn(),
                     connManager?.getActiveNetworkInfo()?.getType())
 
+    fun booksFromInstaller(inst: Installer, exclude: List<String>) =
+            inst.getBooks().filterNot { exclude contains it.getInitials() }
+
     fun installerFromBook(b: Book): Observable<Installer> = Observable.just(
             availableModules.filter {
                 it.flatMap { it.value } contains b