Skip to content

Remove usage of MainDispatcherRule in PagingDataDifferTest - #562

Closed
veyndan wants to merge 2 commits into
androidx:androidx-mainfrom
veyndan:270612487/rm-MainDispatcherRule
Closed

Remove usage of MainDispatcherRule in PagingDataDifferTest#562
veyndan wants to merge 2 commits into
androidx:androidx-mainfrom
veyndan:270612487/rm-MainDispatcherRule

Conversation

@veyndan

@veyndan veyndan commented Jun 6, 2023

Copy link
Copy Markdown
Contributor

Opted to just pass in an EmptyCoroutineContext to PagingDataDiffer instead of inlining MainDispathcherRule, as it's clearer to follow. The removal of loadDispatcher was necessary for this to pass. I didn't trace why this had to be removed, but imo it makes the tests easier to read.

Test: ./gradlew test connectedCheck
Bug: 270612487

@claraf3

claraf3 commented Jun 6, 2023

Copy link
Copy Markdown
Member

loadDispatcher wasn't working because without Dispatchers.setMain(TestDispatcher), loadDispatcher would be using a different scheduler than testScope. It would pass if we initiated it with StandardTestDispatcher(testScope.testScheduler). But I'm fine if we remove loadDispatcher too!

pagingSourceFactory = {
TestPagingSource(
loadDelay = 1000,
loadContext = coroutineContext,

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.

I think we no longer need to set loadContext here

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.

Ooo nice, just removed it in the latest commit!

@veyndan
veyndan force-pushed the 270612487/rm-MainDispatcherRule branch 2 times, most recently from 5e44def to 00f6b6b Compare June 12, 2023 12:41
@claraf3

claraf3 commented Jun 15, 2023

Copy link
Copy Markdown
Member

Hi @veyndan there are merge conflicts on this PR, probably because I had just merged in the other PRs. Can you please try rebasing this?

@veyndan
veyndan force-pushed the 270612487/rm-MainDispatcherRule branch from 00f6b6b to 1f5051a Compare June 18, 2023 19:47
@veyndan

veyndan commented Jun 18, 2023

Copy link
Copy Markdown
Contributor Author

Rebased!

@veyndan
veyndan deleted the 270612487/rm-MainDispatcherRule branch June 19, 2023 19:14
harryzz pushed a commit to harryzz/compose-multiplatform-core that referenced this pull request Jul 28, 2026
Opted to just pass in an `EmptyCoroutineContext` to `PagingDataDiffer` instead of inlining `MainDispathcherRule`, as it's clearer to follow. The removal of `loadDispatcher` was necessary for this to pass. I didn't trace why this had to be removed, but imo it makes the tests easier to read.

Test: ./gradlew test connectedCheck
Bug: 270612487

This is an imported pull request from androidx#562.

Resolves JetBrains#562
Github-Pr-Head-Sha: 1f5051a
GitOrigin-RevId: 469c543
Change-Id: Ic697668277be5ad8d7f99c244a5977127f4f96ab
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.

2 participants