Ensure tests are only run on iPhone 8 #692
Conversation
SwiftFormat found issues:
Generated by 🚫 Danger |
Library/TestHelpers/TestCase.swift
Outdated
| @@ -25,6 +25,12 @@ internal class TestCase: FBSnapshotTestCase { | |||
|
|
|||
| override func setUp() { | |||
| super.setUp() | |||
|
|
|||
| let preferredDeviceName = "iPhone 8" | |||
| if !UIDevice.current.name.contains(preferredDeviceName) { | |||
There was a problem hiding this comment.
Should we be getting this from AppEnvironment.current so that we can potentially inject our own value down the road (and maybe for consistency)?
There was a problem hiding this comment.
I don't think this is really for AppEnvironment, that's only for dependency injection and I don't know that we'd want to inject stuff in unit tests for unit tests 🤔 I think this is like a safety thing much like we have in our swiftlint.yml to fail if we leave self.recordMode = true in the test.
There was a problem hiding this comment.
The test seems to fail on iPhone 8 / iOS 11 .. which is actually good because there could be minor differences due to UIKit's rendering so I think right now this is the right way to go!
Thanks @justinswart this is super helpful!
justinswart
dismissed stale reviews from dusi and Scollaco
Please check the improvement
📲 What
Ensures that we only run tests and record screenshots on the iPhone 8 simulator.
If for whatever reason we need to run unit tests on a different device, we can just comment out this line.
🤔 Why
It's happened once or twice in the past that we accidentally record a bunch of screenshots on the wrong device which results in thousands of new screenshots being committed to the repo. This is just a safety catch to help us prevent that.
🛠 How
Put a check at the top of
TestCasesetUp()function which is called before all tests run.✅ Acceptance criteria