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.

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 <FirebaseCore/FIRAnalyticsConfiguration.h>
 
 @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<NSString *, NSObject *> *)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() {