Core: fix and cleanup notification center tests (#7244)

* Fix log diagnostics tests and remove unused notification

* Replace deprecated OCMock methods with notification expectations

* Bump Core and CoreDiagnostics versions

* Defer Core version update until release

Co-authored-by: Paul Beusterien <[email protected]>

Author: maksymmalyhin

Example/CoreDiagnostics/Tests/FIRCoreDiagnosticsTest.m

@@ -31,7 +31,6 @@
 
 #import "Firebase/CoreDiagnostics/FIRCDLibrary/Protogen/nanopb/firebasecore.nanopb.h"
 
-extern NSString *const kFIRAppDiagnosticsNotification;
 extern NSString *const kFIRLastCheckinDateKey;
 
 static NSString *const kGoogleAppID = @"1:123:ios:123abc";

FirebaseCore.podspec

@@ -47,7 +47,7 @@ Firebase Core includes FIRApp and FIROptions which provide central configuration
   s.tvos.framework = 'UIKit'
   s.dependency 'GoogleUtilities/Environment', '~> 7.0'
   s.dependency 'GoogleUtilities/Logger', '~> 7.0'
-  s.dependency 'FirebaseCoreDiagnostics', '~> 7.0'
+  s.dependency 'FirebaseCoreDiagnostics', '~> 7.4'
 
   s.pod_target_xcconfig = {
     'GCC_C_LANGUAGE_STANDARD' => 'c99',

FirebaseCore/Sources/FIRApp.m

@@ -73,8 +73,6 @@
 NSString *const kFIRGlobalAppDataCollectionEnabledPlistKey =
     @"FirebaseDataCollectionDefaultEnabled";
 
-NSString *const kFIRAppDiagnosticsNotification = @"FIRAppDiagnosticsNotification";
-
 NSString *const kFIRAppDiagnosticsConfigurationTypeKey = @"ConfigType";
 NSString *const kFIRAppDiagnosticsErrorKey = @"Error";
 NSString *const kFIRAppDiagnosticsFIRAppKey = @"FIRApp";

FirebaseCore/Sources/Private/FIRAppInternal.h

@@ -52,11 +52,6 @@ extern NSString *const kFIRGlobalAppDataCollectionEnabledDefaultsKeyFormat;
  */
 extern NSString *const kFIRGlobalAppDataCollectionEnabledPlistKey;
 
-/**
- * A notification fired containing diagnostic information when SDK errors occur.
- */
-extern NSString *const kFIRAppDiagnosticsNotification;
-
 /** @var FIRAuthStateDidChangeInternalNotification
  @brief The name of the @c NSNotificationCenter notification which is posted when the auth state
  changes (e.g. a new token has been produced, a user logs in or out). The object parameter of

FirebaseCore/Tests/Unit/FIRAnalyticsConfigurationTest.m

@@ -19,8 +19,6 @@
 #import "FirebaseCore/Sources/FIRAnalyticsConfiguration.h"
 
 @interface FIRAnalyticsConfigurationTest : FIRTestCase
-/// An observer for NSNotificationCenter.
-@property(nonatomic, strong) id observerMock;
 
 @property(nonatomic, strong) NSNotificationCenter *notificationCenter;
 @end
@@ -30,12 +28,10 @@ @implementation FIRAnalyticsConfigurationTest
 - (void)setUp {
   [super setUp];
 
-  _observerMock = OCMObserverMock();
   _notificationCenter = [NSNotificationCenter defaultCenter];
 }
 
 - (void)tearDown {
-  _observerMock = nil;
   _notificationCenter = nil;
 
   [super tearDown];
@@ -72,14 +68,4 @@ - (void)testSettingAnalyticsCollectionPersistence {
   [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

FirebaseCore/Tests/Unit/FIRAppTest.m

@@ -53,10 +53,12 @@ + (nullable NSNumber *)readDataCollectionSwitchFromUserDefaultsForApp:(FIRApp *)
 @interface FIRAppTest : FIRTestCase
 
 @property(nonatomic) id appClassMock;
-@property(nonatomic) id observerMock;
 @property(nonatomic) id mockCoreDiagnosticsConnector;
 @property(nonatomic) NSNotificationCenter *notificationCenter;
 
+/// If `YES` then throws when `logCoreTelemetryWithOptions:` method is called.
+@property(nonatomic) BOOL assertNoLogCoreTelemetry;
+
 @end
 
 @implementation FIRAppTest
@@ -67,13 +69,17 @@ - (void)setUp {
   [FIRApp resetApps];
   // TODO: Don't mock the class we are testing.
   _appClassMock = OCMClassMock([FIRApp class]);
-  _observerMock = OCMObserverMock();
   _mockCoreDiagnosticsConnector = OCMClassMock([FIRCoreDiagnosticsConnector class]);
 
   [FIROptionsMock mockFIROptions];
 
+  self.assertNoLogCoreTelemetry = NO;
   OCMStub(ClassMethod([self.mockCoreDiagnosticsConnector logCoreTelemetryWithOptions:[OCMArg any]]))
-      .andDo(^(NSInvocation *invocation){
+      .andDo(^(NSInvocation *invocation) {
+        if (self.assertNoLogCoreTelemetry) {
+          XCTFail(@"Method `-[mockCoreDiagnosticsConnector logCoreTelemetryWithOptions:]` must not "
+                  @"be called");
+        }
       });
 
   // TODO: Remove all usages of defaultCenter in Core, then we can instantiate an instance here to
@@ -90,8 +96,6 @@ - (void)tearDown {
 
   [_appClassMock stopMocking];
   _appClassMock = nil;
-  [_notificationCenter removeObserver:_observerMock];
-  _observerMock = nil;
   _notificationCenter = nil;
   _mockCoreDiagnosticsConnector = nil;
 
@@ -105,12 +109,12 @@ - (void)testConfigure {
 
   NSDictionary *expectedUserInfo = [self expectedUserInfoWithAppName:kFIRDefaultAppName
                                                         isDefaultApp:YES];
-  [self expectNotificationForObserver:self.observerMock
-                     notificationName:kFIRAppReadyToConfigureSDKNotification
-                               object:[FIRApp class]
-                             userInfo:expectedUserInfo];
+  XCTestExpectation *notificationExpectation =
+      [self expectNotificationNamed:kFIRAppReadyToConfigureSDKNotification
+                             object:[FIRApp class]
+                           userInfo:expectedUserInfo];
   XCTAssertNoThrow([FIRApp configure]);
-  OCMVerifyAll(self.observerMock);
+  [self waitForExpectations:@[ notificationExpectation ] timeout:0.1];
 
   FIRApp *app = [FIRApp defaultApp];
   XCTAssertNotNil(app);
@@ -140,17 +144,18 @@ - (void)testConfigureWithOptions {
 
   NSDictionary *expectedUserInfo = [self expectedUserInfoWithAppName:kFIRDefaultAppName
                                                         isDefaultApp:YES];
-  [self expectNotificationForObserver:self.observerMock
-                     notificationName:kFIRAppReadyToConfigureSDKNotification
-                               object:[FIRApp class]
-                             userInfo:expectedUserInfo];
+
+  XCTestExpectation *notificationExpectation =
+      [self expectNotificationNamed:kFIRAppReadyToConfigureSDKNotification
+                             object:[FIRApp class]
+                           userInfo:expectedUserInfo];
 
   // Use a valid instance of options.
   FIROptions *options = [[FIROptions alloc] initWithGoogleAppID:kGoogleAppID
                                                     GCMSenderID:kGCMSenderID];
   options.clientID = kClientID;
   XCTAssertNoThrow([FIRApp configureWithOptions:options]);
-  OCMVerifyAll(self.observerMock);
+  [self waitForExpectations:@[ notificationExpectation ] timeout:0.1];
 
   // Verify the default app instance is created.
   FIRApp *app = [FIRApp defaultApp];
@@ -177,12 +182,12 @@ - (void)testConfigureWithNameAndOptions {
 
   NSDictionary *expectedUserInfo = [self expectedUserInfoWithAppName:kFIRTestAppName1
                                                         isDefaultApp:NO];
-  [self expectNotificationForObserver:self.observerMock
-                     notificationName:kFIRAppReadyToConfigureSDKNotification
-                               object:[FIRApp class]
-                             userInfo:expectedUserInfo];
+  XCTestExpectation *notificationExpectation =
+      [self expectNotificationNamed:kFIRAppReadyToConfigureSDKNotification
+                             object:[FIRApp class]
+                           userInfo:expectedUserInfo];
   XCTAssertNoThrow([FIRApp configureWithName:kFIRTestAppName1 options:options]);
-  OCMVerifyAll(self.observerMock);
+  [self waitForExpectations:@[ notificationExpectation ] timeout:0.1];
 
   XCTAssertTrue([FIRApp allApps].count == 1);
   FIRApp *app = [FIRApp appNamed:kFIRTestAppName1];
@@ -199,16 +204,12 @@ - (void)testConfigureWithMultipleApps {
                                                      GCMSenderID:kGCMSenderID];
   options1.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];
-  [[self.observerMock expect] notificationWithName:kFIRAppReadyToConfigureSDKNotification
-                                            object:[FIRApp class]
-                                          userInfo:expectedUserInfo1];
+  XCTestExpectation *configExpectation1 =
+      [self expectNotificationNamed:kFIRAppReadyToConfigureSDKNotification
+                             object:[FIRApp class]
+                           userInfo:expectedUserInfo1];
   XCTAssertNoThrow([FIRApp configureWithName:kFIRTestAppName1 options:options1]);
   XCTAssertTrue([FIRApp allApps].count == 1);
 
@@ -220,13 +221,16 @@ - (void)testConfigureWithMultipleApps {
 
   NSDictionary *expectedUserInfo2 = [self expectedUserInfoWithAppName:kFIRTestAppName2
                                                          isDefaultApp:NO];
-  [[self.observerMock expect] notificationWithName:kFIRAppReadyToConfigureSDKNotification
-                                            object:[FIRApp class]
-                                          userInfo:expectedUserInfo2];
+  XCTestExpectation *configExpectation2 =
+      [self expectNotificationNamed:kFIRAppReadyToConfigureSDKNotification
+                             object:[FIRApp class]
+                           userInfo:expectedUserInfo2];
 
-  [self.observerMock setExpectationOrderMatters:YES];
   XCTAssertNoThrow([FIRApp configureWithName:kFIRTestAppName2 options:options2]);
-  OCMVerifyAll(self.observerMock);
+
+  [self waitForExpectations:@[ configExpectation1, configExpectation2 ]
+                    timeout:0.1
+               enforceOrder:YES];
 
   XCTAssertTrue([FIRApp allApps].count == 2);
   FIRApp *app = [FIRApp appNamed:kFIRTestAppName2];
@@ -358,19 +362,21 @@ - (void)testDeleteApp {
   XCTAssertNotNil(FIR_COMPONENT(FIRTestProtocolEagerCached, app.container));
   XCTAssertNil(FIR_COMPONENT(FIRTestProtocol, app.container));
 
-  [self expectNotificationForObserver:self.observerMock
-                     notificationName:kFIRAppDeleteNotification
-                               object:[FIRApp class]
-                             userInfo:[OCMArg any]];
-  XCTestExpectation *expectation =
+  XCTestExpectation *notificationExpectation =
+      [self expectationForNotification:kFIRAppDeleteNotification
+                                object:[FIRApp class]
+                    notificationCenter:self.notificationCenter
+                               handler:nil];
+
+  XCTestExpectation *deleteExpectation =
       [self expectationWithDescription:@"Deleting the app should succeed."];
   [app deleteApp:^(BOOL success) {
     XCTAssertTrue(success);
-    [expectation fulfill];
+    [deleteExpectation fulfill];
   }];
 
-  [self waitForExpectations:@[ expectation ] timeout:1];
-  OCMVerifyAll(self.observerMock);
+  [self waitForExpectations:@[ notificationExpectation, deleteExpectation ] timeout:1];
+
   XCTAssertTrue([FIRApp allApps].count == 0);
 
   // Check no new library instances created after the app delete.
@@ -725,24 +731,22 @@ - (void)testGlobalDataCollectionNoDiagnosticsSent {
   FIRApp *app = [[FIRApp alloc] initInstanceWithName:NSStringFromSelector(_cmd) options:options];
   app.dataCollectionDefaultEnabled = NO;
 
-  // Add an observer for the diagnostics notification. Currently no object is sent, but in the
-  // future that could change so leave it as OCMOCK_ANY.
-  [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);
 
-  // Ensure configure doesn't fire a notification.
-  [FIRApp configure];
+  // Don't expect the diagnostics data to be sent.
+  self.assertNoLogCoreTelemetry = YES;
 
-  // The observer mock is strict and will raise an exception when an unexpected notification is
-  // received.
-  OCMVerifyAll(self.observerMock);
+  // The diagnostics data is expected to be sent on `UIApplicationDidBecomeActiveNotification` when
+  // data collection is enabled.
+  [FIRApp configure];
+  [self.notificationCenter postNotificationName:[self appDidBecomeActiveNotificationName]
+                                         object:nil];
+  // Wait for some time because diagnostics is logged asynchronously.
+  OCMVerifyAllWithDelay(self.mockCoreDiagnosticsConnector, 1);
 }
 
 #pragma mark - Analytics Flag Tests
@@ -930,12 +934,18 @@ - (void)testCoreDiagnosticsLoggedWhenAppDidBecomeActive {
 
 #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];
+- (XCTestExpectation *)expectNotificationNamed:(NSNotificationName)name
+                                        object:(nullable id)object
+                                      userInfo:(NSDictionary *)userInfo {
+  XCTestExpectation *notificationExpectation =
+      [self expectationForNotification:name
+                                object:object
+                    notificationCenter:self.notificationCenter
+                               handler:^BOOL(NSNotification *_Nonnull notification) {
+                                 return [userInfo isEqualToDictionary:notification.userInfo];
+                               }];
+
+  return notificationExpectation;
 }
 
 - (NSDictionary<NSString *, NSObject *> *)expectedUserInfoWithAppName:(NSString *)name

FirebaseCoreDiagnostics.podspec

@@ -1,6 +1,6 @@
 Pod::Spec.new do |s|
   s.name             = 'FirebaseCoreDiagnostics'
-  s.version          = '7.3.0'
+  s.version          = '7.4.0'
   s.summary          = 'Firebase Core Diagnostics'
 
   s.description      = <<-DESC