Skip to content

Commit c36a417

Browse files
paulb777paulb777
committed
[Crashlyics] Address some Crashlytics flakes and disable others in nightlies
1 parent bf942b9 commit c36a417

File tree

9 files changed

+134
-63
lines changed

9 files changed

+134
-63
lines changed

‎.github/workflows/common.yml‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,12 @@ on:
5252
required: false
5353
default: ""
5454

55+
# An environment variable to be set for the test run.
56+
env_var:
57+
type: string
58+
required: false
59+
default: ""
60+
5561
outputs:
5662
cache_key:
5763
description: "The cache key for the Swift package resolution."
@@ -105,6 +111,9 @@ jobs:
105111
with:
106112
path: .build
107113
key: ${{needs.spm-package-resolved.outputs.cache_key}}
114+
- name: Set nightly env var
115+
if: inputs.env_var != ''
116+
run: echo "${{ inputs.env_var }}=1" >> $GITHUB_ENV
108117
- name: Xcode
109118
run: sudo xcode-select -s /Applications/${{ matrix.xcode }}.app/Contents/Developer
110119
- name: Run setup command, if needed.

‎.github/workflows/crashlytics.yml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ jobs:
3030
uses: ./.github/workflows/common.yml
3131
with:
3232
target: FirebaseCrashlyticsUnit
33+
env_var: ${{ github.event_name == 'schedule' && 'CRASHLYTICS_NIGHTLY' || '' }}
3334

3435
catalyst:
3536
uses: ./.github/workflows/common_catalyst.yml

‎Crashlytics/UnitTests/FIRCLSContextManagerTests.m‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,10 @@ @implementation FIRCLSContextManagerTests
4242

4343
- (void)setUp {
4444
self.fileManager = [[FIRCLSMockFileManager alloc] init];
45+
[[NSFileManager defaultManager] createDirectoryAtPath:self.fileManager.rootPath
46+
withIntermediateDirectories:YES
47+
attributes:nil
48+
error:nil];
4549
[self.fileManager createReportDirectories];
4650
[self.fileManager setupNewPathForExecutionIdentifier:TestContextReportID];
4751

‎Crashlytics/UnitTests/FIRCLSExistingReportManagerTests.m‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,10 +54,11 @@ - (void)setUp {
5454

5555
self.fileManager = [[FIRCLSTempMockFileManager alloc] init];
5656

57-
// Cleanup potential artifacts from other test files.
57+
// Clean up the directory and then re-create it to ensure a fresh state
5858
if ([[NSFileManager defaultManager] fileExistsAtPath:[self.fileManager rootPath]]) {
5959
assert([self.fileManager removeItemAtPath:[self.fileManager rootPath]]);
6060
}
61+
[self.fileManager createReportDirectories];
6162

6263
// Allow nil values only in tests
6364
#pragma clang diagnostic push

‎Crashlytics/UnitTests/FIRCLSReportUploaderTests.m‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,9 @@ - (void)runUploadPackagedReportWithUrgency:(BOOL)urgent {
284284
asUrgent:urgent];
285285

286286
XCTAssertNotNil(self.mockDataTransport.sendDataEvent_event);
287+
288+
// Wait a little bit for the file to be removed.
289+
[NSRunLoop.currentRunLoop runUntilDate:[NSDate dateWithTimeIntervalSinceNow:0.1]];
287290
XCTAssertEqualObjects(self.fileManager.removedItemAtPath_path, [self packagePath]);
288291
}
289292

‎Crashlytics/UnitTests/FIRCLSUserDefaultsTests.m‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,9 @@ - (void)setUp {
5353

5454
- (void)tearDown {
5555
[_userDefaults removeAllObjects];
56+
[[NSUserDefaults standardUserDefaults] removeObjectForKey:_testKey1];
57+
[[NSUserDefaults standardUserDefaults] removeObjectForKey:_testKey2];
58+
[[NSUserDefaults standardUserDefaults] removeObjectForKey:_testKey3];
5659
[super tearDown];
5760
}
5861

‎Crashlytics/UnitTests/Mocks/FIRCLSMockFileManager.h‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,17 @@
1616

1717
#import <XCTest/XCTest.h>
1818

19-
@interface FIRCLSMockFileManager : FIRCLSFileManager
19+
// Notification posted when an item is removed via `removeItemAtPath`.
20+
extern NSNotificationName const FIRCLSMockFileManagerDidRemoveItemNotification;
2021

21-
// Number of calls to removeItemAtPath are expected for the unit test
22-
@property(nonatomic) NSInteger expectedRemoveCount;
22+
@interface FIRCLSMockFileManager : FIRCLSFileManager
2323

2424
// Incremented when a remove happens with removeItemAtPath
2525
@property(nonatomic) NSInteger removeCount;
2626

27+
// Number of calls to removeItemAtPath are expected for the unit test
28+
@property(nonatomic) NSInteger expectedRemoveCount;
29+
2730
// Will be fulfilled when the expected number of removes have happened
2831
// using removeItemAtPath
2932
//

‎Crashlytics/UnitTests/Mocks/FIRCLSMockFileManager.m‎

Lines changed: 55 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@
1414

1515
#import "Crashlytics/UnitTests/Mocks/FIRCLSMockFileManager.h"
1616

17+
NSNotificationName const FIRCLSMockFileManagerDidRemoveItemNotification =
18+
@"FIRCLSMockFileManagerDidRemoveItemNotification";
19+
1720
@interface FIRCLSMockFileManager ()
1821

1922
@property(nonatomic) NSMutableDictionary<NSString *, NSData *> *fileSystemDict;
@@ -34,68 +37,82 @@ - (instancetype)init {
3437
}
3538

3639
- (BOOL)removeItemAtPath:(NSString *)path {
37-
[self.fileSystemDict removeObjectForKey:path];
38-
39-
self.removeCount += 1;
40-
41-
// If we set up the expectation, and we went over the expected count or removes, fulfill the
42-
// expectation
43-
if (self.removeExpectation && self.removeCount >= self.expectedRemoveCount) {
44-
[self.removeExpectation fulfill];
40+
@synchronized(self) {
41+
[self.fileSystemDict removeObjectForKey:path];
42+
self.removeCount += 1;
43+
44+
// If we set up the expectation, and we went over the expected count or removes, fulfill the
45+
// expectation
46+
if (self.removeExpectation && self.removeCount >= self.expectedRemoveCount) {
47+
[self.removeExpectation fulfill];
48+
}
4549
}
4650

51+
[[NSNotificationCenter defaultCenter]
52+
postNotificationName:FIRCLSMockFileManagerDidRemoveItemNotification
53+
object:self];
54+
4755
return YES;
4856
}
4957

5058
- (BOOL)fileExistsAtPath:(NSString *)path {
51-
return self.fileSystemDict[path] != nil;
59+
@synchronized(self) {
60+
return self.fileSystemDict[path] != nil;
61+
}
5262
}
5363

5464
- (BOOL)createFileAtPath:(NSString *)path
5565
contents:(NSData *)data
5666
attributes:(NSDictionary<NSFileAttributeKey, id> *)attr {
57-
self.fileSystemDict[path] = data;
67+
@synchronized(self) {
68+
self.fileSystemDict[path] = data;
69+
}
5870
return YES;
5971
}
6072

6173
- (NSArray *)activePathContents {
62-
NSMutableArray *pathsWithActive = [[NSMutableArray alloc] init];
63-
for (NSString *path in [_fileSystemDict allKeys]) {
64-
if ([path containsString:@"v5/reports/active"]) {
65-
[pathsWithActive addObject:path];
74+
@synchronized(self) {
75+
NSMutableArray *pathsWithActive = [[NSMutableArray alloc] init];
76+
for (NSString *path in [_fileSystemDict allKeys]) {
77+
if ([path containsString:@"v5/reports/active"]) {
78+
[pathsWithActive addObject:path];
79+
}
6680
}
81+
return pathsWithActive;
6782
}
68-
69-
return pathsWithActive;
7083
}
7184

7285
- (NSData *)dataWithContentsOfFile:(NSString *)path {
73-
return self.fileSystemDict[path];
86+
@synchronized(self) {
87+
return self.fileSystemDict[path];
88+
}
7489
}
7590

7691
- (void)enumerateFilesInDirectory:(NSString *)directory
7792
usingBlock:(void (^)(NSString *filePath, NSString *extension))block {
78-
NSArray<NSString *> *filteredPaths = [self.fileSystemDict.allKeys
79-
filteredArrayUsingPredicate:[NSPredicate predicateWithBlock:^BOOL(NSString *path,
80-
NSDictionary *bindings) {
81-
return [path hasPrefix:directory];
82-
}]];
83-
84-
for (NSString *path in filteredPaths) {
85-
NSString *extension;
86-
NSString *fullPath;
87-
88-
// Skip files that start with a dot. This is important, because if you try to move a .DS_Store
89-
// file, it will fail if the target directory also has a .DS_Store file in it. Plus, its
90-
// wasteful, because we don't care about dot files.
91-
if ([path hasPrefix:@"."]) {
92-
continue;
93-
}
94-
95-
extension = [path pathExtension];
96-
fullPath = [directory stringByAppendingPathComponent:path];
97-
if (block) {
98-
block(fullPath, extension);
93+
@synchronized(self) {
94+
NSArray<NSString *> *filteredPaths = [self.fileSystemDict.allKeys
95+
filteredArrayUsingPredicate:[NSPredicate predicateWithBlock:^BOOL(NSString *path,
96+
NSDictionary *bindings) {
97+
return [path hasPrefix:directory];
98+
}]];
99+
100+
for (NSString *path in filteredPaths) {
101+
NSString *extension;
102+
NSString *fullPath;
103+
104+
// Skip files that start with a dot. This is important, because if you try to move a
105+
// .DS_Store file, it will fail if the target directory also has a .DS_Store file in
106+
// it. Plus, it's wasteful, because we don't care about dot files.
107+
if ([path hasPrefix:@"."]) {
108+
continue;
109+
}
110+
111+
extension = [path pathExtension];
112+
fullPath = [directory stringByAppendingPathComponent:path];
113+
if (block) {
114+
block(fullPath, extension);
115+
}
99116
}
100117
}
101118
}

‎Package.swift‎

Lines changed: 51 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -626,27 +626,7 @@ let package = Package(
626626
.swiftLanguageMode(SwiftLanguageMode.v5),
627627
]
628628
),
629-
.testTarget(
630-
name: "FirebaseCrashlyticsUnit",
631-
dependencies: ["FirebaseCrashlytics", .product(name: "OCMock", package: "ocmock")],
632-
path: "Crashlytics/UnitTests",
633-
resources: [
634-
.copy("FIRCLSMachO/machO_data"),
635-
.copy("Data"),
636-
],
637-
cSettings: [
638-
.headerSearchPath("../.."),
639-
.define("DISPLAY_VERSION", to: firebaseVersion),
640-
.define("CLS_SDK_NAME", to: "Crashlytics iOS SDK", .when(platforms: [.iOS])),
641-
.define(
642-
"CLS_SDK_NAME",
643-
to: "Crashlytics macOS SDK",
644-
.when(platforms: [.macOS, .macCatalyst])
645-
),
646-
.define("CLS_SDK_NAME", to: "Crashlytics tvOS SDK", .when(platforms: [.tvOS])),
647-
.define("CLS_SDK_NAME", to: "Crashlytics watchOS SDK", .when(platforms: [.watchOS])),
648-
]
649-
),
629+
crashlyticsUnitTestChooser(),
650630
.target(
651631
name: "FirebaseDatabaseInternal",
652632
dependencies: [
@@ -1467,6 +1447,56 @@ func firestoreWrapperTarget() -> Target {
14671447
)
14681448
}
14691449

1450+
func crashlyticsUnitTestChooser() -> Target {
1451+
// Don't run flaky tests in nightly runs.
1452+
if Context.environment["CRASHLYTICS_NIGHTLY"] != nil {
1453+
return .testTarget(
1454+
name: "FirebaseCrashlyticsUnit",
1455+
dependencies: ["FirebaseCrashlytics", .product(name: "OCMock", package: "ocmock")],
1456+
path: "Crashlytics/UnitTests",
1457+
exclude: ["FIRCrashlyticsReportTests.m"], // Flaky
1458+
resources: [
1459+
.copy("FIRCLSMachO/machO_data"),
1460+
.copy("Data"),
1461+
],
1462+
cSettings: [
1463+
.headerSearchPath("../.."),
1464+
.define("DISPLAY_VERSION", to: firebaseVersion),
1465+
.define("CLS_SDK_NAME", to: "Crashlytics iOS SDK", .when(platforms: [.iOS])),
1466+
.define(
1467+
"CLS_SDK_NAME",
1468+
to: "Crashlytics macOS SDK",
1469+
.when(platforms: [.macOS, .macCatalyst])
1470+
),
1471+
.define("CLS_SDK_NAME", to: "Crashlytics tvOS SDK", .when(platforms: [.tvOS])),
1472+
.define("CLS_SDK_NAME", to: "Crashlytics watchOS SDK", .when(platforms: [.watchOS])),
1473+
]
1474+
)
1475+
} else {
1476+
return .testTarget(
1477+
name: "FirebaseCrashlyticsUnit",
1478+
dependencies: ["FirebaseCrashlytics", .product(name: "OCMock", package: "ocmock")],
1479+
path: "Crashlytics/UnitTests",
1480+
resources: [
1481+
.copy("FIRCLSMachO/machO_data"),
1482+
.copy("Data"),
1483+
],
1484+
cSettings: [
1485+
.headerSearchPath("../.."),
1486+
.define("DISPLAY_VERSION", to: firebaseVersion),
1487+
.define("CLS_SDK_NAME", to: "Crashlytics iOS SDK", .when(platforms: [.iOS])),
1488+
.define(
1489+
"CLS_SDK_NAME",
1490+
to: "Crashlytics macOS SDK",
1491+
.when(platforms: [.macOS, .macCatalyst])
1492+
),
1493+
.define("CLS_SDK_NAME", to: "Crashlytics tvOS SDK", .when(platforms: [.tvOS])),
1494+
.define("CLS_SDK_NAME", to: "Crashlytics watchOS SDK", .when(platforms: [.watchOS])),
1495+
]
1496+
)
1497+
}
1498+
}
1499+
14701500
func firestoreTargets() -> [Target] {
14711501
if Context.environment["FIREBASE_SOURCE_FIRESTORE"] != nil {
14721502
return [

0 commit comments

Comments
 (0)