search

erikdoe / ocmock

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

Fix multiple expectation logging #498

Open dmaclach opened 3 years ago

dmaclach commented 3 years ago

Previously multiple expectations were not being logged correctly and would be missing names. You would get: OCClassMockObject(NSString): 2 expected methods were not invoked:.

erikdoe commented 3 years ago

I'm not sure I understand this change. The original code does append the names of the methods, and when I run the tests from this PR against the original code they pass. In other words: the tests from this PR do not require the code changes from this PR.

In detail: the names of the methods are appended in line 531. This is at the top-level of the loop iterating over all stubs/expectations. The only possibility that this line is not reached is that the if statemenet in line 521 evaluates to true, but that happens when the stub isn't an expectation.

Under which circumstances did you see the missing expectations without a list of the names?

dmaclach commented 3 years ago

I honestly can't remember. At that point I was trying to extract a bunch of changes I had made into small bits I could submit for review. I may have gone overboard here. Apologies for not doing a better job catching the error case better in the test. I can only assume I had a case I had found, wrote what I thought was a correct fix and then wrote a test but didn't verify that the test actually represented the case I was running into. Perhaps worthwhile keeping the test (because I believe it tests an untested branch) but dropping the change to OCMockObject?

erikdoe commented 3 years ago

Sounds like a plan. I will merge the test only.