Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why display scale traits for iOS devices are commented out? #427

Open
darrarski opened this issue Feb 5, 2021 · 5 comments
Open

Why display scale traits for iOS devices are commented out? #427

darrarski opened this issue Feb 5, 2021 · 5 comments

Comments

@darrarski
Copy link

When snapshot testing iOS views I am using display scale trait depending on the device which layout I am using:

assertSnapshot(
  matching: /* SwiftUI.View */,
  as: .image(
    layout: .device(config: .iPhoneSe),
    traits: .init(displayScale: 2) // ← because iPhone Se has display with 2x scale
)

assertSnapshot(
  matching: /* UIKit.UIViewController */,
  as: .image(
    on: .iPhoneX,
    traits: .init(displayScale: 3) // ← because iPhone X has display with 3x scale
  )
)

This allows snapshotting the view with the provided layout using any simulator. For example, I can record the above snapshots using the iPhone Se simulator and then assert them on the iPhone X simulator. Without providing a display scale traits, like above, the assertion would fail because snapshot images would have the scale of the current simulator. Providing display scale traits solves this problem.

I noticed that layout configurations implemented in the SnapshotTesting library already contains display scale traits for each iOS device layout. However, they are commented out, so I need to provide them explicitly when writing assertions.

https://github.com/pointfreeco/swift-snapshot-testing/blob/main/Sources/SnapshotTesting/Common/View.swift#L354

I wonder what is the reason behind commenting out the display scale traits in the source code?

@stephencelis
Copy link
Member

@darrarski Do you remember what lines you were referring to? Unfortunately your link is locked to a branch that has since changed and not a SHA.

@darrarski
Copy link
Author

@stephencelis oh, sorry, here is the link locked to SHA:

The line numbers shifted on the main branch, but the commented out code is still there:

@ZevEisenberg
Copy link

bump - I also have this question

@gohanlon
Copy link
Contributor

gohanlon commented Jun 20, 2022

Setting the displayScale trait appropriately isn't enough to make the currently running device runtime produce view snapshots that match some other device runtime — it merely makes the rendered image closer to what you'd expect. I think those commented lines were added when the maintainers were exploring ways to overcome this limitation. They were probably left in as a hint at how to capture more accurate, yet still inaccurate, snapshots.

One example I've experienced: Fonts won't render with the correct optical sizing adjustments, which is a significant difference on its own, but can also cause line wrapping differences between devices and across OS versions.

I use snapshot testing for verification, so accuracy is very important in my use. The only way that I'm aware of to capture accurate view snapshots is to capture snapshots on that device's simulator. (Noted in the README, and I've suggested broadening via #592.)

I wrote the following strategy to take a view snapshot using the currently running device:

#if canImport(UIKit)
  import SnapshotTesting
  import SwiftUI

  public func assertCurrentDeviceSnapshot<SnapshotContent: View>(
    for view: SnapshotContent,
    precision: Float = 1,
    record: Bool = false,
    file: StaticString = #file,
    testName: String = #function,
    line: UInt = #line
  ) {
    var transaction = Transaction(animation: nil)
    transaction.disablesAnimations = true
    withTransaction(transaction) {
      let safeArea: UIEdgeInsets
      switch UIDevice.current.name {
      case "iPhone 8":
        safeArea = .init(top: 20, left: 0, bottom: 0, right: 0)

      case "iPhone 13 mini":
        safeArea = .init(top: 50, left: 0, bottom: 34, right: 0)

      case "iPhone 13", "iPhone 13 Pro", "iPhone 13 Pro Max":
        safeArea = .init(top: 47, left: 0, bottom: 34, right: 0)

      case "iPad mini (6th generation)":
        safeArea = .init(top: 24, left: 0, bottom: 20, right: 0)

      case "iPad Pro (12.9-inch) (5th generation)":
        fatalError("Haven't yet added the safe area for device: iPad Pro (12.9-inch) (5th generation)")

      case "iPhone SE (3rd generation)":
        safeArea = .init(top: 20.0, left: 0.0, bottom: 0.0, right: 0.0)

      case "iPod touch (7th generation)":
        safeArea = .init(top: 20, left: 0, bottom: 0, right: 0)

      default:
        fatalError("Don't know safe area for device: \(UIDevice.current.name)")
      }

      assertSnapshot(
        matching: view,
        as: .image(
          precision: precision,
          layout: .device(config: .init(safeArea: safeArea, size: UIScreen.main.bounds.size))
        ),
        named: "\(UIDevice.current.name).\(UIDevice.current.systemVersion)",
        record: record,
        file: file,
        testName: testName,
        line: line
      )
    }
  }
#endif

That strategy captures snapshots to the conventional location, named with the device and OS, e.g.:

testPromptSignIn.iPhone-SE-3rd-generation-15-4.png

I then capture snapshots by testing on several devices in parallel:

$ xcodebuild test -scheme "MySnapshotTests" \
  -destination "platform=iOS Simulator,OS=15.4,name=iPhone 8" \
  -destination "platform=iOS Simulator,OS=15.4,name=iPhone 13 mini" \
  -destination "platform=iOS Simulator,OS=15.4,name=iPhone 13" \
  -destination "platform=iOS Simulator,OS=15.4,name=iPhone 13 Pro" \
  -destination "platform=iOS Simulator,OS=15.4,name=iPhone 13 Pro Max" \
  -destination "platform=iOS Simulator,OS=15.4,name=iPad mini (6th generation)" \
  -destination "platform=iOS Simulator,OS=15.4,name=iPad Pro (12.9-inch) (5th generation)" \
  -destination "platform=iOS Simulator,OS=15.4,name=iPhone SE (3rd generation)" \
  -destination "platform=iOS Simulator,OS=15.4,name=iPod touch (7th generation)" \
  -maximum-concurrent-test-simulator-destinations 9

While this works and could be fast on good hardware, it's actually quite slow even on great hardware due to an xcodebuild test issue. (See #591, haven't tested Xcode 14b1 yet but no response yet to my filed Feedback: FB9972000.)

There are some limitations to this strategy, too. To reduce duplication and properly assert correctness, I still need some way to designate a particular snapshot as cannon across multiple similar devices, and also across different OS versions. And then, support for a more specific device/OS snapshot to be added when that particular device/OS should in fact differ from the more general snapshot.

@ZevEisenberg
Copy link

Ah yep, the font hinting thing makes sense. It's likely to always be a pile of (loving, carefully considered) hacks until Apple supports something like this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants