Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conflict between class mocks and partial mocks #173

Open
mglidden opened this issue Jan 20, 2015 · 9 comments
Open

Conflict between class mocks and partial mocks #173

mglidden opened this issue Jan 20, 2015 · 9 comments

Comments

@mglidden
Copy link
Contributor

It seems that adding a partial mock will disable an existing class mock. We had a test case similar to the one below, where we wanted to mock access to a shared object and then mock a method on that shared object.

@interface TestClassWithClassMethods : NSObject
@property (copy, nonatomic) NSString *identifier;
@end

@implementation TestClassWithClassMethods
+ (instancetype)sharedObject {
  static TestClassWithClassMethods *sharedObject = nil;
  static dispatch_once_t onceToken;

  dispatch_once(&onceToken, ^{
    sharedObject = [[self alloc] init];
    sharedObject.identifier = @"shared object";
  });
  return sharedObject;
}
@end


@interface OCMockObjectClassMethodMockingTests : XCTestCase
@end

@implementation OCMockObjectClassMethodMockingTests
- (void)testClassAndPartialMock {
  TestClassWithClassMethods *mockObject = [[TestClassWithClassMethods alloc] init];
  mockObject.identifier = @"mock object";
  id mock = [OCMockObject mockForClass:[TestClassWithClassMethods class]];
  [[[[mock stub] classMethod] andReturn:mockObject] sharedObject];

  [[[OCMPartialMock([TestClassWithClassMethods sharedObject]) stub] andReturn:@"mocked method"] identifier];

  NSString *identifier = [TestClassWithClassMethods sharedObject].identifier;
  XCTAssertEqualObjects(identifier, @"mocked method", @"expected \"mocked method\", got \"%@\"", identifier);
}
@end

When setting up the partial mock:

[[[OCMPartialMock([TestClassWithClassMethods sharedObject]) stub] andReturn:@"mocked method"] identifier];

the sharedObject is the one we mocked out with the class mock, and it ends up with the partial mock as expected. However, the class mock is also disabled, so future calls to sharedObject go through to the real shared object instead of hitting the mocks.

I saw that you can only have one class mock on a class at a time (http://ocmock.org/reference/#advanced-topics) – is that true for a partial mock and class mock as well? If so, it would be really useful to have an assert to help catch that.

@erikdoe
Copy link
Owner

erikdoe commented Jan 21, 2015

This is expected behaviour. Partial mocks are class mocks and OCPartialMockObject inherits from OCClassMockObject. The documentation clearly says that only one mock object should mock class methods on a given class. I know this is awkward but the alternatives seem worse.

On the bright side, there is a simple solution in your case: simply use the partial mock to stub the class method:

- (void)testClassAndPartialMock {
  TestClassWithClassMethods *mockObject = [[TestClassWithClassMethods alloc] init];
  mockObject.identifier = @"mock object";

  id mock = OCMPartialMock(mockObject)
  [[[[mock stub] classMethod] andReturn:mockObject] sharedObject];
  [[[mock stub] andReturn:@"mocked method"] identifier];

  NSString *identifier = [TestClassWithClassMethods sharedObject].identifier;
  XCTAssertEqualObjects(identifier, @"mocked method", @"expected \"mocked method\", got \"%@\"", identifier);
}

@mglidden
Copy link
Contributor Author

What would you think about adding an assert when two class mocks are added to the same class? This issue caused a really subtle and hard to track bug in our test suite. I'd be happy to provide the patch.

@erikdoe
Copy link
Owner

erikdoe commented Jan 22, 2015

There is an obvious place in the code where to do this: https://github.com/erikdoe/ocmock/blob/master/Source/OCMock/OCClassMockObject.m#L85-88

However, there is a reason why the code is as it is. It is basically the consequence of another design decision. In OCMock you use the same kind of mock object to mock class methods and instance methods. And because it is often necessary to have two mock objects for the same class, when mocking instance methods, not allowing two mocks for the same class is just not practical.

The ideal solution would be to allow two mock objects to mock class methods on the same class. That's not so straight forward. Have a look at the code and see if you can think of a solution for that case.

@mglidden
Copy link
Contributor Author

Here's a work-in-progress branch for getting two class mocks working at once: https://github.com/mglidden/ocmock/commits/mglidden-double-the-mocks

There are still a few things to fix (expectations don't work yet, and releasing the original class mock won't stop its stubs from firing) but I think the basic approach will work.

@erikdoe
Copy link
Owner

erikdoe commented Feb 17, 2015

Thanks for the outline of a solution. It does look like it can be made to work along these lines. That said, have you had a chance to look into this further and address the known problems? I'm asking because I have been super busy otherwise, and I'm not sure I can take this over from where it is at right now, at least not for a while.

@erikdoe
Copy link
Owner

erikdoe commented Feb 24, 2015

In the meantime it occurred to me that it is possible to copy the stubs from the previous mock for the class to the new one. Somehow like this:

- (void)prepareClassForClassMethodMocking
{
    ...

    /* copy class method stubs from other mock */
    if(otherMock != nil)
    {
        for(OCMInvocationStub *s in [otherMock stubs])
        {
            if([s recordedAsClassMethod])
                [self addStub:s];
        }
    }
}

Of course, this is not entirely correct because now the lifetime of these stubs would be determined by the new mock and not the one that they were added with. However, it would solve this common problem and we're "safe" because the documentation simply says the effect of using two mocks to manipulate the same class is undefined. We'd just change the undefined behaviour Any opinions? Anything that I'm missing?

@mglidden
Copy link
Contributor Author

mglidden commented Mar 9, 2015

That would work, but I feel that the new behavior might be more confusing that the current one, since stopMocking wouldn't work on the first one anymore (and calling it on the second would would disable the first one). We ran into this issue when a global mock (setup at the start of our test suite) conflicted with a local mock for a single test. This new behavior would have still given us a bug.

I'm close to a solution – just a couple more bugs to squash.

@judos
Copy link

judos commented Dec 20, 2016

I am confused. This doesn't work:

TMLoginManager *loginManager = [TMLoginManager new];
// paste here
id classMock = OCMClassMock([TMLoginManager class]);
OCMStub([classMock sharedInstance]).andReturn(loginManager);
id loginManagerMock = OCMPartialMock(loginManager); // cut
XCTAssertEqual([TMLoginManager sharedInstance], loginManager);

But if you move the line (see comments) then it works just fine.

This is expected behaviour.

I find this very unintuitiv. If it can be improved I would love to have it different than now.

@erikdoe
Copy link
Owner

erikdoe commented Dec 21, 2016

As explained above, I would also like to improve it but I don't know how. The alternatives seem worse at this stage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants