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

Prevent PicassoExecutorService race condition in DispatcherTest #2341

Merged
merged 1 commit into from May 26, 2023

Conversation

gamepro65
Copy link
Collaborator

Been seeing a pattern of flaky DispatcherTests along the lines of

com.squareup.picasso3.DispatcherTest > performSubmitWithTwoDifferentRequestsQueuesHunters FAILED
    value of: map.size()
    expected: 2
    but was : 1
    map was : {}
        at com.squareup.picasso3.DispatcherTest.performSubmitWithTwoDifferentRequestsQueuesHunters(DispatcherTest.kt:110)

With the debugger I was able to see that PicassoExecutorService is actually spinning up a new thread and executing / completing before our test asserts causing the failure / race condition. Tried swapping PicassoExecutorService out but its required for some tests, so using a spy to prevent creating / executing other threads.

private val bitmap1 = makeBitmap()

@Before fun setUp() {
initMocks(this)
`when`(context.applicationContext).thenReturn(context)
doReturn(mock(Future::class.java)).`when`(executorService).submit(any(Runnable::class.java))
Copy link
Member

Choose a reason for hiding this comment

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

Do you guys not bring in Mockito Kotlin?

Copy link
Collaborator

Choose a reason for hiding this comment

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

not at the moment, cuz we'd like to remove all mocks, but that will take a bit of refactoring to achieve.

@jrodbx jrodbx merged commit e040c93 into master May 26, 2023
@jrodbx jrodbx deleted the cdrury/flakyDispatcherTest branch May 26, 2023 21:18
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.

None yet

3 participants