UIAlertController Failure on version 2.2.0

[farshadtx]

Hey Jared,

We recently updated our Carthage Repo add decided to go with the latest version of Fleet(ver 1.0.2 to ver 2.2.1).

Unit tests will crash because of a casting on UIViewController which is due to tapping on an action on Alert Controller.

We downgraded to 2.1.0 and it looks it is fine. I will try to provide more info about it later. (just added this here so we can keep tracking it)

Thanks

[jwfriese]

@farshadtx - Thanks for filing the issue. I spent a bit of time trying to reproduce this problem, and could not. I'd love to see example code if you have it available. Answers to the following questions would help me diagnose the problem as well:

• What UIViewController casting is crashing? Is it an attempt to cast the UIAlertController that has the alert action? Is it the presenting UIViewController? Or is it some other one?
• Does the UIAlertAction handler use the passed-in UIAlertAction object (the one that the handler gets set on)?

[farshadtx]

@jwfriese - Sorry for short explanation on the issue.

The best way I can describe it is, imagine there is a navigation controller which currently has ViewController A on it's stack. then we will present an alert on this view controller with 2 actions. by tapping one of the actions we suppose to close the alert and push (show) to ViewController B. (I tried 🥇 )

I tracked the issue down till I figured, casting UIAlertController looks fine and we can get actions out of it. but tapping on actions not working (not dismiss the alert and push to the next window). that is the cast I was talking about. because the topmostViewController will return the UIAlertController.

I tested the production code and it's working fine. Here is production code:

// MARK: - Actions
    @IBAction func didTapMakeItAComboButton(_ sender: UIButton) {
        actionSheetPresenter.comboSize(self) { comboSize in  // present a action sheet and the callback is to handle tap on actions
            self.dismiss(animated: true, completion: nil) // dismiss the action sheet which is not working in unit tests

            self.getComboDetailsAndSegue(comboSize) // get info and then segue to the next page
        }
    }

    @IBAction func didTapNoThanksButton(_ sender: AnyObject) {
        performSegue(withIdentifier: Segues.customization, sender: comboUpsell.entree)
    }

    // MARK: - Private Functions
    private func getComboDetailsAndSegue(_ comboSize: ComboSize) {
        let comboTypeToIdMap = [ComboSize.small: self.comboUpsell.smallComboId,
                                ComboSize.medium: self.comboUpsell.mediumComboId,
                                ComboSize.large: self.comboUpsell.largeComboId]
        let comboTypeId = comboTypeToIdMap[comboSize]!

        _ = self.comboRepository.get(comboTypeId, storeName: self.store.name).then { result in
            switch result {
            case .success(let comboItem):
                self.sdkAdapter.execute {
                    self.performSegue(withIdentifier: Segues.customization, sender: comboItem)
                }
            default: break
            }
        }
    }

Here is the test context:

    context("...") {
        beforeEach {
            let buttons = ["Go Medium", "Go Large", "Make it a Small"]
            let randomIndex = Int(arc4random_uniform(UInt32(buttons.count)))
            comboSizeActionSheet.tapAlertAction(withTitle: buttons[randomIndex])
        }

        context("and the combo details service resolves successfully") {
            let comboItem = TestComboItem(name: "COMBO")

            beforeEach {
                comboPromise.resolve(ValueResult.success(comboItem))
            }

            it("seques to the customization page with a modified combo item") {
                let expectedCustomizationViewController = Fleet.getApplicationScreen()?.topmostViewController as! CustomizationViewController // here is the cast that crash
                expect(expectedCustomizationViewController).to(beIdenticalTo(fakeCustomizationViewController))
                expect(expectedCustomizationViewController.customizableItem).to(beIdenticalTo(comboItem))
                expect(expectedCustomizationViewController.customizationType).to(equal(CustomizationType.add))
            }

            it("calls the combo customization service") {
                expect(fakeComboRepository.getCallCount).to(equal(1))
            }
        }
    }

[jwfriese]

@farshadtx - Thank you so much for putting all this effort into reproducing some code for me to work with. I am looking into this right now and will get back to you as soon as I can!

Here is the Tracker story: https://www.pivotaltracker.com/story/show/137963403

[jwfriese]

@farshadtx - What follows is a long explanation that covers four things: 1) Why Fleet version <= 2.1.0 works in your case 2) Why Fleet version >= 2.2.0 does not work 3) Why I am inclined not to change it back and most importantly, 4) What you might be able to do in your tests to get in alignment with the new behavior

---

1) In all versions of Fleet prior to 2.2.0, the implementation of the swizzled dismiss method on UIViewController looked like the following:

    func fleet_dismiss(animated: Bool, completion: (() -> ())?) {
        let viewControllerToDismiss = self.fleet_property_presentedViewController

        self.fleet_property_presentedViewController = nil
        viewControllerToDismiss?.fleet_property_presentingViewController = nil

        if let completion = completion {
            completion()
        }

        fleet_dismiss(animated: animated, completion: nil)
    }

There are two things to note here: First, the completion block fires immediately in this implementation, before proceeding with the functional steps of dismissing the view controller. Second, the participating view controllers "lie" about their presentedViewController and presentingViewController properties.

The second point above is why your tests pass with versions of Fleet older than 2.2.0. The production code here:

    @IBAction func didTapMakeItAComboButton(_ sender: UIButton) {
        actionSheetPresenter.comboSize(self) { comboSize in  // present a action sheet and the callback is to handle tap on actions
            self.dismiss(animated: true, completion: nil) // dismiss the action sheet which is not working in unit tests

            self.getComboDetailsAndSegue(comboSize) // get info and then segue to the next page
        }
    }

has that call to dismiss, and your tests were able to assume that the view stack would immediately reflect that change.

2) Now, let's look at the implementation of the swizzled dismiss method on UIViewController in 2.2.0:

    func fleet_dismiss(animated: Bool, completion: (() -> ())?) {
        fleet_dismiss(animated: false, completion: completion)
    }

You'll see that there is almost nothing in there. Really, all it does is turn off animations on dismisses, which essentially means that the functional effect of a dismiss on the view hierarchy takes effect immediately, but only once the UI thread gets around to processing that dismiss request.

Critically, it does not fire the completion handler before dismissing, and does not fake out the view controllers' answers to presentedViewController and presentingViewController. UIKit takes control of those again.

3) After investigating this issue, writing a couple more exploratory tests, and comparing the behavior of the different versions of Fleet in question in a couple other apps that use it, I think that the behavior of Fleet in 2.2.1 and above is correct. Here's why:

• The version of Fleet older than 2.2.0 fired completion handlers for both present and dismiss calls before those transitions had a chance to even start. This was simply a bug in Fleet for its entire existence. Any completion handlers in test that expected a transition to have completed would simply not work.

As an example of what I'm talking about, here's some production code for a UIAlertAction's handler that simply pops the top view controller off the navigation stack:

dismissAction = UIAlertAction(title: "OK", style: .default) { _ in
    // Note that this production code requires the UIAlertController to be gone already
    let _ = self.navigationController?.popViewController(animated: true)
}

and here is the test that drove out that behavior:

describe("When the user dismisses the success alert") {
    it("pops itself off the navigation controller") {
        let screen = Fleet.getApplicationScreen()
        var didTapOK = false
        let assertOKTappedBehavior = { () -> Bool in
            if didTapOK {
                // This always fails, because the call to dismiss in the action handler will 
                // have fired too soon --when the UIAlertController is still on the top of the stack
                return screen?.topmostViewController === navigationController
            }

            if let alert = screen?.topmostViewController as? UIAlertController {
                alert.tapAlertAction(withTitle: "OK")
                didTapOK = true
            }

            return false
        }

        expect(assertOKTappedBehavior()).toEventually(beTrue())
    }
}

I put comments in the two code snippets above to try to clarify things a little bit, but I know that there's still a lot of context missing around it. The above test failed, because the UIAlertAction behavior of always dismissing the alert before executing its handler combined with a handler that expected the alert to be off the view stack already was not being accurately handled by Fleet. Fleet fired the completion handler of the UIAlertAction before actually processing the dismissal of the alert, and thus the completion handler's expectation that the alert would be gone -- which is correct -- was not respected in test.

Put simply, Fleet made UIKit behave badly in test, and 2.2.0 fixes this by removing all of the swizzling voodoo that happened in UIViewController while still giving you the benefit of consistent UI transition behavior in your tests. What is lost is the behavior that essentially circumvented the real UI thread, and allowed you to assert on the structure of the view controller hierarchy before the transitions were actually being processed by the UI thread.

4) In your tests, what you'll have to start accounting for is the need for the UI thread to have processed a transition before you see that transition's effects reflected in the view hierarchy. With Nimble, this is mostly straightforward. It basically means presentation assertions will look like this:

let currentTopViewController: () -> UIViewController? = {
    return Fleet.getApplicationScreen()?.topmostViewController
}

// Note the use of `toEventually` here instead of `to`
expect(currentTopViewController()).toEventually(beIdenticalTo(expectedViewController))

instead of like this:

expect(Fleet.getApplicationScreen()?.topmostViewController).to(beIdenticalTo(expectedViewController))

I have not seen your failing test, so it's hard for me to say exactly what you might have to change. I hope that this update does not break too many things, and would love to help you fix up that failing test in whatever way I can. There is definitely a way to write that test so that it accurately bites at things the way they happen in prod, and using the new version of the framework. We can remote pair if you'd like. You can PM or email me the test if you'd like.

Does all the above make sense? I'd like to clarify any confusions if possible.

[farshadtx]

Sorry for late response but it looks problem is solved now. I will mark it closed.