Skip to content

Commit

Permalink
De-flake loading_slot tests (#122)
Browse files Browse the repository at this point in the history
* De-flake PicassoTest.loading_slot
* Remove coroutines-test from Picasso
* De-flake CoilTest.loading_slot
  • Loading branch information
chrisbanes committed Oct 15, 2020
1 parent 260341e commit f6d27a9
Show file tree
Hide file tree
Showing 4 changed files with 117 additions and 55 deletions.
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 0 additions & 2 deletions picasso/build.gradle
Expand Up @@ -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
Expand Down
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)

Expand Down
@@ -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()
}
}
}

0 comments on commit f6d27a9

Please sign in to comment.