Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use onIdle to avoid a race in FlowTests #5202

Merged
merged 2 commits into from
Jul 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions integration/ktx/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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 {
Expand All @@ -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()
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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)
}
}
4 changes: 2 additions & 2 deletions settings.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down