swift-snapshot-testing icon indicating copy to clipboard operation
swift-snapshot-testing copied to clipboard

Image precision default value variable

Open ejensen opened this issue 1 year ago • 5 comments

Overview

A follow-up on https://github.com/pointfreeco/swift-snapshot-testing/pull/628#issuecomment-1246768428

This PR adds global variables that control the default image precision and perceptualPrecision parameter values when no value is specified in the Snapshotting.image() method.

This makes it simple to configure snapshot tests with precision values that accommodate subtle differences between device rendering. Such as the imperceptible differences in Intel and Apple Silicon hardware accelerated anti-aliasing, shadow, and blur rendering.

Alternatives considered:

Integrators could add overloading extensions that override Snapshotting.image() parameter defaults. But, this requires more code to be written by the integrator and is more susceptible to breakage and infinite recursion.

ejensen avatar Sep 22 '22 00:09 ejensen

I think this makes sense, though as you note in the alternative, nothing prevents folks from defining these overrides.

stephencelis avatar Sep 22 '22 00:09 stephencelis

We have overrides and I think it makes more sense. Global variables can be confusing/misleading sometimes.

I would love to see the second PR with the difference reporting for failures! 🙏🏼

Kaspik avatar Sep 22 '22 10:09 Kaspik

I would love to see the second PR with the difference reporting for failures! 🙏🏼

The second PR was merged and included in the 1.10.0 release.

ejensen avatar Sep 22 '22 13:09 ejensen

Bruh, I'm slow. Thanks!

Kaspik avatar Sep 22 '22 13:09 Kaspik

@ejensen Let's keep this open as a draft to gather feedback, but if others agree that a workaround is better than more global config surface area, we can close it out.

In the future, we'd love to encode more data into snapshot formats by default, and then strategies could use this to do a "best guess" for diffing. We probably won't have time to explore such a thing anytime soon, though.

stephencelis avatar Sep 22 '22 16:09 stephencelis