pointfreeco / swift-snapshot-testing

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

Right to left layout testing. #168

Open crayment opened 5 years ago

crayment commented 5 years ago

Specifying UITraitCollection(layoutDirection: .rightToLeft) in a ViewImageConfig for image snapshots doesn't seem to work. System controls and even my own views continue to lay out in the left to right mode. I imagine this is out of your control and would require Apple to support?

mbrandonw commented 5 years ago

Hi @crayment! The right-to-left trait collections should work as you expect. I just put together this gist to show that it does work for a simple VC with a horizontal stack view:

https://gist.github.com/mbrandonw/465ae3a165982f874cd690a090b042ca

If it doesn't work in snapshot testing, then we may have a bug, or maybe there's something we aren't understanding about how right-to-left UI works. We'll look into it more, and let us know if you find anything.

Thanks!

crayment commented 5 years ago

Thanks for the response and the gist! Your playground does show that leading and trailing anchors seem to be respected with that setup. However with some simple modifications you can see that UIControl's don't seem to be respecting the RTL trait. I have done a bit more digging replicating some of my UI in your playground and uploaded to a gist here

Here is an image of the live view with some text overlaid:

One thing I noticed is that as soon as I interact with the UISwitch it fixes itself (starts behaving in RTL mode). I'm wondering if there is a similar trigger that would fix the text field and label.

mbrandonw commented 5 years ago

Oof dang, that's a bummer. I'll investigate a bit more and let you know if I come up with anything.

pinda commented 5 years ago

@crayment were you able to resolve this issue? running into a similar issue, hoping you could point me in the right direction?

crayment commented 5 years ago

@pinda I wasn't able to find a solution unfortunately. I have filed a radar - feel free to dupe! :)

smileyborg commented 5 years ago

@crayment Thanks for filing the bug report. As a workaround, you might try setting the semanticContentAttribute property of the views to .forceRightToLeft instead. And the earlier that you can do that, the less likely it is that the view might end up running code which reads that value to configure something, where the view doesn't properly invalidate later if that value changes.

crayment commented 5 years ago

@smileyborg I did experiment with the semanticContentAttribute property a bit. I even remember writing some ugly code that crawled through the view hierarchy setting it on everything. I don't remember exactly why but I ended up giving up on it. I think there were certain things (maybe UINavigationBar) that just wouldn't do what I expected. If you are having success with that maybe I should give it another chance though. Thanks for the suggestion!

SebastianOsinski commented 5 years ago

Hi! I think I found promising solution for RTL testing. Xcode 11 introduces Test plans. We can create test plan for snapshot tests with two configurations:

With two configurations, snapshot tests are ran twice with different languages. It doesn't override traits so bug mentioned above does not affect RTL layout and it looks as expected.

The only problem is that both runs of tests expect reference snapshots with the same name. I solved that problem by defining custom environment variable CONFIGURATION_NAME in each configuration with appropriate values. Then I read this variable in tests and append it to test name I put in testName parameter in assertSnapshot method. I tried to snoop around default environment variables to see if there is existing one with configuration name, but it seems that Apple doesn't provide it.

I think it would be nice if library supported it directly, e.g. it would support optional SNAPSHOT_CONFIG_NAME environment variable which would be read in verifySnapshot method and used as prefix (or even a subdirectory) for reference and snapshot image file names. What do you think @stephencelis @mbrandonw? Would such addition be welcome?

SebastianOsinski commented 5 years ago

After further testing it looks the aforementioned bug persists even with test plans, but it behaves differently:

It looked so promising 😢.

SebastianOsinski commented 5 years ago

Eureka! After even more digging I found that in the end overriding language in plans works correctly. The issue is that the library always overrides layout direction to .leftToRight (in View.swift there is UITraitCollection extension with methods returning traits for each iPhone model) which combined with RTL configuration triggers the same iOS bug @crayment reported but in opposite fashion. So my first post is still valid but we would need to apply some more changes to the library. Specifically, remove .init(layoutDirection: .leftToRight), from each base array in View.swift. (same as displayGamut and displayScale are commented out).

This is a breaking change of course, but this override didn't really work correctly so I think it would be beneficial to remove it.

I can create PR for aforementioned Environment variable and this trait override removal.

drogel commented 1 year ago

Hi! If it helps, I managed to find a (very hacky) way to force RTL directions on a per-test basis, without requiring changes in the snapshot testing library, without test plans, and without needing to change the simulator language. Ideally, I would want an API in which I have some sort of parameter that I could pass to assertSnapshot to perform a snapshot test on an RTL or LTR environment, and this is the closest I could come up with. The only thing is that I couldn't make it work for UITextFields for some reason; they still render the content as if it were LTR always.

It involves setting the appearance().semanticContentAttribute of all UIKit subclasses to .forceRightToLeft. Here's an example you can run to test it.

See example ```swift import SnapshotTesting import XCTest final class TestView: UIView { private let verticalStackView: UIStackView = { let stackView = UIStackView() stackView.axis = .vertical stackView.translatesAutoresizingMaskIntoConstraints = false stackView.spacing = 4 return stackView }() private let textField: UITextField = { let textField = UITextField() textField.borderStyle = .roundedRect textField.text = "This is a test text field content" return textField }() private let horizontalStackView: UIStackView = { let stackView = UIStackView() stackView.axis = .horizontal stackView.spacing = 4 return stackView }() private let label: UILabel = { let label = UILabel() label.text = "This is a test label that is actually quite long so it will probably force a line break." label.numberOfLines = 2 return label }() private let toggleSwitch = UISwitch() init() { super.init(frame: .zero) setUpViewHierarchy() } @available(*, unavailable) required init?(coder _: NSCoder) { fatalError("init(coder:) has not been implemented") } private func setUpViewHierarchy() { backgroundColor = .white addSubview(verticalStackView) verticalStackView.addArrangedSubview(textField) verticalStackView.addArrangedSubview(horizontalStackView) horizontalStackView.addArrangedSubview(label) horizontalStackView.addArrangedSubview(toggleSwitch) NSLayoutConstraint.activate([ verticalStackView.topAnchor.constraint(equalTo: topAnchor, constant: 4), verticalStackView.leadingAnchor.constraint(equalTo: leadingAnchor, constant: 4), verticalStackView.trailingAnchor.constraint(equalTo: trailingAnchor, constant: -4), verticalStackView.bottomAnchor.constraint(equalTo: bottomAnchor, constant: -4), toggleSwitch.widthAnchor.constraint(equalToConstant: 60) ]) } } final class RightToLeftViewTests: XCTestCase { private var sut: TestView! override func setUp() { super.setUp() isRecording = true } func test_withLeftToRight_shouldRenderExpectedContent() { setUpSUT(forceRightToLeft: false) assertSnapshot(matching: sut, as: .image(size: .init(width: 300, height: 110))) } func test_withRightToLeft_shouldRenderExpectedContent() { setUpSUT(forceRightToLeft: true) assertSnapshot(matching: sut, as: .image(size: .init(width: 300, height: 110))) } } private extension RightToLeftViewTests { func setUpSUT(forceRightToLeft: Bool) { sut = setUpUIView(forceRightToLeft: forceRightToLeft) { TestView() } } } extension XCTestCase { func setUpUIView(forceRightToLeft: Bool, uiViewSetUp: () -> ViewUnderTest) -> ViewUnderTest { setUpAppearance(forceRightToLeft: forceRightToLeft) return uiViewSetUp() } } private extension XCTestCase { func setUpAppearance(forceRightToLeft: Bool) { UIView.appearance().semanticContentAttribute = forceRightToLeft ? .forceRightToLeft : .unspecified UIButton.appearance().semanticContentAttribute = forceRightToLeft ? .forceRightToLeft : .unspecified UIStackView.appearance().semanticContentAttribute = forceRightToLeft ? .forceRightToLeft : .unspecified UILabel.appearance().semanticContentAttribute = forceRightToLeft ? .forceRightToLeft : .unspecified UIColorWell.appearance().semanticContentAttribute = forceRightToLeft ? .forceRightToLeft : .unspecified UIDatePicker.appearance().semanticContentAttribute = forceRightToLeft ? .forceRightToLeft : .unspecified UIPageControl.appearance().semanticContentAttribute = forceRightToLeft ? .forceRightToLeft : .unspecified UISegmentedControl.appearance().semanticContentAttribute = forceRightToLeft ? .forceRightToLeft : .unspecified UISlider.appearance().semanticContentAttribute = forceRightToLeft ? .forceRightToLeft : .unspecified UIStepper.appearance().semanticContentAttribute = forceRightToLeft ? .forceRightToLeft : .unspecified UISwitch.appearance().semanticContentAttribute = forceRightToLeft ? .forceRightToLeft : .unspecified UITextField.appearance().semanticContentAttribute = forceRightToLeft ? .forceRightToLeft : .unspecified UITextView.appearance().semanticContentAttribute = forceRightToLeft ? .forceRightToLeft : .unspecified UISearchTextField.appearance().semanticContentAttribute = forceRightToLeft ? .forceRightToLeft : .unspecified UINavigationBar.appearance().semanticContentAttribute = forceRightToLeft ? .forceRightToLeft : .unspecified UISearchBar.appearance().semanticContentAttribute = forceRightToLeft ? .forceRightToLeft : .unspecified UIToolbar.appearance().semanticContentAttribute = forceRightToLeft ? .forceRightToLeft : .unspecified UITabBar.appearance().semanticContentAttribute = forceRightToLeft ? .forceRightToLeft : .unspecified } } ```

Here are the results: test_withLeftToRight_shouldRenderExpectedContent 1 test_withRightToLeft_shouldRenderExpectedContent 1