[go: up one dir, main page]

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

Remove usage of MainDispatcherRule in PagingDataDifferTest #562

Closed

Conversation

veyndan
Copy link
Contributor
@veyndan veyndan commented Jun 6, 2023

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
Copy link
Member
claraf3 commented Jun 6, 2023

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
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
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
Copy link
Member
claraf3 commented Jun 15, 2023

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
Copy link
Contributor Author
veyndan commented Jun 18, 2023

Rebased!

@veyndan veyndan deleted the 270612487/rm-MainDispatcherRule branch June 19, 2023 19:14
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