Comments (16)
FYI, we've actually written an OCMockito helper that tracks all mocks that are created and automatically calls stopMocking()
on them during tearDown
. I wonder if this would be something that we could add to OCMockito? We could then adapt it to call disableMocking()
on all mocks, followed by stopMocking()
on all mocks.
i.e. as well as stopMocking(mock)
, we could add stopAllMocks()
which would perform the operations on all mocks created up to that point.
I guess it would have to be an "add-on" though - some users may like the flexibility of stopping mocks at certain points in their tests.
from ocmockito.
@cerihughes I'd be interested in stopAllMocks()
. But I'm also curious… what does your automatic solution look like? How does it work?
from ocmockito.
@jonreid: Agreed - I think stopAllMocks()
is a much nicer approach. It means that:
disableMocking(mock)
andstopMocking(mock)
don't need to be part of the public API any more, they'd just be implementation details -stopAllMocks()
would make sure to call them in the correct order on all tracked mocks.- we get a nice point to also make sure we also clean up the static instances in the OCMockito runtime (I've been playing around with unit tests that assert that all objects created during the test are also deallocated, and noticed that some of the static OCMockito instances are being reported as leaks).
Our automatic solution for tracking mocks isn't that smart... we just define our own XCTestCase
subclass and duplicate the mock creation methods on there. Those methods just call the mock()
equivalents and put the object in a collection before returning it. OCMockito could implement a similar solution without having to redefine the API. I'll see if I have time this week to create a PR.
Thanks!
Cer
from ocmockito.
I've spent some time investigating this more thoroughly and have realised that even with the PR #140 the problem hasn't gone away. I wrote a rough test case that captures all of the different scenarios for mocks being used in dealloc
and couldn't find a way to get them all to pass.
There's 1 simple scenario that even with the current codebase, is impossible to prevent from crashing:
- We have some code under test that calls a property's method in
dealloc
, passingself
into that method, e.g.[_observableObject removeObserver:self];
- We inject a mock instance of
_observableObject
into the code under test - We don't interact with that mock in any way during the test
- We deallocate the code under test before disabling and stopping the mock.
In this test, there's no way I can see of guaranteeing that the mock object is disabled and stopped before deallocation of the code under test without the tester explicitly knowing about this problem, understanding the problem, and writing the test in such a way that the mock is stopped before the code under test is deallocated. IMO, that's too big an ask for a test writer.
The crux of the problem is that [_observableObject removeObserver:self];
triggers the following method invocations from dealloc
(from bottom up):
[NSInvocation retainArguments]
[NSInvocation mkt_retainArgumentsWithWeakTarget]
[MKTInvocationContainer setInvocationForPotentialStubbing:]
[MKTBaseMockObject prepareInvocationForStubbing:]
[MKTBaseMockObject forwardInvocation:]
...................
. (NSProxy magic) .
...................
[_observableObject removeObserver:]
[DeallocationTestsRunner dealloc]
In this scenario, one of the arguments in the invocation is the object being deallocated, so we increase the retain count on this object during deallocation. When we then go to tear down the mock, we release it again, triggering a deallocation of an already deallocated object.
So, how do we get around this? I had a couple of ideas:
- Update
mkt_retainArgumentsWithWeakTarget
to figure out that it's been called directly from adealloc
call. Figure out what the object being deallocated is, and if it's one of the arguments, don't retain it. - Take more control over memory management - e.g. swizzle
[NSObject alloc]
so that we can keep track of all objects created in a OCMockito "session", and make sure they're not released until we've disabled and stopped all mocks.
I made a quick and dirty attempt at the 1st solution using the existing logic in MKTCallStackElement
. It kinda worked (I got it to detect that it was being called from a dealloc
method) but the runtime performance was horrible (inspecting the call stack for every invocation of a mock is very time consuming).
The second solution seems more achievable but we'd need to introduce the concept of a OCMockito "session". This could be done with functions similar to the existing API, e.g. startMockito()
(which could swizzle alloc
) and stopMockito()
(which would unswizzle alloc
). Calling these functions in a test would be optional (so not really a change to the existing API) and tests that don't use them should still work (assuming that don't reference mocks in dealloc
, but this case is broken anyway). Note that this solution has quite a lot of overlap with another PR I've submitted (which is also flawed for the same reasons) : #143 One downside of this is that we can't use any mocks to test dealloc
logic in code under test, but that's already a restriction we have.
Of course, this also opens up lots of potential for things to go very badly wrong. What if a test calls startMockito()
but doesn't call stopMockito()
. We can't detect that and it would cause untold havoc :)
At this point, it might be worth reverting #140 as it only solves part of the problem, and also introduces a change to the API.
I'd be very interested in working with you on this, but it's probably best to talk about the direction you want the API to take first. Are you comfortable with the concept of "starting" and "stopping" mockito (i.e. a more memory-managed solution), or do you think there's more value trying to figure out a solution that can detect if dealloc
is being called? Or are there any other ways to solve this that you can think of?
Finally, some good news: I ran the same tests using OCMock and it failed in exactly the same way :)
from ocmockito.
... or of course, the other option is to just document these corner cases and make sure testers know how to fix their tests when the crash occurs.
from ocmockito.
That's an interesting piece of information about OCMock! That helps nudge my opinion toward your last comment: "fix with documentation" rather than trying to avoid the problem in OCMockito.
from ocmockito.
Would you be willing to add a section to https://github.com/jonreid/OCMockito/wiki/OCMockito-FAQ ?
from ocmockito.
I would like to roll another release. @cerihughes how do you feel about disableMocking
…
- Still a keeper?
- Unending edge cases, not worth it?
from ocmockito.
Hey @jonreid! I think the latter... it hasn't made the problem go away, so let's get rid of it.
I'll write something for the FAQ when I get a chance, but it might not be for a week or so.
Thanks!
from ocmockito.
hey @cerihughes! Any news on progress of this issue / pull request?
I'm working with a large iOS unit test suite (+/- 9500 tests) and am experiencing the same issues you describe.
For my making a build of OCMockito with your proposed changes included, along with a system to keep track of all mocks seems to improve it greatly!
Unfortunately I also see crashes when running in an CI environment, but wasn't able yet to reproduce on my development machine.
If you've made any progress that would be awesome!
from ocmockito.
I'm interested in the idea of something that can register when mocks are created. Here's my main design goal:
It should be optional. That is, existing tests should work fine without this mock-cleaner-upper.
One way of achieving this would be to create a subclass of XCTestCase. When mocks are created, they can notice their context and register themselves. Cleanup would happen automatically on tearDown
.
Thoughts?
from ocmockito.
Hey @RuudPuts ! No progress from me I'm afraid. We've pretty much decided to not use mocks to test any scenarios showing the dealloc
problem. It's quite an edge case for us.
from ocmockito.
@jonreid - I'm a little hesitant about putting too much logic in XCTestCase
subclasses. There are sometimes cases when you want to work with multiple test frameworks, e.g. KIF for functional UI testing and FBSnapshotTestCase for visual UI testing, and if all these frameworks mandate that you use an XCTestCase
subclass, we run into problems.
Maybe a better compromise would to to provide all the logic outside of the XCTestCase
, but also provide a XCTestCase
subclass that uses it, and does all the necessary work in setUp
and tearDown
for those that want it (like KIF does IIRC). Best of both worlds :)
from ocmockito.
@cerihughes That makes sense. I wonder if we can make something so that: when a new mock is created, it can determine whether or not this list of of managed mocks even exists. If there is no list, do nothing. If there is a list, register itself.
Something like that.
from ocmockito.
I do agree on the mock tracker being optional, although I do think any test suite can benefit from it. Placing it in XCTestCase itself could cause problems when combining with other frameworks. An approach I've taken now is in a XCTestCase subclass define new my_mock() my_mockProtocol() etc methods, which create the mock using OCMockto's mock() mockProtocol() and store the mock created in the mock-cleaner-upper.
From the teardown of the XCTestCase then the mock-cleaner-upper is called upon to cleanup all mocks. I've tried to achieve the same with a test observer, although I didn't get that to work unfortunately.
@cerihughes I'm going to see if I can discover and fix the crashes still present after this pull request, although I can't seem to reproduce them always. Any insights you might still have I'd appreciate!
from ocmockito.
One way of achieving this would be to create a subclass of XCTestCase. When mocks are created, they can notice their context and register themselves. Cleanup would happen automatically on tearDown.
As of Xcode 9, this could maybe be done without subclassing XCTestCase
by using -addTeardownBlock:
. Teardown blocks stack up during a test run, then they get run in last-in, first-out order before the -tearDown
method gets called on the test case instance.
from ocmockito.
Related Issues (20)
- Swift Static Declaration of 'times' follows non-static declaration HOT 1
- Expected nil, but was nil HOT 3
- MKTArgumentCaptor to HCArgumentCaptor migration HOT 2
- verify macro replaces argument type with id HOT 2
- Unable to @import OCMockitoIOS HOT 4
- Is there a way to assert method throws? HOT 1
- Stubbing the protocol return as Nil HOT 3
- Cannot use OCMockito because of OCHamcrest. HOT 2
- Feature request: possibility to call through HOT 1
- Argument captor capturing outer function HOT 1
- Import OCHamcrest as a module breaks backward compatibility and feels unnecessary HOT 1
- Mock internal calls of a another class methods HOT 4
- Feature Request: Apple Silicon binary release HOT 9
- Catalyst Support for OCMockito.xcframework HOT 8
- verify doesn't fail when parameter is mocked HOT 11
- Stubs return nil when verifying HOT 3
- Could not build OCMockito using Carthage HOT 3
- Building with Carthage fails HOT 1
- Use OCHamcrest 9.0.1 via all dependency managers
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from ocmockito.