Closed ejensen closed 10 months ago
Thank you! I spent a bunch of time recently trying to debug why we were having false positive on our CI/CD pipeline, I was able to narrow it down to trying to use perceptual precision on a virtualized MacOS and ended up putting some other bandaids in place to fix it temporarily.
I just recently switched to the revision with this change and what I see in the CI logs is that every failing test is now taking 12 seconds and more where without using precision (or setting it to 1.0) it takes ~2s
Having the same issue here using Xcode Cloud.
It's outputting No Metal renderer available
in the logs but all the tests passes.
Even when we break the UI completely, trying to force it to fail, it still passes, so I believe it's generating all-black images there as well.
I don't think any more that my issue is related to Metal not being available. In that case I would expect 100% failures, yet some tests are still failing on CI as expected but some are not.
I don't think any more that my issue is related to Metal not being available. In that case I would expect 100% failures, yet some tests are still failing on CI as expected but some are not.
With my investigations here, when this "No Metal renderer available" runs, it renders a completely blank image for any snapshot, either the newly taken or the reference one. So, the images will always match.
It'll only fail when having different dimensions/frames. Maybe it's your case
It'll only fail when having different dimensions/frames
Yes, you are most likely right about that!
@ejensen is there any changes you could get back to this? Since it's either false positives or degraded performance will have to revert to using intel machines for recording snapshots but don't want to do that if you have an idea how to solve this.
ejensen is there any changes you could get back to this? Since it's either false positives or degraded performance will have to revert to using intel machines for recording snapshots but don't want to do that if you have an idea how to solve this.
Commit 8d402bf adds the Metal-based method back. There is not a straightforward way to determine if the (virtualized) machine supports Metal, so there is a tricky bit of conditional logic that would benefit from testing on more machine configurations.
@ejensen thanks for the update, although I don't think it will help with performance when Metal is not supported. Is there a way to use a similar filter from CoreImage?
Hello, I am having the issue described here: https://github.com/pointfreeco/swift-snapshot-testing/issues/710, and this PR really fixed the problem. Great works! Is it possible to merge it as soon as possible?
@ejensen In your original PR you had used the CIColorThreshold
filter for comparison and then switched to using Metal so you could target older versions of macOS. What do you think about going back to that? We're having this same problem in our CI environment, but it would be nice to retain the speed benefits since we use macOS 12.0 on CI.
@ejensen In your https://github.com/pointfreeco/swift-snapshot-testing/pull/628 you had used the CIColorThreshold filter for comparison and then switched to using Metal so you could target older versions of macOS. What do you think about going back to that?
Sounds like we should be able to cover all bases if we add one more availability check:
MPS
is available, use that.macOS 11.0+
is available, use CIColorThreshold
.Thanks for this! And sorry for the delay. Strategy-specific PRs tend to take on a lot of complexity due to all the branching and platforms involved. We finally took some time to look things over and take the branch for a spin and it looks like a definite improvement over the buggy behavior we previously had.
Overview
Remove Metal usage since some virtualized environments don’t support it and silently fail (only logging something in the console). Replaces the CoreImage operations that require Metal with CPU-based calculation.
Context
Metal is supported by all iOS/tvOS devices (2013 models or later) and Macs (2012 models or later). Older devices that don't support Metal also do not support iOS/tvOS 13 and macOS 10.15 which are the minimum versions of swift-snapshot-testing. However, some virtualized Mac environments do not have a GPU and therefore do not support Metal. In this case, macOS falls back to a CPU-based OpenGL ES renderer that silently fails when Metal commands are issued. The output of the silent failure is a solid black image. And since the threshold filter returns black pixels when it passes, the solid black image from the silent failure causes false positives.
Related PRs