From 09d61fba3eee1e203621c193f9dcef96123d11e6 Mon Sep 17 00:00:00 2001 From: Paul Beusterien Date: Thu, 27 Nov 2025 08:21:06 -0800 Subject: [PATCH 01/18] [Crashlytics] Fix flaky tests --- Crashlytics/UnitTests/Mocks/FIRCLSMockFileManager.m | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Crashlytics/UnitTests/Mocks/FIRCLSMockFileManager.m b/Crashlytics/UnitTests/Mocks/FIRCLSMockFileManager.m index b89d05bbff8..955e701d065 100644 --- a/Crashlytics/UnitTests/Mocks/FIRCLSMockFileManager.m +++ b/Crashlytics/UnitTests/Mocks/FIRCLSMockFileManager.m @@ -41,7 +41,9 @@ - (BOOL)removeItemAtPath:(NSString *)path { // 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]; + dispatch_async(dispatch_get_main_queue(), ^{ + [self.removeExpectation fulfill]; + }); } return YES; From c6c90adffbf7eb7a08007900e86b3f34606766f3 Mon Sep 17 00:00:00 2001 From: Paul Beusterien Date: Thu, 27 Nov 2025 10:06:58 -0800 Subject: [PATCH 02/18] Second try --- Crashlytics/UnitTests/FIRCLSSettingsTests.m | 22 ++++++++++++------- .../UnitTests/Mocks/FIRCLSMockFileManager.h | 12 +++------- .../UnitTests/Mocks/FIRCLSMockFileManager.m | 13 +++++------ 3 files changed, 23 insertions(+), 24 deletions(-) diff --git a/Crashlytics/UnitTests/FIRCLSSettingsTests.m b/Crashlytics/UnitTests/FIRCLSSettingsTests.m index e2bfb58068f..3cc2aa21bf9 100644 --- a/Crashlytics/UnitTests/FIRCLSSettingsTests.m +++ b/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:self.fileManager + handler:nil]; + expectation.expectedFulfillmentCount = expectedRemoveCount; [self.settings cacheSettingsWithGoogleAppID:googleAppID currentTimestamp:currentTimestamp]; - [self waitForExpectations:@[ self.fileManager.removeExpectation ] timeout:1]; + [self waitForExpectations:@[ expectation ] timeout:1]; } - (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:self.fileManager + handler:nil]; + expectation.expectedFulfillmentCount = expectedRemoveCount; [self.settings reloadFromCacheWithGoogleAppID:googleAppID currentTimestamp:currentTimestamp]; - [self waitForExpectations:@[ self.fileManager.removeExpectation ] timeout:5.0]; + [self waitForExpectations:@[ expectation ] timeout:5.0]; } - (void)testActivatedSettingsCached { diff --git a/Crashlytics/UnitTests/Mocks/FIRCLSMockFileManager.h b/Crashlytics/UnitTests/Mocks/FIRCLSMockFileManager.h index 3bff289bf03..2b70854198b 100644 --- a/Crashlytics/UnitTests/Mocks/FIRCLSMockFileManager.h +++ b/Crashlytics/UnitTests/Mocks/FIRCLSMockFileManager.h @@ -16,18 +16,12 @@ #import -@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 diff --git a/Crashlytics/UnitTests/Mocks/FIRCLSMockFileManager.m b/Crashlytics/UnitTests/Mocks/FIRCLSMockFileManager.m index 955e701d065..23ef8dba744 100644 --- a/Crashlytics/UnitTests/Mocks/FIRCLSMockFileManager.m +++ b/Crashlytics/UnitTests/Mocks/FIRCLSMockFileManager.m @@ -14,6 +14,9 @@ #import "Crashlytics/UnitTests/Mocks/FIRCLSMockFileManager.h" +NSNotificationName const FIRCLSMockFileManagerDidRemoveItemNotification = + @"FIRCLSMockFileManagerDidRemoveItemNotification"; + @interface FIRCLSMockFileManager () @property(nonatomic) NSMutableDictionary *fileSystemDict; @@ -38,13 +41,9 @@ - (BOOL)removeItemAtPath:(NSString *)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) { - dispatch_async(dispatch_get_main_queue(), ^{ - [self.removeExpectation fulfill]; - }); - } + [[NSNotificationCenter defaultCenter] + postNotificationName:FIRCLSMockFileManagerDidRemoveItemNotification + object:self]; return YES; } From 18d6d485c597f0e837e3d9060a5e8d15fb6b315d Mon Sep 17 00:00:00 2001 From: Paul Beusterien Date: Thu, 27 Nov 2025 11:38:50 -0800 Subject: [PATCH 03/18] build fixes --- Crashlytics/UnitTests/FIRCLSSettingsTests.m | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/Crashlytics/UnitTests/FIRCLSSettingsTests.m b/Crashlytics/UnitTests/FIRCLSSettingsTests.m index 3cc2aa21bf9..f50530bee38 100644 --- a/Crashlytics/UnitTests/FIRCLSSettingsTests.m +++ b/Crashlytics/UnitTests/FIRCLSSettingsTests.m @@ -211,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]; @@ -244,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]; @@ -278,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]; From 21e20adf65f4a5980ae92de14bab5a369906690d Mon Sep 17 00:00:00 2001 From: Paul Beusterien Date: Thu, 27 Nov 2025 12:06:41 -0800 Subject: [PATCH 04/18] fixes --- Crashlytics/UnitTests/FIRCLSSettingsTests.m | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Crashlytics/UnitTests/FIRCLSSettingsTests.m b/Crashlytics/UnitTests/FIRCLSSettingsTests.m index f50530bee38..0a09381aa11 100644 --- a/Crashlytics/UnitTests/FIRCLSSettingsTests.m +++ b/Crashlytics/UnitTests/FIRCLSSettingsTests.m @@ -131,7 +131,7 @@ - (void)cacheSettingsWithGoogleAppID:(NSString *)googleAppID XCTestExpectation *expectation = [self expectationForNotification:FIRCLSMockFileManagerDidRemoveItemNotification - object:self.fileManager + object:nil handler:nil]; expectation.expectedFulfillmentCount = expectedRemoveCount; @@ -147,7 +147,7 @@ - (void)reloadFromCacheWithGoogleAppID:(NSString *)googleAppID XCTestExpectation *expectation = [self expectationForNotification:FIRCLSMockFileManagerDidRemoveItemNotification - object:self.fileManager + object:nil handler:nil]; expectation.expectedFulfillmentCount = expectedRemoveCount; @@ -381,7 +381,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); From 3e59f61ce88af346afbdd4dd4fc723b6cc8dc3f1 Mon Sep 17 00:00:00 2001 From: Paul Beusterien Date: Thu, 27 Nov 2025 12:31:12 -0800 Subject: [PATCH 05/18] timeout increases --- Crashlytics/UnitTests/FIRCLSSettingsTests.m | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Crashlytics/UnitTests/FIRCLSSettingsTests.m b/Crashlytics/UnitTests/FIRCLSSettingsTests.m index 0a09381aa11..61e7ab1f984 100644 --- a/Crashlytics/UnitTests/FIRCLSSettingsTests.m +++ b/Crashlytics/UnitTests/FIRCLSSettingsTests.m @@ -137,7 +137,7 @@ - (void)cacheSettingsWithGoogleAppID:(NSString *)googleAppID [self.settings cacheSettingsWithGoogleAppID:googleAppID currentTimestamp:currentTimestamp]; - [self waitForExpectations:@[ expectation ] timeout:1]; + [self waitForExpectations:@[ expectation ] timeout:5.0]; } - (void)reloadFromCacheWithGoogleAppID:(NSString *)googleAppID @@ -153,7 +153,7 @@ - (void)reloadFromCacheWithGoogleAppID:(NSString *)googleAppID [self.settings reloadFromCacheWithGoogleAppID:googleAppID currentTimestamp:currentTimestamp]; - [self waitForExpectations:@[ expectation ] timeout:5.0]; + [self waitForExpectations:@[ expectation ] timeout:10.0]; } - (void)testActivatedSettingsCached { From 8c1c5bb797b4d2ad35115079457616924c5fcb83 Mon Sep 17 00:00:00 2001 From: Paul Beusterien Date: Thu, 27 Nov 2025 15:11:30 -0800 Subject: [PATCH 06/18] fix another flake --- Crashlytics/UnitTests/FIRCLSUserDefaultsTests.m | 1 + 1 file changed, 1 insertion(+) diff --git a/Crashlytics/UnitTests/FIRCLSUserDefaultsTests.m b/Crashlytics/UnitTests/FIRCLSUserDefaultsTests.m index e310d85121c..3f9c11eeb43 100644 --- a/Crashlytics/UnitTests/FIRCLSUserDefaultsTests.m +++ b/Crashlytics/UnitTests/FIRCLSUserDefaultsTests.m @@ -53,6 +53,7 @@ - (void)setUp { - (void)tearDown { [_userDefaults removeAllObjects]; + [[NSUserDefaults standardUserDefaults] removeObjectForKey:_testKey1]; [super tearDown]; } From 7a6d04fe47617c75da2b56e01f2014e248564ad7 Mon Sep 17 00:00:00 2001 From: Paul Beusterien Date: Thu, 27 Nov 2025 16:23:13 -0800 Subject: [PATCH 07/18] locks --- .../UnitTests/Mocks/FIRCLSMockFileManager.m | 73 +++++++++++-------- 1 file changed, 42 insertions(+), 31 deletions(-) diff --git a/Crashlytics/UnitTests/Mocks/FIRCLSMockFileManager.m b/Crashlytics/UnitTests/Mocks/FIRCLSMockFileManager.m index 23ef8dba744..e2f7e63fe60 100644 --- a/Crashlytics/UnitTests/Mocks/FIRCLSMockFileManager.m +++ b/Crashlytics/UnitTests/Mocks/FIRCLSMockFileManager.m @@ -37,7 +37,9 @@ - (instancetype)init { } - (BOOL)removeItemAtPath:(NSString *)path { - [self.fileSystemDict removeObjectForKey:path]; + @synchronized(self) { + [self.fileSystemDict removeObjectForKey:path]; + } self.removeCount += 1; @@ -49,54 +51,63 @@ - (BOOL)removeItemAtPath:(NSString *)path { } - (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 *)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 *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 *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); + } } } } From 240a56147c0f1bdfad62e346f16f746dc3b3a4f9 Mon Sep 17 00:00:00 2001 From: Paul Beusterien Date: Fri, 28 Nov 2025 07:44:13 -0800 Subject: [PATCH 08/18] increase timeouts --- Crashlytics/UnitTests/FIRCLSSettingsTests.m | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Crashlytics/UnitTests/FIRCLSSettingsTests.m b/Crashlytics/UnitTests/FIRCLSSettingsTests.m index 61e7ab1f984..c833a292ea4 100644 --- a/Crashlytics/UnitTests/FIRCLSSettingsTests.m +++ b/Crashlytics/UnitTests/FIRCLSSettingsTests.m @@ -137,7 +137,7 @@ - (void)cacheSettingsWithGoogleAppID:(NSString *)googleAppID [self.settings cacheSettingsWithGoogleAppID:googleAppID currentTimestamp:currentTimestamp]; - [self waitForExpectations:@[ expectation ] timeout:5.0]; + [self waitForExpectations:@[ expectation ] timeout:10.0]; } - (void)reloadFromCacheWithGoogleAppID:(NSString *)googleAppID @@ -153,7 +153,7 @@ - (void)reloadFromCacheWithGoogleAppID:(NSString *)googleAppID [self.settings reloadFromCacheWithGoogleAppID:googleAppID currentTimestamp:currentTimestamp]; - [self waitForExpectations:@[ expectation ] timeout:10.0]; + [self waitForExpectations:@[ expectation ] timeout:20.0]; } - (void)testActivatedSettingsCached { From dbba8ce14e6abc0a99c9108e950b29970be577fd Mon Sep 17 00:00:00 2001 From: Paul Beusterien Date: Fri, 28 Nov 2025 07:48:37 -0800 Subject: [PATCH 09/18] review --- Crashlytics/UnitTests/FIRCLSUserDefaultsTests.m | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Crashlytics/UnitTests/FIRCLSUserDefaultsTests.m b/Crashlytics/UnitTests/FIRCLSUserDefaultsTests.m index 3f9c11eeb43..2fcf0b6aed9 100644 --- a/Crashlytics/UnitTests/FIRCLSUserDefaultsTests.m +++ b/Crashlytics/UnitTests/FIRCLSUserDefaultsTests.m @@ -54,6 +54,8 @@ - (void)setUp { - (void)tearDown { [_userDefaults removeAllObjects]; [[NSUserDefaults standardUserDefaults] removeObjectForKey:_testKey1]; + [[NSUserDefaults standardUserDefaults] removeObjectForKey:_testKey2]; + [[NSUserDefaults standardUserDefaults] removeObjectForKey:_testKey3]; [super tearDown]; } From b3be77b4afdbfa73880be745b067051f24fbd9dc Mon Sep 17 00:00:00 2001 From: Paul Beusterien Date: Fri, 28 Nov 2025 08:23:55 -0800 Subject: [PATCH 10/18] review --- Crashlytics/UnitTests/FIRCLSSettingsTests.m | 4 ++-- Crashlytics/UnitTests/Mocks/FIRCLSMockFileManager.m | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/Crashlytics/UnitTests/FIRCLSSettingsTests.m b/Crashlytics/UnitTests/FIRCLSSettingsTests.m index c833a292ea4..0359b76295a 100644 --- a/Crashlytics/UnitTests/FIRCLSSettingsTests.m +++ b/Crashlytics/UnitTests/FIRCLSSettingsTests.m @@ -137,7 +137,7 @@ - (void)cacheSettingsWithGoogleAppID:(NSString *)googleAppID [self.settings cacheSettingsWithGoogleAppID:googleAppID currentTimestamp:currentTimestamp]; - [self waitForExpectations:@[ expectation ] timeout:10.0]; + [self waitForExpectations:@[ expectation ] timeout:4.0]; } - (void)reloadFromCacheWithGoogleAppID:(NSString *)googleAppID @@ -153,7 +153,7 @@ - (void)reloadFromCacheWithGoogleAppID:(NSString *)googleAppID [self.settings reloadFromCacheWithGoogleAppID:googleAppID currentTimestamp:currentTimestamp]; - [self waitForExpectations:@[ expectation ] timeout:20.0]; + [self waitForExpectations:@[ expectation ] timeout:6.0]; } - (void)testActivatedSettingsCached { diff --git a/Crashlytics/UnitTests/Mocks/FIRCLSMockFileManager.m b/Crashlytics/UnitTests/Mocks/FIRCLSMockFileManager.m index e2f7e63fe60..82af331f88f 100644 --- a/Crashlytics/UnitTests/Mocks/FIRCLSMockFileManager.m +++ b/Crashlytics/UnitTests/Mocks/FIRCLSMockFileManager.m @@ -39,10 +39,9 @@ - (instancetype)init { - (BOOL)removeItemAtPath:(NSString *)path { @synchronized(self) { [self.fileSystemDict removeObjectForKey:path]; + self.removeCount += 1; } - self.removeCount += 1; - [[NSNotificationCenter defaultCenter] postNotificationName:FIRCLSMockFileManagerDidRemoveItemNotification object:self]; From d86bcf95adc2d67a09a4485c4a13ae97d16b7b51 Mon Sep 17 00:00:00 2001 From: Paul Beusterien Date: Fri, 28 Nov 2025 09:04:01 -0800 Subject: [PATCH 11/18] fixes --- Crashlytics/UnitTests/FIRCLSSettingsTests.m | 4 ++-- Crashlytics/UnitTests/Mocks/FIRCLSMockFileManager.m | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Crashlytics/UnitTests/FIRCLSSettingsTests.m b/Crashlytics/UnitTests/FIRCLSSettingsTests.m index 0359b76295a..0dc59753f66 100644 --- a/Crashlytics/UnitTests/FIRCLSSettingsTests.m +++ b/Crashlytics/UnitTests/FIRCLSSettingsTests.m @@ -137,7 +137,7 @@ - (void)cacheSettingsWithGoogleAppID:(NSString *)googleAppID [self.settings cacheSettingsWithGoogleAppID:googleAppID currentTimestamp:currentTimestamp]; - [self waitForExpectations:@[ expectation ] timeout:4.0]; + [self waitForExpectations:@[ expectation ] timeout:16.0]; } - (void)reloadFromCacheWithGoogleAppID:(NSString *)googleAppID @@ -153,7 +153,7 @@ - (void)reloadFromCacheWithGoogleAppID:(NSString *)googleAppID [self.settings reloadFromCacheWithGoogleAppID:googleAppID currentTimestamp:currentTimestamp]; - [self waitForExpectations:@[ expectation ] timeout:6.0]; + [self waitForExpectations:@[ expectation ] timeout:14.0]; } - (void)testActivatedSettingsCached { diff --git a/Crashlytics/UnitTests/Mocks/FIRCLSMockFileManager.m b/Crashlytics/UnitTests/Mocks/FIRCLSMockFileManager.m index 82af331f88f..6b968f713d0 100644 --- a/Crashlytics/UnitTests/Mocks/FIRCLSMockFileManager.m +++ b/Crashlytics/UnitTests/Mocks/FIRCLSMockFileManager.m @@ -97,7 +97,7 @@ - (void)enumerateFilesInDirectory:(NSString *)directory // 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. + // it. Plus, it's wasteful, because we don't care about dot files. if ([path hasPrefix:@"."]) { continue; } From db323110b357811b304cfb7d7e2e602ffccc4fb1 Mon Sep 17 00:00:00 2001 From: Paul Beusterien Date: Fri, 28 Nov 2025 09:28:31 -0800 Subject: [PATCH 12/18] wrap unresolved flake in ifdef --- Crashlytics/UnitTests/FIRCLSSettingsTests.m | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Crashlytics/UnitTests/FIRCLSSettingsTests.m b/Crashlytics/UnitTests/FIRCLSSettingsTests.m index 0dc59753f66..7ecf992bc1e 100644 --- a/Crashlytics/UnitTests/FIRCLSSettingsTests.m +++ b/Crashlytics/UnitTests/FIRCLSSettingsTests.m @@ -326,6 +326,7 @@ - (void)testGoogleAppIDChanged { XCTAssertEqual(self.settings.errorLogBufferSize, 64 * 1000); } +#ifdef FLAKY_TEST // This is a weird case where we got settings, but never created a cache key for it. We are // treating this as if the cache was invalid and re-fetching in this case. - (void)testActivatedSettingsMissingCacheKey { @@ -357,6 +358,7 @@ - (void)testActivatedSettingsMissingCacheKey { XCTAssertEqual(self.settings.onDemandBackoffBase, 1.5); XCTAssertEqual(self.settings.onDemandBackoffStepDuration, 6); } +#endif // FLAKY_TEST // These tests are partially to make sure the SDK doesn't crash when it // has corrupted settings. From 7783cf6cac79a0709ceed8fb7de12e44acdf1b8a Mon Sep 17 00:00:00 2001 From: Paul Beusterien Date: Fri, 28 Nov 2025 09:48:05 -0800 Subject: [PATCH 13/18] Disable two more flakes --- Crashlytics/UnitTests/FIRCLSSettingsTests.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Crashlytics/UnitTests/FIRCLSSettingsTests.m b/Crashlytics/UnitTests/FIRCLSSettingsTests.m index 7ecf992bc1e..a964671a28b 100644 --- a/Crashlytics/UnitTests/FIRCLSSettingsTests.m +++ b/Crashlytics/UnitTests/FIRCLSSettingsTests.m @@ -358,7 +358,6 @@ - (void)testActivatedSettingsMissingCacheKey { XCTAssertEqual(self.settings.onDemandBackoffBase, 1.5); XCTAssertEqual(self.settings.onDemandBackoffStepDuration, 6); } -#endif // FLAKY_TEST // These tests are partially to make sure the SDK doesn't crash when it // has corrupted settings. @@ -425,6 +424,7 @@ - (void)testCorruptCacheKey { XCTAssertEqual(self.settings.onDemandBackoffBase, 1.5); XCTAssertEqual(self.settings.onDemandBackoffStepDuration, 6); } +#endif // FLAKY_TEST - (void)testNewReportEndpointSettings { NSString *settingsJSON = From 9f5ffeb2d6fae3c539102e80c9c0d45985303edb Mon Sep 17 00:00:00 2001 From: Paul Beusterien Date: Fri, 28 Nov 2025 11:17:57 -0800 Subject: [PATCH 14/18] More flake fixes --- Crashlytics/UnitTests/FIRCLSContextManagerTests.m | 4 ++++ Crashlytics/UnitTests/FIRCLSExistingReportManagerTests.m | 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/Crashlytics/UnitTests/FIRCLSContextManagerTests.m b/Crashlytics/UnitTests/FIRCLSContextManagerTests.m index f4c863b05a5..944d04c7da6 100644 --- a/Crashlytics/UnitTests/FIRCLSContextManagerTests.m +++ b/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]; diff --git a/Crashlytics/UnitTests/FIRCLSExistingReportManagerTests.m b/Crashlytics/UnitTests/FIRCLSExistingReportManagerTests.m index ce073a8914b..265290f3307 100644 --- a/Crashlytics/UnitTests/FIRCLSExistingReportManagerTests.m +++ b/Crashlytics/UnitTests/FIRCLSExistingReportManagerTests.m @@ -54,10 +54,11 @@ - (void)setUp { 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 From cae9e4373682b7fd38cb7ba435308a7226261edc Mon Sep 17 00:00:00 2001 From: Paul Beusterien Date: Fri, 28 Nov 2025 18:31:04 -0800 Subject: [PATCH 15/18] better --- .../Components/FIRCLSBinaryImage.m | 22 ++++++++----------- .../UnitTests/FIRCLSContextManagerTests.m | 6 ++--- 2 files changed, 12 insertions(+), 16 deletions(-) diff --git a/Crashlytics/Crashlytics/Components/FIRCLSBinaryImage.m b/Crashlytics/Crashlytics/Components/FIRCLSBinaryImage.m index c7e1a602e83..e6713063924 100644 --- a/Crashlytics/Crashlytics/Components/FIRCLSBinaryImage.m +++ b/Crashlytics/Crashlytics/Components/FIRCLSBinaryImage.m @@ -57,23 +57,19 @@ void FIRCLSBinaryImageInit(void) { memset(&_firclsContext.writable->binaryImage, 0, sizeof(_firclsContext.writable->binaryImage)); _firclsContext.writable->binaryImage.file.fd = -1; - dispatch_async(FIRCLSGetBinaryImageQueue(), ^{ - if (!FIRCLSUnlinkIfExists(_firclsContext.readonly->binaryimage.path)) { - FIRCLSSDKLog("Unable to reset the binary image log file %s\n", strerror(errno)); - } + if (!FIRCLSUnlinkIfExists(_firclsContext.readonly->binaryimage.path)) { + FIRCLSSDKLog("Unable to reset the binary image log file %s\n", strerror(errno)); + } - bool needsClosing; // unneeded - if (!FIRCLSBinaryImageOpenIfNeeded(&needsClosing)) { - FIRCLSSDKLog("Error: Unable to open the binary image log file during init\n"); - } - }); + bool needsClosing; // unneeded + if (!FIRCLSBinaryImageOpenIfNeeded(&needsClosing)) { + FIRCLSSDKLog("Error: Unable to open the binary image log file during init\n"); + } + + FIRCLSFileClose(&_firclsContext.writable->binaryImage.file); _dyld_register_func_for_add_image(FIRCLSBinaryImageAddedCallback); _dyld_register_func_for_remove_image(FIRCLSBinaryImageRemovedCallback); - - dispatch_async(FIRCLSGetBinaryImageQueue(), ^{ - FIRCLSFileClose(&_firclsContext.writable->binaryImage.file); - }); } static bool FIRCLSBinaryImageOpenIfNeeded(bool* needsClosing) { diff --git a/Crashlytics/UnitTests/FIRCLSContextManagerTests.m b/Crashlytics/UnitTests/FIRCLSContextManagerTests.m index 944d04c7da6..a0cfc1af74d 100644 --- a/Crashlytics/UnitTests/FIRCLSContextManagerTests.m +++ b/Crashlytics/UnitTests/FIRCLSContextManagerTests.m @@ -43,9 +43,9 @@ @implementation FIRCLSContextManagerTests - (void)setUp { self.fileManager = [[FIRCLSMockFileManager alloc] init]; [[NSFileManager defaultManager] createDirectoryAtPath:self.fileManager.rootPath - withIntermediateDirectories:YES - attributes:nil - error:nil]; + withIntermediateDirectories:YES + attributes:nil + error:nil]; [self.fileManager createReportDirectories]; [self.fileManager setupNewPathForExecutionIdentifier:TestContextReportID]; From 17fdac8b5394b11b427fe08e028a5f6d9d13cc1a Mon Sep 17 00:00:00 2001 From: Paul Beusterien Date: Sat, 29 Nov 2025 07:20:45 -0800 Subject: [PATCH 16/18] Mark another flaky test --- Crashlytics/UnitTests/FIRCLSSettingsTests.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Crashlytics/UnitTests/FIRCLSSettingsTests.m b/Crashlytics/UnitTests/FIRCLSSettingsTests.m index a964671a28b..22c2a1861e6 100644 --- a/Crashlytics/UnitTests/FIRCLSSettingsTests.m +++ b/Crashlytics/UnitTests/FIRCLSSettingsTests.m @@ -296,6 +296,7 @@ - (void)testCacheExpiredFromAppVersion { XCTAssertEqual(self.settings.errorLogBufferSize, 128000); } +#ifdef FLAKY_TEST - (void)testGoogleAppIDChanged { NSError *error = nil; [self writeSettings:FIRCLSTestSettingsInverse error:&error]; @@ -326,7 +327,6 @@ - (void)testGoogleAppIDChanged { XCTAssertEqual(self.settings.errorLogBufferSize, 64 * 1000); } -#ifdef FLAKY_TEST // This is a weird case where we got settings, but never created a cache key for it. We are // treating this as if the cache was invalid and re-fetching in this case. - (void)testActivatedSettingsMissingCacheKey { From 6f1683523764a345023dac7c80ec4c0b9190439a Mon Sep 17 00:00:00 2001 From: Paul Beusterien Date: Sat, 29 Nov 2025 08:55:13 -0800 Subject: [PATCH 17/18] Fix flakes in FIRCrashlyticsReportTests.m --- .../Crashlytics/Models/FIRCLSInternalReport.m | 3 +++ .../Private/FIRCLSInternalReport_Private.h | 25 +++++++++++++++++++ .../UnitTests/FIRCrashlyticsReportTests.m | 5 ++++ 3 files changed, 33 insertions(+) create mode 100644 Crashlytics/Crashlytics/Private/FIRCLSInternalReport_Private.h diff --git a/Crashlytics/Crashlytics/Models/FIRCLSInternalReport.m b/Crashlytics/Crashlytics/Models/FIRCLSInternalReport.m index 35160d1cbc1..82de3e1df27 100644 --- a/Crashlytics/Crashlytics/Models/FIRCLSInternalReport.m +++ b/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]; } diff --git a/Crashlytics/Crashlytics/Private/FIRCLSInternalReport_Private.h b/Crashlytics/Crashlytics/Private/FIRCLSInternalReport_Private.h new file mode 100644 index 00000000000..4c75c911439 --- /dev/null +++ b/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 diff --git a/Crashlytics/UnitTests/FIRCrashlyticsReportTests.m b/Crashlytics/UnitTests/FIRCrashlyticsReportTests.m index f8fd98ff47f..7c58670a589 100644 --- a/Crashlytics/UnitTests/FIRCrashlyticsReportTests.m +++ b/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] From 33a701dfdf41dddba230adc48869d64724ea7650 Mon Sep 17 00:00:00 2001 From: Paul Beusterien Date: Sat, 29 Nov 2025 09:40:02 -0800 Subject: [PATCH 18/18] address another flake --- Crashlytics/UnitTests/FIRCLSReportUploaderTests.m | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Crashlytics/UnitTests/FIRCLSReportUploaderTests.m b/Crashlytics/UnitTests/FIRCLSReportUploaderTests.m index 4dbca18e44c..7e9862d2832 100644 --- a/Crashlytics/UnitTests/FIRCLSReportUploaderTests.m +++ b/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]); }