Skip to content

Remove most usages of TestDispatcher in paging-common tests - #523

Closed
veyndan wants to merge 2 commits into
androidx:androidx-mainfrom
veyndan:275416222/rm-TestDispatcher-usages
Closed

Remove most usages of TestDispatcher in paging-common tests#523
veyndan wants to merge 2 commits into
androidx:androidx-mainfrom
veyndan:275416222/rm-TestDispatcher-usages

Conversation

@veyndan

@veyndan veyndan commented Apr 19, 2023

Copy link
Copy Markdown
Contributor

Removes all usages of TestDispatcher in the Paging 3 parts of paging-common tests, and some usages of TestDispatcher in the Paging 2 parts of paging-common tests (PagedListTest.kt and ContiguousPagedListTest.kt are left).

Test: ./gradlew test connectedCheck
Bug: 275416222

// using advanceTimeBy instead of advanceUntilIdle, otherwise this invalid APPEND + subsequent
// REFRESH will auto run consecutively and we won't be able to assert them incrementally
loadDispatcher.queue.poll()?.run()
loadDispatcher.scheduler.advanceTimeBy(1001)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default loadDelay of TestPagingSource is 1000ms. We want to advance the first one only, so any value between [1001, 1999] would be valid.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, thanks!

@claraf3

claraf3 commented Apr 19, 2023

Copy link
Copy Markdown
Member

The presubmit fails on paging-compose is our issue (as with your other PRs). Don't worry about it.

@claraf3
claraf3 self-requested a review April 19, 2023 17:57
@veyndan
veyndan force-pushed the 275416222/rm-TestDispatcher-usages branch from 8df39fb to df63e58 Compare April 20, 2023 21:45
@veyndan
veyndan force-pushed the 275416222/rm-TestDispatcher-usages branch from df63e58 to 9b82469 Compare April 24, 2023 08:30
initialKey = initialKey,
pagingSourceFactory = {
TestPagingSource(
loadDelay = 0,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If our tests are relying on a specific loadDelay, might be more reliable to declare a specific loadDelay so its not flaky to default impl changes. Perhaps set delay to 1000 here as well?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. Updated PR to explicitly set loadDelay = 1000.

@veyndan
veyndan deleted the 275416222/rm-TestDispatcher-usages branch April 26, 2023 18:33
harryzz pushed a commit to harryzz/compose-multiplatform-core that referenced this pull request Jul 28, 2026
Removes _all_ usages of `TestDispatcher` in the Paging 3 parts of `paging-common` tests, and _some_ usages of `TestDispatcher` in the Paging 2 parts of `paging-common` tests (`PagedListTest.kt` and `ContiguousPagedListTest.kt` are left).

Test: ./gradlew test connectedCheck
Bug: 275416222

This is an imported pull request from androidx#523.

Resolves JetBrains#523
Github-Pr-Head-Sha: 0cf2b19
GitOrigin-RevId: 5aca127
Change-Id: I8cba2977a9ea040fde43fbf58547ab704a26051c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants