test_basicRouteWithEmptyComponents fails on iOS 8

[sweetverbs]

All tests pass fine when run in iOS 7, but in iOS 8 the above tests fails on the assert

XCTAssertTrue([USER_ID isEqualToString:@" "], @"Should have an empty ID");

It looks like USER_ID is coming through as an empty string. I tried poking around, but couldn't see why this would be happening.

[sweetverbs]

Hi,

I had a play around with this, and it seems to be a problem with the test itself. I set up another test:

- (void) test_replaceOldParamWithNew {
  [self.router map:@"users/:user_id" toController:[UserController class]];
  [self.router open:@"users/4"];

  XCTAssertTrue([USER_ID isEqualToString:@"4"]);

  [self.router open:@"users/3"];

  XCTAssertTrue([USER_ID isEqualToString:@"3"]);
}

This also fails on iOS8. Placing a breakpoint in the UserController init method, I can see that it's actually being initialised properly, which means that the problem is most likely to do with evaluating the USER_ID macro. I suspect that, by the time the macro is evaluated, the new UserController isn't the top view controller, and so we're still getting the results from the old one.

[NorthStar]

It is not about whether or not it is on 'top'. It is not there on the router at all.

After the 2nd open, there appears to be only one view controller in the array of view controllers associated with the router's navigation controller (and it is the old one). The macro is evaluated at the right time though (so it is not a problem of lazy eval).

I think the code looks fine, yet for some reason pushing view controller does not add to navigation controller. I saw a post and changed the second open to not animated; now it works:

- (void)test_basicRouteWithEmptyComponents {
    [self.router map:@"users/:user_id/:user_name" toController:[UserController class]];

    [self.router open:@"users//clay"];

    STAssertTrue([USER_ID isEqualToString:@""], @"Should have an empty ID");
    STAssertTrue([USER_NAME isEqualToString:@"clay"], @"Name should be Clay");

    //completely different params
    [self.router open:@"users/ /mimee" animated:NO];

    STAssertTrue([USER_ID isEqualToString:@" "], @"Should have a space ID");
    STAssertTrue([USER_NAME isEqualToString:@"mimee"], @"Name should be Mimee");
}

I am unsure if it is an Apple bug. If it is @clayallsopp might want to consider setting default animation to NO.

[sweetverbs]

Ah that's really interesting. I assumed the problem was in the tests as it doesn't seem to be affecting my app, but maybe I need to do a little more bug hunting...

I've confirmed that all tests pass when the second open: is changed to open: animated: but I'm not sure whether I should change the tests before creating a pull request for my changes. I'm hesitant to create a pull request while tests are failing (even if they're not caused by the added code), but equally I don't want to add the animated:NO to the tests, as it feels like ignoring the root cause. Thoughts?

[NorthStar]

If you want to force the functionalities to pass, you can just have 2 separate tests for the empty string and the space string. I don't think the test was previously meant to do 2 opens anyway.

[sweetverbs]

Yeah, okay. I'll rewrite some of the tests tonight and then create a pull request. Thanks. :)