pointfreeco / swift-snapshot-testing

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

When enabling drawHierarchyInKeyWindow, view's safeAreaInsets may not be set correctly #728

Open RLEE0731 opened 1 year ago

RLEE0731 commented 1 year ago

Describe the bug

For our use case, we need to enable drawHierarchyInKeyWindow to test 3rd party UI framework and generate visual diff results. As such, we're extending swift-snapshot-testing/Sources/SnapshotTesting/Snapshotting/UIViewController's image function, and passing in the boolean to enable drawHierarchyInKeyWindow:

extension Snapshotting where Value == UIViewController, Format == UIImage {
    /*
     Copied from swift-snapshot-testing/Sources/SnapshotTesting/Snapshotting/UIViewController
     Exposing drawHierarchyInKeyWindow
     */
    public static func image(
        drawHierarchyInKeyWindow: Bool,
        on config: ViewImageConfig,
        precision: Float = 1,
        perceptualPrecision: Float = 0.98,
        size: CGSize? = nil,
        traits: UITraitCollection = .init()
    )
    -> Snapshotting {
        print("config's safe area: \(config.safeArea)")
        return SimplySnapshotting.image(precision: precision, perceptualPrecision: perceptualPrecision, scale: traits.displayScale).asyncPullback { viewController in
            snapshotView(
                config: size.map { .init(safeArea: config.safeArea, size: $0, traits: config.traits) } ?? config,
                drawHierarchyInKeyWindow: drawHierarchyInKeyWindow,
                traits: traits,
                view: viewController.view,
                viewController: viewController
            )
        }
    }
}

With drawHierarchyInKeyWindow enabled, when running snapshot tests on a mix of iPhone and iPad strategies in landscape, it seems that if previous device's safe area is .zero, it breaks the the next device's safe area.

For example, from the provided project:

final class MyAppTests: XCTestCase {

    func testSnapshot() throws {        
        assertSnapshots(matching: NamesTableViewController(), as: [
            "ipad-mini-landscape" : .image(drawHierarchyInKeyWindow: true, on: .iPadMini(.landscape)),
            "iphone-se-landscape" : .image(drawHierarchyInKeyWindow: true, on: .iPhoneSe(.landscape))
        ])
    }
}

If the snapshot test runs the iPad Mini strategy first (which has 20 top offset), then the test will correctly generate the ref and test result view with the proper safe area guides. However, if the iPhone strategy is run first (which has 0 top offset), the following iPad run will also inherit the 0 offset.

This seems to originate from the following place:

// in Sources/SnapshotTesting/Common/View

func prepareView(
  config: ViewImageConfig,
  drawHierarchyInKeyWindow: Bool,
  traits: UITraitCollection,
  view: UIView,
  viewController: UIViewController
  ) -> () -> Void {
  ...
  let dispose = add(traits: traits, viewController: viewController, to: window)
  ...
}

private func add(traits: UITraitCollection, viewController: UIViewController, to window: UIWindow) -> () -> Void
{
  ...
  viewController.didMove(toParent: rootViewController)

  window.rootViewController = rootViewController // <-----

  rootViewController.beginAppearanceTransition(true, animated: false)
  ...
}

In successful test run (iPad runs before iPhone), setting the window's rootViewController shows the safe area guides are applied. However, when iPhone test runs before iPad, the safe area guide seems to get zeroed out.

To Reproduce MyApp.zip

Expected behavior The safe area guide should adjust when running multiple strategies with different orientations.

Screenshots This is the expected snapshot:

image

When run after iPhone strategy, the generated iPad Mini snapshot shows no offset:

image

The test then fails because the entire view is shifted due to the missing offset

image

Environment

Additional context I may be using this incorrectly to enable drawHierarchyInKeyWindow, and would love to hear any suggestion on an alternative approach 🙏

Jeff-Meadows commented 1 year ago

I wonder if the bug is actually here (https://github.com/pointfreeco/swift-snapshot-testing/blob/main/Sources/SnapshotTesting/Common/View.swift#L964) - since there's a special case for zero safeArea that doesn't seem to be undone (via the dispose pattern) like most of the other changes.