Draft
Conversation
tcmulcahy
force-pushed
the
tomm/layoutlib-16.2.3-java21
branch
from
7521364 to
30985eb
Compare
Bump Paparazzi to layoutlib 16.2.3 and layoutlib-api 32.1.0. This is the baseline for Java 21 support, and it intentionally exposes the API removals that the rest of this stack adapts to. Test plan: - Ran `JAVA_HOME=$(/usr/libexec/java_home -v 21) ./gradlew :paparazzi-gradle-plugin:test --tests app.cash.paparazzi.gradle.PaparazziPluginTest.accessibilityErrorsLogged --no-daemon` at this commit and saw compile failures for removed layoutlib APIs like `prepareThread`, `cleanupThread`, and `currentRootView`, confirming the dependency bump exposed the compatibility work handled by the next commits.
Layoutlib 16.2.3 removed the Bridge thread lifecycle APIs, so Paparazzi has to stop calling them before the upgraded library can compile. Test plan: - Ran `JAVA_HOME=$(/usr/libexec/java_home -v 21) ./gradlew :paparazzi-gradle-plugin:test --tests app.cash.paparazzi.gradle.PaparazziPluginTest.accessibilityErrorsLogged --no-daemon` at the parent of this commit and saw unresolved `prepareThread` / `cleanupThread` errors in `PaparazziSdk.kt`. - Ran the same command at this commit and the remaining compile failure moved to the removed `currentRootView` API in `AccessibilityRenderExtension.kt`.
Layoutlib 16.2.3 also removed `WindowManagerImpl.currentRootView`. Replace it with a `WindowManagerGlobal`-based popup-root lookup so accessibility rendering can still find dialog and popup windows. Test plan: - Ran `JAVA_HOME=$(/usr/libexec/java_home -v 21) ./gradlew :paparazzi-gradle-plugin:test --tests app.cash.paparazzi.gradle.PaparazziPluginTest.accessibilityErrorsLogged --no-daemon` at the parent of this commit and saw unresolved `currentRootView` / `layoutParams` errors in `AccessibilityRenderExtension.kt`. - Ran the same command at this commit and it passed.
Clear the platform `AnimationHandler` singleton between snapshots so animation callbacks from one render do not leak into the next test case. Test plan: - Ran `JAVA_HOME=$(/usr/libexec/java_home -v 21) ./gradlew :paparazzi:test --tests app.cash.paparazzi.PaparazziTest.resetsAnimationHandler --no-daemon` at the parent of this commit and saw `expected: null but was: android.animation.AnimationHandler@...`. - Ran the same command at this commit and it passed.
Layoutlib 16.2.3's HWUI path rejects a frame timestamp of zero, so snapshots that start at time 0 need a positive initial frame time before they can render reliably. Test plan: - Ran `JAVA_HOME=$(/usr/libexec/java_home -v 21) ./gradlew :paparazzi-gradle-plugin:test --tests app.cash.paparazzi.gradle.PaparazziPluginTest.compose --no-daemon` at this commit and it passed. - Ran `JAVA_HOME=$(/usr/libexec/java_home -v 21) ./gradlew :sample:testDebugUnitTest --tests app.cash.paparazzi.sample.LottieTest.lottie --no-daemon` at this commit and it passed.
Compose needs the full vsync path so ripples, render-thread animators, and synthetic input all advance under layoutlib 16.2.3 the same way they do on device. Test plan: - Ran `JAVA_HOME=$(/usr/libexec/java_home -v 21) ./gradlew :paparazzi-gradle-plugin:test --tests app.cash.paparazzi.gradle.PaparazziPluginTest.compose --no-daemon` at this commit and it passed.
Compose SHRINK snapshots with overlay windows need an extra warm-up measure pass so popup/dialog content gets a non-zero host size before the real render. Test plan: - Ran `JAVA_HOME=$(/usr/libexec/java_home -v 21) ./gradlew :sample:testDebugUnitTest --tests app.cash.paparazzi.sample.ComposeDialogShrinkTest.test --no-daemon` at the parent of this commit and saw `java.lang.IllegalArgumentException: Width (0) and height (0) cannot be <= 0`. - Ran the same command at this commit and it passed.
Layoutlib 16.2.3 shifts animation callback timing slightly, so the timing assertions in `PaparazziTest` need to match the new interpolation behavior. Test plan: - Ran `JAVA_HOME=$(/usr/libexec/java_home -v 21) ./gradlew :paparazzi:test --tests app.cash.paparazzi.PaparazziTest.animationEvents --no-daemon` at the parent of this commit and saw timing assertion failures like `expected ... onAnimationStart time=2000 ... but was ... onAnimationStart time=2250 ...`. - Ran the same command at this commit and it passed.
Nested Compose popups need their own root and `WindowManager.LayoutParams` preserved so accessibility collection can see the same structure that layoutlib renders. Test plan: - Ran `JAVA_HOME=$(/usr/libexec/java_home -v 21) ./gradlew :paparazzi-gradle-plugin:test --tests app.cash.paparazzi.gradle.PaparazziPluginTest.accessibilityRendering --no-daemon` at the parent of this commit and saw `AccessibilityRenderingTest > modalBottomSheetMaterial3 FAILED` and `AccessibilityRenderingTest > dropDownMaterial3 FAILED`. - Ran the same command at this commit and it passed.
Refresh the overwrite-on-max-percent-difference golden to match the corrected timing/rendering behavior from the earlier layoutlib 16.2.3 changes. Test plan: - Ran `JAVA_HOME=$(/usr/libexec/java_home -v 21) ./gradlew :paparazzi-gradle-plugin:test --tests app.cash.paparazzi.gradle.PaparazziPluginTest.overwriteSnapshotOnMaxPercentDiff --no-daemon` at this commit and it passed.
Add the missing baseline snapshot and resource file that the rerun-resource-change coverage needs in order to exercise the updated layoutlib path. Test plan: - Ran `JAVA_HOME=$(/usr/libexec/java_home -v 21) ./gradlew :paparazzi-gradle-plugin:test --tests app.cash.paparazzi.gradle.PaparazziPluginTest.rerunRecordOnResourceChange --no-daemon` at the parent of this commit and saw the rerun-resource-change fixture fail before it could exercise the intended resource-change path. - Ran the same command at this commit and it advanced to the new fixture files, but still needed the later fixture-state reset to avoid `FileAlreadyExistsException`.
Record the legitimate ripple-image shift introduced by the corrected Compose timing/vsync path. Test plan: - Ran `JAVA_HOME=$(/usr/libexec/java_home -v 21) ./gradlew :paparazzi-gradle-plugin:test --tests app.cash.paparazzi.gradle.PaparazziPluginTest.compose --no-daemon` at the parent of this commit and saw `ComposeRippleTest > ripple FAILED`. - Ran the same command at this commit and it passed.
Keep the Compose-specific timing path gated to actual Compose views so plain view snapshots do not try to load Compose classes when Compose is only present on the classpath transitively. Test plan: - Ran `JAVA_HOME=$(/usr/libexec/java_home -v 21) ./gradlew :paparazzi-gradle-plugin:test --tests app.cash.paparazzi.gradle.PaparazziPluginTest.verifyGif --no-daemon` at the parent of this commit and saw `KeypadViewTest > testViews FAILED` with `java.lang.NoClassDefFoundError` at `KeypadViewTest.kt:41`. - Ran the same command at this commit and the failure moved to a snapshot assertion instead, confirming the plain-view GIF path no longer tried to load `ComposeView`.
Compose still needs a positive frame timestamp for HWUI, but the visible `SystemClock` should stay aligned with the requested snapshot time so delay- and time-based code keeps observing the right instant. Test plan: - Ran `JAVA_HOME=$(/usr/libexec/java_home -v 21) ./gradlew :paparazzi-gradle-plugin:test --tests app.cash.paparazzi.gradle.PaparazziPluginTest.verifyCoroutineDelay --no-daemon` at the parent of this commit and saw `CoroutineDelayMainTest > delayUsesMainDispatcher FAILED` with `java.lang.AssertionError` at `CoroutineDelayMainTest.kt:42`. - Ran the same command at this commit and it passed.
Record the expected GIF output after the plain-view/Compose clock fixes so the verify-gif fixture matches the corrected rendering path. Test plan: - Ran `JAVA_HOME=$(/usr/libexec/java_home -v 21) ./gradlew :paparazzi-gradle-plugin:test --tests app.cash.paparazzi.gradle.PaparazziPluginTest.verifyGif --no-daemon` at the parent of this commit and saw `KeypadViewTest > testViews FAILED` with `java.lang.AssertionError` at `KeypadViewTest.kt:41`. - Ran the same command at this commit and it passed.
Make the plugin-fixture BuildConfig use `VERSION_NAME` directly so fixture builds resolve the just-published in-repo Paparazzi artifact instead of whatever transient `project.version` happens to be during configuration. Test plan: - Ran `JAVA_HOME=$(/usr/libexec/java_home -v 21) ./gradlew :paparazzi-gradle-plugin:generateBuildConfigClasses --no-daemon` at this commit and verified `paparazzi-gradle-plugin/build/generated/sources/buildConfig/main/app/cash/paparazzi/gradle/BuildConfig.kt` contains `VERSION = "2.0.0-SNAPSHOT"`. - Ran `JAVA_HOME=$(/usr/libexec/java_home -v 21) ./gradlew :paparazzi-gradle-plugin:test --tests app.cash.paparazzi.gradle.PaparazziPluginTest.androidApplicationPlugin --no-daemon` at this commit and it passed.
The new in-repo rerun-resource-change fixture leaves files behind between runs, so the test has to clear its snapshot/resource directories before copying its inputs. Test plan: - Ran `JAVA_HOME=$(/usr/libexec/java_home -v 21) ./gradlew :paparazzi-gradle-plugin:test --tests app.cash.paparazzi.gradle.PaparazziPluginTest.rerunRecordOnResourceChange --no-daemon` at the parent of this commit and saw `kotlin.io.FileAlreadyExistsException: ... colors.xml: The destination file already exists.`. - Ran the same command at this commit and it passed.
Refresh the `dontRecord` golden to the current layoutlib 16.2.3 render so it no longer sits at a 19.661458333% diff against a 20.0% overwrite cutoff. That left only about 0.34 percentage points of slack, so small renderer drift could flip one environment into overwriting the golden while another stayed just under the threshold.\n\nAlso restore `record` to an intentionally stale golden and read its own last-modified timestamp in the assertion. The previous fixture had `record.png` matching the current render exactly, and the test was comparing `record` against `dontRecord`'s timestamp, so local passes could hide the fact that the overwrite path was no longer being exercised.\n\nTest plan:\n- Ran `JAVA_HOME=/Library/Java/JavaVirtualMachines/jdk-21.0.8.jdk/Contents/Home ./gradlew :paparazzi-gradle-plugin:test --tests app.cash.paparazzi.gradle.PaparazziPluginTest.overwriteSnapshotOnMaxPercentDiff --no-daemon`
Layoutlib 16.2.3 changes the SHRINK-mode AlertDialog render in the sample module, so the checked-in fixture no longer matches the current output. Refresh the golden to keep the sample verification aligned with the renderer shipped on this branch. Test plan: - Ran `JAVA_HOME=/Library/Java/JavaVirtualMachines/jdk-21.0.8.jdk/Contents/Home ./gradlew :sample:verifyPaparazziDebug --tests app.cash.paparazzi.sample.ComposeDialogShrinkTest.test --no-daemon` before this change and saw `Images differ (by 77.986099%)`. - Ran the same command after refreshing the golden and it passed.
The sample keypad's recorded GIF drifted from the layoutlib 16.2.3 renderer, so now fails even though the test is still exercising the same animation path. Refresh the checked-in golden to match the renderer used on this branch. Test plan: - Ran `JAVA_HOME=/Library/Java/JavaVirtualMachines/jdk-21.0.8.jdk/Contents/Home ./gradlew :sample:verifyPaparazziDebug --tests app.cash.paparazzi.sample.KeypadViewTest.testViews --no-daemon` before this change and saw `14 frames differed by more than 0.010000%`. - Ran the same command after refreshing the golden and it passed.
Layoutlib 16.2.3 changes the rendered output of the sample Lottie animations, so both checked-in GIF fixtures in LottieTest drift from the current renderer. Refresh those two goldens together to keep the sample verification aligned with this branch. Test plan: - Ran JAVA_HOME=$(/usr/libexec/java_home -v 21) ./gradlew :sample:verifyPaparazziDebug --tests app.cash.paparazzi.sample.LottieTest.lottie --tests app.cash.paparazzi.sample.LottieTest.lottie2 --no-daemon before this change and saw both tests fail with frame-diff assertions. - Ran the same command after refreshing the goldens and it passed.
tcmulcahy
force-pushed
the
tomm/layoutlib-16.2.3-java21
branch
from
30985eb to
5d18180
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Upgrade layoutlib to 16.2.3 and layoutlib-api to 32.1.0. Fix breaking changes.
Here is the commit that introduced the 16.2.2 prebuilt: https://android.googlesource.com/platform/prebuilts/studio/layoutlib/+/2a1f75b7665fe7510c86b80b92c1e878b8a5f1
It specifies it was built the following commit, which is accessible: https://android.googlesource.com/platform/frameworks/layoutlib/+/e211004e241ca515cb25381adf57f6e792e54646
Here is the commit that introduced the 16.2.3 prebuilt: https://android.googlesource.com/platform/prebuilts/studio/layoutlib/+/0f5c8c2736213742c7b150c9f2fe72c2dee202
It specifies that it was built from the following commit, which is non-accessible:
https://android.googlesource.com/platform/frameworks/layoutlib/+/d0eb3c1279abd884bc1cd7f1aaf57f7feb1f7e19
Since the source for 16.2.3 is non-accessible, we have to disassemble to see the breaking changes:
layoutlib 16.2.3 removed the prepareThread/cleanupThread APIs:
and also removed the getCurrentRootView API: