spy_on NSDate crashes on Xcode 10

[aaa-abrowne]

After updating to Xcode 10, we saw our tests crashing when swizzling Foundation methods like [NSDate date].

objc[30382]: no class for metaclass 0x11588e700

*** Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: '+[NSDate as_spied_class:]: unrecognized selector sent to class 0x1153656d8'
*** First throw call stack:
(
	0   CoreFoundation                      0x00000001150b229b __exceptionPreprocess + 331
	1   libobjc.A.dylib                     0x000000010d15b735 objc_exception_throw + 48
	2   CoreFoundation                      0x00000001150d0ea4 +[NSObject(NSObject) doesNotRecognizeSelector:] + 132
	3   CoreFoundation                      0x00000001150b6fb6 ___forwarding___ + 1446
	4   CoreFoundation                      0x00000001150b8e88 _CF_forwarding_prep_0 + 120
	5   aaa-ace-mobileTests-b-objc          0x0000000139354e83 -[CDRSpy hash] + 211
	6   CoreFoundation                      0x000000011507fc23 __21+[__NSSetI __new::::]_block_invoke + 51
	7   CoreFoundation                      0x000000011507fb1a +[__NSSetI __new::::] + 474
	8   CoreFoundation                      0x00000001150d5ed4 +[NSSet setWithObjects:count:] + 52
	9   CoreFoundation                      0x00000001150d6eab -[NSSet setByAddingObjectsFromSet:] + 763
	10  DTXConnectionServices               0x00000001372baf44 -[DTXProxyChannel _allowedClassesForReturnValues] + 109
	11  DTXConnectionServices               0x00000001372bb722 __42-[DTXProxyChannel _sendInvocationMessage:]_block_invoke + 260
	12  DTXConnectionServices               0x00000001372c9e76 __51-[DTXChannel _scheduleMessage:tracker:withHandler:]_block_invoke.751 + 101
	13  libdispatch.dylib                   0x0000000116ec151d _dispatch_call_block_and_release + 12
	14  libdispatch.dylib                   0x0000000116ec2587 _dispatch_client_callout + 8
	15  libdispatch.dylib                   0x0000000116ec9058 _dispatch_lane_serial_drainTest Case '-[_ValidationTextViewControllerSpec Initialization_with_masking_disabled_as_saves_what_is_initialized_into_properties_Loading_the_view_With_rightImage_as_when_the_view_loads_validate_When_the_validation_object_is_valid_should_hide_the_error_label]' started.
 + 720
	16  libdispatch.dylib                   0x0000000116ec9b9b _dispatch_lane_invoke + 401
	17  libdispatch.dylib                   0x0000000116ed29c6 _dispatch_workloop_worker_thread + 645
	18  libsystem_pthread.dylib             0x00000001172a2fd2 _pthread_wqthread + 980
	19  libsystem_pthread.dylib             0x00000001172a2be9 start_wqthread + 13

[aaa-abrowne]

Workaround: replacing Cedar's spy_on with OCMock no longer produces a crash.

id dateMock = OCMClassMock([NSDate class]);
OCMStub(ClassMethod([dateMock date])).andReturn(fakeTodayDate);

[aaa-abrowne]

spy_on also does not work for other Foundation classes like NSData and NSDictionary.

[tjarratt]

Hello @aaa-abrowne ! Thanks for writing up such a clear issue, you've made it really easy for me to understand at a high-level what's happening.

If you're curious, I'm fairly sure this is happening because the core of Cocoa and Foundation is slowly being rewritten in swift. Cedar makes the assumption that anything it spies on is a proper objective-c object, and spying on Swift objects is not supported. The impressive objective-c shim that exists does a good job of hiding the work Apple is surely doing here, but it's inevitable things like this will happen as Apple slowly starts changing the core of Foundation and other frameworks.

All that aside, I think this is really unlikely to be fixed. Even knowing that OCMock supports it, I think that's highly undesirable. In terms of best use of mocking frameworks, something you'll hear occasionally is the precept "Don't mock types you don't own", or stated in other words "Only mock interfaces that you have defined yourself in your proper application. Do not mock external interfaces". You can find this discussed in much detail on the internet if you'd like to learn more about this element of using mocks.

Additionally, I'd consider an NSDate to be a value type. It's a nice type that encapsulates a timestamp and has methods for operating on that date. It's effectively a really nice wrapper around an integer that is interpreted as a date. Given that, I'd like to share that it's not considered a good practice to mock "value types" - you can find some additional articles on this subject as well.

In terms of how to best apply mocks and spies in an Objective-C or Swift codebase, I haven't ever found a lot of materials (I'd love to write a blogpost on this topic) but I did find a great discussion on StackOverflow that is really aligned with the solutions I've found helpful in my codebases in the past.

Using solutions where you define the needs of your objects in terms of the domain specific messages and data they need, rather than directly using the built-in classes provided by Apple can help you arrive at solutions that are more flexible, easier to test, easier to understand, although they can be perceived as less idiomatic occasionally (although I don't personally find that to be such a bad thing. Idiomatic does not mean "best solution" in my opinion).

All that said, I'd like to try to find a reasonable way to resolve your problem. It looks like in this stack trace, it failed because you have some mocked NSDates and then you add them to an NSSet, which fails because NSSet assumes they will respond to the -hash selector, although it does not. I think it could be useful if Cedar were to emit a warning if you spied on a class that started with the "NS" prefix, or certain other prefixes that are known to be used by Apple. This warning could link to a wiki page here that has some of this information I've shared and tell you that this will soon become an error. I could imagine in a future release this could become an error, and Cedar would (by default) refuse to spy on these classes, because of the potential for this problem.

What are your thoughts on such a solution ? Do you feel such a warning with helpful messaging around what to do here would help you to write better tests ?

[aaa-abrowne]

Thanks for the timely response. We're currently supporting legacy code that extensively uses spy_on on Foundation and UIKit objects. A warning would be helpful to guide better practices.

[tjarratt]

Cool thanks for the quick response @aaa-abrowne. There's an outside chance that I may have contributed to some of those spying practices on Foundation and UIKit objects, so if that's true I beg your eternal forgiveness for the sins I have committed against best practices.

I will say, while I was writing this up, I was reflecting on some of the challenges we face (as TDD practitioners that practice good OO hygiene) while attempting to use the richness of Cocoa in a codebase with tests. The sheer convenience of methods on classes such as NSDate really make it harder to see when these problems are occurring. It's only come with time and experience that it's been obvious to me at a glance that an NSDate is a value type, and as such, shouldn't be mocked. The fact that the API is so rich and very quickly gives you complicated behaviors (often from a singleton class using its static methods) makes it hard to consider whether a collaborating object would be useful, since it often obfuscates what is going on.

Probably something to think about when working in a codebase with rich usage of Foundation and UIKit.

Out of curiousity, are there any other Apple frameworks that you are using that would be useful to avoid mocking ? I've used the PassKit and CoreLocation frameworks a lot in the past, and I don't think I would ever want to mock out anything from there, for example.