Fix redirect context dismissing SafariVC on foreground#833
Fix redirect context dismissing SafariVC on foreground#833bdorfman-stripe merged 3 commits intomasterfrom
Conversation
1. Make to to unsub from things before the test ends so that future notifications don't cause failures in previously run tests (this can be removed when STPURLCallbackHandler has weak delegates because then sut should get dealloc'd and auto unsubscribe at test end). 2. OCMReject must be set up _before_ the test code is run, unlike OCMVerify which must be run _after_ 3. Remove test for dealloc which wasn't actually doing anything and can't be easily tested anyway (and also is wrong because redirect context currently retains itself due to callback handler). 4. Add test to verify that foreground notifications don't dimiss when using SFSafariVC.
| @@ -1 +1,2 @@ | |||
| github "facebook/ios-snapshot-test-case" | |||
| github "erikdoe/ocmock" | |||
There was a problem hiding this comment.
weird that this wasn't there before :/
There was a problem hiding this comment.
Yeah I don't know what happened. It was still working with bootstrap cause it was in Cartfile.resolved
bg-stripe
left a comment
bg-stripe
left a comment
There was a problem hiding this comment.
nice, just one suggestion for a comment
| Note: You MUST pass in the actual context object here and not the mock or the | ||
| unsubscibe will silently fail. | ||
| */ | ||
| - (void)unsubscribeContext:(STPRedirectContext *)context { |
Tests/Tests/STPRedirectContextTest.m
Outdated
| /* | ||
| NOTE: | ||
|
|
||
| If you are adding a test unsubscribe from notifications is called before your |
There was a problem hiding this comment.
maybe "If you are adding a test use unsubscribeContext: to unsubscribe the context being tested before the test ends"
There was a problem hiding this comment.
Ah yeah I also accidentally a word here. Should be "make sure unsubscribe from notifications is called". You theoretically don't need to manually call it if your test is already verifying it is called (Which many do). You can also just directly call unsubscribe on it if you aren't explicitly OCMRejecting that method instead of using unsubscribeContext:. I'll rewrite this comment to be more clear.
|
Comment updated. Will merge when ci passes again |
- Uses backend response instead of hardcoding strings for the time estimate on the consent screen - Adds privacy policy HTML text to the consent screen - Use smaller font for time estimate / privacy policy Additional changes: - Created a generic `HTMLView` that could be reused between both the consent body text and the privacy policy text - Renamed `IdentityHTMLView` -> `HTMLViewWithIconLabels` to disambiguate with the new `HTMLView`
3 participants
When using SafariVC don't dismiss on foreground notification, just on url received (or when the user manually closes). Continue listening for foreground notification when actually redirecting out of the app.
Also fixes a bunch of redirect test code. See commit desc for more details.