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

Add preview test #97

Merged
merged 27 commits into from
Sep 15, 2022
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
f483c93
add paparazzi & showcase
sobaya-0141 Sep 2, 2022
48802bb
add test module
sobaya-0141 Sep 2, 2022
df249fe
add preview parameter
sobaya-0141 Sep 2, 2022
5fd9475
fixed spotless
sobaya-0141 Sep 2, 2022
94224d0
fixed sdk32
sobaya-0141 Sep 3, 2022
d133d4c
test record snapshots
sobaya-0141 Sep 3, 2022
23ac965
Merge branch 'main' into add_preview_test
sobaya-0141 Sep 3, 2022
01b8392
remove record snapshots
sobaya-0141 Sep 3, 2022
1809b9c
use snapshot1.1.0
sobaya-0141 Sep 4, 2022
93208e4
Just use 31
takahirom Sep 4, 2022
9373776
add workflow
sobaya-0141 Sep 5, 2022
f3248d6
Merge branch 'main' into add_preview_test
sobaya-0141 Sep 5, 2022
19bef16
remove screenshot test in Build.yml
sobaya-0141 Sep 5, 2022
bc17d49
Merge branch 'main' into add_preview_test
sobaya-0141 Sep 6, 2022
50e642b
fixded review comment
sobaya-0141 Sep 6, 2022
38f01f1
update screenshots
sobaya-0141 Sep 6, 2022
69ae467
Merge branch 'main' into add_preview_test
sobaya-0141 Sep 6, 2022
dff7c71
revert UpdateScreenshots.yml
sobaya-0141 Sep 6, 2022
a31ddd4
Merge branch 'main' into add_preview_test
sobaya-0141 Sep 8, 2022
cdea948
fixed build error
sobaya-0141 Sep 8, 2022
b487928
Merge branch 'main' into add_preview_test
takahirom Sep 14, 2022
0d8f6ef
update setup-java version
sobaya-0141 Sep 14, 2022
f6debee
Fix JDK setup error
aqua-ix Sep 14, 2022
8dce810
Fix showkase build error
aqua-ix Sep 14, 2022
eece508
Migrate to primitive plugins
aqua-ix Sep 14, 2022
513aaa2
Remove unused attributes
aqua-ix Sep 14, 2022
a675e42
Merge pull request #467 from aqua-ix/refine-preview-test
takahirom Sep 15, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 59 additions & 0 deletions .github/workflows/ScreenShotTest.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
on:
push:
branches:
- main
Copy link

@hkusu hkusu Sep 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

push:
  branches:
    - main

This may not be necessary, because it should work only at the time of pull request as it is only for pull requests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're absolutely right.

pull_request:

jobs:
screenshot-test:
runs-on: ubuntu-latest
timeout-minutes: 60

steps:
- name: Checkout
uses: actions/checkout@v3

- name: Validate Gradle Wrapper
uses: gradle/wrapper-validation-action@v1

- name: Copy CI gradle.properties
run: mkdir -p ~/.gradle ; cp .github/ci-gradle.properties ~/.gradle/gradle.properties

- name: Set up JDK 11
uses: actions/setup-java@v1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] Would you keep using v1 action? or just forget changing to v3?
I can see the v3.x.x in the action releases

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to know the your thought if you have. 🙇

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you are right, it should be v3.x.x.:pray:

with:
java-version: 11

- name: Setup Gradle
uses: gradle/gradle-build-action@v2

- name: Download Screenshots
id: download-artifact
uses: dawidd6/action-download-artifact@v2
with:
workflow: UpdateScreenshots.yml
name: screenshots
path: preview-screenshots/src/test/snapshots/images

- name: Run screenshot tests
id: verifyPaparazziDebug
continue-on-error: true
run: ./gradlew preview-screenshots:verifyPaparazziDebug

- name: Error Screenshot tests
continue-on-error: true
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may not be necessary, because the verifyPaparazziDebug step specifies continue-on-error: true, so it will always succeed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I forgot to delete this one:sweat_smile:

if: ${{ hashFiles('preview-screenshots/out/failures/*.png') != '' }}
uses: actions/upload-artifact@v3
with:
name: scrennshot-test-results
path: preview-screenshots/out/failures

- name: Comment PR
continue-on-error: true
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may not be necessary, too.

if: ${{ hashFiles('preview-screenshots/out/failures/*.png') != '' }}
uses: thollander/actions-comment-pull-request@v1
with:
message: |
There are differences in Compose previews. Please check your build and download the diff artifact.
https://github.com/DroidKaigi/conference-app-2022/actions/workflows/ScreenShotTest.yml
Copy link

@hkusu hkusu Sep 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be kinder to write https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your comment.
I will fix it the way you taught me.

GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
25 changes: 25 additions & 0 deletions .github/workflows/UpdateScreenshots.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
on:
pull_request:
branches:
- main
types: [closed]

jobs:
update-screenshot:
runs-on: ubuntu-latest
if: github.event.pull_request.merged == true

steps:
- uses: actions/checkout@v3
- uses: actions/setup-java@v1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with:
java-version: '11'
- name: Run Update Screenshots
run: ./gradlew recordPaparazziDebug

- name: Upload Screenshots
uses: actions/upload-artifact@v3
with:
name: screenshots
path: preview-screenshots/src/test/snapshots/images
retention-days: 15
5 changes: 5 additions & 0 deletions core-ui/build.gradle.kts
Original file line number Diff line number Diff line change
@@ -1,14 +1,19 @@
// TODO: Remove once https://youtrack.jetbrains.com/issue/KTIJ-19369 is fixed
@Suppress("DSL_SCOPE_VIOLATION")
plugins {
id("droidkaigi.primitive.android")
id("droidkaigi.primitive.android.kotlin")
id("droidkaigi.primitive.android.compose")
id("droidkaigi.primitive.android.hilt")
id("droidkaigi.primitive.molecule")
id("droidkaigi.primitive.spotless")
alias(libs.plugins.ksp)
Copy link
Member

@takahirom takahirom Sep 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you create "droidkaigi.primitive.android.compose.showkase"?

}

android.namespace = "io.github.droidkaigi.confsched2022.template.core.ui"

dependencies {
implementation(libs.accompanistPager)
implementation(libs.showkase.runtime)
ksp(libs.showkase.processor)
}
6 changes: 6 additions & 0 deletions feature-contributors/build.gradle.kts
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
// TODO: Remove once https://youtrack.jetbrains.com/issue/KTIJ-19369 is fixed
@Suppress("DSL_SCOPE_VIOLATION")
plugins {
id("droidkaigi.convention.androidfeature")
alias(libs.plugins.ksp)
}

android.namespace = "io.github.droidkaigi.confsched2022.feature.contributors"
Expand All @@ -23,4 +26,7 @@ dependencies {
androidTestImplementation(libs.composeUiTestJunit4)
debugImplementation(libs.composeUiTooling)
debugImplementation(libs.composeUiTestManifest)

implementation(libs.showkase.runtime)
ksp(libs.showkase.processor)
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ import io.github.droidkaigi.confsched2022.designsystem.theme.KaigiScaffold
import io.github.droidkaigi.confsched2022.designsystem.theme.KaigiTheme
import io.github.droidkaigi.confsched2022.designsystem.theme.KaigiTopAppBar
import io.github.droidkaigi.confsched2022.feature.contributors.ContributorsUiModel.ContributorsState.Loaded
import io.github.droidkaigi.confsched2022.model.Contributor
import io.github.droidkaigi.confsched2022.model.fakes
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[spotless] reported by reviewdog 🐶

Suggested change
import io.github.droidkaigi.confsched2022.model.Contributor
import io.github.droidkaigi.confsched2022.model.fakes


@Composable
fun ContributorsScreenRoot(
Expand Down Expand Up @@ -95,6 +97,13 @@ fun Contributors(
@Composable
fun ContributorsPreview() {
KaigiTheme {
ContributorsScreenRoot()
Contributors(
uiModel = ContributorsUiModel(
Loaded(
contributors = Contributor.fakes()
)
),
onNavigationIconClick = {}
)
}
}
6 changes: 6 additions & 0 deletions feature-sessions/build.gradle.kts
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
// TODO: Remove once https://youtrack.jetbrains.com/issue/KTIJ-19369 is fixed
@Suppress("DSL_SCOPE_VIOLATION")
plugins {
id("droidkaigi.convention.androidfeature")
alias(libs.plugins.ksp)
}

android.namespace = "io.github.droidkaigi.confsched2022.feature.sessions"
Expand All @@ -26,4 +29,7 @@ dependencies {
androidTestImplementation(libs.composeUiTestJunit4)
debugImplementation(libs.composeUiTooling)
debugImplementation(libs.composeUiTestManifest)

implementation(libs.showkase.runtime)
ksp(libs.showkase.processor)
}
9 changes: 9 additions & 0 deletions gradle/libs.versions.toml
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ kluent = "1.68"
coilCompose="2.2.0"
koin = "3.2.0"
androidXChromeCustomTabs = "1.3.0"
ksp = "1.7.10-1.0.6"
paparazzi = "1.1.0-SNAPSHOT"
showkase = "1.0.0-beta13"
testParameterInjector = "1.8"

[libraries]
androidGradlePlugin = { group = "com.android.tools.build", name = "gradle", version.ref = "androidGradlePlugin" }
Expand Down Expand Up @@ -120,9 +124,14 @@ androidxTestEspressoEspressoCore = { module = "androidx.test.espresso:espresso-c
hiltAndroidTesting = { group = "com.google.dagger", name = "hilt-android-testing", version.ref = "dagger" }
robolectric = { module = "org.robolectric:robolectric", version.ref = "robolectric" }
kluentAndroid = {module = "org.amshove.kluent:kluent-android", version.ref = "kluent" }
showkase-runtime = { group = "com.airbnb.android", name = "showkase", version.ref = "showkase" }
showkase-processor = { group = "com.airbnb.android", name = "showkase-processor", version.ref = "showkase" }
testParameterInjector = { group = "com.google.testparameterinjector", name = "test-parameter-injector", version.ref = "testParameterInjector" }

[plugins]
androidGradlePlugin = { id = "com.android.application", version.ref = "androidGradlePlugin" }
androidGradleLibraryPlugin = { id = "com.android.library", version.ref = "androidGradlePlugin" }
kotlinPlugin = { id = "org.jetbrains.kotlin.android", version.ref = "kotlin" }
hiltGradlePlugin = { id = "com.google.dagger.hilt.android", version.ref = "dagger" }
ksp = { id = "com.google.devtools.ksp", version.ref = "ksp" }
paparazzi = { id = "app.cash.paparazzi", version.ref = "paparazzi" }
1 change: 1 addition & 0 deletions preview-screenshots/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
/build
47 changes: 47 additions & 0 deletions preview-screenshots/build.gradle.kts
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// TODO: Remove once https://youtrack.jetbrains.com/issue/KTIJ-19369 is fixed
@Suppress("DSL_SCOPE_VIOLATION")
plugins {
id("droidkaigi.primitive.android")
id("droidkaigi.primitive.android.kotlin")
id("droidkaigi.primitive.android.compose")
id("droidkaigi.primitive.molecule")
id("droidkaigi.primitive.spotless")
alias(libs.plugins.ksp)
alias(libs.plugins.paparazzi)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you create "droidkaigi.primitive.android.paparazzi"?

}

android.namespace = "io.github.droidkaigi.confsched2022.template.preview.screenshots"

androidComponents {
// Disable release builds for this test-only library, no need to run screenshot tests more than
// once
beforeVariants(selector().withBuildType("release")) { builder ->
builder.enable = false
}
beforeVariants(selector().withBuildType("prod")) { builder ->
builder.enable = false
}
}

dependencies {

implementation(project(":core-ui"))
implementation(project(":feature-contributors"))
implementation(project(":feature-sessions"))

implementation(libs.composeUi)
implementation(libs.showkase.runtime)
ksp(libs.showkase.processor)

testImplementation(projects.coreTesting)
testImplementation(libs.testParameterInjector)
}

tasks.named("check") {
dependsOn("verifyPaparazziDemoDebug")
}

tasks.withType<Test>().configureEach {
// Increase memory for Paparazzi tests
maxHeapSize = "2g"
}
Empty file.
21 changes: 21 additions & 0 deletions preview-screenshots/proguard-rules.pro
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# Add project specific ProGuard rules here.
# You can control the set of applied configuration files using the
# proguardFiles setting in build.gradle.
#
# For more details, see
# http://developer.android.com/guide/developing/tools/proguard.html

# If your project uses WebView with JS, uncomment the following
# and specify the fully qualified class name to the JavaScript interface
# class:
#-keepclassmembers class fqcn.of.javascript.interface.for.webview {
# public *;
#}

# Uncomment this to preserve the line number information for
# debugging stack traces.
#-keepattributes SourceFile,LineNumberTable

# If you keep the line number information, uncomment this to
# hide the original source file name.
#-renamesourcefileattribute SourceFile
4 changes: 4 additions & 0 deletions preview-screenshots/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<?xml version="1.0" encoding="utf-8"?>
<manifest xmlns:android="http://schemas.android.com/apk/res/android">

</manifest>
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
package io.github.droidkaigi.confsched2022

import androidx.compose.foundation.layout.Box
import androidx.compose.runtime.Composable
import androidx.compose.runtime.CompositionLocalProvider
import androidx.compose.ui.platform.LocalDensity
import androidx.compose.ui.platform.LocalInspectionMode
import androidx.compose.ui.unit.Density
import app.cash.paparazzi.DeviceConfig
import app.cash.paparazzi.Paparazzi
import app.cash.paparazzi.androidHome
import app.cash.paparazzi.detectEnvironment
import com.airbnb.android.showkase.models.Showkase
import com.airbnb.android.showkase.models.ShowkaseBrowserComponent
import com.google.testing.junit.testparameterinjector.TestParameter
import com.google.testing.junit.testparameterinjector.TestParameter.TestParameterValuesProvider
import com.google.testing.junit.testparameterinjector.TestParameterInjector
import org.junit.Rule
import org.junit.Test
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for confirmation 👀 , is Paparazzi possible to working with JUnit5? 🤔 if possible, other engineer can use modern toolchain...! ✨

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't check this one.
I would like to get it working first.:pray:

import org.junit.runner.RunWith

class ComponentPreview(
private val showkaseBrowserComponent: ShowkaseBrowserComponent
) {
val content: @Composable () -> Unit = showkaseBrowserComponent.component
override fun toString(): String = showkaseBrowserComponent.componentKey
}

@RunWith(TestParameterInjector::class)
class PreviewScreenshotTests {

object PreviewProvider : TestParameterValuesProvider {
override fun provideValues(): List<ComponentPreview> =
Showkase.getMetadata().componentList.map(::ComponentPreview)
}

enum class BaseDeviceConfig(
val deviceConfig: DeviceConfig,
) {
NEXUS_5(DeviceConfig.NEXUS_5),
PIXEL_5(DeviceConfig.PIXEL_5),
PIXEL_C(DeviceConfig.PIXEL_C),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] How about consider to adding the Pixel6 or Nexus 10 ? The Pixel6 is latest device, The Nexus 10 is a one of the tablet devices(I'm not sure we are suppressed to support fro Table devices)

Copy link
Contributor

@fumiya-kume fumiya-kume Sep 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, sorry. I never know the Pixel C is tablet device...! bit old device than my engineer life...!

}

@get:Rule
val paparazzi = Paparazzi(
environment = detectEnvironment().copy(
platformDir = "${androidHome()}/platforms/android-31",
compileSdkVersion = 31
),
maxPercentDifference = 0.0,
)

@Test
fun preview_tests(
@TestParameter(valuesProvider = PreviewProvider::class) componentPreview: ComponentPreview,
@TestParameter baseDeviceConfig: BaseDeviceConfig,
@TestParameter(value = ["1.0", "1.5"]) fontScale: Float
) {
paparazzi.unsafeUpdateConfig(
baseDeviceConfig.deviceConfig.copy(
softButtons = false,
)
)
paparazzi.snapshot {
CompositionLocalProvider(
LocalInspectionMode provides true,
LocalDensity provides Density(
density = LocalDensity.current.density,
fontScale = fontScale
)
) {
Box {
componentPreview.content()
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package io.github.droidkaigi.confsched2022

import com.airbnb.android.showkase.annotation.ShowkaseRoot
import com.airbnb.android.showkase.annotation.ShowkaseRootModule

@ShowkaseRoot
class ScreenshotShowkaseModule : ShowkaseRootModule
6 changes: 4 additions & 2 deletions settings.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ pluginManagement {
gradlePluginPortal()
google()
mavenCentral()
// maven("https://oss.sonatype.org/content/repositories/snapshots/")
maven("https://oss.sonatype.org/content/repositories/snapshots/")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we make it a plugin, we'll be able to remove this? 👀

}
}
enableFeaturePreview("TYPESAFE_PROJECT_ACCESSORS")
Expand All @@ -14,6 +14,7 @@ dependencyResolutionManagement {
repositories {
google()
mavenCentral()
maven("https://oss.sonatype.org/content/repositories/snapshots/")
// for datastore-okio
// maven(url = "https://androidx.dev/snapshots/builds/8938977/artifacts/repository") {
// content {
Expand All @@ -36,7 +37,8 @@ include(
":core-designsystem",
":core-data",
":core-testing",
":core-model"
":core-model",
":preview-screenshots"
)

// for iOS framework name
Expand Down