[Crashlyics] Address some Crashlytics flakes and disable others in nightly tests

.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.

.github/workflows/crashlytics.yml

@@ -30,18 +30,21 @@ jobs:
     uses: ./.github/workflows/common.yml
     with:
       target: FirebaseCrashlyticsUnit
+      env_var: ${{ github.event_name == 'schedule' && 'CRASHLYTICS_NIGHTLY' || '' }}
 
   catalyst:
     uses: ./.github/workflows/common_catalyst.yml
     with:
       product: FirebaseCrashlytics
       target: FirebaseCrashlytics-Unit-unit
+      buildonly: true
 
   pod_lib_lint:
     uses: ./.github/workflows/common_cocoapods.yml
     with:
       product: FirebaseCrashlytics
-      buildonly_platforms: tvOS, macOS, watchOS
+      # Run CI tests only on SPM.
+      buildonly_platforms: iOS, tvOS, macOS, watchOS
 
   quickstart:
     uses: ./.github/workflows/common_quickstart.yml

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

@@ -282,8 +282,13 @@ - (void)runUploadPackagedReportWithUrgency:(BOOL)urgent {
   [self.uploader uploadPackagedReportAtPath:[self packagePath]
                         dataCollectionToken:FIRCLSDataCollectionToken.validToken
                                    asUrgent:urgent];
-
   XCTAssertNotNil(self.mockDataTransport.sendDataEvent_event);
+
+  // Poll until the file is removed, with a timeout to prevent an infinite loop.
+  NSDate *timeoutDate = [NSDate dateWithTimeIntervalSinceNow:1.0];
+  while (self.fileManager.removedItemAtPath_path == nil && [timeoutDate timeIntervalSinceNow] > 0) {
+    [NSRunLoop.currentRunLoop runUntilDate:[NSDate dateWithTimeIntervalSinceNow:0.01]];
+  }
   XCTAssertEqualObjects(self.fileManager.removedItemAtPath_path, [self packagePath]);
 }
 

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

@@ -34,68 +34,77 @@ - (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];
+    }
   }
-
   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);
+      }
     }
   }
 }

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,35 @@ func firestoreWrapperTarget() -> Target {
   )
 }
 
+func crashlyticsUnitTestChooser() -> Target {
+  // Don't run flaky tests in nightly runs.
+  let exclusions: [String] = Context.environment["CRASHLYTICS_NIGHTLY"] != nil ?
+    ["FIRCrashlyticsReportTests.m"] : []
+
+  return .testTarget(
+    name: "FirebaseCrashlyticsUnit",
+    dependencies: ["FirebaseCrashlytics", .product(name: "OCMock", package: "ocmock")],
+    path: "Crashlytics/UnitTests",
+    exclude: exclusions,
+    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 [