Is anybody going to merge this PR?

This PR is still in the queue. The change itself is small, namely using the object_getClass function rather than the class method to get the class of objects that are instances of subclasses of NSManagedObject. @carllindberg asked why this wouldn't be done for all cases, and this does sound logical. However, doing it for all cases makes a unit test break (see #163). To be honest, I don't really understand why this is the case and I haven't had a chance to look into it yet. If someone has a good explanation...

To expand on my earlier comment: We can't do objc_getClass for all cases, because KVO works by:

• Changing the class at runtime, so if you have FooClass, KVO replaces that to FooClass_NSKVONotifying.
• It also stubs out class to return the original class (FooClass).
• objc_getClass returns the "real" class (e.g., as someInstance->isa used to), so that means the OCMock subclass would end up replacing the methods on the dynamic KVO subclass, so we lose KVO notifications.

However, for Core Data, it works by:

• Taking your original class (let's say ManagedClass), and dynamically subclassing it to ManagedClass_EntityName. It then sets the class of the object to ManagedClass_EntityName. This dynamic subclass handles, among other things: dynamic properties and methods for your model, and KVO.
• Core Data also overrides class to return the original ManagedClass class.
• This means that OCMock would end up previously replacing the dynamic class with a class that's a subclass of your original ManagedClass class, which breaks Core Data. Instead, we now subclass Core Data's underlying class. This keeps Core Data & KVO working for managed objects.

Does that help?

Okay, that clarifies a bit. I understand the need for using object_getClass to get the real class in the NSManagedClass case. In fact, I agree with Carl that from a purely theoretical point of view I don't see a reason why we should actually ever use the class method instead of object_getClass. That, however, brings us to the issue with KVO.

Obviously I agree that using object_getClass would make OCMock create its dynamic subclass as a subclass of the KVO dynamic subclass. So far so good. What I still don't understand is why this is bad. You say that "OCMock would end up replacing the methods on the dynamic KVO subclass". What do you mean by this? OCMock doesn't really remove functionality.

For example, let's take a method foo in a class Bar. When creating a partial mock for an instance of Bar, OCMock creates a dynamic subclass of Bar in which it creates a method named ocmock_replaced_foo, pointing it to the original implementation of foo. OCMock also makes foo point to the forwarder mechanism. After that, when foo is invoked, the call ends in the following code:

Unless the mock is instructed (via stub/expect/reject) to do something handleInvocation: will return NO and the original implementation of foo is called. So, unless we specifically stub a KVO method, they should be called. Where's the point that breaks KVO?

If I remember, the issue with partial mocking a KVO class is that there is some global state keyed by the object's isa (since that is a per-instance class just like partial mocks) and when you change it, the KVO mechanism can crash when you call any of the modified methods since all of the associated state is no longer found with the new isa pointer. The way it works now, the partial mocking will blow away the KVO class completely so that notifications won't work while mocked, as all of the altered methods are in a class which is no longer referenced by the object. The KVO class gets restored at -stopMocking, so KVO cleanup can work afterwards. If you partial mock first and then KVO, I think it works, provided you remove all KVO observers (thus restoring the partial mock isa) before -stopMocking is called. There is probably a unit test to make sure that things at least don't crash if you partial mock a KVO object.

I guess the question is what we want to do when faced with dynamic-subclass-replace-isa-override-stubsclassmethod situations (i.e. where [object class] != object_getClass(object) ). We want to go one way with NSManagedObject and the other with KVO. The current change only alters the behavior for NSManagedObject, but doesn't change the default. Maybe though we want to check for KVO explicity and use [object class] in that case, and object_getClass() in all others (thus changing the default). Checking for [object observationInfo] != NULL should do it. The question is what happens with other such situations -- not sure there are any others used by Apple, but who knows about third party code. It's arguable though that using the dynamic subclass is usually better (as it is with NSManagedObject).

I'm going to close this out, @alanf has a new PR that incorporates feedback.