Add tests for changes from #937: ignoring 3rd party load failures while redirecting #945
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…le redirecting I didn't realize that `STPRedirectContext` was as well tested as it is. I ran into test failures when executing on iOS 10 (our CI executes against iOS 11), and realized this needed to be updated. Recap of behavior change: only report errors in initial load on iOS 11+, and only if the latest URL was a `stripe.com` URL. When unit tests were run against iOS 11, this was still true, and the existing test passed. This adds two more tests: for pre-iOS 11 and for iOS 11 with a redirect to non-stripe.com. Both of those cases should not report an error, and should not close the Safari View Controller. We don't have `guard-let` in Obj-C, nor can we check `!@available(...)`, but I have emulated that structure at the beginning of these tests. I think it's worth it, and Joey didn't think it was unreasonable.
joeydong-stripe
approved these changes
May 24, 2018
| }; | ||
| OCMVerify([mockVC presentViewController:[OCMArg checkWithBlock:checker] | ||
| animated:YES | ||
| completion:[OCMArg any]]); |
There was a problem hiding this comment.
do we need to call [self unsubscribeContext:context] here too?
There was a problem hiding this comment.
Great catch. I found another (pre-existing) test case that wasn't unsubscribing, and have updated them both.
Per documentation at the top of the class: > [If] Your `sut` doesn't call unsubscribe and you explicitly `OCMReject` it firing > [Then] call `[self unsubscribeContext:context]` at the end of your test (use the original > `context` object here and _NOT_ the `sut` or it will not work). I missed that in one of the tests I just added, and while auditing all the tests I found another one that wasn't unsubscribing.
joeydong-stripe
approved these changes
May 24, 2018
mludowise-stripe
pushed a commit
that referenced
this pull request
Apr 4, 2022
* Update checkbox for new mocks * update snapshots * Use compBackground when unselected * Use fill color contrasting color
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Recap of behavior change: only report errors in initial load on iOS 11+, and only if
the latest URL was a
stripe.comURL.When unit tests were run against iOS 11, this was still true, and the existing test passed.
This adds two more tests: for pre-iOS 11 and for iOS 11 with a redirect to non-stripe.com.
Both of those cases should not report an error, and should not close the Safari View
Controller.
Motivation
I didn't realize that
STPRedirectContextwas as well tested as it is. I ran into testfailures when executing on iOS 10 (our CI executes against iOS 11), and realized this
needed to be updated.
Testing
These are just additional tests, so I ran the test suite on iOS 10 + 11 simulators.
PS: We don't have
guard-letin Obj-C, nor can we check!@available(...), but I haveemulated that structure at the beginning of these tests. I think it's worth it, and
Joey didn't think it was unreasonable.