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

[feature] Element Screenshot #2012

Closed
ethanshar opened this issue Apr 21, 2020 · 12 comments · Fixed by #2226
Closed

[feature] Element Screenshot #2012

ethanshar opened this issue Apr 21, 2020 · 12 comments · Fixed by #2226

Comments

@ethanshar
Copy link

Is your feature request related to a problem? Please describe.
Recently we used screenshot tests with detox to detect breaking UI changes in our components.
In order to do that we compared screenshots of the entire screen that showcase the component.
Unfortunately, our showcase screens contain other elements other than the tested component (titles, controls, etc..).
If something has changed with those elements the screenshot test failed.
It turned those tests to be very flaky so we turned them off for now.

Describe the solution you'd like
A great solution will be to capture a screenshot of a specific element (by testID), this will help isolate to test case and prevent flakiness.

Describe alternatives you've considered
none.

Additional context
I discussed this some time ago with @amitdwix and @rotemmiz, I understand this is applicable.
Would be very beneficial for UI testing.

@LeoNatan
Copy link
Contributor

The problem with such tests is that you are relying on many different systems, and the results are bound to be flaky. You are relying on two different layout systems (UIKit/Android and RN) and two different drawing systems (UIKit/Android and RN). Other technical limitations will make it even worse for you: iOS, for example, is unable to properly screenshot certain elements, and screenshots change with every version of iOS.

@rotemmiz
Copy link
Member

We'll start with Android to see if it even makes sense, as an experimental feature. It's fairly easy to create such a screenshot with Android

@LeoNatan
Copy link
Contributor

It's not a problem to support on iOS as well. XCUITest supports it too. I'm just not sure it's wise to encourage something as part of detox which is flaky in nature. We've resisted this in the past for good reason.

@ethanshar
Copy link
Author

@LeoNatan Regarding what you said

The problem with such tests is that you are relying on many different systems, and the results are bound to be flaky. You are relying on two different layout systems (UIKit/Android and RN) and two different drawing systems (UIKit/Android and RN). Other technical limitations will make it even worse for you: iOS, for example, is unable to properly screenshot certain elements, and screenshots change with every version of iOS.

Clearly we will have separate screenshot for each system.
And even if we can measure most of the basic UI elements - it's good enough for us.
If you don't want to commit to this feature, let's release it as an experimental feature, like Rotem mentioned.

@LeoNatan
Copy link
Contributor

LeoNatan commented Apr 23, 2020

I don't mean comparing between Android and iOS. I mean comparing across iOS versions and across RN versions is already too flaky. iOS simulator now uses Metal to render, so even different GPU drivers for the host (VM GPU driver) may have influence on the output. Too flaky.

@rotemmiz
Copy link
Member

You may be right, but it's still worth a try

@henrymoulton
Copy link

henrymoulton commented Apr 24, 2020

FWIW, an automated system that takes the artifacts, uploads them to S3 and then allows designers to review changes to components would be useful for my organisation.

Automated visual testing is probably going to have levels of flakiness, allowing designers to review changes to components in a similar fashion to Code Review is useful.

It's quite common for component libraries to be designed in Figma, and then implemented as a separate NPM repository

e.g.
https://github.com/newsuk/times-components
https://github.com/artsy/palette

@LeoNatan
Copy link
Contributor

LeoNatan commented May 7, 2020

Why snapshot tests are bad:

https://twitter.com/gspiers/status/1240951050896539649?s=21

@LeoNatan
Copy link
Contributor

LeoNatan commented May 7, 2020

@stale
Copy link

stale bot commented Jun 6, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
If you believe the issue is still relevant, please test on the latest Detox and report back.

Thank you for your contributions!

For more information on bots in this reporsitory, read this discussion.

@ethanshar
Copy link
Author

Hey @LeoNatan @rotemmiz
I agree that snapshot testing might be flaky and not the best solution. (I know, we tried it)
But first, they don't replace our e2e tests in any way.
Second, I believe it is up to the users' responsibility to decide if they want to use it or not.
I don't think we need to argue on whether those kind of tests are good or bad.

Btw, the whole purpose of this specific feature request is take snapshot of specific element which should reduce visual background noise and give better results.

@LeoNatan
Copy link
Contributor

LeoNatan commented Jun 7, 2020

Providing the element snapshot is not a problem in and of itself. The problem is encouraging bad practice, which I don't want to do in Detox. This is by all means our responsibility to argue.

We're already seeing issues on Android where same problems occur as iOS:
https://stackoverflow.com/questions/62177822/mobile-emulator-simulator-detox-screenshot-artifacts-differ-when-run-on-differen

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants