[go: nahoru, domu]
Skip to content

Commit

Permalink
[GH] Replace most runBlocking calls with runTest in paging-common
Browse files Browse the repository at this point in the history 
The below test is the only remaining instance of `runBlocking` in `paging-common`. The rest of the test uses a lot of Java specific stuff, and in Multiplatform Paging, I hadn't attempted to multiplatformize it yet.

https://github.com/androidx/androidx/blob/88877d72f62df51fc9dd040da322c9046aa81f67/paging/paging-common/src/test/kotlin/androidx/paging/SingleRunnerTest.kt#L151-L190

Test: ./gradlew test connectedCheck
Bug: 270612487

This is an imported pull request from #555.

Resolves #555
Github-Pr-Head-Sha: 4749aac
GitOrigin-RevId: 3a59fb4
Change-Id: Ic8275ee6a26e870d71cbf4bc57275db84b14b031
  • Loading branch information
@veyndan @copybara-github
veyndan authored and copybara-github committed Jun 2, 2023
1 parent fa5d1a8 commit 143a6f5
Show file tree
Hide file tree
Showing 12 changed files with 308 additions and 333 deletions.
116 changes: 55 additions & 61 deletions paging/paging-common/src/test/kotlin/androidx/paging/CachedPageEventFlowLeakTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,19 +21,21 @@ import kotlin.test.Ignore
import kotlin.test.Test
import kotlinx.coroutines.CompletableDeferred
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.cancel
import kotlinx.coroutines.cancelAndJoin
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.collectLatest
import kotlinx.coroutines.flow.takeWhile
import kotlinx.coroutines.launch
import kotlinx.coroutines.runBlocking
import kotlinx.coroutines.test.runTest
import org.junit.runner.RunWith
import org.junit.runners.JUnit4

/**
* reproduces b/203594733
*/
@OptIn(ExperimentalCoroutinesApi::class)
@RunWith(JUnit4::class)
public class CachedPageEventFlowLeakTest {
private val gcHelper = GarbageCollectionTestHelper()
Expand Down Expand Up @@ -136,17 +138,15 @@ public class CachedPageEventFlowLeakTest {

@Ignore("b/206837348")
@Test
public fun dontLeakCachedPageEventFlows_finished() {
public fun dontLeakCachedPageEventFlows_finished() = runTest {
val scope = CoroutineScope(EmptyCoroutineContext)
val flow = pager.flow.cachedIn(scope, tracker)
runBlocking {
collectPages(
flow = flow,
generationCount = 20,
doneInvalidating = null,
finishCollecting = true
)
}
collectPages(
flow = flow,
generationCount = 20,
doneInvalidating = null,
finishCollecting = true
)
gcHelper.assertLiveObjects(
// see b/204125064
// this should ideally be 0 but right now, we keep the previous generation's state
Expand All @@ -159,72 +159,66 @@ public class CachedPageEventFlowLeakTest {
}

@Test
public fun dontLeakNonCachedFlow_finished() {
runBlocking {
collectPages(
flow = pager.flow,
generationCount = 10,
doneInvalidating = null,
finishCollecting = true
)
}
public fun dontLeakNonCachedFlow_finished() = runTest {
collectPages(
flow = pager.flow,
generationCount = 10,
doneInvalidating = null,
finishCollecting = true
)
gcHelper.assertEverythingIsCollected()
}

@Test
public fun dontLeakPreviousPageInfo_stillCollecting() {
public fun dontLeakPreviousPageInfo_stillCollecting() = runTest {
// reproduces b/204125064
runBlocking {
val doneInvalidating = CompletableDeferred<Unit>()
val collection = launch {
collectPages(
flow = pager.flow,
generationCount = 10,
doneInvalidating = doneInvalidating,
finishCollecting = false
)
}
// make sure we collected enough generations
doneInvalidating.await()
gcHelper.assertLiveObjects(
// see b/204125064
// this should ideally be 0 but right now, we keep the previous generation's state
// to be able to find anchor for the new position but we don't clear it yet. It can
// only be cleared after the new generation loads a page.
Item::class to 20
val doneInvalidating = CompletableDeferred<Unit>()
val collection = launch {
collectPages(
flow = pager.flow,
generationCount = 10,
doneInvalidating = doneInvalidating,
finishCollecting = false
)
collection.cancelAndJoin()
}
// make sure we collected enough generations
doneInvalidating.await()
gcHelper.assertLiveObjects(
// see b/204125064
// this should ideally be 0 but right now, we keep the previous generation's state
// to be able to find anchor for the new position but we don't clear it yet. It can
// only be cleared after the new generation loads a page.
Item::class to 20
)
collection.cancelAndJoin()
}

// Broken: b/206981029
@Ignore
@Test
public fun dontLeakPreviousPageInfoWithCache_stillCollecting() {
public fun dontLeakPreviousPageInfoWithCache_stillCollecting() = runTest {
// reproduces b/204125064
val scope = CoroutineScope(EmptyCoroutineContext)
val flow = pager.flow.cachedIn(scope, tracker)
// reproduces b/204125064
runBlocking {
val doneInvalidating = CompletableDeferred<Unit>()
val collection = launch {
collectPages(
flow = flow,
generationCount = 10,
doneInvalidating = doneInvalidating,
finishCollecting = false
)
}
// make sure we collected enough generations
doneInvalidating.await()
gcHelper.assertLiveObjects(
// see b/204125064
// this should ideally be 0 but right now, we keep the previous generation's state
// to be able to find anchor for the new position but we don't clear it yet. It can
// only be cleared after the new generation loads a page.
Item::class to 20,
CachedPageEventFlow::class to 1
val doneInvalidating = CompletableDeferred<Unit>()
val collection = launch {
collectPages(
flow = flow,
generationCount = 10,
doneInvalidating = doneInvalidating,
finishCollecting = false
)
collection.cancelAndJoin()
}
// make sure we collected enough generations
doneInvalidating.await()
gcHelper.assertLiveObjects(
// see b/204125064
// this should ideally be 0 but right now, we keep the previous generation's state
// to be able to find anchor for the new position but we don't clear it yet. It can
// only be cleared after the new generation loads a page.
Item::class to 20,
CachedPageEventFlow::class to 1
)
collection.cancelAndJoin()
}
}
15 changes: 4 additions & 11 deletions paging/paging-common/src/test/kotlin/androidx/paging/CachingTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import kotlinx.coroutines.flow.mapLatest
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.flow.toList
import kotlinx.coroutines.launch
import kotlinx.coroutines.runBlocking
import kotlinx.coroutines.test.TestScope
import kotlinx.coroutines.test.UnconfinedTestDispatcher
import kotlinx.coroutines.test.runCurrent
Expand Down Expand Up @@ -365,25 +364,21 @@ class CachingTest {
}

@Test
fun pagesAreClosedProperty() {
fun pagesAreClosedProperty() = testScope.runTest {
val job = SupervisorJob()
val subScope = CoroutineScope(job + Dispatchers.Default)
val pageFlow = buildPageFlow().cachedIn(subScope, tracker)
assertThat(tracker.pageEventFlowCount()).isEqualTo(0)
assertThat(tracker.pageDataFlowCount()).isEqualTo(0)
val items = runBlocking {
pageFlow.collectItemsUntilSize(9)
}
val items = pageFlow.collectItemsUntilSize(9)
val firstList = buildItems(
version = 0,
generation = 0,
start = 0,
size = 9
)
assertThat(tracker.pageDataFlowCount()).isEqualTo(1)
val items2 = runBlocking {
pageFlow.collectItemsUntilSize(21)
}
val items2 = pageFlow.collectItemsUntilSize(21)
assertThat(items2).isEqualTo(
buildItems(
version = 0,
Expand All @@ -395,9 +390,7 @@ class CachingTest {
assertThat(tracker.pageEventFlowCount()).isEqualTo(0)
assertThat(tracker.pageDataFlowCount()).isEqualTo(1)
assertThat(items).isEqualTo(firstList)
runBlocking {
job.cancelAndJoin()
}
job.cancelAndJoin()
assertThat(tracker.pageEventFlowCount()).isEqualTo(0)
assertThat(tracker.pageDataFlowCount()).isEqualTo(0)
}
Expand Down
Loading

0 comments on commit 143a6f5

Please sign in to comment.