Support DelayedPreviews (not tested, just initial push) #633

sergio-sastre · 2025-01-18T12:32:43Z

No description provided.

takahirom · 2025-01-19T04:17:41Z

👀

takahirom · 2025-01-19T04:34:07Z

The build failed because of a setting in the root build.gradle file:

tasks.withType(org.jetbrains.kotlin.gradle.tasks.KotlinCompile).configureEach {
  compilerOptions {
    freeCompilerArgs.addAll(
      "-Xopt-in=com.github.takahirom.roborazzi.InternalRoborazziApi",
      "-Xopt-in=com.github.takahirom.roborazzi.ExperimentalRoborazziApi",
    )
  }
}

I've addressed this by excluding the "roborazzi-annotations" module from this setting.

takahirom · 2025-01-19T05:17:06Z

I understand this is difficult, especially regarding ComposeRule. I'm exploring solutions. Please bear with me.

github-actions · 2025-01-19T05:40:42Z

Snapshot diff report

File name	Image
com.github.takahirom .preview.tests.Previ ewsKt.PreviewDelayed _compare.png

takahirom · 2025-01-19T05:42:40Z

@sergio-sastre
Thank you for your contribution! I've made some minor changes to utilize RoborazziComposeOptions, which I believe results in a slight improvement. However, we may need to consider the preview changes. 👀

https://github.com/takahirom/roborazzi/pull/633/files/4c4646c6e01373c1becf9bfc9c49b4c52deb0499..2764156a105defb07e7b21376de596e2d46f08d6

takahirom · 2025-01-19T07:15:29Z

I think we might have multiple activities for each capture. I'm looking a little further into this.

sergio-sastre · 2025-01-19T08:17:41Z

Great work!
Let me know your thoughts after exploring and we can discuss how to better solve the problem 😊

takahirom · 2025-01-19T10:49:59Z

Thanks! I'm considering whether the name "DelayedPreview" is appropriate.
Currently, using this implementation prevents us from utilizing the activityTheme() option, which isn't used by the PreviewScreenshots parameter. While this shouldn't pose an immediate issue, it limits the robustness of the RoborazziComposeActivityScenarioCreatorOption API. I'm open to suggestions for improvement but currently lack a concrete solution.

sergio-sastre · 2025-01-19T15:19:26Z

Thanks! I'm considering whether the name "DelayedPreview" is appropriate.
Currently, using this implementation prevents us from utilizing the activityTheme() option, which isn't used by the PreviewScreenshots parameter. While this shouldn't pose an immediate issue, it limits the robustness of the RoborazziComposeActivityScenarioCreatorOption API. I'm open to suggestions for improvement but currently lack a concrete solution.

I've pushed a preview with the DelayedPreview annotation and the current implementation does not seem to work.
I'd say, let's focus first on having a working implementation, and then see what works or doesn't work and search for possible solutions

sergio-sastre · 2025-01-19T16:36:28Z

...anner-support/src/main/java/com/github/takahirom/roborazzi/RoborazziPreviewScannerSupport.kt

@@ -143,18 +197,28 @@ interface ComposePreviewTester<T : Any> {

 @ExperimentalRoborazziApi
 class AndroidComposePreviewTester : ComposePreviewTester<AndroidPreviewInfo> {
+  private val composeTestRule by lazy { createAndroidComposeRule<RoborazziActivity>() }


@takahirom
Actually, it does not matter whether lazy or not since it is called in EVERY test.

I was afraid that using this TestRule in every test would increase the execution time, but I haven't noticed any significant increase though... I believe that is different from Activity Scenario, which launches an Activity and that does add execution time indeed...

It could also be that my laptop (Mac M2) is very fast and the amount of preview tests is not that high (less than 15...). I'm unsure about this

I believe test isolation is important. I think it's good to have a separate Activity for each test, as we do now. Of course, it depends on how slow it is, but I'm fine with it.

I believe that too. However, even with this lazy, the ComposeTestRule is still being created in EVERY test, because it is called in other parts of the AndroidPreviewTester ( I’ve debugged it).
I’d also like it to be created when necessary … any ideas on how to avoid that?

sergio-sastre · 2025-01-19T16:38:12Z

roborazzi-compose/src/main/java/com/github/takahirom/roborazzi/RoborazziCompose.kt

        @SuppressLint("VisibleForTests")
        val viewRootForTest = composeView.getChildAt(0) as ViewRootForTest
+        doBeforeCapture()


@takahirom
I'm open to other solutions, but it does have to happen at this point. That was the reason why it was not working previously: the advanceTimeBy() was being before the Composable was set as content in the Activity

sergio-sastre · 2025-01-19T16:44:03Z

...nnotations/src/main/java/com/github/takahirom/roborazzi/annotations/DelayCaptureRoboImage.kt

+// TODO -> maybe add also parameter for ignoreFrames, as used in mainClock.advanceTime()
+// TODO -> Make BINARY annotation only applicable to Methods
+// TODO -> Docu: mention about the 16ms frame in Android
+annotation class DelayCaptureRoboImage(val delayInMillis: Long)


@takahirom
Feel free to give feedback on the name ;)

I'm thinking whether it'd also make sense to add the capability to take screenshots after a set of time intervals... what could be interesting to verify animations.

I think in that case, we could create a different annotation for that like
IntervalCaptureRoboImage(intervalsInMillis: arrayOf(500, 100, 200))

If you like the idea, we could make it either in this PR or afterwards, up to you :)

Thanks. I think the name CaptureRoboImage is a little misleading when used for DelayCaptureRoboImage, because we filter Previews by @Preview and not by @DelayCaptureRoboImage. So I've renamed it to RoboManualAdvance .

As for IntervalCaptureRoboImage, could we perhaps have a repeatable annotation?
When considering IntervalCaptureRoboImage, the use of CaptureRoboImage name is understandable though.

@RoboManualAdvance(advanceTimeMillis = 100) @RoboManualAdvance(advanceTimeMillis = 200) @RoboManualAdvance(advanceTimeMillis = 500)

Thanks for the suggestion!
I am also not very convinced about RoboManualAdvance, I think it does not tell its purpose clearly.

Its purpose is to advance the clock before capturing a screenshot.
I think something similar to
AdvanceClockBeforeCapture or the like makes more sense.

as of repeatable annotations, ComposablePreviewScanner still does not support them as stated in the corresponding method:

https://github.com/sergio-sastre/ComposablePreviewScanner/blob/ea9962eada536d81b9c8cdfa3df586df03c182a9/core/src/main/java/sergio/sastre/composable/preview/scanner/core/preview/ComposablePreview.kt#L34

That lies on me since it is a bit tricky 😅.
However, I agree that is a better solution.
I can create an issue and try to address it to support that in the future

I‘m planning to release support for repeated annotations in ComposablePreviewScanner 0.5.1.

However, what would be your expectation?
Would „duplicate“ previews for each repeated annotation, or would be more like a getRepeatedAnnotation() and get a list with all annotations?
I‘m more inclined to the last one, but I‘m wondering how we could integrate it in this case 🤔

Thanks. It might be good to consider when we should add another annotation.

@RoboManualAdvance(advanceTimeMillis = 100) @RoboManualAdvance(advanceTimeMillis = 200) @RoboFoo(true) @RoboFoo(false)

We might want to take a screenshot when RoboFoo is true and RoboManualAdvance is 100. So it might be good to have one annotation like @RoboComposeOption()? For example, we could use @RoboComposeOption(autoAdvance = false, advanceTimeMillis = 100, foo = true).

initial push (not tested)

4c4646c

sergio-sastre mentioned this pull request Jan 18, 2025

Render compose component with clock based logic #605

Open

Exclude roborazzi-annotations from allprojects

7ac7567

takahirom added 2 commits January 19, 2025 14:31

Use RoborazziComposeOptions instead of doBeforeCaptureRoboImage

b87c1de

Fix delayed time logic

2764156

takahirom added 2 commits January 19, 2025 14:54

Use JDK 17

13c6c7b

Use JDK 17

31da1d4

Use ComposeTestRule for Preview Activity

2f0b89d

Push example for Preview with Delay

629241e

Push first working solution

6071e3c

sergio-sastre commented Jan 19, 2025

View reviewed changes

takahirom added 2 commits January 20, 2025 09:53

Add some content for checking PreviewDelayed

07986a5

Rename DelayCaptureRoboImage to RoboManualAdvance

1364461

takahirom mentioned this pull request Jan 20, 2025

Roborazzi 1.37.0 and above brokes almost all screenshots in our app #628

Closed

sergio-sastre mentioned this pull request Jan 22, 2025

Add support for repeatable Annotations sergio-sastre/ComposablePreviewScanner#40

Open

Merge branch 'main' into origin/feature/takahirom#605

f05360a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support DelayedPreviews (not tested, just initial push) #633

Support DelayedPreviews (not tested, just initial push) #633

sergio-sastre commented Jan 18, 2025

takahirom commented Jan 19, 2025

takahirom commented Jan 19, 2025

takahirom commented Jan 19, 2025

github-actions bot commented Jan 19, 2025 •

edited

Loading

takahirom commented Jan 19, 2025 •

edited

Loading

takahirom commented Jan 19, 2025

sergio-sastre commented Jan 19, 2025

takahirom commented Jan 19, 2025

sergio-sastre commented Jan 19, 2025

sergio-sastre Jan 19, 2025 •

edited

Loading

takahirom Jan 20, 2025

sergio-sastre Jan 20, 2025

sergio-sastre Jan 19, 2025

sergio-sastre Jan 19, 2025 •

edited

Loading

takahirom Jan 20, 2025 •

edited

Loading

sergio-sastre Jan 20, 2025

sergio-sastre Jan 22, 2025

takahirom Jan 23, 2025 •

edited

Loading

Support DelayedPreviews (not tested, just initial push) #633

Are you sure you want to change the base?

Support DelayedPreviews (not tested, just initial push) #633

Conversation

sergio-sastre commented Jan 18, 2025

takahirom commented Jan 19, 2025

takahirom commented Jan 19, 2025

takahirom commented Jan 19, 2025

github-actions bot commented Jan 19, 2025 • edited Loading

takahirom commented Jan 19, 2025 • edited Loading

takahirom commented Jan 19, 2025

sergio-sastre commented Jan 19, 2025

takahirom commented Jan 19, 2025

sergio-sastre commented Jan 19, 2025

sergio-sastre Jan 19, 2025 • edited Loading

Choose a reason for hiding this comment

takahirom Jan 20, 2025

Choose a reason for hiding this comment

sergio-sastre Jan 20, 2025

Choose a reason for hiding this comment

sergio-sastre Jan 19, 2025

Choose a reason for hiding this comment

sergio-sastre Jan 19, 2025 • edited Loading

Choose a reason for hiding this comment

takahirom Jan 20, 2025 • edited Loading

Choose a reason for hiding this comment

sergio-sastre Jan 20, 2025

Choose a reason for hiding this comment

sergio-sastre Jan 22, 2025

Choose a reason for hiding this comment

takahirom Jan 23, 2025 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Jan 19, 2025 •

edited

Loading

takahirom commented Jan 19, 2025 •

edited

Loading

sergio-sastre Jan 19, 2025 •

edited

Loading

sergio-sastre Jan 19, 2025 •

edited

Loading

takahirom Jan 20, 2025 •

edited

Loading

takahirom Jan 23, 2025 •

edited

Loading