Skip to content

Commit ef0f51e

Browse files
charlotteliangcharlotteliang
authored
Avoid using OCMock on NSOperation (#6315)
1 parent 7a85401 commit ef0f51e

File tree

2 files changed

+85
-86
lines changed

2 files changed

+85
-86
lines changed

‎Example/InstanceID/Tests/FIRInstanceIDTokenManagerTest.m

Lines changed: 59 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,43 @@ - (void)finishWithResult:(FIRInstanceIDTokenOperationResult)result
5353

5454
@end
5555

56+
// A Fake operation that we have control over the returned error.
57+
// We are not using mocks here because we have no way of forcing NSOperationQueues to release
58+
// their operations, and this means that there is always going to be a race condition between
59+
// when we "stop" our partial mock vs when NSOperationQueue attempts to access the mock object on a
60+
// separate thread. We had mocks previously.
61+
@interface FIRInstanceIDTokenDeleteOperationFake : FIRInstanceIDTokenDeleteOperation
62+
@property(nonatomic, copy) NSError *error;
63+
@end
64+
65+
@implementation FIRInstanceIDTokenDeleteOperationFake
66+
67+
- (void)performTokenOperation {
68+
if (self.error) {
69+
[self finishWithResult:FIRInstanceIDTokenOperationError token:nil error:self.error];
70+
} else {
71+
[self finishWithResult:FIRInstanceIDTokenOperationSucceeded token:kToken error:self.error];
72+
}
73+
}
74+
75+
@end
76+
77+
@interface FIRInstanceIDTokenFetchOperationFake : FIRInstanceIDTokenFetchOperation
78+
@property(nonatomic, copy) NSError *error;
79+
@end
80+
81+
@implementation FIRInstanceIDTokenFetchOperationFake
82+
83+
- (void)performTokenOperation {
84+
if (self.error) {
85+
[self finishWithResult:FIRInstanceIDTokenOperationError token:nil error:self.error];
86+
} else {
87+
[self finishWithResult:FIRInstanceIDTokenOperationSucceeded token:kToken error:self.error];
88+
}
89+
}
90+
91+
@end
92+
5693
@interface FIRInstanceIDTokenManager (ExposedForTests)
5794

5895
- (BOOL)checkTokenRefreshPolicyForIID:(NSString *)IID;
@@ -165,19 +202,12 @@ - (void)testNewTokenSuccess {
165202
NSDictionary *tokenOptions = [NSDictionary dictionary];
166203

167204
// Create a fake operation that always returns success
168-
FIRInstanceIDTokenFetchOperation *operation =
169-
[[FIRInstanceIDTokenFetchOperation alloc] initWithAuthorizedEntity:kAuthorizedEntity
170-
scope:kScope
171-
options:tokenOptions
172-
checkinPreferences:self.fakeCheckin
173-
instanceID:[OCMArg any]];
174-
id mockOperation = OCMPartialMock(operation);
175-
[[[mockOperation stub] andDo:^(NSInvocation *invocation) {
176-
[invocation.target finishWithResult:FIRInstanceIDTokenOperationSucceeded
177-
token:kToken
178-
error:nil];
179-
}] performTokenOperation];
180-
205+
FIRInstanceIDTokenFetchOperationFake *operation =
206+
[[FIRInstanceIDTokenFetchOperationFake alloc] initWithAuthorizedEntity:kAuthorizedEntity
207+
scope:kScope
208+
options:tokenOptions
209+
checkinPreferences:self.fakeCheckin
210+
instanceID:[OCMArg any]];
181211
XCTestExpectation *operationFinishExpectation =
182212
[self expectationWithDescription:@"operationFinishExpectation"];
183213
operation.completionBlock = ^{
@@ -203,11 +233,6 @@ - (void)testNewTokenSuccess {
203233
}];
204234

205235
[self waitForExpectations:@[ tokenExpectation, operationFinishExpectation ] timeout:1];
206-
207-
// Make sure the partial mock stops mocking before `operation` is deallocated to avoid crash.
208-
[mockOperation stopMocking];
209-
// Keep 'operation' alive, so it's not prematurely destroyed
210-
XCTAssertNotNil(operation);
211236
}
212237

213238
/**
@@ -223,18 +248,12 @@ - (void)testNewTokenSaveFailure {
223248
self.fakeKeyChain.cannotWriteToKeychain = YES;
224249

225250
// Create a fake operation that always returns success
226-
FIRInstanceIDTokenFetchOperation *operation =
227-
[[FIRInstanceIDTokenFetchOperation alloc] initWithAuthorizedEntity:kAuthorizedEntity
228-
scope:kScope
229-
options:tokenOptions
230-
checkinPreferences:self.fakeCheckin
231-
instanceID:[OCMArg any]];
232-
id mockOperation = OCMPartialMock(operation);
233-
[[[mockOperation stub] andDo:^(NSInvocation *invocation) {
234-
[invocation.target finishWithResult:FIRInstanceIDTokenOperationSucceeded
235-
token:kToken
236-
error:nil];
237-
}] performTokenOperation];
251+
FIRInstanceIDTokenFetchOperationFake *operation =
252+
[[FIRInstanceIDTokenFetchOperationFake alloc] initWithAuthorizedEntity:kAuthorizedEntity
253+
scope:kScope
254+
options:tokenOptions
255+
checkinPreferences:self.fakeCheckin
256+
instanceID:[OCMArg any]];
238257

239258
XCTestExpectation *operationFinishExpectation =
240259
[self expectationWithDescription:@"operationFinishExpectation"];
@@ -260,11 +279,6 @@ - (void)testNewTokenSaveFailure {
260279
}];
261280

262281
[self waitForExpectations:@[ tokenExpectation, operationFinishExpectation ] timeout:1];
263-
264-
// Make sure the partial mock stops mocking before `operation` is deallocated to avoid crash.
265-
[mockOperation stopMocking];
266-
// Keep 'operation' alive, so it's not prematurely destroyed
267-
XCTAssertNotNil(operation);
268282
}
269283

270284
/**
@@ -278,24 +292,19 @@ - (void)testNewTokenFailure {
278292
NSDictionary *tokenOptions = [NSDictionary dictionary];
279293

280294
// Create a fake operation that always returns failure
281-
FIRInstanceIDTokenFetchOperation *operation =
282-
[[FIRInstanceIDTokenFetchOperation alloc] initWithAuthorizedEntity:kAuthorizedEntity
283-
scope:kScope
284-
options:tokenOptions
285-
checkinPreferences:self.fakeCheckin
286-
instanceID:[OCMArg any]];
287-
id mockOperation = OCMPartialMock(operation);
288-
[[[mockOperation stub] andDo:^(NSInvocation *invocation) {
289-
NSError *someError = [[NSError alloc] initWithDomain:@"InstanceIDUnitTest" code:0 userInfo:nil];
290-
[invocation.target finishWithResult:FIRInstanceIDTokenOperationError token:nil error:someError];
291-
}] performTokenOperation];
295+
FIRInstanceIDTokenFetchOperationFake *operation =
296+
[[FIRInstanceIDTokenFetchOperationFake alloc] initWithAuthorizedEntity:kAuthorizedEntity
297+
scope:kScope
298+
options:tokenOptions
299+
checkinPreferences:self.fakeCheckin
300+
instanceID:[OCMArg any]];
301+
operation.error = [[NSError alloc] initWithDomain:@"InstanceIDUnitTest" code:0 userInfo:nil];
292302

293303
XCTestExpectation *operationFinishExpectation =
294304
[self expectationWithDescription:@"operationFinishExpectation"];
295305
operation.completionBlock = ^{
296306
[operationFinishExpectation fulfill];
297307
};
298-
299308
// Return our fake operation when asked for an operation
300309
[[[self.mockTokenManager stub] andReturn:operation]
301310
createFetchOperationWithAuthorizedEntity:[OCMArg any]
@@ -314,11 +323,6 @@ - (void)testNewTokenFailure {
314323
}];
315324

316325
[self waitForExpectations:@[ tokenExpectation, operationFinishExpectation ] timeout:1];
317-
318-
// Make sure the partial mock stops mocking before `operation` is deallocated to avoid crash.
319-
[mockOperation stopMocking];
320-
// Keep 'operation' alive, so it's not prematurely destroyed
321-
XCTAssertNotNil(operation);
322326
}
323327

324328
/**
@@ -329,16 +333,12 @@ - (void)testDeleteTokenSuccess {
329333
[self expectationWithDescription:@"Delete handler invoked."];
330334

331335
// Create a fake operation that always succeeds
332-
FIRInstanceIDTokenDeleteOperation *operation = [[FIRInstanceIDTokenDeleteOperation alloc]
336+
FIRInstanceIDTokenDeleteOperationFake *operation = [[FIRInstanceIDTokenDeleteOperationFake alloc]
333337
initWithAuthorizedEntity:kAuthorizedEntity
334338
scope:kScope
335339
checkinPreferences:self.fakeCheckin
336340
instanceID:[OCMArg any]
337341
action:FIRInstanceIDTokenActionDeleteToken];
338-
id mockOperation = OCMPartialMock(operation);
339-
[[[mockOperation stub] andDo:^(NSInvocation *invocation) {
340-
[invocation.target finishWithResult:FIRInstanceIDTokenOperationSucceeded token:nil error:nil];
341-
}] performTokenOperation];
342342

343343
XCTestExpectation *operationFinishExpectation =
344344
[self expectationWithDescription:@"operationFinishExpectation"];
@@ -363,11 +363,6 @@ - (void)testDeleteTokenSuccess {
363363
}];
364364

365365
[self waitForExpectations:@[ deleteExpectation, operationFinishExpectation ] timeout:1];
366-
367-
// Make sure the partial mock stops mocking before `operation` is deallocated to avoid crash.
368-
[mockOperation stopMocking];
369-
// Keep 'operation' alive, so it's not prematurely destroyed
370-
XCTAssertNotNil(operation);
371366
}
372367

373368
/**
@@ -378,17 +373,13 @@ - (void)testDeleteTokenFailure {
378373
[self expectationWithDescription:@"Delete handler invoked."];
379374

380375
// Create a fake operation that always fails
381-
FIRInstanceIDTokenDeleteOperation *operation = [[FIRInstanceIDTokenDeleteOperation alloc]
376+
FIRInstanceIDTokenDeleteOperationFake *operation = [[FIRInstanceIDTokenDeleteOperationFake alloc]
382377
initWithAuthorizedEntity:kAuthorizedEntity
383378
scope:kScope
384379
checkinPreferences:self.fakeCheckin
385380
instanceID:[OCMArg any]
386381
action:FIRInstanceIDTokenActionDeleteToken];
387-
id mockOperation = OCMPartialMock(operation);
388-
[[[mockOperation stub] andDo:^(NSInvocation *invocation) {
389-
NSError *someError = [[NSError alloc] initWithDomain:@"InstanceIDUnitTest" code:0 userInfo:nil];
390-
[invocation.target finishWithResult:FIRInstanceIDTokenOperationError token:nil error:someError];
391-
}] performTokenOperation];
382+
operation.error = [[NSError alloc] initWithDomain:@"InstanceIDUnitTest" code:0 userInfo:nil];
392383

393384
XCTestExpectation *operationFinishExpectation =
394385
[self expectationWithDescription:@"operationFinishExpectation"];
@@ -413,11 +404,6 @@ - (void)testDeleteTokenFailure {
413404
}];
414405

415406
[self waitForExpectations:@[ deleteExpectation, operationFinishExpectation ] timeout:1];
416-
417-
// Make sure the partial mock stops mocking before `operation` is deallocated to avoid crash.
418-
[mockOperation stopMocking];
419-
// Keep 'operation' alive, so it's not prematurely destroyed
420-
XCTAssertNotNil(operation);
421407
}
422408

423409
#pragma mark - Cached Token Invalidation

‎Example/InstanceID/Tests/FIRInstanceIDTokenOperationsTest.m

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,23 @@ @interface FIRInstallationsAuthTokenResult (Tests)
5252
- (instancetype)initWithToken:(NSString *)token expirationDate:(NSDate *)expirationDate;
5353
@end
5454

55+
// A Fake operation that allows us to check that perform was called.
56+
// We are not using mocks here because we have no way of forcing NSOperationQueues to release
57+
// their operations, and this means that there is always going to be a race condition between
58+
// when we "stop" our partial mock vs when NSOperationQueue attempts to access the mock object on a
59+
// separate thread. We had mocks previously.
60+
@interface FIRInstanceIDTokenOperationFake : FIRInstanceIDTokenOperation
61+
@property(nonatomic, assign) BOOL performWasCalled;
62+
@end
63+
64+
@implementation FIRInstanceIDTokenOperationFake
65+
66+
- (void)performTokenOperation {
67+
self.performWasCalled = YES;
68+
}
69+
70+
@end
71+
5572
@interface FIRInstanceIDTokenOperationsTest : XCTestCase
5673

5774
@property(strong, readonly, nonatomic) FIRInstanceIDAuthService *authService;
@@ -180,33 +197,29 @@ - (void)testThatAnAlreadyCancelledOperationFinishesWithoutStarting {
180197
[self expectationWithDescription:@"Operation finished as cancelled"];
181198
XCTestExpectation *didNotCallPerform =
182199
[self expectationWithDescription:@"Did not call performTokenOperation"];
183-
__block BOOL performWasCalled = NO;
184200

185201
int64_t tenHoursAgo = FIRInstanceIDCurrentTimestampInMilliseconds() - 10 * 60 * 60 * 1000;
186202
FIRInstanceIDCheckinPreferences *checkinPreferences =
187203
[self setCheckinPreferencesWithLastCheckinTime:tenHoursAgo];
188204

189-
FIRInstanceIDTokenOperation *operation =
190-
[[FIRInstanceIDTokenOperation alloc] initWithAction:FIRInstanceIDTokenActionFetch
191-
forAuthorizedEntity:kAuthorizedEntity
192-
scope:kScope
193-
options:nil
194-
checkinPreferences:checkinPreferences
195-
instanceID:self.instanceID];
205+
FIRInstanceIDTokenOperationFake *operation =
206+
[[FIRInstanceIDTokenOperationFake alloc] initWithAction:FIRInstanceIDTokenActionFetch
207+
forAuthorizedEntity:kAuthorizedEntity
208+
scope:kScope
209+
options:nil
210+
checkinPreferences:checkinPreferences
211+
instanceID:self.instanceID];
212+
operation.performWasCalled = NO;
196213
[operation addCompletionHandler:^(FIRInstanceIDTokenOperationResult result,
197214
NSString *_Nullable token, NSError *_Nullable error) {
198215
if (result == FIRInstanceIDTokenOperationCancelled) {
199216
[cancelledExpectation fulfill];
200217
}
201218

202-
if (!performWasCalled) {
219+
if (!operation.performWasCalled) {
203220
[didNotCallPerform fulfill];
204221
}
205222
}];
206-
id mockOperation = OCMPartialMock(operation);
207-
[[[mockOperation stub] andDo:^(NSInvocation *invocation) {
208-
performWasCalled = YES;
209-
}] performTokenOperation];
210223

211224
[operation cancel];
212225
[operation start];

0 commit comments

Comments
 (0)