-
Notifications
You must be signed in to change notification settings - Fork 108
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
Implemented Showkase + Paparazzi artifact to automate screenshot testing #294
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow this is cool! Can't wait to test it out! 😍
@@ -489,3 +507,8 @@ internal enum class ShowkaseGeneratedMetadataType { | |||
TYPOGRAPHY | |||
} | |||
|
|||
internal enum class ScreenshotTestType { | |||
SHOWKASE, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the shot
one right? Would it make sense to name this SHOT_SHOWKASE
maybe? :)
|
||
android { | ||
namespace 'com.airbnb.android.showkase.screenshot.testing.paparazzi' | ||
compileSdk 32 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it should be part of a separate PR or something, but should we bump the compile sdk and target sdk to 33 now? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neat! Generally looks good to me, only question is if some of the code-gen can be moved to the base interface / abstract class to simplify maintenence
fileBuilder.build().writeTo(environment.filer, mode = XFiler.Mode.Aggregating) | ||
} | ||
|
||
private fun TypeSpec.Builder.addPreviewProvider() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it necessary to code gen this (and some of the other objects?) They seem purely static from what I can see
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@BenSchwab That was necessary so that users could override the behavior of some of the methods declared inside the CompanionObject
interface. If I were to move these objects directly inside the interface, I'd need to change the interface to an abstract class, but at that point, I won't be able to allow overriding the behavior which is vital. Is there another way to structure this? 🤔
@@ -16,7 +16,7 @@ import java.io.File | |||
* Temporarily set this to true to have the test runner update test resource file expected outputs | |||
* instead of failing tests on mismatch. Use this to easily update expected outputs. | |||
*/ | |||
const val UPDATE_TEST_OUTPUTS = false | |||
const val UPDATE_TEST_OUTPUTS = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not 100% sure about this but should we set this to false in the master branch? Thinking since now, it will generate the test output always and not verify that the output is actually correct 🤔
Wdyt? @vinaygaba :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made #300 to address this :)
This PR implements the
showkase-screenshot-testing-paparazzi
artifact that enables complete automation of screenshot testing for your codebase using Showkase + Paparazzi.Assuming you were already using Showkase and had previews of your
@Composable
functions (by using either@Preview
or@ShowkaseComposable
), this is all the extra setup you will need to add screenshot testing to your codebase using Showkase + PaparazziWith just 3 lines of code, all the previews from your codebase will be screenshot tested using Paparazzi with appropriate defaults 🤯💥
You might have a use case where you also want to screenshot test your components in dark mode or in RTL. Doing so is equally trivial. Just implement the appropriate functions to override the default behavior.
This will now ensure that all the previews are also screenshot tested in RTL and dark mode configurations. This creates a test matrix so all permutations are screenshot tested, just like you'd expect.
Generated File
@airbnb/showkase-maintainers