[Crashlytics] Fix flaky tests

Crashlytics/Crashlytics/Models/FIRCLSInternalReport.m

@@ -16,6 +16,7 @@
 // experiment
 
 #import "Crashlytics/Crashlytics/Models/FIRCLSInternalReport.h"
+#import "Crashlytics/Crashlytics/Private/FIRCLSInternalReport_Private.h"
 
 #import "Crashlytics/Crashlytics/Helpers/FIRCLSFile.h"
 #import "Crashlytics/Crashlytics/Helpers/FIRCLSLogger.h"
@@ -53,6 +54,8 @@ @interface FIRCLSInternalReport () {
 
 @implementation FIRCLSInternalReport
 
+@synthesize operationQueue = _operationQueue;
+
 + (instancetype)reportWithPath:(NSString *)path {
   return [[self alloc] initWithPath:path];
 }

Crashlytics/Crashlytics/Private/FIRCLSInternalReport_Private.h

@@ -0,0 +1,25 @@
+// Copyright 2025 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#import "Crashlytics/Crashlytics/Models/FIRCLSInternalReport.h"
+
+NS_ASSUME_NONNULL_BEGIN
+
+@interface FIRCLSInternalReport ()
+
+@property(nonatomic, readonly) NSOperationQueue *operationQueue;
+
+@end
+
+NS_ASSUME_NONNULL_END

Crashlytics/UnitTests/FIRCLSContextManagerTests.m

@@ -42,6 +42,10 @@ @implementation FIRCLSContextManagerTests
 
 - (void)setUp {
   self.fileManager = [[FIRCLSMockFileManager alloc] init];
+  [[NSFileManager defaultManager] createDirectoryAtPath:self.fileManager.rootPath
+                            withIntermediateDirectories:YES
+                                             attributes:nil
+                                                  error:nil];
   [self.fileManager createReportDirectories];
   [self.fileManager setupNewPathForExecutionIdentifier:TestContextReportID];
 

Crashlytics/UnitTests/FIRCLSExistingReportManagerTests.m

@@ -54,10 +54,11 @@
 
   self.fileManager = [[FIRCLSTempMockFileManager alloc] init];
 
-  // Cleanup potential artifacts from other test files.
+  // Clean up the directory and then re-create it to ensure a fresh state
   if ([[NSFileManager defaultManager] fileExistsAtPath:[self.fileManager rootPath]]) {
     assert([self.fileManager removeItemAtPath:[self.fileManager rootPath]]);
   }
+  [self.fileManager createReportDirectories];
 
   // Allow nil values only in tests
 #pragma clang diagnostic push
@@ -164,7 +165,7 @@
 
   // Reports without events should be deleted
   XCTAssertEqual([[self contentsOfActivePath] count], 0, @"Contents of active path: %@",
                  [self contentsOfActivePath]);
   XCTAssertEqual(self.existingReportManager.unsentReportsCount, 0);
   XCTAssertEqual(self.existingReportManager.newestUnsentReport, nil);
   XCTAssertEqual(self.existingReportManager.existingUnemptyActiveReportPaths.count, 0);

Crashlytics/UnitTests/FIRCLSReportUploaderTests.m

@@ -284,6 +284,9 @@ - (void)runUploadPackagedReportWithUrgency:(BOOL)urgent {
                                    asUrgent:urgent];
 
   XCTAssertNotNil(self.mockDataTransport.sendDataEvent_event);
+
+  // Wait a little bit for the file to be removed.
+  [NSRunLoop.currentRunLoop runUntilDate:[NSDate dateWithTimeIntervalSinceNow:0.1]];
   XCTAssertEqualObjects(self.fileManager.removedItemAtPath_path, [self packagePath]);
 }
 

Crashlytics/UnitTests/FIRCLSSettingsTests.m

@@ -127,27 +127,33 @@ - (BOOL)writeSettings:(const NSString *)settings
 - (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 @@ - (void)testCacheExpiredFromTTL {
   [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 @@ - (void)testCacheExpiredFromBuildInstanceID {
   [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 @@ - (void)testCacheExpiredFromAppVersion {
   [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];
 
@@ -302,6 +296,7 @@ - (void)testCacheExpiredFromAppVersion {
   XCTAssertEqual(self.settings.errorLogBufferSize, 128000);
 }
 
+#ifdef FLAKY_TEST
 - (void)testGoogleAppIDChanged {
   NSError *error = nil;
   [self writeSettings:FIRCLSTestSettingsInverse error:&error];
@@ -387,7 +382,7 @@ - (void)testCorruptCache {
   // 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);
@@ -429,6 +424,7 @@ - (void)testCorruptCacheKey {
   XCTAssertEqual(self.settings.onDemandBackoffBase, 1.5);
   XCTAssertEqual(self.settings.onDemandBackoffStepDuration, 6);
 }
+#endif  // FLAKY_TEST
 
 - (void)testNewReportEndpointSettings {
   NSString *settingsJSON =

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/FIRCrashlyticsReportTests.m

@@ -19,6 +19,7 @@
 #import "Crashlytics/Crashlytics/Components/FIRCLSGlobals.h"
 #import "Crashlytics/Crashlytics/Helpers/FIRCLSFile.h"
 #import "Crashlytics/Crashlytics/Models/FIRCLSInternalReport.h"
+#import "Crashlytics/Crashlytics/Private/FIRCLSInternalReport_Private.h"
 #import "Crashlytics/Crashlytics/Private/FIRCrashlyticsReport_Private.h"
 #import "Crashlytics/Crashlytics/Public/FirebaseCrashlytics/FIRCrashlyticsReport.h"
 
@@ -244,6 +245,7 @@ - (void)testCustomKeysLimits {
     NSString *key = [NSString stringWithFormat:@"key_%i", i];
     [report setCustomValue:@"hello" forKey:key];
   }
+  [report.internalReport.operationQueue waitUntilAllOperationsAreFinished];
 
   NSArray *entriesI = FIRCLSFileReadSections(
       [[report.internalReport pathForContentFile:FIRCLSReportUserIncrementalKVFile]
@@ -265,6 +267,7 @@ - (void)testLogsNoExisting {
 
   [report log:@"Normal log without formatting"];
   [report logWithFormat:@"%@, %@", @"First", @"Second"];
+  [report.internalReport.operationQueue waitUntilAllOperationsAreFinished];
 
   NSArray *entries = FIRCLSFileReadSections(
       [[report.internalReport pathForContentFile:FIRCLSReportLogAFile] fileSystemRepresentation],
@@ -283,6 +286,7 @@ - (void)testLogsWithExisting {
 
   [report log:@"Normal log without formatting"];
   [report logWithFormat:@"%@, %@", @"First", @"Second"];
+  [report.internalReport.operationQueue waitUntilAllOperationsAreFinished];
 
   NSArray *entries = FIRCLSFileReadSections(
       [[report.internalReport pathForContentFile:FIRCLSReportLogAFile] fileSystemRepresentation],
@@ -302,6 +306,7 @@ - (void)testLogLimits {
   for (int i = 0; i < 2000; i++) {
     [report log:@"0123456789"];
   }
+  [report.internalReport.operationQueue waitUntilAllOperationsAreFinished];
 
   unsigned long long sizeA = [[[NSFileManager defaultManager]
       attributesOfItemAtPath:[report.internalReport pathForContentFile:FIRCLSReportLogAFile]

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);
+      }
     }
   }
 }