Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing an issue with stubs returning BOOL on 64bit iOS #55

Merged
merged 1 commit into from Mar 11, 2014

Conversation

stigi
Copy link
Contributor

@stigi stigi commented Dec 31, 2013

Looks like on 64bit iOS @encode(BOOL) results in B instead of c (as in 32bit iOS and Mac OS as well as surprise surprise 64bit Mac OS).

Therefore MKTSetReturnValueForInvocation should have a dedicated check for methodReturnType == @encode(BOOL).
This check should be done after checking agains @encode(char) to not break char return types in non 64bit iOS.

Happy New Year
-ullrich

PS: I could reproduce this in my iOS7 only project, but unfortunately wasn't able to run the OCMockito test suite in 64bit, as my Xcode always decided to crash on trying so. I added a test for it anyway.

@NicholasTD07
Copy link

Tonight I ran into the same problem.

I have two tests like these :

- (void)testStubWorksForYES
{
    // given
    [given([sut methodReturnsNO]) willReturnBool:YES];
    // then
    XCTAssertTrue([sut methodReturnsNO], @"should be true.");
}

- (void)testStubWorksForNO
{
    // given
    [given([sut methodReturnsYES]) willReturnBool:NO];
    // then
    XCTAssertFalse([sut methodReturnsYES], @"should be False");
}

When I tested them in 32bit simulator, everything was fine.
But if I test it in 64bit simulator, then - (void)testStubWorksForYES will not pass.

Hope this get fixed soon. If not, could you (Jon) put a note in the ReadMe file? So if anyone else runs into the same problem, they would know why it happens.
Thanks!

@ronnieliew
Copy link

👍

Similarly to @NicholasTD07, I'm also experiencing the bug in the 64-bit simulator.
This is not a trivial bug and agreed that this should minimally be documented in the ReadMe.

@jonreid jonreid merged commit 45eb99c into jonreid:master Mar 11, 2014
@jonreid
Copy link
Owner

jonreid commented Mar 11, 2014

Sorry for the delay on this. Thanks to Ulrich for the fix, and the rest of you for waving your hands until I noticed.

@gomera
Copy link

gomera commented Apr 1, 2014

@jonreid do you know if there is an ETA for the next release that will include this fix ?

@jonreid
Copy link
Owner

jonreid commented Apr 6, 2014

v1.2.0 is now out

@jberkel
Copy link

jberkel commented May 21, 2014

danke @stigi 🍰

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants