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 MainTestClockImpl.onTimeAdvanced #1618

Merged
merged 1 commit into from
Oct 29, 2024

Conversation

m-sasha
Copy link
Member

@m-sasha m-sasha commented Oct 6, 2024

Now that we have a rendering loop in tests, it's no longer necessary to call render every time the test clock is advanced. In fact, it is incorrect to do so. The following test succeeds on Android, but fails on the desktop:

    @Test
    fun advancingClockByLessThanFrameDoesNotRecompose() = runComposeUiTest {
        mainClock.autoAdvance = false
        var value by mutableIntStateOf(1)
        var compositionValue = 0
        setContent {
            compositionValue = value
        }
        assertEquals(1, compositionValue)
        value = 2
        mainClock.advanceTimeBy(1, ignoreFrameDuration = true)
        assertEquals(1, compositionValue)
    }

Following that, our changes to AbstractMainTestClock are no longer needed, and Upstreaming. commonization. ui-test. AbstractMainTestClock is also no longer needed.

Fixes https://youtrack.jetbrains.com/issue/CMP-5776/Upstreaming.-commonization.-ui-test.-AbstractMainTestClock

Testing

Added a new unit test.

Release Notes

Breaking Changes - Multiple Platforms

  • Advancing mainClock such that it doesn't reach the next frame, will no longer cause a recomposition.

@m-sasha m-sasha requested a review from igordmn October 6, 2024 11:59
@m-sasha m-sasha merged commit de01522 into jb-main Oct 29, 2024
6 checks passed
@m-sasha m-sasha deleted the m-sasha/remove-on-time-advanced branch October 29, 2024 19:39
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