Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
File renamed without changes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still find these file names to be way too long to be practical, at least in my personal work flow. Maybe it's worth checking with the team what info they find necessary in a snapshot test? Personally, I don't think things like device type and size category are useful enough to be worth the extra words, but I'm okay with being outvoted.

Even without removing information, there's still some things you could do to make the name more streamlined. The full path of this one is ios-oss/Kickstarter-iOS/Features/LoginTout/Controller/__Snapshots__/testLoginToutView.LoginToutViewControllerTests_testLoginToutView_phone4-7inch_en_dark_UICTContentSizeCategoryAccessibilityXXXL_portrait.png. Notice how this includes the test name twice? I also think it'd be worth describing the content size category more concisely if we're going to include it at all - maybe as accessibilityXXL?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can drop the file/test prefix from the named: value so the filename is just the config (device/lang/style/font/orientation), and let the test’s Snapshots/testLoginToutView/ folder provide the test context. I’ll also shorten the content-size labels—thinking a11yXXXL as a common shorthand for “accessibility” to keep names shorter, but I can spell out “accessibilityXXXL” if you prefer. With these tweaks, a filename would look like phone4_7inch_en_dark_a11yXXXL_portrait.png. If the team wants to trim further (e.g., drop style/font), I can do that too.

File renamed without changes
1 change: 1 addition & 0 deletions Kickstarter-iOS/TestHelpers/Combos.swift
Original file line number Diff line number Diff line change
Expand Up @@ -98,4 +98,5 @@ internal func orthogonalCombos<A, B, C, D, E>(
)
}
}

// swiftlint:enable large_tuple
42 changes: 34 additions & 8 deletions Kickstarter-iOS/TestHelpers/ScreenshotTestHelpers.swift
Original file line number Diff line number Diff line change
Expand Up @@ -113,10 +113,10 @@ internal func assertSnapshot(
forController controller: UIViewController,
withType type: ScreenshotType,
perceptualPrecision: Float? = nil,
record _: Bool = false,
record: Bool = false,
file: StaticString = #file,
testName: String = #function,
line _: UInt = #line
line: UInt = #line
) {
let contentSizeTraits = UITraitCollection(
preferredContentSizeCategory: type.contentSizeCategory
Expand Down Expand Up @@ -149,19 +149,26 @@ internal func assertSnapshot(
return .image
}()

let directory = snapshotDirectory(for: file)

if let failure = verifySnapshot(
of: parent.view,
as: strategy,
named: name,
record: record,
snapshotDirectory: directory,
file: file,
testName: testName,
line: line
) {
XCTFail(
"""
Snapshot failed for \(name)
device=\(type.device.snapshotDescription), lang=\(type.language.rawValue), style=\(type.style.snapshotDescription), font=\(type.contentSizeCategory.rawValue), orientation=\(type.orientation.snapshotDescription)
device=\(type.device.snapshotDescription),
lang=\(type.language.rawValue),
style=\(type.style.snapshotDescription),
font=\(type.contentSizeCategory.rawValue),
orientation=\(type.orientation.snapshotDescription)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great, thanks for adding!

\(failure)
""",
file: file,
Expand All @@ -178,7 +185,7 @@ internal func assertSnapshot(
size: CGSize? = nil,
useIntrinsicSize: Bool = false,
perceptualPrecision: Float? = nil,
record _: Bool = false,
record: Bool = false,
file: StaticString = #file,
testName: String = #function,
line: UInt = #line
Expand Down Expand Up @@ -242,19 +249,26 @@ internal func assertSnapshot(
return .image
}()

let directory = snapshotDirectory(for: file)

if let failure = verifySnapshot(
of: parent.view,
as: strategy,
named: name,
record: record,
snapshotDirectory: directory,
file: file,
testName: testName,
line: line
) {
XCTFail(
"""
Snapshot failed for \(name)
device=\(type.device.snapshotDescription), lang=\(type.language.rawValue), style=\(type.style.snapshotDescription), font=\(type.contentSizeCategory.rawValue), orientation=\(type.orientation.snapshotDescription)
device=\(type.device.snapshotDescription),
lang=\(type.language.rawValue),
style=\(type.style.snapshotDescription),
font=\(type.contentSizeCategory.rawValue),
orientation=\(type.orientation.snapshotDescription)
\(failure)
""",
file: file,
Expand All @@ -271,7 +285,7 @@ internal func assertSnapshot<Content: View>(
size: CGSize? = nil,
useIntrinsicSize: Bool = false,
perceptualPrecision: Float? = nil,
record _: Bool = false,
record: Bool = false,
file: StaticString = #file,
testName: String = #function,
line: UInt = #line
Expand Down Expand Up @@ -366,11 +380,23 @@ private extension Orientation {
}
}

// MARK: - Private helpers
private extension UIUserInterfaceStyle {
var snapshotDescription: String {
switch self {
case .light: return "light"
case .dark: return "dark"
default: return "unspecified"
}
}
}

private func snapshotDirectory(for file: StaticString) -> String {
let fileURL = URL(fileURLWithPath: "\(file)")
return fileURL.deletingLastPathComponent().appendingPathComponent("__Snapshots__").path
}

private func withLanguage(_ language: Language, body: () -> Void) {
AppEnvironment.pushEnvironment(language: language)
body()
AppEnvironment.popEnvironment()
}