Use onIdle to avoid a race in FlowTests

Glide's executor thread notifies the flow in a loop while holding a lock
on the resource. When it finishes, it releases the lock.

If the flow wins the race and runs before Glide's executor releases the
lock, the resource will not be recycled in the remainder of the test
method. If the executor wins the race and releases the lock first, then
the resource will be recycled in the rest of the test method.

To fix this race, I've used Espresso's onIdle and idling resources,
similar to the compose rule.

integration/ktx/build.gradle

@@ -42,6 +42,8 @@ dependencies {
  api project(":library")
  implementation libs.androidx.core.ktx
  implementation libs.coroutines.core
+ testImplementation libs.androidx.espresso
+ testImplementation libs.androidx.espresso.idling
  testImplementation libs.androidx.test.ktx
  testImplementation libs.kotlin.junit
  testImplementation libs.androidx.test.ktx.junit

integration/ktx/src/test/java/com/bumptech/glide/integration/ktx/FlowsTest.kt

@@ -10,6 +10,7 @@ import android.graphics.drawable.ColorDrawable
 import android.graphics.drawable.Drawable
 import android.net.Uri
 import androidx.test.core.app.ApplicationProvider
+import androidx.test.espresso.Espresso.onIdle
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import com.bumptech.glide.Glide
 import com.bumptech.glide.GlideBuilder
@@ -20,6 +21,7 @@ import com.bumptech.glide.load.Options
 import com.bumptech.glide.load.engine.GlideException
 import com.bumptech.glide.load.engine.cache.MemoryCache
 import com.bumptech.glide.load.engine.executor.GlideExecutor
+import com.bumptech.glide.load.engine.executor.GlideIdlingResources
 import com.bumptech.glide.load.model.ModelLoader
 import com.bumptech.glide.load.model.ModelLoaderFactory
 import com.bumptech.glide.load.model.MultiModelLoaderFactory
@@ -47,19 +49,25 @@ import kotlinx.coroutines.launch
 import kotlinx.coroutines.newSingleThreadContext
 import kotlinx.coroutines.test.runTest
 import org.junit.After
+import org.junit.Before
 import org.junit.Rule
 import org.junit.Test
 import org.junit.rules.TemporaryFolder
 import org.junit.runner.RunWith
 
 // newFile throws IOException, which triggers this warning even though there's no reasonable
 // alternative :/.
-@Suppress("BlockingMethodInNonBlockingContext")
+@Suppress("BlockingMethodInNonBlockingContext", "RedundantSuppression")
 @RunWith(AndroidJUnit4::class)
 class FlowsTest {
  private val context = ApplicationProvider.getApplicationContext<Context>()
  @get:Rule val temporaryFolder = TemporaryFolder()
 
+ @Before
+ fun setUp() {
+ GlideIdlingResources.initGlide()
+ }
+
  @After
  fun tearDown() {
  Glide.tearDown()
@@ -303,8 +311,7 @@ class FlowsTest {
  @Test
  fun flow_onClose_clearsTarget() = runTest {
  val inCache = AtomicReference<com.bumptech.glide.load.engine.Resource<*>?>()
- Glide.init(
- context,
+ GlideIdlingResources.initGlide(
  GlideBuilder()
  .setMemoryCache(
  object : MemoryCache {
@@ -327,11 +334,15 @@ class FlowsTest {
  inCache.set(resource)
  return null
  }
-  }
-  )
+ }
+ )
  )
  val data = Glide.with(context).load(newImageFile()).flow(100, 100).firstLoad().toList()
  assertThat(data).isNotEmpty()
+ // Glide's executor (in EngineJob's notify loop) and the coroutine race to close the resource.
+ // If Glide's executor wins, then the coroutine will be able to put the resource in the cache,
+ // but if not, the item won't be cached until after the coroutine starts back up.
+ onIdle()
  assertThat(inCache.get()).isNotNull()
  }
 

integration/ktx/src/test/java/com/bumptech/glide/load/engine/executor/GlideIdlingResourceInit.kt

@@ -0,0 +1,36 @@
+package com.bumptech.glide.load.engine.executor
+
+import androidx.test.core.app.ApplicationProvider
+import androidx.test.espresso.IdlingRegistry
+import androidx.test.espresso.idling.concurrent.IdlingThreadPoolExecutor
+import com.bumptech.glide.Glide
+import com.bumptech.glide.GlideBuilder
+import java.util.concurrent.LinkedBlockingQueue
+import java.util.concurrent.TimeUnit
+
+object GlideIdlingResources {
+
+ fun initGlide(builder: GlideBuilder? = null) {
+ val registry = IdlingRegistry.getInstance()
+ val executor =
+ IdlingThreadPoolExecutor(
+ "glide_test_thread",
+ /* corePoolSize = */ 1,
+ /* maximumPoolSize = */ 1,
+ /* keepAliveTime = */ 1,
+ TimeUnit.SECONDS,
+ LinkedBlockingQueue()
+ ) {
+ Thread(it)
+ }
+ val glideExecutor = GlideExecutor(executor)
+ Glide.init(
+ ApplicationProvider.getApplicationContext(),
+ (builder ?: GlideBuilder())
+ .setSourceExecutor(glideExecutor)
+ .setAnimationExecutor(glideExecutor)
+ .setDiskCacheExecutor(glideExecutor)
+ )
+ registry.register(executor)
+ }
+}

settings.gradle

@@ -54,14 +54,14 @@ dependencyResolutionManagement {
 
  // Versions for dependencies
  version('compose', '1.3.2')
- version('coroutines', '1.6.4')
+ version('coroutines', '1.7.2')
  version('dagger', '2.46.1')
  version('errorprone', '2.18.0')
  version('kotlin', '1.7.0')
  version('mockito', '5.3.1')
  version('retrofit', '2.3.0')
  version('androidx-benchmark', '1.1.1')
- version('androidx-espresso', '3.4.0')
+ version('androidx-espresso', '3.5.1')
  // At least versions 1.5 and later require java 8 desugaring, which Glide can't
  // currently use, so we're stuck on an older version.
  version('androidx-fragment', '1.3.6')