Skip to content

Commit

Permalink
Make RequestDisposableTest less flaky. (#401)
Browse files Browse the repository at this point in the history 
* Make RequestDisposableTest less flaky.

* Test again.

* Test.

* Prevent race conditions.

* Minor cleanup.

* Use different isDisposed implementation.

* Test.

* Debug.

* Fix.

* Restart CI.

* Print stack trace.

* Debug.

* Test.

* Attempt.

* Remove debug code.

* Remove repeat.
  • Loading branch information
@colinrtwhite
colinrtwhite committed May 11, 2020
1 parent 155e3a3 commit 541393d
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 50 deletions.
93 changes: 49 additions & 44 deletions coil-base/src/androidTest/java/coil/request/RequestDisposableTest.kt
View file
Original file line number Diff line number Diff line change
@@ -1,24 +1,24 @@
package coil.request

import android.content.ContentResolver.SCHEME_CONTENT
import android.content.ContentResolver.SCHEME_FILE
import android.content.Context
import android.graphics.Bitmap
import android.graphics.drawable.Drawable
import android.widget.ImageView
import androidx.test.core.app.ApplicationProvider
import coil.ImageLoader
import coil.RealImageLoader
import coil.annotation.ExperimentalCoilApi
import coil.transition.Transition
import coil.transition.TransitionTarget
import coil.bitmappool.BitmapPool
import coil.fetch.AssetUriFetcher.Companion.ASSET_FILE_PATH_ROOT
import coil.size.Size
import coil.transform.Transformation
import coil.util.CoilUtils
import coil.util.requestManager
import coil.util.runBlockingTest
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.FlowPreview
import kotlinx.coroutines.channels.ConflatedBroadcastChannel
import kotlinx.coroutines.flow.asFlow
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.runBlocking
import org.junit.After
import org.junit.Before
import org.junit.Test
Expand All @@ -31,12 +31,14 @@ import kotlin.test.assertTrue
class RequestDisposableTest {

private lateinit var context: Context
private lateinit var imageLoader: RealImageLoader
private lateinit var imageLoader: ImageLoader

@Before
fun before() {
context = ApplicationProvider.getApplicationContext()
imageLoader = ImageLoader(context) as RealImageLoader
imageLoader = ImageLoader.Builder(context)
.memoryCachePolicy(CachePolicy.DISABLED)
.build()
}

@After
Expand All @@ -45,9 +47,11 @@ class RequestDisposableTest {
}

@Test
fun baseTargetRequestDisposable_dispose() {
fun baseTargetRequestDisposable_dispose() = runBlockingTest {
val request = LoadRequest.Builder(context)
.data("$SCHEME_CONTENT://coil/normal.jpg")
.data("$SCHEME_FILE:///$ASSET_FILE_PATH_ROOT/normal.jpg")
.size(100, 100)
.transformations(GateTransformation())
.target { /** Do nothing. */ }
.build()
val disposable = imageLoader.execute(request)
Expand All @@ -59,29 +63,32 @@ class RequestDisposableTest {
}

@Test
fun baseTargetRequestDisposable_await() {
fun baseTargetRequestDisposable_await() = runBlockingTest {
val transformation = GateTransformation()
var result: Drawable? = null
val request = LoadRequest.Builder(context)
.data("$SCHEME_CONTENT://coil/normal.jpg")
.data("$SCHEME_FILE:///$ASSET_FILE_PATH_ROOT/normal.jpg")
.size(100, 100)
.transformations(transformation)
.target { result = it }
.build()
val disposable = imageLoader.execute(request)

assertTrue(disposable is BaseTargetRequestDisposable)
assertNull(result)
runBlocking { disposable.await() }
transformation.open()
disposable.await()
assertNotNull(result)
}

@Test
fun viewTargetRequestDisposable_dispose() {
val transition = GateTransition()
fun viewTargetRequestDisposable_dispose() = runBlockingTest {
val imageView = ImageView(context)
val request = LoadRequest.Builder(context)
.data("$SCHEME_CONTENT://coil/normal.jpg")
.data("$SCHEME_FILE:///$ASSET_FILE_PATH_ROOT/normal.jpg")
// Set a fixed size so we don't suspend indefinitely waiting for the view to be measured.
.size(100, 100)
.transition(transition)
.transformations(GateTransformation())
.target(imageView)
.build()
val disposable = imageLoader.execute(request)
Expand All @@ -93,42 +100,42 @@ class RequestDisposableTest {
}

@Test
fun viewTargetRequestDisposable_await() {
val transition = GateTransition()
fun viewTargetRequestDisposable_await() = runBlockingTest {
val transformation = GateTransformation()
val imageView = ImageView(context)
val request = LoadRequest.Builder(context)
.data("$SCHEME_CONTENT://coil/normal.jpg")
.data("$SCHEME_FILE:///$ASSET_FILE_PATH_ROOT/normal.jpg")
// Set a fixed size so we don't suspend indefinitely waiting for the view to be measured.
.size(100, 100)
.transition(transition)
.transformations(transformation)
.target(imageView)
.build()
val disposable = imageLoader.execute(request)

assertTrue(disposable is ViewTargetRequestDisposable)
assertNull(imageView.drawable)
transition.open()
runBlocking { disposable.await() }
transformation.open()
disposable.await()
assertNotNull(imageView.drawable)
}

@Test
fun viewTargetRequestDisposable_restart() = runBlockingTest {
val transition = GateTransition()
val transformation = GateTransformation()
val imageView = ImageView(context)
val request = LoadRequest.Builder(context)
.data("$SCHEME_CONTENT://coil/normal.jpg")
.data("$SCHEME_FILE:///$ASSET_FILE_PATH_ROOT/normal.jpg")
// Set a fixed size so we don't suspend indefinitely waiting for the view to be measured.
.size(100, 100)
.transition(transition)
.transformations(transformation)
.target(imageView)
.build()
val disposable = imageLoader.execute(request)

assertTrue(disposable is ViewTargetRequestDisposable)
assertFalse(disposable.isDisposed)

transition.open()
transformation.open()
disposable.await()
assertFalse(disposable.isDisposed)

Expand All @@ -144,15 +151,14 @@ class RequestDisposableTest {

@Test
fun viewTargetRequestDisposable_replace() = runBlockingTest {
val transition = GateTransition()
val imageView = ImageView(context)

fun launchNewRequest(): RequestDisposable {
val request = LoadRequest.Builder(context)
.data("$SCHEME_CONTENT://coil/normal.jpg")
.data("$SCHEME_FILE:///$ASSET_FILE_PATH_ROOT/normal.jpg")
// Set a fixed size so we don't suspend indefinitely waiting for the view to be measured.
.size(100, 100)
.transition(transition)
.transformations(GateTransformation())
.target(imageView)
.build()
return imageLoader.execute(request)
Expand All @@ -172,13 +178,12 @@ class RequestDisposableTest {

@Test
fun viewTargetRequestDisposable_clear() = runBlockingTest {
val transition = GateTransition()
val imageView = ImageView(context)
val request = LoadRequest.Builder(context)
.data("$SCHEME_CONTENT://coil/normal.jpg")
.data("$SCHEME_FILE:///$ASSET_FILE_PATH_ROOT/normal.jpg")
// Set a fixed size so we don't suspend indefinitely waiting for the view to be measured.
.size(100, 100)
.transition(transition)
.transformations(GateTransformation())
.target(imageView)
.build()
val disposable = imageLoader.execute(request)
Expand All @@ -192,21 +197,21 @@ class RequestDisposableTest {
* Prevent completing the [LoadRequest] until [open] is called.
* This is to avoid our test assertions racing the image request.
*/
private class GateTransition : Transition {
private class GateTransformation : Transformation {

private val isOpen = MutableStateFlow(false)

private val isOpen = ConflatedBroadcastChannel(false)
override fun key() = GateTransformation::class.java.name

override suspend fun transition(
target: TransitionTarget<*>,
result: RequestResult
) {
override suspend fun transform(pool: BitmapPool, input: Bitmap, size: Size): Bitmap {
// Suspend until the gate is open.
isOpen.asFlow().first { it }
isOpen.first { it }

// Delegate to the empty transition.
Transition.NONE.transition(target, result)
return input
}

fun open() = isOpen.offer(true)
fun open() {
isOpen.value = true
}
}
}
1 change: 1 addition & 0 deletions coil-base/src/androidTest/java/coil/util/SystemCallbacksTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ class SystemCallbacksTest {
// Keep allocating bitmaps until either the image loader is freed or we run out of memory.
bitmaps += createBitmap(500, 500, Bitmap.Config.ARGB_8888)
}
bitmaps.clear()

// Ensure that the next system callback is called.
systemCallbacks.onTrimMemory(TRIM_MEMORY_BACKGROUND)
Expand Down
16 changes: 10 additions & 6 deletions coil-base/src/main/java/coil/memory/ViewTargetRequestManager.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ internal class ViewTargetRequestManager : View.OnAttachStateChangeListener {
// A pending operation that is posting to the main thread to clear the current request.
@Volatile private var pendingClear: Job? = null

// Only accessed from the main thread.
private var isRestart = false
private var skipAttach = true

Expand Down Expand Up @@ -73,11 +74,14 @@ internal class ViewTargetRequestManager : View.OnAttachStateChangeListener {
override fun onViewAttachedToWindow(v: View) {
if (skipAttach) {
skipAttach = false
} else {
currentRequest?.let { request ->
isRestart = true
request.restart()
}
return
}

currentRequest?.let { request ->
// As this is called from the main thread, isRestart will
// be cleared synchronously as part of request.restart().
isRestart = true
request.restart()
}
}

Expand All @@ -93,7 +97,7 @@ internal class ViewTargetRequestManager : View.OnAttachStateChangeListener {
// Return the current request ID if this is a restarted request.
// Restarted requests are always launched from the main thread.
val requestId = currentRequestId
if (requestId != null && isMainThread() && isRestart) {
if (requestId != null && isRestart && isMainThread()) {
return requestId
}

Expand Down

0 comments on commit 541393d

Please sign in to comment.