swift-snapshot-testing
swift-snapshot-testing copied to clipboard
Use iOS 16 ImageRenderer to snapshot SwiftUI Views
The new ImageRenderer
type in iOS 16 is purpose built for converting SwiftUI views into images. Seems like a great fit for snapshot testing.
~~I took the approach of using ImageRenderer
whenever possible for the .image
strategy because it seems likely to produce the best results by default, but I'd be happy to discuss another approach.~~
I wan't able to get the existing images for testSwiftUIView_iOS
to pass with or without my change on any devices I tried on either Xcode 13.5 or Xcode 14. Does it require some specific setup to get the tests passing?
I'm wondering why the .sizeThatFits layout is generating a 5px wider image file?
I couldn't figure out how to generate a matching snapshots to the recorded images even on the main
branch without my changes.
The Makefile looks like the tests were designed to be run on an iPhone 11 Pro Max, but even on the versions of Xcode the CI runs on I couldn't get the tests to pass on main
.
The CI only runs make test-swift
so it hasn't been validating the iOS tests.
FYI using this renderer fixes a crash we are seeing when snapshotting a SwiftUI Map on an iOS 16 simulator.
@stephencelis I figured since I wasn't trying to match the API of .image
it'd make sense to embrace an API that more directly mirrors ImageRenderer
's properties
One of the notable changes is that .imageRenderer
doesn't match the simulator's display scale by default, instead opting for ImageRenderer
s default behavior of rendering a 1 pixel per point.
Thoughts?
@john-flanagan Will try to take this for a spin soon. Also I think you brought up .imageRender
as being a nice name for this, and I agree that it looks nicer in practice:
assertSnapshot(matching: SwiftUIView(), as: .imageRender)
What do you think of renaming?
What do you think of renaming?
I'm good with .imageRender
as well. I'll make the update
@stephencelis (or anyone else following along) I've been playing with the renderer more and I'd love your thoughts on a couple things:
- I think the
isOpaque
argument should be dropped from the current implementation and the system default background should be applied to all of the layouts.
I think this will be more intuitive for consumers since it will behave like SwiftUI Previews where transparent views still display over a default background.
Also the isOpaque
argument renders transparent pixels as black which doesn't seem like a good fit for this purpose.
- Currently we pass
.unspecified
as the defaultProposedViewSize
, which is also the default behavior ofImageRenderer
. As a result a view using.sizeThatFits
layout will be allowed to grow as large as it wants in any direction by default. For example, a really longText
view will take up as much horizontal length as it needs in order to display as a single line of text, even if that goes well beyond the user's selected simulator.
Obviously users can opt-in to device sized rendering using a .device(...)
layout, but what do you think of of making the the proposedSize
parameter optional and falling back to ProposedViewSize(UIScreen.main.bounds.size)
when it's absent?
That way the default behavior would again roughly match SwiftUI Previews where .previewLayout(.sizeThatFits)
attempts to fit the view on the current simulator's screen size, but allow it to shrink if it can.
Both .device
and .fixedSize
layouts would override the proposed size since they're laid out with an explicit .frame
.
The drawback is that the default output of .imageRender
would then be dependent on the user's selected simulator, but it would behave more like .image
and SwiftUI Previews
Hey! Thanks for the great work, I tested it with some views, and I found that it doesn't seem to work with SwiftUI.ScrollViews.
for example:
assertSnapshot(
matching: ScrollView { Text("Hello world") } ,
as: .imageRender(layout: .device(config: .iPhone12), proposedSize: .unspecified)
)
will give a white image as a result :(
Edit: Ok actually, it makes sense when reading the ImageRenderer Documentation
I guess It still makes sense to add this but maybe the documentation should specify that this won't work with any Views backed by UIKit
Hey @stephencelis, I think this PR is in pretty good shape. Mind giving it a once over?
@stephencelis 👋 looks like just an approval is not enough, it asking for "First-time contributors need a maintainer to approve running workflows" :O
Hi @stephencelis
I can't merge this branch. Is there anything else you're looking for out of this PR?
I just tried this out and could verify this to work. It helped to workaround the problem, I've encountered here. @stephencelis What's stopping this from getting merged and being shipped?
@mrackwitz Sorry for the delay in action and communication! We do try to get to core issues and bugs with SnapshotTesting when we have time, but when it comes to bugs and contributions around strategies that can be replaced in user land, we unfortunately have to put those on the back burner right now. New strategies becomes additional surface area that we might have to be on alert for, and changes to existing strategies become vectors for regressions.
We do plan on returning to SnapshotTesting this year with a revamped 2.0.0 that will be modernized, more powerful, and easier to build a plug-in ecosystem around. We also want folks to feel freer to share their own strategies as standalone libraries that can be dropped in rather than everyone being on hold/wait for us to merge them.
@john-flanagan On that note, what do you think of shipping this in a standalone package for now (that we can reference on the README), and when we return to spend more energy iterating on this library later this year we can revisit.
Regardless of a standalone library being available, though, the additions in this PR should be easily pulled into your own projects if they help you!
@stephencelis, I happy to create a new package for the imageRender
strategy. I totally get your reasoning.
One thing to consider though is that some companies make it tough to add new third party dependencies, and adding them from arbitrary Github users can be even more challenging. I don't know how much effort you're interesting in investing in accommodating annoying corporate policies (I doubt very much, and I don't blame you), but there's definitely an advantage to having a wide set of strategies ship with the library itself for some consumers of the library.
That said I just pushed up this repository: https://github.com/john-flanagan/SnapshotTestingImageRender
Thanks @stephencelis for getting back quickly and no offense from my side. I totally see getting things falling through the cracks given the sheer amount of repos you've up here. So I also totally understand your reasoning to switch gears re:community contributions and larger API surface area.
Indeed I had just pulled the imageRender
strategy in a simplified version into a separate file.
But thanks @john-flanagan for following up swiftly, then we can depend on that instead.
Any update on getting this in? Should be able to support macOS 13+ as well