Closed twitterkb closed 4 years ago
Hey there.. sorry you are having issues. Can you expand on how the test is failing? I'd love to see a reduced test case if possible.
a reduced test-case is probably technically "possible", though we have a fairly large code-base, and reducing this down would be pretty time-consuming.
our test is failing during cleanup, typically with an EXC_BAD_ADDRESS either in the stopMocking that gets called from dealloc, or in the .cxx_destruct for something that at first appears unrelated.
i didn't dig much deeper other than to basically git bisect
and discovered which past caused our regression, and then tried simply reverting the patch on the tip of master
and finding that it resolved the problem.
what that implies to me off the top of my head then is that your test is "passing" because the mock is being leaked. I assume your codebase is ARC based. Would you be able to see if patching in https://github.com/erikdoe/ocmock/pull/391 fixes it for you as well?
i tried applying patch #391, and end up with a crash in the exact same place:
`Thread 1: EXC_BAD_INSTRUCTION (code=EXC_i386, INVOP, subcode 0x0)
`
That implies to me that you have and "andDo" block in there somewhere? In your "andDo" block do you extract Arguments or set return values on the invocation?
For extract arguments you need to be sure to use something like:
__unsafe_unretained FooObject *object;
[invocation getArgument:&object atIndex:2];
and for returns, I typically make them __autoreleasing
.
Note that the __unsafe_unretained
applies for not just objective c object types, but CF types, blocks, dispatch_queues, etc. Anything that could get retained/released.
I would also try running with NSZombieEnabled
either via the environment variable, or Instruments.
not sure about the provenance, but it appears the stack-crawl above was produced based on bad DerivedData …
i was able to apply patch #391 properly, clean, and run the tests without failure.
for informational purposes, we do use andDo:
, and also use __unsafe_unretained
for objects we use when retrieving arguments.
and for pragmatic purposes … i am sort of out of spare time for debugging the problem. i had been inclined to move forward with a locally reverted version of #392, and may do so until #391 is merged, or also if further "regressions" pop up.
Thanks @twitterkb for spending the time on it. FWIW I'm busy attempting to move Google's codebase from OCMock 3.4 to OCMock 3.6 hence all the recent patches. I agree that #391 is a crucial fix to OCMock.
It sounds like this will be fixed when #391 is merged. I'll mark it as blocked for now. Please retest when #391 is merged.
Assume fixed with #391.
i had updated to tip-of-master and discovered a regression in a test that performs
id mock = [OCMockObject niceMockForClass:[MyClass class]]; [[[mock stub] andReturn:mock] sharedInstance];
based on the comment in #392
i reverted this patch locally, and the failing test no longer fails.
i have put a comment in pull request #392 that i would like to revert it. also opening this issue to go with it.