From 61d27419aaddc7d09f00532a1a26b48aa1f5a3c2 Mon Sep 17 00:00:00 2001 From: Jordan Demeulenaere Date: Thu, 30 Jun 2022 13:02:31 +0200 Subject: [PATCH 1/2] Small improvements to dialogScreenshotTest {} This CL disables the enter/exit animations of the dialog about to be screenshot tested, and ensures that the dialog is dismissed at the end of the test. Test: atest SystemUIGoogleScreenshotTests Bug: 230832101 Change-Id: I30e26411042bc42181cab38ec706b8ed2a2808f7 --- .../testing/screenshot/ViewScreenshotTestRule.kt | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/packages/SystemUI/screenshot/src/com/android/systemui/testing/screenshot/ViewScreenshotTestRule.kt b/packages/SystemUI/screenshot/src/com/android/systemui/testing/screenshot/ViewScreenshotTestRule.kt index 35812e39677d2..6a80c486d5152 100644 --- a/packages/SystemUI/screenshot/src/com/android/systemui/testing/screenshot/ViewScreenshotTestRule.kt +++ b/packages/SystemUI/screenshot/src/com/android/systemui/testing/screenshot/ViewScreenshotTestRule.kt @@ -60,11 +60,17 @@ class ViewScreenshotTestRule(testSpec: ScreenshotTestSpec) : TestRule { ) { var dialog: Dialog? = null activityRule.scenario.onActivity { activity -> - // Make sure that the dialog draws full screen and fits the whole display instead of the - // system bars. dialog = dialogProvider(activity).apply { + // Make sure that the dialog draws full screen and fits the whole display + // instead of the system bars. window.setDecorFitsSystemWindows(false) + + // Disable enter/exit animations. + create() + window.setWindowAnimations(0) + + // Show the dialog. show() } } @@ -74,7 +80,11 @@ class ViewScreenshotTestRule(testSpec: ScreenshotTestSpec) : TestRule { activityRule.scenario.onActivity { // Check that the content is what we expected. val dialog = dialog ?: error("dialog is null") - screenshotRule.screenshotTest(goldenIdentifier, dialog.window.decorView) + try { + screenshotRule.screenshotTest(goldenIdentifier, dialog.window.decorView) + } finally { + dialog.dismiss() + } } } } From 6e4f63a7722afd5ab291ec264963e0b45472a7ec Mon Sep 17 00:00:00 2001 From: Jordan Demeulenaere Date: Fri, 1 Jul 2022 14:17:33 +0200 Subject: [PATCH 2/2] Do pixel-perfect mapping on x86_64 devices only This CL changes the matcher used for unit screenshot tests so that we do pixel-perfect matching only on x86_64 devices, and use the MSSIM matcher otheriwse. See comment in the code and b/237511747 for more information. Bug: 237511747 Test: atest SystemUIGoogleScreenshotTests Change-Id: Ia8915de33521035d46f1854580db830b11f94e5c --- .../testing/screenshot/ScreenshotTestRule.kt | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/packages/SystemUI/screenshot/src/com/android/systemui/testing/screenshot/ScreenshotTestRule.kt b/packages/SystemUI/screenshot/src/com/android/systemui/testing/screenshot/ScreenshotTestRule.kt index 363ce10fa36cd..564901c2a773e 100644 --- a/packages/SystemUI/screenshot/src/com/android/systemui/testing/screenshot/ScreenshotTestRule.kt +++ b/packages/SystemUI/screenshot/src/com/android/systemui/testing/screenshot/ScreenshotTestRule.kt @@ -20,6 +20,7 @@ import android.app.UiModeManager import android.content.Context import android.graphics.Bitmap import android.graphics.Canvas +import android.os.Build import android.os.UserHandle import android.view.Display import android.view.View @@ -32,6 +33,7 @@ import platform.test.screenshot.GoldenImagePathManager import platform.test.screenshot.PathConfig import platform.test.screenshot.PathElementNoContext import platform.test.screenshot.ScreenshotTestRule +import platform.test.screenshot.matchers.MSSIMMatcher import platform.test.screenshot.matchers.PixelPerfectMatcher /** @@ -55,7 +57,11 @@ class ScreenshotTestRule(private val testSpec: ScreenshotTestSpec) : TestRule { currentDisplay?.name ?: error("currentDisplay is null") }, ) - private val defaultMatcher = PixelPerfectMatcher() + private val matcher = if (shouldUsePerfectMatching()) { + PixelPerfectMatcher() + } else { + MSSIMMatcher() + } private val screenshotRule = ScreenshotTestRule( @@ -67,6 +73,17 @@ class ScreenshotTestRule(private val testSpec: ScreenshotTestSpec) : TestRule { ) ) + private fun shouldUsePerfectMatching(): Boolean { + // Different CPU architectures can sometimes end up rendering differently, so we can't do + // pixel-perfect matching on different architectures using the same golden. Given that our + // presubmits are run on cf_x86_64_phone, our goldens should be perfectly matched on the + // x86_64 architecture and use the Structural Similarity Index on others. + // TODO(b/237511747): Run our screenshot presubmit tests on arm64 instead so that we can + // do pixel perfect matching both at presubmit time and at development time with actual + // devices. + return Build.CPU_ABI == "x86_64" + } + override fun apply(base: Statement, description: Description): Statement { // The statement which call beforeTest() before running the test and afterTest() afterwards. val statement = @@ -147,7 +164,7 @@ class ScreenshotTestRule(private val testSpec: ScreenshotTestSpec) : TestRule { // device to assertBitmapAgainstGolden instead? currentDisplay = testSpec.display currentGoldenIdentifier = goldenIdentifier - screenshotRule.assertBitmapAgainstGolden(bitmap, identifierWithSpec, defaultMatcher) + screenshotRule.assertBitmapAgainstGolden(bitmap, identifierWithSpec, matcher) currentDisplay = null currentGoldenIdentifier = goldenIdentifier }