[Crashlytics] Fix flaky tests

Crashlytics/UnitTests/FIRCLSSettingsTests.m

@@ -127,27 +127,33 @@
 - (void)cacheSettingsWithGoogleAppID:(NSString *)googleAppID
                     currentTimestamp:(NSTimeInterval)currentTimestamp
                  expectedRemoveCount:(NSInteger)expectedRemoveCount {
-  self.fileManager.removeExpectation = [[XCTestExpectation alloc]
-      initWithDescription:@"FIRCLSMockFileManager.removeExpectation.cache"];
   self.fileManager.removeCount = 0;
-  self.fileManager.expectedRemoveCount = expectedRemoveCount;
+
+  XCTestExpectation *expectation =
+      [self expectationForNotification:FIRCLSMockFileManagerDidRemoveItemNotification
+                                object:nil
+                               handler:nil];
+  expectation.expectedFulfillmentCount = expectedRemoveCount;
 
   [self.settings cacheSettingsWithGoogleAppID:googleAppID currentTimestamp:currentTimestamp];
 
-  [self waitForExpectations:@[ self.fileManager.removeExpectation ] timeout:1];
+  [self waitForExpectations:@[ expectation ] timeout:16.0];
 }
 
 - (void)reloadFromCacheWithGoogleAppID:(NSString *)googleAppID
                       currentTimestamp:(NSTimeInterval)currentTimestamp
                    expectedRemoveCount:(NSInteger)expectedRemoveCount {
-  self.fileManager.removeExpectation = [[XCTestExpectation alloc]
-      initWithDescription:@"FIRCLSMockFileManager.removeExpectation.reload"];
   self.fileManager.removeCount = 0;
-  self.fileManager.expectedRemoveCount = expectedRemoveCount;
+
+  XCTestExpectation *expectation =
+      [self expectationForNotification:FIRCLSMockFileManagerDidRemoveItemNotification
+                                object:nil
+                               handler:nil];
+  expectation.expectedFulfillmentCount = expectedRemoveCount;
 
   [self.settings reloadFromCacheWithGoogleAppID:googleAppID currentTimestamp:currentTimestamp];
 
-  [self waitForExpectations:@[ self.fileManager.removeExpectation ] timeout:5.0];
+  [self waitForExpectations:@[ expectation ] timeout:14.0];
 }
 
 - (void)testActivatedSettingsCached {
@@ -205,10 +211,6 @@
   [self writeSettings:FIRCLSTestSettingsActivated error:&error];
   XCTAssertNil(error, "%@", error);
 
-  // 1 delete for clearing the cache key, plus 2 for the deletes from reloading and clearing the
-  // cache and cache key
-  self.fileManager.expectedRemoveCount = 3;
-
   NSTimeInterval currentTimestamp = [NSDate timeIntervalSinceReferenceDate];
   [self.settings cacheSettingsWithGoogleAppID:TestGoogleAppID currentTimestamp:currentTimestamp];
 
@@ -238,10 +240,6 @@
   [self writeSettings:FIRCLSTestSettingsActivated error:&error];
   XCTAssertNil(error, "%@", error);
 
-  // 1 delete for clearing the cache key, plus 2 for the deletes from reloading and clearing the
-  // cache and cache key
-  self.fileManager.expectedRemoveCount = 3;
-
   NSTimeInterval currentTimestamp = [NSDate timeIntervalSinceReferenceDate];
   [self.settings cacheSettingsWithGoogleAppID:TestGoogleAppID currentTimestamp:currentTimestamp];
 
@@ -272,10 +270,6 @@
   [self writeSettings:FIRCLSTestSettingsActivated error:&error];
   XCTAssertNil(error, "%@", error);
 
-  // 1 delete for clearing the cache key, plus 2 for the deletes from reloading and clearing the
-  // cache and cache key
-  self.fileManager.expectedRemoveCount = 3;
-
   NSTimeInterval currentTimestamp = [NSDate timeIntervalSinceReferenceDate];
   [self.settings cacheSettingsWithGoogleAppID:TestGoogleAppID currentTimestamp:currentTimestamp];
 
@@ -387,7 +381,7 @@
   // Cache them, and reload. Since it's corrupted we should delete it all
   [self cacheSettingsWithGoogleAppID:TestGoogleAppID
                     currentTimestamp:currentTimestamp
-                 expectedRemoveCount:2];
+                 expectedRemoveCount:3];
 
   // Should have default values because we deleted the cache and settingsDictionary
   XCTAssertEqual(self.settings.isCacheExpired, YES);

Crashlytics/UnitTests/FIRCLSUserDefaultsTests.m

@@ -53,6 +53,9 @@ - (void)setUp {
 
 - (void)tearDown {
   [_userDefaults removeAllObjects];
+  [[NSUserDefaults standardUserDefaults] removeObjectForKey:_testKey1];
+  [[NSUserDefaults standardUserDefaults] removeObjectForKey:_testKey2];
+  [[NSUserDefaults standardUserDefaults] removeObjectForKey:_testKey3];
   [super tearDown];
 }
 

Crashlytics/UnitTests/Mocks/FIRCLSMockFileManager.h

@@ -16,18 +16,12 @@
 
 #import <XCTest/XCTest.h>
 
-@interface FIRCLSMockFileManager : FIRCLSFileManager
+// Notification posted when an item is removed via `removeItemAtPath`.
+extern NSNotificationName const FIRCLSMockFileManagerDidRemoveItemNotification;
 
-// Number of calls to removeItemAtPath are expected for the unit test
-@property(nonatomic) NSInteger expectedRemoveCount;
+@interface FIRCLSMockFileManager : FIRCLSFileManager
 
 // Incremented when a remove happens with removeItemAtPath
 @property(nonatomic) NSInteger removeCount;
 
-// Will be fulfilled when the expected number of removes have happened
-// using removeItemAtPath
-//
-// Users should initialize this in their test.
-@property(nonatomic, strong) XCTestExpectation *removeExpectation;
-
 @end

Crashlytics/UnitTests/Mocks/FIRCLSMockFileManager.m

@@ -14,6 +14,9 @@
 
 #import "Crashlytics/UnitTests/Mocks/FIRCLSMockFileManager.h"
 
+NSNotificationName const FIRCLSMockFileManagerDidRemoveItemNotification =
+    @"FIRCLSMockFileManagerDidRemoveItemNotification";
+
 @interface FIRCLSMockFileManager ()
 
 @property(nonatomic) NSMutableDictionary<NSString *, NSData *> *fileSystemDict;
@@ -34,68 +37,76 @@ - (instancetype)init {
 }
 
 - (BOOL)removeItemAtPath:(NSString *)path {
-  [self.fileSystemDict removeObjectForKey:path];
-
-  self.removeCount += 1;
-
-  // If we set up the expectation, and we went over the expected count or removes, fulfill the
-  // expectation
-  if (self.removeExpectation && self.removeCount >= self.expectedRemoveCount) {
-    [self.removeExpectation fulfill];
+  @synchronized(self) {
+    [self.fileSystemDict removeObjectForKey:path];
+    self.removeCount += 1;
   }
 
+  [[NSNotificationCenter defaultCenter]
+      postNotificationName:FIRCLSMockFileManagerDidRemoveItemNotification
+                    object:self];
+
   return YES;
 }
 
 - (BOOL)fileExistsAtPath:(NSString *)path {
-  return self.fileSystemDict[path] != nil;
+  @synchronized(self) {
+    return self.fileSystemDict[path] != nil;
+  }
 }
 
 - (BOOL)createFileAtPath:(NSString *)path
                 contents:(NSData *)data
               attributes:(NSDictionary<NSFileAttributeKey, id> *)attr {
-  self.fileSystemDict[path] = data;
+  @synchronized(self) {
+    self.fileSystemDict[path] = data;
+  }
   return YES;
 }
 
 - (NSArray *)activePathContents {
-  NSMutableArray *pathsWithActive = [[NSMutableArray alloc] init];
-  for (NSString *path in [_fileSystemDict allKeys]) {
-    if ([path containsString:@"v5/reports/active"]) {
-      [pathsWithActive addObject:path];
+  @synchronized(self) {
+    NSMutableArray *pathsWithActive = [[NSMutableArray alloc] init];
+    for (NSString *path in [_fileSystemDict allKeys]) {
+      if ([path containsString:@"v5/reports/active"]) {
+        [pathsWithActive addObject:path];
+      }
     }
+    return pathsWithActive;
   }
-
-  return pathsWithActive;
 }
 
 - (NSData *)dataWithContentsOfFile:(NSString *)path {
-  return self.fileSystemDict[path];
+  @synchronized(self) {
+    return self.fileSystemDict[path];
+  }
 }
 
 - (void)enumerateFilesInDirectory:(NSString *)directory
                        usingBlock:(void (^)(NSString *filePath, NSString *extension))block {
-  NSArray<NSString *> *filteredPaths = [self.fileSystemDict.allKeys
-      filteredArrayUsingPredicate:[NSPredicate predicateWithBlock:^BOOL(NSString *path,
-                                                                        NSDictionary *bindings) {
-        return [path hasPrefix:directory];
-      }]];
-
-  for (NSString *path in filteredPaths) {
-    NSString *extension;
-    NSString *fullPath;
-
-    // Skip files that start with a dot.  This is important, because if you try to move a .DS_Store
-    // file, it will fail if the target directory also has a .DS_Store file in it.  Plus, its
-    // wasteful, because we don't care about dot files.
-    if ([path hasPrefix:@"."]) {
-      continue;
-    }
-
-    extension = [path pathExtension];
-    fullPath = [directory stringByAppendingPathComponent:path];
-    if (block) {
-      block(fullPath, extension);
+  @synchronized(self) {
+    NSArray<NSString *> *filteredPaths = [self.fileSystemDict.allKeys
+        filteredArrayUsingPredicate:[NSPredicate predicateWithBlock:^BOOL(NSString *path,
+                                                                          NSDictionary *bindings) {
+          return [path hasPrefix:directory];
+        }]];
+
+    for (NSString *path in filteredPaths) {
+      NSString *extension;
+      NSString *fullPath;
+
+      // Skip files that start with a dot.  This is important, because if you try to move a
+      // .DS_Store file, it will fail if the target directory also has a .DS_Store file in
+      //  it.  Plus, it's wasteful, because we don't care about dot files.
+      if ([path hasPrefix:@"."]) {
+        continue;
+      }
+
+      extension = [path pathExtension];
+      fullPath = [directory stringByAppendingPathComponent:path];
+      if (block) {
+        block(fullPath, extension);
+      }
     }
   }
 }