From c66a38057ef7a05ecec2510881f04ce134bd1ed3 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 25 Aug 2022 11:12:49 +0100 Subject: [PATCH 01/14] adding paparazzi screenshot testing lib setup --- build.gradle | 1 + dependencies_groups.gradle | 6 ++++++ gradle.properties | 1 + vector/build.gradle | 1 + 4 files changed, 9 insertions(+) diff --git a/build.gradle b/build.gradle index 78cc80c291..479a6b6e25 100644 --- a/build.gradle +++ b/build.gradle @@ -33,6 +33,7 @@ buildscript { classpath "org.jetbrains.dokka:dokka-gradle-plugin:1.7.10" classpath "org.jetbrains.kotlinx:kotlinx-knit:0.4.0" classpath 'com.jakewharton:butterknife-gradle-plugin:10.2.3' + classpath 'app.cash.paparazzi:paparazzi-gradle-plugin:1.0.0' // NOTE: Do not place your application dependencies here; they belong // in the individual module build.gradle files } diff --git a/dependencies_groups.gradle b/dependencies_groups.gradle index e44e0bc5c2..a97d80bc7f 100644 --- a/dependencies_groups.gradle +++ b/dependencies_groups.gradle @@ -49,6 +49,7 @@ ext.groups = [ regex: [ ], group: [ + 'app.cash.paparazzi', 'ch.qos.logback', 'com.adevinta.android', 'com.airbnb.android', @@ -150,11 +151,14 @@ ext.groups = [ 'it.unimi.dsi', 'jakarta.activation', 'jakarta.xml.bind', + 'javax.activation', 'javax.annotation', 'javax.inject', + 'javax.xml.bind', 'jline', 'jp.wasabeef', 'junit', + 'kxml2', 'me.saket', 'net.bytebuddy', 'net.java', @@ -183,11 +187,13 @@ ext.groups = [ 'org.hamcrest', 'org.jacoco', 'org.java-websocket', + 'org.jcodec', 'org.jetbrains', 'org.jetbrains.dokka', 'org.jetbrains.intellij.deps', 'org.jetbrains.kotlin', 'org.jetbrains.kotlinx', + 'org.jetbrains.trove4j', 'org.json', 'org.jsoup', 'org.junit', diff --git a/gradle.properties b/gradle.properties index 0e561faa8d..438c27d6f0 100644 --- a/gradle.properties +++ b/gradle.properties @@ -17,6 +17,7 @@ org.gradle.caching=true # Android Settings android.enableJetifier=true android.useAndroidX=true +android.jetifier.ignorelist=android-base-common,common #Project Settings # Change debugPrivateData to true for debugging diff --git a/vector/build.gradle b/vector/build.gradle index 3a923f1b5c..61190a2fa2 100644 --- a/vector/build.gradle +++ b/vector/build.gradle @@ -3,6 +3,7 @@ apply plugin: 'kotlin-android' apply plugin: 'kotlin-parcelize' apply plugin: 'kotlin-kapt' apply plugin: 'dagger.hilt.android.plugin' +apply plugin: 'app.cash.paparazzi' if (project.hasProperty("coverage")) { apply plugin: 'jacoco' From 9fd77044a9f375d6f04d686de407fd049967d364 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 14 Sep 2022 11:50:47 +0100 Subject: [PATCH 02/14] including git lfs config for picking up screenshot test recordings --- .gitattributes | 1 + 1 file changed, 1 insertion(+) create mode 100644 .gitattributes diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 0000000000..0542767eff --- /dev/null +++ b/.gitattributes @@ -0,0 +1 @@ +**/snapshots/**/*.png filter=lfs diff=lfs merge=lfs -text From 9ef497502541b78df1be150ed86e63ac4aedd905 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 14 Sep 2022 13:18:01 +0100 Subject: [PATCH 03/14] adding github action to validate screenshot files are commited with git lfs --- .github/workflows/validate-lfs.yml | 15 +++++++++++++++ tools/validate_lfs.sh | 31 ++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+) create mode 100644 .github/workflows/validate-lfs.yml create mode 100755 tools/validate_lfs.sh diff --git a/.github/workflows/validate-lfs.yml b/.github/workflows/validate-lfs.yml new file mode 100644 index 0000000000..203ecb0481 --- /dev/null +++ b/.github/workflows/validate-lfs.yml @@ -0,0 +1,15 @@ +name: Validate Git LFS + +on: [pull_request] + +jobs: + build: + runs-on: ubuntu-latest + name: Validate + steps: + - uses: actions/checkout@v3 + with: + lfs: 'true' + + - run: | + ./tools/validate_lfs.sh diff --git a/tools/validate_lfs.sh b/tools/validate_lfs.sh new file mode 100755 index 0000000000..3777a009f5 --- /dev/null +++ b/tools/validate_lfs.sh @@ -0,0 +1,31 @@ +# +# Copyright (c) 2022 New Vector Ltd +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +#! /bin/bash + +# Compare the output of `git ls-files ':(attr:filter=lfs)'` against `git lfs ls-files` +# If there's no diff we assume the files have been committed using git lfs +diff <(git ls-files ':(attr:filter=lfs)' | sort) <(git lfs ls-files -n | sort) >/dev/null + +ret=$? + +if [[ $ret -ne 0 ]]; then + echo >&2 "Detected files committed without using Git LFS." + echo >&2 "Install git lfs (eg brew install git-lfs) and run 'git lfs install --local' within the root repository directory and re-commit your files." + exit 1 +fi + + From 0f19726fe21e6049b2819d48524a2ba32d29edcc Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 14 Sep 2022 14:05:40 +0100 Subject: [PATCH 04/14] creating custom tasks to record and verify screenshot - introduces a 'screenshot' flag to include/exclude the screenshot tests from the default test runs --- build.gradle | 22 ++++++++++++++++++++++ vector/build.gradle | 3 ++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 479a6b6e25..0e87ab51be 100644 --- a/build.gradle +++ b/build.gradle @@ -296,3 +296,25 @@ dependencyAnalysis { } } } + +task recordScreenshots(type: GradleBuild) { + startParameter.projectProperties.screenshot = "" + tasks = [':vector:recordPaparazziGplayDebug'] +} + +task verifyScreenshots(type: GradleBuild) { + startParameter.projectProperties.screenshot = "" + tasks = [':vector:verifyPaparazziGplayDebug'] +} + +ext.initScreenshotTests = { project -> + project.apply plugin: 'app.cash.paparazzi' + project.android.testOptions.unitTests.all { + def screenshotTestCapture = "**/*ScreenshotTest*" + if (project.hasProperty("screenshot")) { + include screenshotTestCapture + } else { + exclude screenshotTestCapture + } + } +} diff --git a/vector/build.gradle b/vector/build.gradle index 61190a2fa2..ff0d907212 100644 --- a/vector/build.gradle +++ b/vector/build.gradle @@ -3,7 +3,6 @@ apply plugin: 'kotlin-android' apply plugin: 'kotlin-parcelize' apply plugin: 'kotlin-kapt' apply plugin: 'dagger.hilt.android.plugin' -apply plugin: 'app.cash.paparazzi' if (project.hasProperty("coverage")) { apply plugin: 'jacoco' @@ -25,6 +24,8 @@ project.android.buildTypes.all { buildType -> ] } +initScreenshotTests(project) + android { // Due to a bug introduced in Android gradle plugin 3.6.0, we have to specify the ndk version to use // Ref: https://issuetracker.google.com/issues/144111441 From 7f8cb4b1a12f461374db975d5d670a4c53f30696 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 14 Sep 2022 14:12:52 +0100 Subject: [PATCH 05/14] ignoring the screenshot failure results from git --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index f1c0b99b58..aae906afc2 100644 --- a/.gitignore +++ b/.gitignore @@ -22,3 +22,4 @@ /package.json /yarn.lock /node_modules +**/out/failures From b0e2596b586e23e8b438a8dee8e206654a7ac76d Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 14 Sep 2022 15:09:14 +0100 Subject: [PATCH 06/14] including screenshot verification and error result uploads as part of the test step --- .github/workflows/tests.yml | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index fb8e3080ae..3ed21f5568 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -30,6 +30,19 @@ jobs: distribution: 'adopt' java-version: '11' - uses: gradle/gradle-build-action@v2 + + - name: Run screenshot tests + run ./gradlew verifyScreenshots + + - name: Archive Screenshot Results on Error + if: failure() + uses: actions/upload-artifact@v1 + with: + name: screenshot-results + path: | + **/out/failures/ + **/build/reports/tests/*UnitTest/ + - uses: actions/setup-python@v4 with: python-version: 3.8 From 243ca01924e35da6b74d30644ff72416c0b1f7ae Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 14 Sep 2022 15:29:21 +0100 Subject: [PATCH 07/14] including lfs files when checking out during tests workflow --- .github/workflows/tests.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 3ed21f5568..48ec140a64 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -24,6 +24,7 @@ jobs: steps: - uses: actions/checkout@v3 with: + lfs: true fetch-depth: 0 - uses: actions/setup-java@v3 with: From c19b359f439e377eca804b44d9391c0b097cab7c Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 14 Sep 2022 17:46:24 +0100 Subject: [PATCH 08/14] adding docs for creating, recording and verifying screenshot tests --- docs/screenshot_testing.md | 72 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) create mode 100644 docs/screenshot_testing.md diff --git a/docs/screenshot_testing.md b/docs/screenshot_testing.md new file mode 100644 index 0000000000..d892948913 --- /dev/null +++ b/docs/screenshot_testing.md @@ -0,0 +1,72 @@ +# Screenshot testing + + + +* [Overview](#overview) +* [Setup](#setup) +* [Recording](#recording) +* [Verifying](#verifying) +* [Contributing](#contributing) +* [Example](#example) + + + +## Overview + +- Screenshot tests are tests which record the content of a rendered screen and verify subsequent runs to check if the screen renders differently. +- Element uses [Paparazzi](https://github.com/cashapp/paparazzi) to render, record and verify android layouts. +- The screenshot verification occurs on every pull request as part of the `tests.yml` workflow. + +## Setup + +- Install Git LFS through your package manager of choice (`brew install git-lfs` | `yay -S git-lfs`). +- Install the Git LFS hooks into the project. + +```bash +# with element-android as the current working directory +git lfs install --local +``` + +- If installed correctly, `git push` and `git pull` will now include LFS content. + +## Recording + +- `./gradlew recordScreenshots` +- Paparazzi will generate images in `${module}/src/test/snapshots`, which will need to be committed to the repository using Git LFS. + +## Verifying + +- `./gradlew verifyScreenshots` +- In the case of failure, Paparazzi will generate images in `${module}/out/failure`. The images will show the expected and actual screenshots along with a delta of the two images. + +## Contributing + +- When creating a test, the file (and class) name names must include `ScreenshotTest`, eg `ItemScreenshotTest`. +- After creating the new test, record and commit the newly rendered screens. +- `./tools/validate_lfs` can be ran to ensure everything is working correctly with Git LFS, the CI also runs this check. + +### Example + +```kotlin +class PaparazziExampleScreenshotTest { + + @get:Rule + val paparazzi = Paparazzi( + deviceConfig = PIXEL_3, + theme = "Theme.Vector.Light", + ) + + @Test + fun `example paparazzi test`() { + // Inflate the layout + val view = paparazzi.inflate(R.layout.item_radio) + + // Bind data to the view + view.findViewById(R.id.actionTitle).text = paparazzi.resources.getString(R.string.room_settings_all_messages) + view.findViewById(R.id.radioIcon).setImageResource(R.drawable.ic_radio_on) + + // Record the bound view + paparazzi.snapshot(view) + } +} +``` From 1fab6e69c52236aabc3735ce8643ad18f551d071 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 14 Sep 2022 17:48:38 +0100 Subject: [PATCH 09/14] adding origin of bash script url --- tools/validate_lfs.sh | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tools/validate_lfs.sh b/tools/validate_lfs.sh index 3777a009f5..ce121057b6 100755 --- a/tools/validate_lfs.sh +++ b/tools/validate_lfs.sh @@ -1,3 +1,5 @@ +#! /bin/bash + # # Copyright (c) 2022 New Vector Ltd # @@ -13,19 +15,15 @@ # See the License for the specific language governing permissions and # limitations under the License. # - -#! /bin/bash +# Based on https://cashapp.github.io/paparazzi/#git-lfs # Compare the output of `git ls-files ':(attr:filter=lfs)'` against `git lfs ls-files` # If there's no diff we assume the files have been committed using git lfs diff <(git ls-files ':(attr:filter=lfs)' | sort) <(git lfs ls-files -n | sort) >/dev/null ret=$? - if [[ $ret -ne 0 ]]; then echo >&2 "Detected files committed without using Git LFS." echo >&2 "Install git lfs (eg brew install git-lfs) and run 'git lfs install --local' within the root repository directory and re-commit your files." exit 1 fi - - From 469dd46a4520e478a28079871268c940e98a16ea Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 14 Sep 2022 17:54:55 +0100 Subject: [PATCH 10/14] adding changelog entry --- changelog.d/5798.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/5798.misc diff --git a/changelog.d/5798.misc b/changelog.d/5798.misc new file mode 100644 index 0000000000..40185eac0d --- /dev/null +++ b/changelog.d/5798.misc @@ -0,0 +1 @@ +Adds screenshot testing tooling From 7740404b1430d32282ed7289db2c1bba8e107af7 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 14 Sep 2022 18:12:11 +0100 Subject: [PATCH 11/14] using ## heading for the examples block --- docs/screenshot_testing.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/screenshot_testing.md b/docs/screenshot_testing.md index d892948913..93b91cdf67 100644 --- a/docs/screenshot_testing.md +++ b/docs/screenshot_testing.md @@ -45,7 +45,7 @@ git lfs install --local - After creating the new test, record and commit the newly rendered screens. - `./tools/validate_lfs` can be ran to ensure everything is working correctly with Git LFS, the CI also runs this check. -### Example +## Example ```kotlin class PaparazziExampleScreenshotTest { From 7d18d89a611ef992bcbb690c7b9c04310d08c2e8 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 15 Sep 2022 09:56:50 +0100 Subject: [PATCH 12/14] adding missing colon to yml run param --- .github/workflows/tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 48ec140a64..5208a38683 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -33,7 +33,7 @@ jobs: - uses: gradle/gradle-build-action@v2 - name: Run screenshot tests - run ./gradlew verifyScreenshots + run: ./gradlew verifyScreenshots - name: Archive Screenshot Results on Error if: failure() From dc0599f9660e135946ff74eb37920d54cbf1d564 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Fri, 16 Sep 2022 16:14:36 +0100 Subject: [PATCH 13/14] removing jetifier screenshot config as the jetifier is no longer needed --- gradle.properties | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/gradle.properties b/gradle.properties index 438c27d6f0..ded5a43e28 100644 --- a/gradle.properties +++ b/gradle.properties @@ -15,9 +15,8 @@ org.gradle.vfs.watch=true org.gradle.caching=true # Android Settings -android.enableJetifier=true +android.enableJetifier=false android.useAndroidX=true -android.jetifier.ignorelist=android-base-common,common #Project Settings # Change debugPrivateData to true for debugging From c0baa2e8fd7d5b1673d44c73f83db386bd716e5b Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Fri, 16 Sep 2022 16:15:10 +0100 Subject: [PATCH 14/14] making use of the lazy task registration to allow only applying the paparazzi plugin when needed --- build.gradle | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/build.gradle b/build.gradle index 0e87ab51be..7b67c89502 100644 --- a/build.gradle +++ b/build.gradle @@ -297,21 +297,24 @@ dependencyAnalysis { } } -task recordScreenshots(type: GradleBuild) { +tasks.register("recordScreenshots", GradleBuild) { startParameter.projectProperties.screenshot = "" - tasks = [':vector:recordPaparazziGplayDebug'] + tasks = [':vector:recordPaparazziDebug'] } -task verifyScreenshots(type: GradleBuild) { +tasks.register("verifyScreenshots", GradleBuild) { startParameter.projectProperties.screenshot = "" - tasks = [':vector:verifyPaparazziGplayDebug'] + tasks = [':vector:verifyPaparazziDebug'] } ext.initScreenshotTests = { project -> - project.apply plugin: 'app.cash.paparazzi' + def hasScreenshots = project.hasProperty("screenshot") + if (hasScreenshots) { + project.apply plugin: 'app.cash.paparazzi' + } project.android.testOptions.unitTests.all { def screenshotTestCapture = "**/*ScreenshotTest*" - if (project.hasProperty("screenshot")) { + if (hasScreenshots) { include screenshotTestCapture } else { exclude screenshotTestCapture