Fix reference dir #220
Closed
Fix reference dir #220
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed. If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks! |
Add back the logic to only verify screenshots and not record them when referenceDir is present.
tiagoengel
force-pushed
the
add-back-reference-dir
branch
from
21f6860 to
4a8d502
Compare
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
xiphirx
requested changes
May 16, 2019
There was a problem hiding this comment.
Thanks for fixing this!
In regards to broken master, I'm aware of that and its due to two changes (I think): AndroidX and moving towards populating + pulling a single zip instead of individual PNGs.
I'd rather fix master and release instead of patching an old release.
.idea/codeStyleSettings.xml
Outdated
| @@ -1,251 +0,0 @@ | |||
| <?xml version="1.0" encoding="UTF-8"?> | |||
There was a problem hiding this comment.
Sorry, didn't realise this. Done
| @@ -32,6 +32,8 @@ open class ScreenshotsPluginExtension { | |||
| var multipleDevices = false | |||
| /** The python executable to use */ | |||
| var pythonExecutable = "python" | |||
| /** The directory to pull screenshots from in verify only mode */ | |||
There was a problem hiding this comment.
Lets change this to
Suggested change
| /** The directory to pull screenshots from in verify only mode */ | |
| /** The directory to compare screenshots from in verify only mode */ |
|
Sure, no big deal about the current master. I'll keep using my fork in the meantime and try to help when I have some free time. I've addressed all the comments 🙂 |
xiphirx
approved these changes
May 17, 2019
There was a problem hiding this comment.
@xiphirx is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
facebook-github-bot
referenced
this pull request
in facebook/flipper
Mar 23, 2021
Summary: Bumps [core](https://github.com/facebook/screenshot-tests-for-android) from 0.5.0 to 0.13.0. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/facebook/screenshot-tests-for-android/releases">core's releases</a>.</em></p> <blockquote> <h2>0.13.0</h2> <p><code>9e3b940</code> Tentative fix for <a href="https://github.com/facebook/screenshot-tests-for-android/issues/248">https://github.com/facebook/flipper/issues/248</a> (<a href="https://github.com/facebook/screenshot-tests-for-android/issues/249">https://github.com/facebook/flipper/issues/249</a>) <code>49676c1</code> Make accessibility info for the screenshot an optional value (<a href="https://github.com/facebook/screenshot-tests-for-android/issues/250">https://github.com/facebook/flipper/issues/250</a>)</p> <h2>0.12.0</h2> <p><code>2af0200</code> Update to AGP 3.6 <code>6dedde3</code> Update gradle + wrapper to 6.2.x <code>2b37d95</code> Add option to show screenshot difference in html report</p> <h2>0.11.0</h2> <p><code>e00973d</code> Remove use of indirect obsolete api usage <code>4116c6a</code> Updated Gradle to 5.6.2 <code>4885a0e</code> Replaced use of a deprecated Gradle API (<a href="https://github.com/facebook/screenshot-tests-for-android/issues/231">https://github.com/facebook/flipper/issues/231</a>) <code>de7f430</code> Adopt Contributor Covenant <code>e897fe8</code> Added failureDir to record differences when verifying screenshots (<a href="https://github.com/facebook/screenshot-tests-for-android/issues/225">https://github.com/facebook/flipper/issues/225</a>)</p> <h2>0.10.0</h2> <p><code>4e054b0</code> Add accessibility evaluation to detect focusable views without any text to announce. <code>25f3f4c</code> Force Litho to render in accessibility mode, and bump the metadata version number <code>a919928</code> Move over to AndroidX <code>fa4da40</code> Add AccessibilityUtil class with a data structure for the accessibility tree <code>db3ef4e</code> Add the concept of a version to the metadata JSON file <code>83d010d</code> Fix reference dir (<a href="https://github.com/facebook/screenshot-tests-for-android/issues/220">https://github.com/facebook/flipper/issues/220</a>) <code>0b9e8e9</code> Fix screenshots on Android 8.0 <code>2cb4718</code> Support setting max height and width <code>ff2a939</code> Fix <a href="https://github.com/facebook/screenshot-tests-for-android/issues/192">https://github.com/facebook/flipper/issues/192</a> ScreenshotsPluginExtension's addDeps parameter doesn't work <code>f98a24e</code> Expose max pixels so we can adjust our view size if we exceed it. <code>7826f9b</code> Zip up all the output screenshots so that the puller can pull them all at once</p> <h2>0.9.0</h2> <p><code>3983392</code> Loop over serials in ANDROID_SERIAL when pulling images (<a href="https://github.com/facebook/screenshot-tests-for-android/issues/140">https://github.com/facebook/flipper/issues/140</a>) (<a href="https://github.com/facebook/screenshot-tests-for-android/issues/214">https://github.com/facebook/flipper/issues/214</a>) <code>81c8c53</code> Fix display text in the output html of a screenshot test <code>c9a7646</code> Bump min sdk to 14 <code>f7a153c</code> Check for empty children when writing a hierarchy <code>76d8cd1</code> Make tests run on devices with KitKat and earlier <code>b9ea677</code> Add Accessibility Hierarchy to Screenshot Tests <code>8ef0e18</code> Handle absence of Play Services (<a href="https://github.com/facebook/screenshot-tests-for-android/issues/203">https://github.com/facebook/flipper/issues/203</a>) <code>fe98582</code> avoid the crash on processing an anonymous view (<a href="https://github.com/facebook/screenshot-tests-for-android/issues/200">https://github.com/facebook/flipper/issues/200</a>) <code>dcb5e39</code> detailed error propagation for screenshot tests <code>ded2077</code> Close ParcelFileDescriptor when done. (<a href="https://github.com/facebook/screenshot-tests-for-android/issues/195">https://github.com/facebook/flipper/issues/195</a>) <code>d02e224</code> Catch API errors when showing current images in reports <code>39b05df</code> Trigger onGlobalLayoutListener during screenshot tests <code>7609a80</code> View before images when running screenshot tests locally <code>7dba891</code> Correct multipleDevices flag default <code>0f14f96</code> Add setMaxPixels to RecordBuilder interface (<a href="https://github.com/facebook/screenshot-tests-for-android/issues/189">https://github.com/facebook/flipper/issues/189</a>)</p> <h2>0.8.0</h2> <p><code>1411308</code> Add language feature to device name calculator <code>fadd68c</code> Always use the same output directory for reports (<a href="https://github.com/facebook/screenshot-tests-for-android/issues/169">https://github.com/facebook/flipper/issues/169</a>)</p> </blockquote> <p>... (truncated)</p> </details> <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/facebook/screenshot-tests-for-android/blob/master/CHANGELOG.md">core's changelog</a>.</em></p> <blockquote> <h2>0.13.0 (Jul 8 2020)</h2> <ul> <li>Made accessibility node information optional</li> <li>Bugfixes surrounding obtaining accessibility node information</li> </ul> <h2>0.12.0 (Mar 4 2020)</h2> <ul> <li>Added the ability to generate a diff between the old version of a screenshot and the new output in the html report. Currently only works with a configured test image API</li> <li>Added support for AGP 3.6 + Gradle 6.2.x</li> </ul> <h2>0.11.0 (Oct 17 2019)</h2> <ul> <li>Replaced direct usages of deprecated Gradle APIs</li> <li>Adopted the Contributor Covenant</li> <li>Added <code>failureDir</code> which saves the expected, actual and diff images of each failing test when verification fails.</li> </ul> <h2>0.10.0 (Jun 11 2019)</h2> <ul> <li>Added batch downloading of screenshot images instead of pulling individual files</li> <li>Added Accessibility hierarchy information</li> <li>Added ability to specify max sizes for images</li> <li>Fixed addDeps functionality parameter in the plugin</li> <li>Fixed referenceDir functionality in the plugin</li> <li>Fixed an issue on Samsung devices where a crash would occur when faking a WindowAttachment</li> <li>Migrated to AndroidX</li> </ul> <h2>0.9.0 (Apr 1 2019)</h2> <ul> <li>Added a setMaxPixels method to the record builder interface to allow for really large images</li> <li>Added an integration point to allow you to see a version of the given screenshot from a server provided service</li> <li>Fixed an issue where onGlobalLayoutListener wasn'nt being triggered properly</li> <li>Fixed an issue where a parcel file descriptor wasn't being closed</li> <li>Added the ability to dump the accessibility hierarchy</li> <li>Min SDK has been bumped to 14</li> <li>Added the ability to run tests on all connected targets</li> </ul> <h2>0.8.0 (Jul 30 2018)</h2> <ul> <li>Replaced androidTestApi with androidTestImplementation when adding in core dependency via the plugin</li> <li>Fixed a bug where requesting focus prior to being attached to a Window would crash</li> <li>Added the ability to customize the max pixel size restriction</li> <li>Moved generated report to build/ instead of /tmp</li> <li>Added language to the device name calculation for multiple devices</li> </ul> <h2>0.7.0 (Apr 19 2018)</h2> <ul> <li>Added the ability to retrieve the resulting Bitmap for custom use on your RecordBuilder</li> <li>Removed the runtime dependency on Dexmaker, this will resolve any issues of using frameworks such as Mockito in your screenshot tests</li> <li>Added a a check to fail when resultant screenshots are extremely large</li> <li>Rewrote the client plugin to provide screenshot test tasks per applicable variant</li> </ul> </blockquote> <p>... (truncated)</p> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/facebook/screenshot-tests-for-android/commit/8a99f25684f977937797abe4d02bdb3428f90de1"><code>8a99f25</code></a> 0.13.0</li> <li><a href="https://github.com/facebook/screenshot-tests-for-android/commit/80aa7b47d269d68ba45b9f26a03cd30318692d53"><code>80aa7b4</code></a> Fix expected output of integrationTest task</li> <li><a href="https://github.com/facebook/screenshot-tests-for-android/commit/9e3b940e7dee53950633719747da2689535f8ce4"><code>9e3b940</code></a> Tentative fix for <a href="https://github.com/facebook/screenshot-tests-for-android/issues/248">https://github.com/facebook/flipper/issues/248</a> (<a href="https://github.com/facebook/screenshot-tests-for-android/issues/249">https://github.com/facebook/flipper/issues/249</a>)</li> <li><a href="https://github.com/facebook/screenshot-tests-for-android/commit/28dcef98d873923ae21171f66d3a9f36dff46040"><code>28dcef9</code></a> Fix AX output</li> <li><a href="https://github.com/facebook/screenshot-tests-for-android/commit/49676c1eaee5efe9b6ba55584c5859dce062a33d"><code>49676c1</code></a> Make accessibility info for the screenshot an optional value (<a href="https://github.com/facebook/screenshot-tests-for-android/issues/250">https://github.com/facebook/flipper/issues/250</a>)</li> <li><a href="https://github.com/facebook/screenshot-tests-for-android/commit/8d8ee23b9a05bc955ac8f590acbf94cd452911d3"><code>8d8ee23</code></a> Prepare next development version</li> <li><a href="https://github.com/facebook/screenshot-tests-for-android/commit/805a93fdf2ea49af1f324112673f191b08f833ea"><code>805a93f</code></a> 0.12.0</li> <li><a href="https://github.com/facebook/screenshot-tests-for-android/commit/2af0200ea721ed6066630d0f6aaf2df2a048d71a"><code>2af0200</code></a> Update to AGP 3.6</li> <li><a href="https://github.com/facebook/screenshot-tests-for-android/commit/6dedde3f82ff5e1e4aa9095b1a32349935345f92"><code>6dedde3</code></a> Update gradle + wrapper to 6.2.x</li> <li><a href="https://github.com/facebook/screenshot-tests-for-android/commit/c7a4c8b02dccf2a1df21595fc15fc216933d096b"><code>c7a4c8b</code></a> Update Litho, Kotlin, AndroidX and material components</li> <li>Additional commits viewable in <a href="https://github.com/facebook/screenshot-tests-for-android/compare/v0.5.0...0.13.0">compare view</a></li> </ul> </details> <br /> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `dependabot rebase` will rebase this PR - `dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `dependabot merge` will merge this PR after your CI passes on it - `dependabot squash and merge` will squash and merge this PR after your CI passes on it - `dependabot cancel merge` will cancel a previously requested merge and block automerging - `dependabot reopen` will reopen this PR if it is closed - `dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> Pull Request resolved: #2057 Reviewed By: passy Differential Revision: D27230350 Pulled By: priteshrnandgaonkar fbshipit-source-id: d602e7dda009f892cb24ed660db09aebc371912a
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add back the logic to only verify screenshots and not record them when referenceDir is present.
NOTE: I can't make the current project I'm working on work on version 9 (or current master) because of some internal error finding a resource. Would you be open to release a new version for 8 with this fix too? I can send a PR for that branch too, once this is merged.
fixes #166