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

Multiple paparazzi rules in the same test run give NullPointerException #1340

Open
eboudrant opened this issue Mar 14, 2024 · 3 comments
Open
Labels
bug Something isn't working

Comments

@eboudrant
Copy link

Description
We'd like to create our own rule to wrap some boiler plate code and help adopting some good practice by our users. One of it is to build snapshot test that cover font scale on device. Since the fontScale can only be set on the DeviceConfig I ended with a custom test rule that delegate to 2 Paparazzi rules (see code). When the test run we get a NPE because of initialization code.

app.cash.paparazzi.sample.ScaledTextTest > compositeItems FAILED
    java.lang.NullPointerException
        at app.cash.paparazzi.Paparazzi.takeSnapshots(Paparazzi.kt:285)
        at app.cash.paparazzi.Paparazzi.snapshot(Paparazzi.kt:216)
        at app.cash.paparazzi.Paparazzi.snapshot$default(Paparazzi.kt:215)
        at app.cash.paparazzi.Paparazzi.snapshot(Paparazzi.kt:211)
        at app.cash.paparazzi.sample.MyPaparazziRule.snapshot(MyPaparazziRule.kt:39)
        at app.cash.paparazzi.sample.MyPaparazziRule.snapshot$default(MyPaparazziRule.kt:34)
        at app.cash.paparazzi.sample.ScaledTextTest.compositeItems(ScaledTextTest.kt:14)

isInitialized is true for the second Paparazzi rule giving it an invalid state.

Also a side question I guess would be, how would you recommend to approach testing on variants that are at the DeviceConfig level. Ultimately we want our user to write the test once and have multiple snapshot per DeviceConfig property we care about.

Steps to Reproduce
This branch https://github.com/eboudrant/paparazzi/tree/multiple_paparazzi contains a new test app.cash.paparazzi.sample.ScaledTextTest

Expected behavior
Not crash, the test output would be 2 images

Additional information:

  • Paparazzi Version: 1.3.3 (branch created from 1.3.3 tag)
@eboudrant eboudrant added the bug Something isn't working label Mar 14, 2024
@eboudrant
Copy link
Author

eboudrant commented Mar 18, 2024

Btw, I just realized to solve the fontScale (or any attribute from the DeviceFactor) you can use @TestParameter at the test class level (see https://github.com/google/TestParameterInjector).

So paparazzi might just need a better error message than the NullPointerException noted here.

@jrodbx
Copy link
Collaborator

jrodbx commented Mar 18, 2024

Btw, I just realized ... you can use @TestParameter at the test class level...

Correct! We have an example for that here.

So paparazzi might just need a better error message than the NullPointerException noted here.

Agreed. A second instance of Paparazzi rule is invalid so that sounds like the appropriate fix here. Wanna submit a PR?

@jrodbx
Copy link
Collaborator

jrodbx commented Sep 5, 2024

For anyone interested in tackling this, the proposed approach would be to figure out the root cause of the NPE and allow multiple instances of Paparazzi to run without interfering with each other. This is especially important for the SDKifying effort.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants