Skip to content

When the test main clock is advanced, call ComposeScene.render with the current test time - #584

Merged
Alexander Maryanovsky (m-sasha) merged 1 commit into
m-sasha/test-frameworkfrom
m-sasha/sync-main-test-and-frame-clocks
Jun 3, 2023
Merged

Alexander Maryanovsky (m-sasha) merged 1 commit into
m-sasha/test-frameworkfrom
m-sasha/sync-main-test-and-frame-clocks

Conversation

@m-sasha

Copy link
Copy Markdown

Proposed Changes

In order to sync the composition with the with the test main clock, call ComposeScene.render whenever the test main clock is advanced.

Unfortunately, I couldn't figure how to do this like Android does, so I hope this way will be good enough.

Android appears to use a TestMonotonicFrameClock (with the scheduler advanced by the MainTestClock) in place of our BroadcastFrameClock as the parent clock for Recomposer.runRecomposeAndApplyChanges(). This means that when the main test clock advances, Recomposer wakes up (from parentFrameClock.withFrameNanos) and runs a recomposition.

For us, however, using Recomposer.runRecomposeAndApplyChanges() with a TestMonotonicFrameClock is difficult, because it would require ComposeScene.frameClock (currently a BroadcastFrameClock) to be TestMonotonicFrameClock when running in a test context. Unfortunately, ComposeScene is rather tightly coupled with BroadcastFrameClock.

Testing

Test: Added a unit test to verify that when the main test clock is advanced, a recomposition will happen correctly.

@m-sasha

Copy link
Copy Markdown
Author

Note that I had to modify AbstractMainTestClock which is in commonMain. We should probably upstream it, but not urgently, as it's an internal class.

@eymar

Copy link
Copy Markdown
Member

And a general question.

As I understand, currently the only way to sync the Recomposer with the clock is to call waitForIdle which calls renderNextFrame. And with this change, the sync is happening automatically. Is that right?

I want to better understand why do we need to make this sync happen automatically? To align the behaviour with android? Or is there something else?

@m-sasha

Copy link
Copy Markdown
Author

As I understand, currently the only way to sync the Recomposer with the clock is to call waitForIdle which calls renderNextFrame. And with this change, the sync is happening automatically. Is that right?

Yes, but not exactly. waitForIdle advances everything until the system is idle. Here we only want to advance our ComposeScene.frameClock to match the time of the test clock.

I want to better understand why do we need to make this sync happen automatically? To align the behaviour with android? Or is there something else?

Yes. We want to match Android's behavior, described here: https://developer.android.com/jetpack/compose/testing#sync-auto

@m-sasha
Alexander Maryanovsky (m-sasha) force-pushed the m-sasha/sync-main-test-and-frame-clocks branch from b9fdfe4 to 072667c Compare June 3, 2023 07:29
@m-sasha
Alexander Maryanovsky (m-sasha) merged commit b90e6e5 into m-sasha/test-framework Jun 3, 2023
@m-sasha
Alexander Maryanovsky (m-sasha) deleted the m-sasha/sync-main-test-and-frame-clocks branch June 3, 2023 12:06
Igor Demin (igordmn) pushed a commit that referenced this pull request Jul 10, 2023
Test: ./gradlew test connectedCheck

This is an imported pull request from androidx#584.

Resolves #584
Github-Pr-Head-Sha: 514b36c
GitOrigin-RevId: b46f27d
Change-Id: Ia01ea0db5ab29d40b61a1147742c05f650dadacb
Igor Demin (igordmn) pushed a commit that referenced this pull request Nov 15, 2023
…mposeScene.render with the current test time. (#584)
Igor Demin (igordmn) pushed a commit that referenced this pull request Nov 16, 2023
Zahari Zahariev (harryzz) pushed a commit to harryzz/compose-multiplatform-core that referenced this pull request Jul 28, 2026
Zahari Zahariev (harryzz) pushed a commit to harryzz/compose-multiplatform-core that referenced this pull request Jul 28, 2026
Test: ./gradlew test connectedCheck

This is an imported pull request from androidx#584.

Resolves JetBrains#584
Github-Pr-Head-Sha: 514b36c
GitOrigin-RevId: b46f27d
Change-Id: Ia01ea0db5ab29d40b61a1147742c05f650dadacb
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