feat(macOS): deterministically snapshot `NSView`s on any device #533

tillhainbach · 2021-11-06T19:24:16Z

Deterministically snapshot `NSView`s on any device

Summary

This PR builds upon #412 but maintains compatibility with existing API-behaviour. Additionally to fixing backingScaleFactor issues, 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 NSView is determined by its .size, .appearance .window.backingScaleFactor, and window.colorSpace. This PR tackles the later two and adds a GenericWindow subclass of NSWindow which can be configured with a specific backingScaleFactor and colorSpace. The .image snapshot strategy on NSView is extended to allow the caller to optionally set a specific NSAppearance and to render the view in a GenericWindow. This window can be configure to use CI compatible values (eg. backingScaleFactor: 1.0 and colorSpace: .genericRGB). There is even a handy preconfigured static instance GenericWindow.ci.

Added Tests

Two tests are added to showcase the CI-compatibility. These are the CI-compatible version of pre-existing macOS NSView tests:

testNSViewCICompatible
testNSViewWithLayerCICompatible

For passing tests see tillhainbach#3

Caveats/Gotchas

The current implementation for .image for NSView has side-effect on the view which are not undone. This means that the calling order for .image and .recursiveDescription strategy 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:

related to light/dark mode setting on test machine. I refactor the failing tests to explicitly use .aqua (light mode) appearance (the one used for provided target snapshot)
related to number formatting using localised formatting (decimal separator in my case). I changed the number formatter to use the a . 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

closes Improved reproducibility and customizable of NSView snapshots #412
fixes issues with Support SwiftUI snapshots on macOS with NSImage #477. Supporting SwiftUI, however, could be simplified to just pullback to NSView.
closes How to snapshot a view on macOS with provided screen scale? #428
closes Snapshot taken on CI has slight color differences compared to local #313
closes macOS Screenshots with different Resolutions #305

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.

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.

Jeehut · 2022-06-21T18:42:39Z

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?

stephencelis

Looks really good! Thanks for PRing and sorry for the long overdue review.

I've rebased things and gotten CI building again (we want to run against a matrix of Xcode versions, so it'll just be less reliable for now).

stephencelis · 2022-09-22T00:19:10Z

Sources/SnapshotTesting/Snapshotting/NSView.swift

+    perceptualPrecision: Float = 1,
+    size: CGSize? = nil,
+    appearance: NSAppearance? = NSAppearance(named: .aqua),
+    windowForDrawing: GenericWindow? = nil


A few questions about this.

Any reason why this is nil by default and not a GenericWindow() 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 GenericWindow a private concern of the library and instead introduce options for backingScaleFactor and colorSpace? Would we be missing out on other options?

Alternately, could this be a more generic NSWindow and then we could default it to be the deterministic 2x scale and RGB color space?

Obbut · 2023-03-07T13:25:02Z

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

Obbut · 2023-03-07T13:26:14Z

If the comments from @stephencelis are blocking to get this merged, I'd be happy to create a new PR with that feedback implemented

tillhainbach · 2023-03-07T19:54:22Z

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 🥲.

brzzdev · 2023-10-23T11:54:27Z

@Obbut Did you ever get the time to look at this? These look like great changes

as seen in pointfreeco#533

doozMen · 2024-08-08T06:02:08Z

Sources/SnapshotTesting/Snapshotting/CGPath.swift

@@ -123,6 +123,7 @@ extension Snapshotting where Value == CGPath, Format == String {

 private let defaultNumberFormatter: NumberFormatter = {
  let numberFormatter = NumberFormatter()
+  numberFormatter.decimalSeparator = "."


moved this changes into #889

doozMen · 2024-08-08T06:02:14Z

Sources/SnapshotTesting/Snapshotting/NSBezierPath.swift

@@ -87,6 +87,7 @@ extension Snapshotting where Value == NSBezierPath, Format == String {

 private let defaultNumberFormatter: NumberFormatter = {
  let numberFormatter = NumberFormatter()
+  numberFormatter.decimalSeparator = "."


moved this changes into #889

* fix formatting of numbers to have stable snapshots as seen in #533 * fix to make tests compile #887 --------- Co-authored-by: stijn <[email protected]>

Credit goes to pointfreeco/swift-snapshot-testing#533

* Snapshot view to debug UI tests in CI * Always run snapshot upload (to see snapshot on fail) * Standardize AppKitBackend CI rendering env's dpi and color space Credit goes to pointfreeco/swift-snapshot-testing#533

tillhainbach added 13 commits November 5, 2021 16:01

[WIP] ci(workflow): test macos image snapshots on ci

68d27bb

[WIP] ci(workflow): upload failing artifacts

24f7afe

[WIP] ci(workflow): fix artifact path in upload step

e31bd9f

feat(nsview): add option to specify appearance

f8b7dee

fix(cg-path, ns-bezier-path): explicitly set decimal separator

ce077a1

Avoid failing tests if system localisation uses a different decimal separator (e.g. "," for German region)

test(nsview): fix appearance to .aqua

5d08eee

This avoids failing tests if host macOS is in dark mode.

test(navies): add a CI-compatible snapshot test

90e90d7

test(nsview): add snapshots for CI-campatible test

c639e36

feat(nsview): add option to set appearance, backingScaleFactor and co…

c3f2b91

…lorSpace Setting all three values allows for device independent snapshotting for macOS

test(nsview): set size to fixed value

df55d47

test(nsview): fix failing recursiveDescription test

23fa4d6

To get CI test running with `recursiveDescription` the view size must be set explicitly.

refactor(nsview): add convenience GenericWindow preconfigured for CI

02643ce

[WIP] test(nsview): add more ci-compatible nsview-tests

a8ca3ac

tillhainbach force-pushed the fix-macos branch from e203a44 to a8ca3ac Compare November 6, 2021 19:37

tillhainbach mentioned this pull request Nov 6, 2021

Support SwiftUI snapshots on macOS with NSImage #477

Closed

stephencelis and others added 4 commits June 21, 2022 15:23

Merge branch 'main' into fix-macos

fc8cb07

Merge branch 'main' into fix-macos

efe7011

Update ci.yml

9259a19

Remove CI-compatible tests for now

4e92761

stephencelis reviewed Sep 22, 2022

View reviewed changes

firetop0011 approved these changes Feb 8, 2023

View reviewed changes

thecoolwinter mentioned this pull request Jan 16, 2024

Editor View Tests CodeEditApp/CodeEdit#1564

Closed

6 tasks

nickromano mentioned this pull request Apr 5, 2024

tillhainbach's fix for macOS nickromano/swift-snapshot-testing#1

Open

doozMen mentioned this pull request Aug 8, 2024

Possible Strategy to use SwiftUI snapshots on a mac with ImageRenderer #888

Open

doozMen pushed a commit to doozMen/swift-snapshot-testing that referenced this pull request Aug 8, 2024

fix formatting of numbers to have stable snapshots

e470e77

as seen in pointfreeco#533

doozMen mentioned this pull request Aug 8, 2024

Feature/fixed decimal separator #889

Merged

doozMen reviewed Aug 8, 2024

View reviewed changes

stephencelis pushed a commit that referenced this pull request Aug 8, 2024

Feature/fixed decimal separator (#889)

384b1c9

* fix formatting of numbers to have stable snapshots as seen in #533 * fix to make tests compile #887 --------- Co-authored-by: stijn <[email protected]>

stackotter added a commit to stackotter/swift-cross-ui that referenced this pull request Mar 10, 2025

Standardize AppKitBackend CI rendering env's dpi and color space

11fce56

Credit goes to pointfreeco/swift-snapshot-testing#533

stackotter mentioned this pull request Mar 10, 2025

Fix basic AppKitBackend layout test in CI stackotter/swift-cross-ui#112

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(macOS): deterministically snapshot `NSView`s on any device #533

feat(macOS): deterministically snapshot `NSView`s on any device #533

tillhainbach commented Nov 6, 2021 •

edited

Loading

Jeehut commented Jun 21, 2022

stephencelis left a comment

stephencelis Sep 22, 2022

Obbut commented Mar 7, 2023

Obbut commented Mar 7, 2023

tillhainbach commented Mar 7, 2023

brzzdev commented Oct 23, 2023

doozMen Aug 8, 2024

doozMen Aug 8, 2024

feat(macOS): deterministically snapshot NSViews on any device #533

Are you sure you want to change the base?

feat(macOS): deterministically snapshot NSViews on any device #533

Conversation

tillhainbach commented Nov 6, 2021 • edited Loading

Deterministically snapshot NSViews on any device

Summary

In Detail

Added Tests

Caveats/Gotchas

Minor Fixes along the way

Other

Related PRs and Issues

Jeehut commented Jun 21, 2022

stephencelis left a comment

Choose a reason for hiding this comment

stephencelis Sep 22, 2022

Choose a reason for hiding this comment

Obbut commented Mar 7, 2023

Obbut commented Mar 7, 2023

tillhainbach commented Mar 7, 2023

brzzdev commented Oct 23, 2023

doozMen Aug 8, 2024

Choose a reason for hiding this comment

doozMen Aug 8, 2024

Choose a reason for hiding this comment

feat(macOS): deterministically snapshot `NSView`s on any device #533

feat(macOS): deterministically snapshot `NSView`s on any device #533

tillhainbach commented Nov 6, 2021 •

edited

Loading

Deterministically snapshot `NSView`s on any device