pointfreeco / swift-snapshot-testing

📸 Delightful Swift snapshot testing.
https://www.pointfree.co/episodes/ep41-a-tour-of-snapshot-testing
MIT License
3.72k stars 564 forks source link

Image snapshot strategies order and blurred text #222

Open darrarski opened 5 years ago

darrarski commented 5 years ago

Imagine you want to snapshot-test a single UIViewController on multiple devices. You can do it like this:

assertSnapshots(matching: viewController, as: [
    .image(on: .iPhoneSe),
    .image(on: .iPhone8),
    .image(on: .iPhoneX)
])

For some reason (unknown to me), the order of configurations in this case matters. It has to be sorted ascending by the device's screen size and scale. If I put .iPhoneX which is @3x before .iPhoneSe which is @2x the tests will not pass. The failure will be caused by blurred texts in UILabels. Not sure if this is something that can be fixed, but it's easy to work-around by always providing a correctly sorted array of configurations.

However, if you would like to provide custom names for each snapshot configuration, like this:

assertSnapshots(matching: viewController, as: [
    "iPhoneSe": .image(on: .iPhoneSe),
    "iPhone8": .image(on: .iPhone8),
    "iPhoneX": .image(on: .iPhoneX)
])

You can't really guarantee the order in which configurations are used. It's random each time you run tests, which is caused by Swift's Dictionary characteristics. In this case, your tests will randomly fail for the same reason as before - blurred or slightly misplaced texts.

It's hard for me to investigate the issue deeper and find what's the real reason for that odd behavior, but I manage to create a simple workaround. I defined another assertSnapshots function which accepts an array of tuples [(String, Snapshotting<Value, Format>)] instead of dictionary [String: Snapshotting<Value, Format>].

func assertSnapshots<Value, Format>(
    matching value: @autoclosure () throws -> Value,
    as strategies: [(String, Snapshotting<Value, Format>)],
    named name: String? = nil,
    record recording: Bool = false,
    timeout: TimeInterval = 5,
    file: StaticString = #file,
    testName: String = #function,
    line: UInt = #line) {

    try? strategies.forEach { (strategyName, strategy) in
        let name = [name, strategyName].compactMap { $0 }.joined(separator: ".")
        assertSnapshot(
            matching: try value(),
            as: strategy,
            named: name,
            record: recording,
            timeout: timeout,
            file: file,
            testName: testName,
            line: line
        )
    }
}

This way I can control the order in which strategies are used to take snapshots.

assertSnapshots(matching: viewController, as: [
    ("iPhoneSe", .image(on: .iPhoneSe)),
    ("iPhone8", .image(on: .iPhone8)),
    ("iPhoneX", .image(on: .iPhoneX))
])

Please, let me know if proposed workaround could be included in the library so that I can create a pull request.

Perhaps one could guide me with searching for the root of my problem with blurred text on snapshots, but I guess it will take some time to investigate and address. I can live with a workaround for now :-)

stephencelis commented 5 years ago

Hi @darrarski! Thanks for reporting this issue! Any chance you can push a branch with a failing test case? My first thought is that ordering shouldn't matter, so maybe we're not doing enough work to re-layout things.

darrarski commented 5 years ago

Hi @stephencelis! Unfortunately, I can't do it now, as I encountered the issue when working on a commercial, closed-source project. It could be connected to layout, which in my case is quite simple, but have some relative constraints (one view is half the size of another, etc.). I have no auto layout warning though. I will try to reproduce this behavior in example project which I can share here when I find some time.

Sherlouk commented 5 years ago

Hey @darrarski, we actually had almost the exact same issue! I didn't report it though because I thought it was very specific to the way we construct our views.

We actually just ended up having to recreate the UIViewController for every device we run on. This has made it a tonne more reliable and consistent for us with our setup (though it is a little bit slower) YMMV!

bcherry commented 3 years ago

I just found this same issue today and it was baffling! Once I figured out it was due to random ordering I was able to search and find this issue and your handy workaround (which I had been planning to sort out myself). Still an issue in 2021.