Roborazzi Test for Markdown Playground #1064

kateliu20 · 2024-10-28T15:42:44Z

I've added a Roborazzi test for Compose Desktop support (based on the suggestions here). I also added the necessary dependencies for testing and compose.

…i test

kateliu20 · 2024-10-28T15:44:25Z

...pose/playground/src/jvmMain/kotlin/foundry/intellij/compose/playground/MarkdownPlayground.kt

+  singleWindowApplication { AppContent() }
+}
+
+// created function for testing


I added a comment here for now since testing on the function wrapped in a singleWindowApplication seemed to result in an infinite loop (where the test wouldn't end). I made a composable AppContent() function with the same content and added a test tag too.

I don't think we need the comment, feels obvious in source why it's there :)

...layground/src/jvmTest/kotlin/foundry/intellij/compose/playground/MarkdownPlaygroundKtTest.kt

kateliu20 · 2024-10-28T15:45:50Z

platforms/intellij/compose/playground/build.gradle.kts

+      dependencies {
+        implementation(libs.junit)
+        implementation(libs.roborazzi)
+        implementation(compose.desktop.common)


I added compose dependencies to the jvmTest. Before I added these dependencies, I had a java.lang.NoClassDefFoundError.

…-gradle-plugin into kl/roborazzi_markdown_test

kateliu20 · 2024-10-28T16:06:40Z

platforms/intellij/compose/playground/build.gradle.kts

+        implementation(libs.junit)
+        implementation(libs.roborazzi)
+        implementation(compose.desktop.common)
+        implementation(compose.desktop.uiTestJUnit4)


Would this also be the correct compose desktop test dependency to add?

What does the sample in roborazzi use? Probably easiest to just match that to start

It seems here they don't have dependencies in jvmTest, but they have these in the commonMain dependencies:

implementation(compose.runtime) implementation(compose.desktop.currentOs) implementation(project(":roborazzi-painter")) implementation(libs.kotlin.stdlib.jdk8) api libs.compose.ui.test.junit4.desktop implementation libs.compose.ui.graphics.desktop

That's the library itself, we should look at one of their samples. I was eyeing this: https://github.com/takahirom/roborazzi/blob/main/sample-compose-desktop-jvm/build.gradle.kts

ZacSweers

I would expect some screenshots to be checked in, are you going to add them? We should also enable git-lfs and enforce snapshots use it, you can see some example setup here: slackhq/circuit#204

gradle/libs.versions.toml

ZacSweers · 2024-10-28T16:05:03Z

platforms/intellij/compose/playground/build.gradle.kts

@@ -56,3 +66,7 @@ configurations
  .configureEach { attributes { attribute(KotlinPlatformType.attribute, KotlinPlatformType.jvm) } }

 dependencies { lintChecks(libs.composeLints) }
+
+tasks.withType<KotlinCompile>().configureEach {
+  compilerOptions { freeCompilerArgs.add("-Xcontext-receivers") }


Let's add a comment explaining why this is needed, and let's limit it to test JVM compilations.

tasks.named { it.name == "compileJvmTestKotlin" }...

Should be soooomething like that

Do I need to register compileJvmTestKotlin task? It's not in the list of gradlew tasks.

the name might be slightly different, you'll want to find the task name for the task that compiles test jvm sources

jvmTestClasses is not a type of KotlinCompile as far as I know. Do you see this actually being configured there? I looked and I'm think what you want is compileTestKotlinJvm, but if it works without adding this opt-in at all then we can omit it?

I found the task compileTestKotlinJvm while running ./gradlew tasks --all , so I committed it here: e1ebfee. Is this the task we are looking for? I think it didn't show up when just running ./gradlew tasks

yes, in general you are gonna be better off not relying on ./gradlew tasks vs just running a task you know will run it and adding --dry-run to the end, which will just print all the tasks it's gonna run. You can also look at a build scan to see task types.

...pose/playground/src/jvmMain/kotlin/foundry/intellij/compose/playground/MarkdownPlayground.kt

...layground/src/jvmTest/kotlin/foundry/intellij/compose/playground/MarkdownPlaygroundKtTest.kt

.../playground/src/jvmTest/kotlin/foundry/intellij/compose/playground/MarkdownPlaygroundTest.kt

hellohj · 2024-10-28T16:20:17Z

.../playground/src/jvmTest/kotlin/foundry/intellij/compose/playground/MarkdownPlaygroundTest.kt

+class MarkdownPlaygroundKtTest {
+  @OptIn(ExperimentalTestApi::class)
+  @Test
+  fun test() = runTest {


Bonus point. This test will test a light mode by default. If you want to test a dark mode, you can also try @Config(qualifiers = "+night") in another test.

Do you know what dependency I need for this Config? I wasn't able to run the Roborazzi test due to a missing dependency, even though I have robolectric and androidx-testing-core. Is this annotation mobile specific?

hellohj · 2024-10-28T16:25:09Z

.../playground/src/jvmTest/kotlin/foundry/intellij/compose/playground/MarkdownPlaygroundTest.kt

+    runDesktopComposeUiTest {
+      setContent { AppContent() }
+
+      val roborazziOptions =


If you define Roborazzi options/rule globally like this, it will be easy to add more tests and you don't need to pass roborazziOptions whenever you call captureRoboImage() API.

Is RoborazziRule supposed to be provided by the Roborazzi plugin? It was asking me to create my own class for RoborazziRule when I tried to add it.

yes, you need to add this. testing-roborazzi-rules = { module = "io.github.takahirom.roborazzi:roborazzi-junit-rule", version.ref = "roborazzi" }

These are things we use in our slack-android-ng. you may need to see which additional ones are necessary.

testing-roborazzi = { module = "io.github.takahirom.roborazzi:roborazzi", version.ref = "roborazzi" } testing-roborazzi-rules = { module = "io.github.takahirom.roborazzi:roborazzi-junit-rule", version.ref = "roborazzi" } testing-roborazzi-compose = { module = "io.github.takahirom.roborazzi:roborazzi-compose", version.ref = "roborazzi" } testing-roborazzi-core = { module = "io.github.takahirom.roborazzi:roborazzi-core", version.ref = "roborazzi" }

…ons.

kateliu20 · 2024-10-29T15:41:39Z

I would expect some screenshots to be checked in, are you going to add them? We should also enable git-lfs and enforce snapshots use it, you can see some example setup here: slackhq/circuit#204

I'm having trouble getting the screenshots to actually generate. The test is successfully running, but the captureRoboImage() calls aren't producing any image files on my end. I am also trying to call it to a directory but nothing gets produced. I feel like the build files are set up but am I missing something in the test to produce images?

ZacSweers · 2024-10-31T16:02:40Z

platforms/intellij/compose/playground/build.gradle.kts

+}
+
+roborazzi {
+  outputDir = file("src/jvmTest/kotlin/foundry/intellij/compose/playground/snapshots/images")


Is there a property input of this? We should ideally use this in gradle for lazier eval

outputDir.set(layout.projectDirectory.dir("src/jvmTest/kotlin/foundry/intellij/compose/playground/snapshots/images"))

if outputDir is just a File and not a Property<File>, then you can still use the above but just write .asFile.get() at the end

outputDir = layout.projectDirectory.dir("src/jvmTest/kotlin/foundry/intellij/compose/playground/snapshots/images").asFile.get()

ZacSweers · 2024-10-31T16:02:53Z

gradle/libs.versions.toml

@@ -139,11 +143,17 @@ okio-fakefilesystem = { module = "com.squareup.okio:okio-fakefilesystem", versio
 oshi = "com.github.oshi:oshi-core:6.6.5"
 retrofit = { module = "com.squareup.retrofit2:retrofit", version.ref = "retrofit" }
 retrofit-converters-wire = { module = "com.squareup.retrofit2:converter-wire", version.ref = "retrofit" }
+robolectric = { group = "org.robolectric", name = "robolectric", version.ref = "robolectric" }


talked offline but we shouldn't need this

.github/workflows/ci.yml

takahirom · 2024-11-01T16:00:10Z

.github/workflows/ci.yml

@@ -41,7 +44,7 @@ jobs:
      - name: Build and run tests
        id: gradle
        timeout-minutes: 10
-        run: ./gradlew check
+        run: ./gradlew check verifyRoborazzi


This is just a personal observation, but I prefer using verifyRoborazzi**Debug** since the test runs twice - once for debug and once for release.

I apologize,since this might be a JVM module, there may not be any build types available. 🙇

.github/workflows/ci.yml

ZacSweers · 2024-11-11T19:14:01Z

.github/workflows/ci.yml

@@ -54,6 +57,7 @@ jobs:
          name: reports
          path: |
            **/build/reports/**
+            **/src/jvmTest/snapshots/**/*_compare.png


.github/workflows/ci.yml

platforms/intellij/compose/gradle.properties

...pose/playground/src/jvmMain/kotlin/foundry/intellij/compose/playground/MarkdownPlayground.kt

.../playground/src/jvmTest/kotlin/foundry/intellij/compose/playground/MarkdownPlaygroundTest.kt

ZacSweers · 2024-11-11T19:17:08Z

.../playground/src/jvmTest/kotlin/foundry/intellij/compose/playground/MarkdownPlaygroundTest.kt

+    onRoot().captureRoboImage(roborazziOptions = roborazziOptions)
+
+    onNodeWithTag("dark-mode-toggle").performClick()
+
+    onRoot().captureRoboImage(roborazziOptions = roborazziOptions)


...apshots/images/foundry.intellij.compose.playground.MarkdownPlaygroundTest.test_2_compare.png

ZacSweers

in the future let's split this up into multiple commits. Having to search in the commit for the bits referenced by a comment adds a bit of extra work to reviewers 🙂

kateliu20 and others added 9 commits October 24, 2024 13:57

Initial commit, add roborazzi dependencies

4bc6b9e

First attempt at adding SlackRoborazziTest

8f49433

Add compose desktop roborazzi dependency

9d10d3d

Change roborazzi module and version

45ac073

Change main function structure and write basic (but failing) roborazz…

7217eeb

…i test

Add dependencies to jvmTest

b75371f

Modify MarkdownPlayground to have a composable app content for testing.

0f9a04a

Add roborazzi test

0a4b6e9

Merge branch 'main' into kl/roborazzi_markdown_test

376c598

kateliu20 commented Oct 28, 2024

View reviewed changes

...layground/src/jvmTest/kotlin/foundry/intellij/compose/playground/MarkdownPlaygroundKtTest.kt Outdated Show resolved Hide resolved

kateliu20 commented Oct 28, 2024

View reviewed changes

kateliu20 added 3 commits October 28, 2024 11:53

Add modifier to AppContent

c3c167d

Merge branch 'kl/roborazzi_markdown_test' of github.com:slackhq/slack…

d54a281

…-gradle-plugin into kl/roborazzi_markdown_test

Run spotless

09db2b7

kateliu20 requested review from jhperezd, ZacSweers and linhpha October 28, 2024 16:01

kateliu20 commented Oct 28, 2024

View reviewed changes

ZacSweers reviewed Oct 28, 2024

View reviewed changes

Rename test file

8f9b8ea

hellohj reviewed Oct 28, 2024

View reviewed changes

.../playground/src/jvmTest/kotlin/foundry/intellij/compose/playground/MarkdownPlaygroundTest.kt Outdated Show resolved Hide resolved

hellohj reviewed Oct 28, 2024

View reviewed changes

kateliu20 added 3 commits October 28, 2024 12:25

Remove junit from libs.versions.toml and default changeThreshold opti…

60903cf

…ons.

Rename to MarkdownPlayground

3099434

Add robolectric and testing dependencies

3a0b3a7

kateliu20 added 2 commits October 29, 2024 16:00

specify roborazzi screenshots in folder

eabadf3

Add path to build

77a9e74

ZacSweers reviewed Oct 31, 2024

View reviewed changes

.github/workflows/ci.yml Show resolved Hide resolved

ZacSweers reviewed Oct 31, 2024

View reviewed changes

.github/workflows/ci.yml Outdated Show resolved Hide resolved

kateliu20 added 10 commits October 31, 2024 12:04

Remove robolectric dependency

7a13dfd

Add validation task and rearrange tasks

b7b18ff

Modify outputdir

4bcd716

Add snapshot image path

0e4bb3b

Add kotlin test dependency

ab6ec98

Try verifyRoborazziJvm

2e046f4

Try running compare

3974141

remove specific directory and see if verify works.

6eba561

run without gradlew check

48d5397

Add output

815e184

takahirom reviewed Nov 1, 2024

View reviewed changes

kateliu20 and others added 4 commits November 7, 2024 09:34

Upgrade roborazzi for output dirs

79d4622

upgrade roborazzi version

b0a3161

Merge branch 'main' into kl/roborazzi_markdown_test

da67e7b

Remove verifyRoborazzi check

a1895aa

kateliu20 requested review from ZacSweers and hellohj November 7, 2024 14:53

ZacSweers reviewed Nov 11, 2024

View reviewed changes

kateliu20 force-pushed the kl/roborazzi_markdown_test branch from 364ff5b to a1895aa Compare November 11, 2024 23:23

kateliu20 and others added 2 commits November 11, 2024 18:29

Clean up code and remove files

992b80d

Merge branch 'main' into kl/roborazzi_markdown_test

5bc98ea

ZacSweers approved these changes Nov 12, 2024

View reviewed changes

kateliu20 added this pull request to the merge queue Nov 12, 2024

Merged via the queue into main with commit ad748a6 Nov 12, 2024
3 checks passed

kateliu20 deleted the kl/roborazzi_markdown_test branch November 12, 2024 14:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Roborazzi Test for Markdown Playground #1064

Roborazzi Test for Markdown Playground #1064

kateliu20 commented Oct 28, 2024

kateliu20 Oct 28, 2024

ZacSweers Oct 31, 2024

kateliu20 Oct 28, 2024

kateliu20 Oct 28, 2024

ZacSweers Oct 28, 2024

kateliu20 Oct 28, 2024

ZacSweers Oct 29, 2024

ZacSweers left a comment

ZacSweers Oct 28, 2024

kateliu20 Oct 29, 2024

ZacSweers Oct 30, 2024

ZacSweers Oct 30, 2024

kateliu20 Oct 30, 2024 •

edited

Loading

ZacSweers Oct 30, 2024

hellohj Oct 28, 2024

kateliu20 Oct 30, 2024

hellohj Oct 28, 2024

kateliu20 Oct 28, 2024

hellohj Oct 28, 2024

hellohj Oct 28, 2024

kateliu20 commented Oct 29, 2024

ZacSweers Oct 31, 2024

ZacSweers Oct 31, 2024

takahirom Nov 1, 2024

takahirom Nov 1, 2024 •

edited

Loading

ZacSweers Nov 11, 2024

ZacSweers Nov 11, 2024

ZacSweers left a comment

Roborazzi Test for Markdown Playground #1064

Roborazzi Test for Markdown Playground #1064

Conversation

kateliu20 commented Oct 28, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ZacSweers left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kateliu20 Oct 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kateliu20 commented Oct 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

takahirom Nov 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ZacSweers left a comment

Choose a reason for hiding this comment

kateliu20 Oct 30, 2024 •

edited

Loading

takahirom Nov 1, 2024 •

edited

Loading