diff --git a/.github/workflows/common.yml b/.github/workflows/common.yml index 4af29b0f68b..1de6102e1b6 100644 --- a/.github/workflows/common.yml +++ b/.github/workflows/common.yml @@ -52,6 +52,12 @@ on: required: false default: "" + # An environment variable to be set for the test run. + env_var: + type: string + required: false + default: "" + outputs: cache_key: description: "The cache key for the Swift package resolution." @@ -105,6 +111,9 @@ jobs: with: path: .build key: ${{needs.spm-package-resolved.outputs.cache_key}} + - name: Set nightly env var + if: inputs.env_var != '' + run: echo "${{ inputs.env_var }}=1" >> $GITHUB_ENV - name: Xcode run: sudo xcode-select -s /Applications/${{ matrix.xcode }}.app/Contents/Developer - name: Run setup command, if needed. diff --git a/.github/workflows/crashlytics.yml b/.github/workflows/crashlytics.yml index 3969c1eb223..60599d17c3a 100644 --- a/.github/workflows/crashlytics.yml +++ b/.github/workflows/crashlytics.yml @@ -30,6 +30,7 @@ jobs: uses: ./.github/workflows/common.yml with: target: FirebaseCrashlyticsUnit + env_var: ${{ github.event_name == 'schedule' && 'CRASHLYTICS_NIGHTLY' || '' }} catalyst: uses: ./.github/workflows/common_catalyst.yml diff --git a/Crashlytics/UnitTests/FIRCLSContextManagerTests.m b/Crashlytics/UnitTests/FIRCLSContextManagerTests.m index f4c863b05a5..a0cfc1af74d 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 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]); } diff --git a/Crashlytics/UnitTests/FIRCLSUserDefaultsTests.m b/Crashlytics/UnitTests/FIRCLSUserDefaultsTests.m index e310d85121c..2fcf0b6aed9 100644 --- a/Crashlytics/UnitTests/FIRCLSUserDefaultsTests.m +++ b/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]; } diff --git a/Crashlytics/UnitTests/Mocks/FIRCLSMockFileManager.h b/Crashlytics/UnitTests/Mocks/FIRCLSMockFileManager.h index 3bff289bf03..86382211da1 100644 --- a/Crashlytics/UnitTests/Mocks/FIRCLSMockFileManager.h +++ b/Crashlytics/UnitTests/Mocks/FIRCLSMockFileManager.h @@ -16,14 +16,17 @@ #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; +// Number of calls to removeItemAtPath are expected for the unit test +@property(nonatomic) NSInteger expectedRemoveCount; + // Will be fulfilled when the expected number of removes have happened // using removeItemAtPath // diff --git a/Crashlytics/UnitTests/Mocks/FIRCLSMockFileManager.m b/Crashlytics/UnitTests/Mocks/FIRCLSMockFileManager.m index b89d05bbff8..2c37549503f 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; @@ -34,68 +37,82 @@ - (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; + + // 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]; + } } + [[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 *)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, 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); + } } } } diff --git a/Package.swift b/Package.swift index 41afc02d1b4..79f6beb2627 100644 --- a/Package.swift +++ b/Package.swift @@ -626,27 +626,7 @@ let package = Package( .swiftLanguageMode(SwiftLanguageMode.v5), ] ), - .testTarget( - name: "FirebaseCrashlyticsUnit", - dependencies: ["FirebaseCrashlytics", .product(name: "OCMock", package: "ocmock")], - path: "Crashlytics/UnitTests", - resources: [ - .copy("FIRCLSMachO/machO_data"), - .copy("Data"), - ], - cSettings: [ - .headerSearchPath("../.."), - .define("DISPLAY_VERSION", to: firebaseVersion), - .define("CLS_SDK_NAME", to: "Crashlytics iOS SDK", .when(platforms: [.iOS])), - .define( - "CLS_SDK_NAME", - to: "Crashlytics macOS SDK", - .when(platforms: [.macOS, .macCatalyst]) - ), - .define("CLS_SDK_NAME", to: "Crashlytics tvOS SDK", .when(platforms: [.tvOS])), - .define("CLS_SDK_NAME", to: "Crashlytics watchOS SDK", .when(platforms: [.watchOS])), - ] - ), + crashlyticsUnitTestChooser(), .target( name: "FirebaseDatabaseInternal", dependencies: [ @@ -1467,6 +1447,56 @@ func firestoreWrapperTarget() -> Target { ) } +func crashlyticsUnitTestChooser() -> Target { + // Don't run flaky tests in nightly runs. + if Context.environment["CRASHLYTICS_NIGHTLY"] != nil { + return .testTarget( + name: "FirebaseCrashlyticsUnit", + dependencies: ["FirebaseCrashlytics", .product(name: "OCMock", package: "ocmock")], + path: "Crashlytics/UnitTests", + exclude: ["FIRCrashlyticsReportTests.m"], // Flaky + resources: [ + .copy("FIRCLSMachO/machO_data"), + .copy("Data"), + ], + cSettings: [ + .headerSearchPath("../.."), + .define("DISPLAY_VERSION", to: firebaseVersion), + .define("CLS_SDK_NAME", to: "Crashlytics iOS SDK", .when(platforms: [.iOS])), + .define( + "CLS_SDK_NAME", + to: "Crashlytics macOS SDK", + .when(platforms: [.macOS, .macCatalyst]) + ), + .define("CLS_SDK_NAME", to: "Crashlytics tvOS SDK", .when(platforms: [.tvOS])), + .define("CLS_SDK_NAME", to: "Crashlytics watchOS SDK", .when(platforms: [.watchOS])), + ] + ) + } else { + return .testTarget( + name: "FirebaseCrashlyticsUnit", + dependencies: ["FirebaseCrashlytics", .product(name: "OCMock", package: "ocmock")], + path: "Crashlytics/UnitTests", + resources: [ + .copy("FIRCLSMachO/machO_data"), + .copy("Data"), + ], + cSettings: [ + .headerSearchPath("../.."), + .define("DISPLAY_VERSION", to: firebaseVersion), + .define("CLS_SDK_NAME", to: "Crashlytics iOS SDK", .when(platforms: [.iOS])), + .define( + "CLS_SDK_NAME", + to: "Crashlytics macOS SDK", + .when(platforms: [.macOS, .macCatalyst]) + ), + .define("CLS_SDK_NAME", to: "Crashlytics tvOS SDK", .when(platforms: [.tvOS])), + .define("CLS_SDK_NAME", to: "Crashlytics watchOS SDK", .when(platforms: [.watchOS])), + ] + ) + } +} + func firestoreTargets() -> [Target] { if Context.environment["FIREBASE_SOURCE_FIRESTORE"] != nil { return [