De-flake loading_slot tests

coil/src/androidTest/java/dev/chrisbanes/accompanist/coil/CoilTest.kt

@@ -57,7 +57,6 @@ import kotlinx.coroutines.channels.Channel
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.runBlocking
 import kotlinx.coroutines.test.TestCoroutineDispatcher
-import kotlinx.coroutines.test.runBlockingTest
 import kotlinx.coroutines.withTimeout
 import kotlinx.coroutines.withTimeoutOrNull
 import okhttp3.mockwebserver.Dispatcher
@@ -67,7 +66,6 @@ import okhttp3.mockwebserver.RecordedRequest
 import okio.Buffer
 import org.junit.After
 import org.junit.Before
-import org.junit.Ignore
 import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
@@ -443,41 +441,45 @@ class CoilTest {
 
     @OptIn(ExperimentalCoroutinesApi::class)
     @Test
-    @Ignore("Flakey: https://github.com/chrisbanes/accompanist/issues/105")
     fun loading_slot() {
-        val dispatcher = TestCoroutineDispatcher()
         val loadLatch = CountDownLatch(1)
 
-        dispatcher.runBlockingTest {
-            pauseDispatcher()
-
-            composeTestRule.setContent {
-                CoilImage(
-                    request = ImageRequest.Builder(ContextAmbient.current)
-                        .data(server.url("/image"))
-                        .dispatcher(dispatcher)
-                        .build(),
-                    modifier = Modifier.preferredSize(128.dp, 128.dp),
-                    // Disable memory cache. If the item is in the cache, the fetch is
-                    // synchronous and the dispatcher pause has no effect
-                    imageLoader = noCacheImageLoader(),
-                    loading = { Text(text = "Loading") },
-                    onRequestCompleted = { loadLatch.countDown() }
-                )
-            }
+        // Create a test dispatcher and immediately pause it
+        val dispatcher = TestCoroutineDispatcher()
+        dispatcher.pauseDispatcher()
 
-            // Assert that the loading component is displayed
-            composeTestRule.onNodeWithText("Loading").assertIsDisplayed()
+        val context = InstrumentationRegistry.getInstrumentation().targetContext
+        val imageLoader = ImageLoader.Builder(context)
+            // Load on our test dispatcher
+            .dispatcher(dispatcher)
+            // Disable memory cache. If the item is in the cache, the fetch is
+            // synchronous and the dispatcher pause has no effect
+            .memoryCachePolicy(CachePolicy.DISABLED)
+            .build()
 
-            // Now resume the dispatcher to start the Coil request
-            dispatcher.resumeDispatcher()
+        composeTestRule.setContent {
+            CoilImage(
+                data = server.url("/image"),
+                imageLoader = imageLoader,
+                modifier = Modifier.preferredSize(128.dp, 128.dp),
+                loading = { Text(text = "Loading") },
+                onRequestCompleted = { loadLatch.countDown() }
+            )
         }
 
+        // Assert that the loading component is displayed
+        composeTestRule.onNodeWithText("Loading").assertIsDisplayed()
+
+        // Now resume the dispatcher to start the Coil request
+        dispatcher.resumeDispatcher()
+
         // We now wait for the request to complete
         loadLatch.await(5, TimeUnit.SECONDS)
 
         // And assert that the loading component no longer exists
         composeTestRule.onNodeWithText("Loading").assertDoesNotExist()
+
+        dispatcher.cleanupTestCoroutines()
     }
 
     @Test

picasso/build.gradle

@@ -95,8 +95,6 @@ dependencies {
 
     androidTestImplementation Libs.OkHttp.mockWebServer
 
-    androidTestImplementation Libs.Coroutines.test
-
     androidTestImplementation Libs.AndroidX.Compose.test
     androidTestImplementation Libs.AndroidX.Compose.ui
     androidTestImplementation Libs.AndroidX.Test.rules

picasso/src/androidTest/java/dev/chrisbanes/accompanist/picasso/PicassoTest.kt

@@ -41,14 +41,13 @@ import androidx.ui.test.onNodeWithText
 import com.google.common.truth.Truth.assertThat
 import com.squareup.picasso.MemoryPolicy
 import com.squareup.picasso.NetworkPolicy
+import com.squareup.picasso.Picasso
 import dev.chrisbanes.accompanist.imageloading.ImageLoadState
 import dev.chrisbanes.accompanist.picasso.test.R
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.channels.Channel
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.runBlocking
-import kotlinx.coroutines.test.TestCoroutineDispatcher
-import kotlinx.coroutines.test.runBlockingTest
 import kotlinx.coroutines.withTimeout
 import kotlinx.coroutines.withTimeoutOrNull
 import okhttp3.mockwebserver.Dispatcher
@@ -58,7 +57,6 @@ import okhttp3.mockwebserver.RecordedRequest
 import okio.Buffer
 import org.junit.After
 import org.junit.Before
-import org.junit.Ignore
 import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
@@ -359,38 +357,38 @@ class PicassoTest {
             .assertPixels { Color.Cyan }
     }
 
-    @OptIn(ExperimentalCoroutinesApi::class)
     @Test
-    @Ignore("Flakey: https://github.com/chrisbanes/accompanist/issues/105")
     fun loading_slot() {
-        val dispatcher = TestCoroutineDispatcher()
         val loadLatch = CountDownLatch(1)
 
-        dispatcher.runBlockingTest {
-            pauseDispatcher()
-
-            composeTestRule.setContent {
-                PicassoImage(
-                    data = server.url("/image"),
-                    modifier = Modifier.preferredSize(128.dp, 128.dp),
-                    // Disable any caches. If the item is in the cache, the fetch is
-                    // synchronous which means the Loading state is skipped
-                    requestBuilder = {
-                        networkPolicy(NetworkPolicy.NO_CACHE)
-                        memoryPolicy(MemoryPolicy.NO_CACHE)
-                    },
-                    loading = { Text(text = "Loading") },
-                    onRequestCompleted = { loadLatch.countDown() }
-                )
-            }
-
-            // Assert that the loading component is displayed
-            composeTestRule.onNodeWithText("Loading").assertIsDisplayed()
+        // Create an executor which is paused, and build a Picasso instance which uses it
+        val executor = SingleThreadPausableExecutor(paused = true)
+        val picasso = Picasso.Builder(InstrumentationRegistry.getInstrumentation().targetContext)
+            .executor(executor)
+            .build()
 
-            // Now resume the dispatcher to start the Coil request
-            dispatcher.resumeDispatcher()
+        composeTestRule.setContent {
+            PicassoImage(
+                data = server.url("/image"),
+                picasso = picasso,
+                modifier = Modifier.preferredSize(128.dp, 128.dp),
+                // Disable any caches. If the item is in the cache, the fetch is
+                // synchronous which means the Loading state is skipped
+                requestBuilder = {
+                    networkPolicy(NetworkPolicy.NO_CACHE)
+                    memoryPolicy(MemoryPolicy.NO_CACHE)
+                },
+                loading = { Text(text = "Loading") },
+                onRequestCompleted = { loadLatch.countDown() }
+            )
         }
 
+        // Assert that the loading component is displayed
+        composeTestRule.onNodeWithText("Loading").assertIsDisplayed()
+
+        // Now resume the executor and let the Picasso request run
+        executor.resume()
+
         // We now wait for the request to complete
         loadLatch.await(5, TimeUnit.SECONDS)
 

picasso/src/androidTest/java/dev/chrisbanes/accompanist/picasso/SingleThreadPauseableExecutor.kt

@@ -0,0 +1,64 @@
+/*
+ * Copyright 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package dev.chrisbanes.accompanist.picasso
+
+import java.util.concurrent.LinkedBlockingQueue
+import java.util.concurrent.ThreadPoolExecutor
+import java.util.concurrent.TimeUnit
+import java.util.concurrent.locks.ReentrantLock
+
+/**
+ * This is copied from the javadoc on [ThreadPoolExecutor].
+ */
+internal class SingleThreadPausableExecutor(paused: Boolean = false) : ThreadPoolExecutor(
+    1, 1, 0L, TimeUnit.MILLISECONDS, LinkedBlockingQueue()
+) {
+    private var isPaused = paused
+    private val pauseLock = ReentrantLock()
+    private val unpaused = pauseLock.newCondition()
+
+    override fun beforeExecute(t: Thread, r: Runnable) {
+        super.beforeExecute(t, r)
+        pauseLock.lock()
+        try {
+            while (isPaused) unpaused.await()
+        } catch (ie: InterruptedException) {
+            t.interrupt()
+        } finally {
+            pauseLock.unlock()
+        }
+    }
+
+    fun pause() {
+        pauseLock.lock()
+        try {
+            isPaused = true
+        } finally {
+            pauseLock.unlock()
+        }
+    }
+
+    fun resume() {
+        pauseLock.lock()
+        try {
+            isPaused = false
+            unpaused.signalAll()
+        } finally {
+            pauseLock.unlock()
+        }
+    }
+}