Make sure that perceptuallyCompare fails if Metal is not available#702
Open
mschuetz72 wants to merge 2 commits into
Open
Make sure that perceptuallyCompare fails if Metal is not available#702mschuetz72 wants to merge 2 commits into
perceptuallyCompare fails if Metal is not available#702mschuetz72 wants to merge 2 commits into
Conversation
peteralt
approved these changes
Feb 9, 2023
|
+1 for this. I wish I'd seen this sooner as I did a parallel investigation in #710 on Bitrise. |
|
Hi, We are facing the same issues in our company, seeing that this fix is already in place and working in this branch, are you planning on pushing it soon to the main branch then release it ? this would truly save a lot of people some headaches! Thanks! |
Contributor
|
This would be very useful. I just ran into this on CircleCI, I had no idea our snapshot tests weren't doing anything on CI even though recording was working correctly 🤦🏻 |
NachoSoto
referenced
this pull request
Dec 19, 2023
* wip * wip * wip * wip * wip * wip * wip * wip * wip * wip * wip * fix * beta 6 * wip * wip * wip * wip * wip * wip * wip * wip * wip * wip * wip * wip * fix * wip * wip * wip * wip * wip * wip * wip * wip * wip * small things * wip * DocC + swift-format (#765) * DocC and swift-format support * wip * wip * wip --------- Co-authored-by: Brandon Williams <mbrandonw@hey.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
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.
What
Metal is currently unavailable on Github Actions. The
perceptuallyComparediffing algorithm relies on Metal render to obtain the level of correspondence to compare it with the given threshold. Snapshot assertions will falsely succeed, even if a given image does not match the stored reference image.Why
The snapshot assertion should fail if it hasn't got the requirements to execute the tests. Currently, the only indication that something's wrong is hidden inside the logs, stating
I'm aware that there are other initiatives, such as #666, which would be perfect, but until this is solved I think it's important to make people aware that the assertions might be wrong when relying on virtual machines without Metal support.
any real issue, any real inconsistency will be undetected when using
perceptuallyCompareand running on a virtual machine without Metal support.How
Added a pre-condition check to
perceptuallyComparewhich will fail when Metal is unavailable.Testing
Any image assertion using
perceptuallyComparerunning on an environment without Metal support. See also https://github.com/mschuetz-viz/DemoSnapshotTest