feat(macOS): deterministically snapshot NSViews on any device
#533
Conversation
Avoid failing tests if system localisation uses a different decimal separator (e.g. "," for German region)
This avoids failing tests if host macOS is in dark mode.
…lorSpace Setting all three values allows for device independent snapshotting for macOS
To get CI test running with `recursiveDescription` the view size must be set explicitly.
tillhainbach
force-pushed
the
fix-macos
branch
from
e203a44
to
a8ca3ac
Compare
|
What's the state of this? Is there any plan to getting this merged? Looks like there's no SwiftUI support on macOS yet. With Apple getting serious with SwiftUI on macOS now with Ventura (the new Settings app was written with SwiftUI it looks like), I'd love to see proper support for SwiftUI in this lib as well. Are there any blockers right now? |
| perceptualPrecision: Float = 1, | ||
| size: CGSize? = nil, | ||
| appearance: NSAppearance? = NSAppearance(named: .aqua), | ||
| windowForDrawing: GenericWindow? = nil |
There was a problem hiding this comment.
A few questions about this.
- Any reason why this is
nilby default and not aGenericWindow()that produces more deterministic snapshots? Is it just to keep compatibility with existing snaps? We're prepping a release soon that has other bug fixes that may break some existing snapshots, so we are OK making it quite clear in the release notes that upon upgrading, folks should regenerate and audit their snapshot suites. - Is it possible to make
GenericWindowa private concern of the library and instead introduce options forbackingScaleFactorandcolorSpace? Would we be missing out on other options? - Alternately, could this be a more generic
NSWindowand then we could default it to be the deterministic 2x scale and RGB color space?
|
Are there any plans to get this merged? For anyone finding and needing this before it is merged, I created a branch in fork which basically combines this PR as well as #477: https://github.com/Obbut/swift-snapshot-testing/tree/updated-swiftui-macos |
|
If the comments from @stephencelis are blocking to get this merged, I'd be happy to create a new PR with that feedback implemented |
Would be great if you can take over. I was wanting to address this but I can't find the time since I haven't been working on anything swift related 🥲. |
|
@Obbut Did you ever get the time to look at this? These look like great changes |
Deterministically snapshot
NSViews on any deviceSummary
This PR builds upon #412 but maintains compatibility with existing API-behaviour. Additionally to fixing
backingScaleFactorissues, it also fixes the color space issues discussed in #477. Therefore all relevant rendering parameters can be set in a deterministic way allowing macOS snapshot test to run on any device including CI.In Detail
As far as I can tell from experimenting, the rendering of
NSViewis determined by its.size,.appearance.window.backingScaleFactor, andwindow.colorSpace. This PR tackles the later two and adds aGenericWindowsubclass ofNSWindowwhich can be configured with a specificbackingScaleFactorandcolorSpace. The.imagesnapshot strategy onNSViewis extended to allow the caller to optionally set a specificNSAppearanceand to render the view in aGenericWindow. This window can be configure to use CI compatible values (eg.backingScaleFactor: 1.0andcolorSpace: .genericRGB). There is even a handy preconfigured static instanceGenericWindow.ci.Added Tests
Two tests are added to showcase the CI-compatibility. These are the CI-compatible version of pre-existing macOS NSView tests:
testNSViewCICompatibletestNSViewWithLayerCICompatibleFor passing tests see tillhainbach#3
Caveats/Gotchas
The current implementation for
.imageforNSViewhas side-effect on the view which are not undone. This means that the calling order for.imageand.recursiveDescriptionstrategy matters. I added a note to the doc comment.Minor Fixes along the way
I noticed some test failures on my local machine that where:
.aqua(light mode) appearance (the one used for provided target snapshot).as the decimal separator.Other
This PR adds an additional step to the GitHub actions which uploads any failing snapshot artefacts. This is great for debugging failures on CI.
Related PRs and Issues
NSViewsnapshots #412NSImage#477. Supporting SwiftUI, however, could be simplified to just pullback to NSView.PS: I left the WIP and intermediate commits to improve comprehensibility for review but will squash them once this PR is approved to reduce the noise.