search

erikdoe / ocmock

Mock objects for Objective-C
http://ocmock.org
Apache License 2.0
2.16k stars 606 forks source link

Add exception for mocking class methods after stopMocking called. #400

Closed dmaclach closed 4 years ago

dmaclach commented 4 years ago

Adding class methods after stopMocking was called could cause extremely confusing results including crashes at runtime.

twitterkb commented 4 years ago

hoping this might help us track down some issues with our tests. thanks.

erikdoe commented 4 years ago

So, this does something quite different than the version for instance methods, added in https://github.com/erikdoe/ocmock/commit/3474a033aed40ac9458e5733b224ac0690fafde3

The instance method version throws when you try to use the mock, the change here throws when you try to set up a new stub for a mock on which stop mocking has been called.

Of course, I understand that (short of something like NSZombie) there is no way to warn the user that they are using a class method when the mock for the class has been stopped, but I do wonder whether it would make sense to add this new behaviour to instance methods as well.

Also, this really shouldn't be in the macro tests (because it's not specific to macros).

dmaclach commented 4 years ago

Fixed up the test location. With regards to your question about doing this for instance methods would you like to have it done in this PR, or maybe have an issue filed and follow up in a separate PR?

erikdoe commented 4 years ago
There are a number of different concerns. I think it looks like this at the moment:
checks		 instance methods class methods
invoke methods implemented not possible
verify implemented not possible?
stub/expect/reject not implemented implemented in this PR

When I look at this, I think it would make sense to make the changes for instance methods in this PR.

dmaclach commented 4 years ago

I'll try and take a look at it today.

dmaclach commented 4 years ago

Good call Erik. You can actually catch both cases at one chokepoint.

dmaclach commented 4 years ago

Just in case it wasn't clear, I think I've answered all open issues on this one.

erikdoe commented 4 years ago

Sorry, I had missed this somehow.