Skip to content

Commit 91ecdd6

Browse files
ravimandalaravimandala
authored
Fixing minor issues and doing some minor refactoring/clean-up (#444)
* Avoid NPE when appBundlePath is nil in BPConfiguration * Minor logging changes * Better error handling
1 parent 4e11c74 commit 91ecdd6

File tree

4 files changed

+37
-21
lines changed

4 files changed

+37
-21
lines changed

‎bluepill/src/BPApp.m

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,8 @@ + (instancetype)appWithConfig:(BPConfiguration *)config
106106
for (NSString *testName in config.tests) {
107107
BPTestPlan *testPlan = [config.tests objectForKey:testName];
108108
BPXCTestFile *xcTestFile = [BPXCTestFile BPXCTestFileFromBPTestPlan:testPlan withName:testName andError:errPtr];
109+
if (*errPtr)
110+
return nil;
109111
[loadedTests addObject:xcTestFile];
110112
}
111113

‎bp/src/BPConfiguration.m

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -710,23 +710,25 @@ - (BOOL)validateConfigWithError:(NSError *__autoreleasing *)errPtr {
710710
return NO;
711711
}
712712

713-
if (self.appBundlePath && (![[NSFileManager defaultManager] fileExistsAtPath: self.appBundlePath isDirectory:&isdir] || !isdir)) {
714-
BP_SET_ERROR(errPtr, @"%@ not found.", self.appBundlePath);
715-
return NO;
716-
}
717-
NSString *path = [self.appBundlePath stringByAppendingPathComponent:@"Info.plist"];
718-
NSDictionary *dic = [NSDictionary dictionaryWithContentsOfFile:path];
713+
if (self.appBundlePath) {
714+
if (![[NSFileManager defaultManager] fileExistsAtPath: self.appBundlePath isDirectory:&isdir] || !isdir) {
715+
BP_SET_ERROR(errPtr, @"%@ not found.", self.appBundlePath);
716+
return NO;
717+
}
718+
NSString *path = [self.appBundlePath stringByAppendingPathComponent:@"Info.plist"];
719+
NSDictionary *dic = [NSDictionary dictionaryWithContentsOfFile:path];
719720

720-
if (!dic) {
721-
BP_SET_ERROR(errPtr, @"Could not read %@, perhaps you forgot to run xcodebuild build-for-testing?", path);
722-
}
721+
if (!dic) {
722+
BP_SET_ERROR(errPtr, @"Could not read %@, perhaps you forgot to run xcodebuild build-for-testing?", path);
723+
}
723724

724-
NSString *platform = [dic objectForKey:@"DTPlatformName"];
725-
if (platform && ![platform isEqualToString:@"iphonesimulator"]) {
726-
BP_SET_ERROR(errPtr, @"Wrong platform in %@. Expected 'iphonesimulator', found '%@'", self.appBundlePath, platform);
727-
return NO;
725+
NSString *platform = [dic objectForKey:@"DTPlatformName"];
726+
if (platform && ![platform isEqualToString:@"iphonesimulator"]) {
727+
BP_SET_ERROR(errPtr, @"Wrong platform in %@. Expected 'iphonesimulator', found '%@'", self.appBundlePath, platform);
728+
return NO;
729+
}
728730
}
729-
731+
730732
if (self.outputDirectory) {
731733
if ([[NSFileManager defaultManager] fileExistsAtPath:self.outputDirectory isDirectory:&isdir]) {
732734
if (!isdir) {

‎bp/src/Bluepill.m

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@ - (void)retry {
121121
if (![self canRetryOnError] || self.failureTolerance <= 0) {
122122
// If there is no more retries, set the final exitCode to current context's exitCode
123123
self.finalExitStatus |= self.context.finalExitStatus;
124+
[BPUtils printInfo:ERROR withString:@"No retries left. Giving up."];
124125
[BPUtils printInfo:INFO withString:@"%s:%d finalExitStatus = %@", __FILE__, __LINE__, [BPExitStatusHelper stringFromExitStatus:self.finalExitStatus]];
125126
self.exitLoop = YES;
126127
return;
@@ -158,9 +159,9 @@ - (void)recover {
158159
// If error retry reach to the max, then return
159160
if (![self canRetryOnError]) {
160161
self.finalExitStatus |= self.context.finalExitStatus;
162+
[BPUtils printInfo:ERROR withString:@"No retries left. Giving up."];
161163
[BPUtils printInfo:INFO withString:@"%s:%d finalExitStatus = %@", __FILE__, __LINE__, [BPExitStatusHelper stringFromExitStatus:self.finalExitStatus]];
162164
self.exitLoop = YES;
163-
[BPUtils printInfo:ERROR withString:@"Too many retries have occurred. Giving up."];
164165
return;
165166
}
166167

@@ -186,9 +187,9 @@ - (void)recover {
186187
- (void)proceed {
187188
if (![self canRetryOnError]) {
188189
self.finalExitStatus |= self.context.finalExitStatus;
190+
[BPUtils printInfo:ERROR withString:@"No retries left. Giving up."];
189191
[BPUtils printInfo:INFO withString:@"%s:%d finalExitStatus = %@", __FILE__, __LINE__, [BPExitStatusHelper stringFromExitStatus:self.finalExitStatus]];
190192
self.exitLoop = YES;
191-
[BPUtils printInfo:ERROR withString:@"Too many retries have occurred. Giving up."];
192193
return;
193194
}
194195
self.retries += 1;
@@ -591,7 +592,9 @@ - (void)deleteSimulatorOnlyTaskWithContext:(BPExecutionContext *)context {
591592
*/
592593
- (void)finishWithContext:(BPExecutionContext *)context {
593594
context.finalExitStatus |= context.exitStatus;
594-
[BPUtils printInfo:INFO withString:@"Attempt's Exit Status: %@", [BPExitStatusHelper stringFromExitStatus:context.exitStatus]];
595+
[BPUtils printInfo:INFO withString:@"Attempt's Exit Status: %@, Bundle exit status: %@",
596+
[BPExitStatusHelper stringFromExitStatus:context.exitStatus],
597+
[BPExitStatusHelper stringFromExitStatus:context.finalExitStatus]];
595598

596599
switch (context.exitStatus) {
597600
// BP exit handler
@@ -628,19 +631,24 @@ - (void)finishWithContext:(BPExecutionContext *)context {
628631
return;
629632

630633
case BPExitStatusAppCrashed:
631-
// Remember the app crash and report whether a retry passes or not
634+
// Crashed test is considered fatal and shall not be retried
632635
self.finalExitStatus |= context.exitStatus;
633636
NEXT([self proceed]);
634637
return;
635638

636639
case BPExitStatusSimulatorDeleted:
637640
case BPExitStatusSimulatorReuseFailed:
638641
self.finalExitStatus |= context.finalExitStatus;
639-
[BPUtils printInfo:INFO withString:@"%s:%d finalExitStatus = %@", __FILE__, __LINE__, [BPExitStatusHelper stringFromExitStatus:self.finalExitStatus]];
642+
[BPUtils printInfo:INFO withString:@"%s:%d finalExitStatus = %@",
643+
__FILE__, __LINE__,
644+
[BPExitStatusHelper stringFromExitStatus:self.finalExitStatus]];
640645
self.exitLoop = YES;
641646
return;
642647
}
643-
[BPUtils printInfo:ERROR withString:@"%s:%d YOU SHOULDN'T BE HERE. exitStatus = %@, finalExitStatus = %@", __FILE__, __LINE__, [BPExitStatusHelper stringFromExitStatus:context.exitStatus], [BPExitStatusHelper stringFromExitStatus:context.finalExitStatus]];
648+
[BPUtils printInfo:ERROR withString:@"%s:%d YOU SHOULDN'T BE HERE. exitStatus = %@, finalExitStatus = %@",
649+
__FILE__, __LINE__,
650+
[BPExitStatusHelper stringFromExitStatus:context.exitStatus],
651+
[BPExitStatusHelper stringFromExitStatus:context.finalExitStatus]];
644652
}
645653

646654
// MARK: Helpers
@@ -666,7 +674,9 @@ - (BOOL)canRetryOnError {
666674
if (self.retries > maxErrorRetryCount) {
667675
// If retries strictly exceeds the max error retry, then we must have incremented it beyond the limit somehow.
668676
// It is safe to halt retries here, but log to alert unexpected behavior.
669-
[BPUtils printInfo:ERROR withString:@"Current retry count (%d) exceeded maximum retry count (%d)!", (int) self.retries, (int) maxErrorRetryCount];
677+
[BPUtils printInfo:ERROR withString:@"Current retry count (%d) exceeded maximum retry count (%d)!",
678+
(int) self.retries,
679+
(int) maxErrorRetryCount];
670680
}
671681
return false;
672682
}

‎bp/tests/Resource Files/testConfig-busted.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
"VoyagerTests"
55
],
66
"error-retries": 0,
7+
"num-sims": 2,
8+
"unsafe-skip-xcode-version-check": true,
79
"test": "/tmp",
810
"output-dir": "/tmp/simulator",
911
"test-bundle-path": "/usr/bin",

0 commit comments

Comments
 (0)