search

facebook / screenshot-tests-for-android

Generate fast deterministic screenshots during Android instrumentation tests
http://facebook.github.io/screenshot-tests-for-android
Apache License 2.0
1.74k stars 229 forks source link

Make accessibility info for the screenshot an optional value #250

Closed pedrovgs closed 4 years ago

pedrovgs commented 4 years ago

:pushpin: References

:tophat: What is the goal?

Make the usage of the accessibility report feature optional and configurable from the public API.

In order to properly understand the motivations behind this PR and get full context we'd recommend you reading issue #248 and the PR #249.

How is it being implemented?

We've just updated and documented the public RecordBuilder API and RecourdBuilderImpl implementation to be able to handle a new parameter related to the usage of the accessibility feature. Once this was ready, we've modified the record process to do not include the accessibility information if this configuration flag is disabled. In order to do not introduce any breaking change the parameter default value enables this feature. Last but not least, I've added end to end coverage in the sample project ensuring the library works with this optional parameter disabled.

How can it be tested?

I've created a new test in the sample project disabling this flag to ensure the library is still working in an end to end scenario. However, I don't see any CI tool configured checking the codestyle of the code or evaluating these tests so I can only ensure this worked in my machine 😃

Important considerations and questions.

We already agree this is not a fix for the original issue but a workaround to be able to use the library with views where the accessibility information generation is not working. As the error is placed in a library we don't have access to and there is no stable version fixing the exception throw in the original library we think this would be a nice workaround for this feature.

pedrovgs commented 4 years ago

Hey @xiphirx there are two Facebook Internal checks failing in this PR but I can't access the details in order to fix it. Could you please provide some feedback about how to fix them so we can get them passing and publish the fix in a new patch release?

xiphirx commented 4 years ago

Ignore those, doesn't impact the PR :)

facebook-github-bot commented 4 years ago

@xiphirx merged this pull request in facebook/screenshot-tests-for-android@49676c1eaee5efe9b6ba55584c5859dce062a33d.