From 8d399c78bda9529832d6ecd70a6c4c564c62da6d Mon Sep 17 00:00:00 2001 From: Ryan Wilson Date: Wed, 27 Jun 2018 09:42:09 -0400 Subject: Better mocking for Core Unit Tests (#1467) * Better mocking for Core Unit Tests This includes NSNotificationCenter and NSUserDefaults to prevent flaky tests and unit tests interfering with each other. * Share variable for intervals being set. --- Example/Core/Tests/FIRAnalyticsConfigurationTest.m | 100 ++++++++++++++------ Example/Core/Tests/FIRAppTest.m | 104 ++++++++++++++------- Example/Core/Tests/FIRLoggerTest.m | 22 ++--- Firebase/Core/FIRLogger.m | 22 ++++- 4 files changed, 170 insertions(+), 78 deletions(-) diff --git a/Example/Core/Tests/FIRAnalyticsConfigurationTest.m b/Example/Core/Tests/FIRAnalyticsConfigurationTest.m index a66ad06..8cf9da4 100644 --- a/Example/Core/Tests/FIRAnalyticsConfigurationTest.m +++ b/Example/Core/Tests/FIRAnalyticsConfigurationTest.m @@ -20,19 +20,25 @@ #import @interface FIRAnalyticsConfigurationTest : FIRTestCase -/// A mock for [NSNotificationCenter defaultCenter]. -@property(nonatomic, strong) id notificationCenterMock; +/// An observer for NSNotificationCenter. +@property(nonatomic, strong) id observerMock; + +@property(nonatomic, strong) NSNotificationCenter *notificationCenter; @end @implementation FIRAnalyticsConfigurationTest - (void)setUp { [super setUp]; - _notificationCenterMock = OCMPartialMock([NSNotificationCenter defaultCenter]); + + _observerMock = OCMObserverMock(); + _notificationCenter = [NSNotificationCenter defaultCenter]; } - (void)tearDown { - [_notificationCenterMock stopMocking]; + _observerMock = nil; + _notificationCenter = nil; + [super tearDown]; } @@ -44,49 +50,73 @@ /// Test that setting the minimum session interval on the singleton fires a notification. - (void)testMinimumSessionIntervalNotification { + // Pick a value to set as the session interval and verify it's in the userInfo dictionary of the + // posted notification. + NSNumber *sessionInterval = @2601; + + // Set up the expectation for the notification. FIRAnalyticsConfiguration *config = [FIRAnalyticsConfiguration sharedInstance]; - [config setMinimumSessionInterval:2601]; NSString *notificationName = kFIRAnalyticsConfigurationSetMinimumSessionIntervalNotification; - OCMVerify([self.notificationCenterMock postNotificationName:notificationName - object:config - userInfo:@{ - notificationName : @2601 - }]); + [self expectNotificationForObserver:self.observerMock + notificationName:notificationName + object:config + userInfo:@{notificationName : sessionInterval}]; + + // Trigger the notification. + [config setMinimumSessionInterval:[sessionInterval integerValue]]; + + // Verify the observer mock. + OCMVerifyAll(self.observerMock); } /// Test that setting the minimum session timeout interval on the singleton fires a notification. - (void)testSessionTimeoutIntervalNotification { + // Pick a value to set as the timeout interval and verify it's in the userInfo dictionary of the + // posted notification. + NSNumber *timeoutInterval = @1000; + + // Set up the expectation for the notification. FIRAnalyticsConfiguration *config = [FIRAnalyticsConfiguration sharedInstance]; - [config setSessionTimeoutInterval:1000]; NSString *notificationName = kFIRAnalyticsConfigurationSetSessionTimeoutIntervalNotification; - OCMVerify([self.notificationCenterMock postNotificationName:notificationName - object:config - userInfo:@{ - notificationName : @1000 - }]); + [self expectNotificationForObserver:self.observerMock + notificationName:notificationName + object:config + userInfo:@{notificationName : timeoutInterval}]; + + // Trigger the notification. + [config setSessionTimeoutInterval:[timeoutInterval integerValue]]; + + /// Verify the observer mock. + OCMVerifyAll(self.observerMock); } - (void)testSettingAnalyticsCollectionEnabled { - // The ordering matters for these notifications. - [self.notificationCenterMock setExpectationOrderMatters:YES]; - - // Test setting to enabled. + // Test setting to enabled. The ordering matters for these notifications. FIRAnalyticsConfiguration *config = [FIRAnalyticsConfiguration sharedInstance]; NSString *notificationName = kFIRAnalyticsConfigurationSetEnabledNotification; + [self.notificationCenter addMockObserver:self.observerMock name:notificationName object:config]; + + [self.observerMock setExpectationOrderMatters:YES]; + [[self.observerMock expect] notificationWithName:notificationName + object:config + userInfo:@{ + notificationName : @YES + }]; + + // Test setting to enabled. [config setAnalyticsCollectionEnabled:YES]; - OCMVerify([self.notificationCenterMock postNotificationName:notificationName - object:config - userInfo:@{ - notificationName : @YES - }]); + + // Expect the second notification. + [[self.observerMock expect] notificationWithName:notificationName + object:config + userInfo:@{ + notificationName : @NO + }]; // Test setting to disabled. [config setAnalyticsCollectionEnabled:NO]; - OCMVerify([self.notificationCenterMock postNotificationName:notificationName - object:config - userInfo:@{ - notificationName : @NO - }]); + + OCMVerifyAll(self.observerMock); } - (void)testSettingAnalyticsCollectionPersistence { @@ -114,4 +144,14 @@ [userDefaultsMock stopMocking]; } +#pragma mark - Private Test Helpers + +- (void)expectNotificationForObserver:(id)observer + notificationName:(NSNotificationName)name + object:(nullable id)object + userInfo:(nullable NSDictionary *)userInfo { + [self.notificationCenter addMockObserver:self.observerMock name:name object:object]; + [[observer expect] notificationWithName:name object:object userInfo:userInfo]; +} + @end diff --git a/Example/Core/Tests/FIRAppTest.m b/Example/Core/Tests/FIRAppTest.m index 549c1ab..656f046 100644 --- a/Example/Core/Tests/FIRAppTest.m +++ b/Example/Core/Tests/FIRAppTest.m @@ -52,8 +52,9 @@ NSString *const kFIRTestAppName2 = @"test-app-name-2"; @property(nonatomic) id appClassMock; @property(nonatomic) id optionsInstanceMock; -@property(nonatomic) id notificationCenterMock; +@property(nonatomic) id observerMock; @property(nonatomic) FIRApp *app; +@property(nonatomic) NSNotificationCenter *notificationCenter; @end @@ -65,13 +66,19 @@ NSString *const kFIRTestAppName2 = @"test-app-name-2"; [FIRApp resetApps]; _appClassMock = OCMClassMock([FIRApp class]); _optionsInstanceMock = OCMPartialMock([FIROptions defaultOptions]); - _notificationCenterMock = OCMPartialMock([NSNotificationCenter defaultCenter]); + _observerMock = OCMObserverMock(); + + // TODO: Remove all usages of defaultCenter in Core, then we can instantiate an instance here to + // inject instead of using defaultCenter. + _notificationCenter = [NSNotificationCenter defaultCenter]; } - (void)tearDown { [_appClassMock stopMocking]; [_optionsInstanceMock stopMocking]; - [_notificationCenterMock stopMocking]; + [_notificationCenter removeObserver:_observerMock]; + _observerMock = nil; + _notificationCenter = nil; [super tearDown]; } @@ -79,11 +86,12 @@ NSString *const kFIRTestAppName2 = @"test-app-name-2"; - (void)testConfigure { NSDictionary *expectedUserInfo = [self expectedUserInfoWithAppName:kFIRDefaultAppName isDefaultApp:YES]; - OCMExpect([self.notificationCenterMock postNotificationName:kFIRAppReadyToConfigureSDKNotification - object:[FIRApp class] - userInfo:expectedUserInfo]); + [self expectNotificationForObserver:self.observerMock + notificationName:kFIRAppReadyToConfigureSDKNotification + object:[FIRApp class] + userInfo:expectedUserInfo]; XCTAssertNoThrow([FIRApp configure]); - OCMVerifyAll(self.notificationCenterMock); + OCMVerifyAll(self.observerMock); self.app = [FIRApp defaultApp]; XCTAssertNotNil(self.app); @@ -108,12 +116,13 @@ NSString *const kFIRTestAppName2 = @"test-app-name-2"; NSDictionary *expectedUserInfo = [self expectedUserInfoWithAppName:kFIRDefaultAppName isDefaultApp:YES]; - OCMExpect([self.notificationCenterMock postNotificationName:kFIRAppReadyToConfigureSDKNotification - object:[FIRApp class] - userInfo:expectedUserInfo]); + [self expectNotificationForObserver:self.observerMock + notificationName:kFIRAppReadyToConfigureSDKNotification + object:[FIRApp class] + userInfo:expectedUserInfo]; // default options XCTAssertNoThrow([FIRApp configureWithOptions:[FIROptions defaultOptions]]); - OCMVerifyAll(self.notificationCenterMock); + OCMVerifyAll(self.observerMock); self.app = [FIRApp defaultApp]; XCTAssertNotNil(self.app); @@ -130,12 +139,13 @@ NSString *const kFIRTestAppName2 = @"test-app-name-2"; options.APIKey = kCustomizedAPIKey; NSDictionary *expectedUserInfo = [self expectedUserInfoWithAppName:kFIRDefaultAppName isDefaultApp:YES]; - OCMExpect([self.notificationCenterMock postNotificationName:kFIRAppReadyToConfigureSDKNotification - object:[FIRApp class] - userInfo:expectedUserInfo]); + [self expectNotificationForObserver:self.observerMock + notificationName:kFIRAppReadyToConfigureSDKNotification + object:[FIRApp class] + userInfo:expectedUserInfo]; XCTAssertNoThrow([FIRApp configureWithOptions:options]); - OCMVerifyAll(self.notificationCenterMock); + OCMVerifyAll(self.observerMock); self.app = [FIRApp defaultApp]; XCTAssertNotNil(self.app); @@ -158,11 +168,12 @@ NSString *const kFIRTestAppName2 = @"test-app-name-2"; NSDictionary *expectedUserInfo = [self expectedUserInfoWithAppName:kFIRTestAppName1 isDefaultApp:NO]; - OCMExpect([self.notificationCenterMock postNotificationName:kFIRAppReadyToConfigureSDKNotification - object:[FIRApp class] - userInfo:expectedUserInfo]); + [self expectNotificationForObserver:self.observerMock + notificationName:kFIRAppReadyToConfigureSDKNotification + object:[FIRApp class] + userInfo:expectedUserInfo]; XCTAssertNoThrow([FIRApp configureWithName:kFIRTestAppName1 options:[FIROptions defaultOptions]]); - OCMVerifyAll(self.notificationCenterMock); + OCMVerifyAll(self.observerMock); XCTAssertTrue([FIRApp allApps].count == 1); self.app = [FIRApp appNamed:kFIRTestAppName1]; @@ -179,11 +190,16 @@ NSString *const kFIRTestAppName2 = @"test-app-name-2"; FIROptions *newOptions = [options copy]; newOptions.deepLinkURLScheme = kDeepLinkURLScheme; + // Set up notification center observer for verifying notifications. + [self.notificationCenter addMockObserver:self.observerMock + name:kFIRAppReadyToConfigureSDKNotification + object:[FIRApp class]]; + NSDictionary *expectedUserInfo1 = [self expectedUserInfoWithAppName:kFIRTestAppName1 isDefaultApp:NO]; - OCMExpect([self.notificationCenterMock postNotificationName:kFIRAppReadyToConfigureSDKNotification - object:[FIRApp class] - userInfo:expectedUserInfo1]); + [[self.observerMock expect] notificationWithName:kFIRAppReadyToConfigureSDKNotification + object:[FIRApp class] + userInfo:expectedUserInfo1]; XCTAssertNoThrow([FIRApp configureWithName:kFIRTestAppName1 options:newOptions]); XCTAssertTrue([FIRApp allApps].count == 1); self.app = [FIRApp appNamed:kFIRTestAppName1]; @@ -196,11 +212,13 @@ NSString *const kFIRTestAppName2 = @"test-app-name-2"; NSDictionary *expectedUserInfo2 = [self expectedUserInfoWithAppName:kFIRTestAppName2 isDefaultApp:NO]; - OCMExpect([self.notificationCenterMock postNotificationName:kFIRAppReadyToConfigureSDKNotification - object:[FIRApp class] - userInfo:expectedUserInfo2]); + [[self.observerMock expect] notificationWithName:kFIRAppReadyToConfigureSDKNotification + object:[FIRApp class] + userInfo:expectedUserInfo2]; + + [self.observerMock setExpectationOrderMatters:YES]; XCTAssertNoThrow([FIRApp configureWithName:kFIRTestAppName2 options:customizedOptions]); - OCMVerifyAll(self.notificationCenterMock); + OCMVerifyAll(self.observerMock); XCTAssertTrue([FIRApp allApps].count == 2); self.app = [FIRApp appNamed:kFIRTestAppName2]; @@ -241,12 +259,15 @@ NSString *const kFIRTestAppName2 = @"test-app-name-2"; [FIRApp configure]; self.app = [FIRApp defaultApp]; XCTAssertTrue([FIRApp allApps].count == 1); + [self expectNotificationForObserver:self.observerMock + notificationName:kFIRAppDeleteNotification + object:[FIRApp class] + userInfo:[OCMArg any]]; [self.app deleteApp:^(BOOL success) { XCTAssertTrue(success); }]; - OCMVerify([self.notificationCenterMock postNotificationName:kFIRAppDeleteNotification - object:[FIRApp class] - userInfo:[OCMArg any]]); + + OCMVerifyAll(self.observerMock); XCTAssertTrue(self.app.alreadySentDeleteNotification); XCTAssertTrue([FIRApp allApps].count == 0); } @@ -671,16 +692,27 @@ NSString *const kFIRTestAppName2 = @"test-app-name-2"; - (void)testGlobalDataCollectionNoDiagnosticsSent { [FIRApp configure]; + // Add an observer for the diagnostics notification - both with and without an object to ensure it + // catches it either way. Currently no object is sent, but in the future that could change. + [self.notificationCenter addMockObserver:self.observerMock + name:kFIRAppDiagnosticsNotification + object:nil]; + [self.notificationCenter addMockObserver:self.observerMock + name:kFIRAppDiagnosticsNotification + object:OCMOCK_ANY]; + // Stub out reading from user defaults since stubbing out the BOOL has issues. If the data // collection switch is disabled, the `sendLogs` call should return immediately and not fire a // notification. OCMStub([self.appClassMock readDataCollectionSwitchFromUserDefaultsForApp:OCMOCK_ANY]) .andReturn(@NO); - OCMReject([self.notificationCenterMock postNotificationName:kFIRAppDiagnosticsNotification - object:OCMOCK_ANY - userInfo:OCMOCK_ANY]); + NSError *error = [NSError errorWithDomain:@"com.firebase" code:42 userInfo:nil]; [[FIRApp defaultApp] sendLogsWithServiceName:@"Service" version:@"Version" error:error]; + + // The observer mock is strict and will raise an exception when an unexpected notification is + // received. + OCMVerifyAll(self.observerMock); } #pragma mark - Analytics Flag Tests @@ -767,6 +799,14 @@ NSString *const kFIRTestAppName2 = @"test-app-name-2"; #pragma mark - private +- (void)expectNotificationForObserver:(id)observer + notificationName:(NSNotificationName)name + object:(nullable id)object + userInfo:(nullable NSDictionary *)userInfo { + [self.notificationCenter addMockObserver:observer name:name object:object]; + [[observer expect] notificationWithName:name object:object userInfo:userInfo]; +} + - (NSDictionary *)expectedUserInfoWithAppName:(NSString *)name isDefaultApp:(BOOL)isDefaultApp { return @{ diff --git a/Example/Core/Tests/FIRLoggerTest.m b/Example/Core/Tests/FIRLoggerTest.m index c1ba37b..b871244 100644 --- a/Example/Core/Tests/FIRLoggerTest.m +++ b/Example/Core/Tests/FIRLoggerTest.m @@ -31,6 +31,8 @@ extern const char *kFIRLoggerASLClientFacilityName; extern void FIRResetLogger(void); +extern void FIRSetLoggerUserDefaults(NSUserDefaults *defaults); + extern aslclient getFIRLoggerClient(void); extern dispatch_queue_t getFIRClientQueue(void); @@ -43,7 +45,7 @@ static NSString *const kMessageCode = @"I-COR000001"; @property(nonatomic) NSString *randomLogString; -@property(nonatomic, strong) id userDefaultsMock; +@property(nonatomic, strong) NSUserDefaults *defaults; @end @@ -53,14 +55,15 @@ static NSString *const kMessageCode = @"I-COR000001"; [super setUp]; FIRResetLogger(); - // Stub NSUserDefaults for tracking the error and warning count. - _userDefaultsMock = OCMPartialMock([NSUserDefaults standardUserDefaults]); + // Stub NSUserDefaults for cleaner testing. + _defaults = [[NSUserDefaults alloc] initWithSuiteName:@"com.firebase.logger_test"]; + FIRSetLoggerUserDefaults(_defaults); } - (void)tearDown { [super tearDown]; - [_userDefaultsMock stopMocking]; + _defaults = nil; } // Test some stable variables to make sure they weren't accidently changed. @@ -92,8 +95,7 @@ static NSString *const kMessageCode = @"I-COR000001"; FIRLogError(kFIRLoggerCore, kMessageCode, @"Some error."); // Assert. - NSNumber *debugMode = - [[NSUserDefaults standardUserDefaults] objectForKey:kFIRPersistedDebugModeKey]; + NSNumber *debugMode = [self.defaults objectForKey:kFIRPersistedDebugModeKey]; XCTAssertNil(debugMode); XCTAssertFalse(getFIRLoggerDebugMode()); @@ -111,8 +113,7 @@ static NSString *const kMessageCode = @"I-COR000001"; FIRLogError(kFIRLoggerCore, kMessageCode, @"Some error."); // Assert. - NSNumber *debugMode = - [[NSUserDefaults standardUserDefaults] objectForKey:kFIRPersistedDebugModeKey]; + NSNumber *debugMode = [self.defaults objectForKey:kFIRPersistedDebugModeKey]; XCTAssertTrue(debugMode.boolValue); XCTAssertTrue(getFIRLoggerDebugMode()); @@ -123,14 +124,13 @@ static NSString *const kMessageCode = @"I-COR000001"; - (void)testInitializeASLForDebugModeWithUserDefaults { // Stub. NSNumber *debugMode = @YES; - OCMStub([self.userDefaultsMock boolForKey:kFIRPersistedDebugModeKey]) - .andReturn(debugMode.boolValue); + [self.defaults setBool:debugMode.boolValue forKey:kFIRPersistedDebugModeKey]; // Test. FIRLogError(kFIRLoggerCore, kMessageCode, @"Some error."); // Assert. - debugMode = [[NSUserDefaults standardUserDefaults] objectForKey:kFIRPersistedDebugModeKey]; + debugMode = [self.defaults objectForKey:kFIRPersistedDebugModeKey]; XCTAssertTrue(debugMode.boolValue); XCTAssertTrue(getFIRLoggerDebugMode()); } diff --git a/Firebase/Core/FIRLogger.m b/Firebase/Core/FIRLogger.m index ae14e9f..2784ae9 100644 --- a/Firebase/Core/FIRLogger.m +++ b/Firebase/Core/FIRLogger.m @@ -64,6 +64,10 @@ static aslclient sFIRLoggerClient; static dispatch_queue_t sFIRClientQueue; +/// NSUserDefaults that should be used to store and read variables. If nil, `standardUserDefaults` +/// will be used. +static NSUserDefaults *sFIRLoggerUserDefaults; + static BOOL sFIRLoggerDebugMode; // The sFIRAnalyticsDebugMode flag is here to support the -FIRDebugEnabled/-FIRDebugDisabled @@ -113,14 +117,17 @@ void FIRLoggerInitializeASL() { sFIRAnalyticsDebugMode = NO; sFIRLoggerMaximumLevel = FIRLoggerLevelNotice; - NSUserDefaults *userDefaults = [NSUserDefaults standardUserDefaults]; - BOOL debugMode = [userDefaults boolForKey:kFIRPersistedDebugModeKey]; + // Use the standard NSUserDefaults if it hasn't been explicitly set. + if (sFIRLoggerUserDefaults == nil) { + sFIRLoggerUserDefaults = [NSUserDefaults standardUserDefaults]; + } + BOOL debugMode = [sFIRLoggerUserDefaults boolForKey:kFIRPersistedDebugModeKey]; if ([arguments containsObject:kFIRDisableDebugModeApplicationArgument]) { // Default mode - [userDefaults removeObjectForKey:kFIRPersistedDebugModeKey]; + [sFIRLoggerUserDefaults removeObjectForKey:kFIRPersistedDebugModeKey]; } else if ([arguments containsObject:kFIREnableDebugModeApplicationArgument] || debugMode) { // Debug mode - [userDefaults setBool:YES forKey:kFIRPersistedDebugModeKey]; + [sFIRLoggerUserDefaults setBool:YES forKey:kFIRPersistedDebugModeKey]; asl_set_filter(sFIRLoggerClient, ASL_FILTER_MASK_UPTO(ASL_LEVEL_DEBUG)); sFIRLoggerDebugMode = YES; } @@ -190,7 +197,12 @@ __attribute__((no_sanitize("thread"))) BOOL FIRIsLoggableLevel(FIRLoggerLevel lo #ifdef DEBUG void FIRResetLogger() { sFIRLoggerOnceToken = 0; - [[NSUserDefaults standardUserDefaults] removeObjectForKey:kFIRPersistedDebugModeKey]; + [sFIRLoggerUserDefaults removeObjectForKey:kFIRPersistedDebugModeKey]; + sFIRLoggerUserDefaults = nil; +} + +void FIRSetLoggerUserDefaults(NSUserDefaults *defaults) { + sFIRLoggerUserDefaults = defaults; } aslclient getFIRLoggerClient() { -- cgit v1.2.3