Additional form factory tests

[gbirch-stripe]

Summary

Soon we will be removing form specs. Additional unit tests that formalize form behavior will help ensure that this migration is done safely and accurately

Motivation

Testing

Changelog

[gbirch-stripe]

TODO: pull latest form_specs from backend and update tests accordingly

[gbirch-stripe]

TODO: look into related snapshot test coverage

[yuki-stripe]

Check out Elements+TestHelpers, might be able to use getTextFieldElement("Full name") instead here.

[yuki-stripe]

If you haven't already, check out LPMConfirmFlowTests - I think there's some overlap. They use default billing configuration and assert the form contains the expected fields (for all of PI, PI+SFU, SI).

[yuki-stripe]

They don't test never billing details, however. I kinda wish there was a way to test that more simply? Maybe like a single testAllFormsRespectBillingCollectionNever that loops through every pm type and applies the same check to the respective form? Just spitballing.

[gbirch-stripe]

Hmm yeah I think those may entirely cover this

[gbirch-stripe]

Ok it looks like The LPMConfirmFlowTests cover that the form is functional and the snapshot tests cover that the form fields look right (though they don't include these LPMs). IMO the best path forward to reduce redundancy but ensure coverage would be to not add these unit tests (leave the unit tests as-is) and add snapshot tests for these PMs. Thoughts?

[yuki-stripe]

IDK about snapshot tests for each PM. Like if each PM form's view is just a composition of elements, and all of those elements are (or should be) fully snapshot tested already, is there additional value in snapshot testing each form?

[gbirch-stripe]

I think the main thing is that it's much easier to write a snapshot test that covers everything. E.g. the form fields being unified with a header vs separate (as per the slack discussion). I'm open to these being unit tests instead. My main concern is making sure every payment method form is codified as thoroughly as possible before changes are made to the definitions

[gbirch-stripe]

This could have been covered by other means as well, but one side effect of the lack of LPM snapshot test coverage is AU BECS using the wrong font which I am fixing here. There were previously no tests that covered the visuals of this form

[yuki-stripe]

I don't love that snapshot tests test are ~change detectors much unrelated stuff. But there's some useful possible things they'd catch (if the author/reviewer notices!) that unit tests wouldn't.

It's a bit of a shame because I think these forms are particularly well suited to unit tests. Like if the method being tested just outputs a FormElement composed of elements e.g. makeIDEAL outputs this (for a PI):

<FormElement: 0x000000010665d3d0>
└─ <SectionElement: 0x000000010664c660>; title = nil
   └─ <TextFieldElement: 0x000000010664c4f0>; label = Full name; text = ; validationState = invalid(error: StripeUICore.TextFieldElement.Error.empty(localizedDescription: ""), shouldDisplay: false)
└─ <SectionElement: 0x000000010665d260>; title = nil
   └─ <DropdownFieldElement: 0x000000010664caf0>; label = iDEAL Bank; validationState = valid; rawData = abn_amro

It'd be sweet if we could assert that the form is equal to this 'view model' without actually comparing pixels.

--

I'm not sure how to catch the AU Becs case or generally ensure all UI respects appearance. We could have a 'testFormWithCustomAppearance()` snapshot test but in practice this doesn't work very well, it's hard to tell if some color/shape in the snapshot is expected or not and I've seen countless instances where we've failed to notice a snapshot test showing a problem.