Fail if bp binary is not found adjacent to bluepill and also some ref…

…actoring (#380)

Do not look for `bp` binary on `PATH` and simply fail if it is not found adjacent to `bluepill` binary.

Also did some refactoring, cleanup and improvements like...
1. Moved a helper method to utils
2. Enhancing error messages and better error handling
3. Fixing asserts in tests
4. Renaming variables

bluepill/src/BPApp.m

@@ -67,7 +67,7 @@ @implementation BPApp
                                        andXcodePath:(NSString *)xcodePath
                                           withError:(NSError *__autoreleasing *)errPtr {
 
-    NSMutableArray<BPXCTestFile *> *allTests = [[NSMutableArray alloc] init];
+    NSMutableArray<BPXCTestFile *> *allXCTestFiles = [[NSMutableArray alloc] init];
     NSUInteger errorCount = 0;
     for (NSString *key in xcTestRunDict) {
         if ([key isEqualToString:@"__xctestrun_metadata__"]) {
@@ -83,39 +83,39 @@ @implementation BPApp
             errorCount++;
             continue;
         }
-        [allTests addObject:xcTestFile];
+        [allXCTestFiles addObject:xcTestFile];
     }
     if (errorCount) {
         BP_SET_ERROR(errPtr, @"Failed to load some test bundles");
         return nil;
     }
-    return allTests;
+    return allXCTestFiles;
 }
 
 + (instancetype)appWithConfig:(BPConfiguration *)config
                     withError:(NSError *__autoreleasing *)errPtr {
 
     BPApp *app = [[BPApp alloc] init];
-    NSMutableArray<BPXCTestFile *> *allTests = [[NSMutableArray alloc] init];
-
+    NSMutableArray<BPXCTestFile *> *allXCTestFiles = [[NSMutableArray alloc] init];
     if (config.xcTestRunDict) {
         NSAssert(config.xcTestRunPath, @"");
         [BPUtils printInfo:INFO withString:@"Using xctestrun configuration"];
         NSArray<BPXCTestFile *> *loadedTests = [BPApp testsFromXCTestRunDict:config.xcTestRunDict
-                                                           andXCTestRunPath:config.xcTestRunPath
+                                                            andXCTestRunPath:config.xcTestRunPath
                                                                 andXcodePath:config.xcodePath
                                                                    withError:errPtr];
         if (loadedTests == nil) {
             return nil;
         }
 
-        [allTests addObjectsFromArray:loadedTests];
+        [allXCTestFiles addObjectsFromArray:loadedTests];
     } else if (config.appBundlePath) {
         NSAssert(config.appBundlePath, @"no app bundle and no xctestrun file");
-        [allTests addObjectsFromArray:[BPApp testsFromAppBundle:config.appBundlePath
-                                              andTestBundlePath:config.testBundlePath
-                                             andUITargetAppPath:config.testRunnerAppPath
-                                                      withError:errPtr]];
+        [BPUtils printInfo:WARNING withString:@"Using broken configuration, consider using .xctestrun files"];
+        [allXCTestFiles addObjectsFromArray:[BPApp testsFromAppBundle:config.appBundlePath
+                                                    andTestBundlePath:config.testBundlePath
+                                                   andUITargetAppPath:config.testRunnerAppPath
+                                                            withError:errPtr]];
     } else {
         BP_SET_ERROR(errPtr, @"xctestrun file must be given, see usage.");
         return nil;
@@ -126,21 +126,21 @@ + (instancetype)appWithConfig:(BPConfiguration *)config
             BPXCTestFile *testFile = [BPXCTestFile BPXCTestFileFromXCTestBundle:testBundle
                                                                andHostAppBundle:config.appBundlePath
                                                              andUITargetAppPath:config.testRunnerAppPath
-                                                                    withError:errPtr];
-            [allTests addObject:testFile];
+                                                                      withError:errPtr];
+            [allXCTestFiles addObject:testFile];
         }
     }
 
     if (config.additionalUITestBundles) {
         for (NSString *testBundle in config.additionalUnitTestBundles) {
             BPXCTestFile *testFile = [BPXCTestFile BPXCTestFileFromXCTestBundle:testBundle
                                                                andHostAppBundle:config.testRunnerAppPath
-                                                                    withError:errPtr];
-            [allTests addObject:testFile];
+                                                                      withError:errPtr];
+            [allXCTestFiles addObject:testFile];
         }
     }
 
-    app.testBundles = allTests;
+    app.testBundles = allXCTestFiles;
     return app;
 }
 

bluepill/src/BPPacker.m

@@ -7,8 +7,9 @@
 //  distributed under the License is distributed on an "AS IS" BASIS,
 //  WITHOUT WARRANTIES OF ANY KIND, either express or implied.  See the License for the specific language governing permissions and limitations under the License.
 
-#import "BPPacker.h"
+#import "bp/src/BPXCTestFile.h"
 #import "bp/src/BPUtils.h"
+#import "BPPacker.h"
 
 @implementation BPPacker
 
@@ -56,7 +57,7 @@ @implementation BPPacker
             }
         }
     }
-    
+
     NSUInteger testsPerGroup = MAX(1, totalTests / numBundles);
     NSMutableArray<BPXCTestFile *> *bundles = [[NSMutableArray alloc] init];
     for (BPXCTestFile *xctFile in sortedXCTestFiles) {
@@ -107,10 +108,13 @@ @implementation BPPacker
     }
 
     // load the config file
-    NSDictionary *testTimes = [self loadConfigFile:config.testTimeEstimatesJsonFile withError:errPtr];
-    if ((errPtr && *errPtr) || !testTimes) {
+    NSDictionary *testTimes = [BPUtils loadSimpleJsonFile:config.testTimeEstimatesJsonFile withError:errPtr];
+    if (errPtr && *errPtr) {
         [BPUtils printInfo:ERROR withString:@"%@", [*errPtr localizedDescription]];
         return NULL;
+    } else if (!testTimes) {
+        [BPUtils printInfo:ERROR withString:@"Invalid test execution time data"];
+        return NULL;
     }
 
     NSMutableDictionary *estimatedTimesByTestFilePath = [[NSMutableDictionary alloc] init];
@@ -161,14 +165,4 @@ @implementation BPPacker
     return sortedBundles;
 }
 
-+ (NSDictionary *)loadConfigFile:(NSString *)file withError:(NSError **)errPtr{
-    NSData *data = [NSData dataWithContentsOfFile:file
-                                          options:NSDataReadingMappedIfSafe
-                                            error:errPtr];
-    if (!data) return nil;
-
-    return [NSJSONSerialization JSONObjectWithData:data
-                                           options:kNilOptions
-                                             error:errPtr];
-}
 @end

bluepill/src/BPReportCollector.m

@@ -59,8 +59,7 @@ + (void)collectReportsFromPath:(NSString *)reportsPath
         NSNumber *isDirectory = nil;
         if (![url getResourceValue:&isDirectory forKey:NSURLIsDirectoryKey error:&error]) {
             [BPUtils printInfo:ERROR withString:@"Failed to get resource from url %@", url];
-        }
-        else if (![isDirectory boolValue]) {
+        } else if (![isDirectory boolValue]) {
             if ([[url pathExtension] isEqualToString:@"xml"]) {
                 [BPUtils printInfo:DEBUGINFO withString:@"JUnit collecting: %@", [url path]];
                 NSString *path = [url path];

bluepill/src/BPRunner.h

@@ -16,7 +16,7 @@
 @property (nonatomic, strong) BPConfiguration *config;
 @property (nonatomic, strong) NSString *bpExecutable;
 @property (nonatomic, strong) NSMutableArray *nsTaskList;
-@property (nonatomic, strong) NSMutableDictionary* testHostForSimUDID;
+@property (nonatomic, strong) NSDictionary *testHostSimTemplates;
 
 /*!
  * @discussion get a BPRunnner to run tests
@@ -40,11 +40,12 @@
                     andNumber:(NSUInteger)number
                     andDevice:(NSString *)deviceID
            andCompletionBlock:(void (^)(NSTask * ))block;
+
 /**
  @discussion start running tests
  @return 1: test failures 0: pass -1: failed to run tests
  */
 - (int)runWithBPXCTestFiles:(NSArray<BPXCTestFile *>*)xcTestFiles;
-- (void) interrupt;
 
+- (void) interrupt;
 @end

bluepill/src/BPRunner.m

@@ -7,23 +7,23 @@
 //  distributed under the License is distributed on an "AS IS" BASIS,
 //  WITHOUT WARRANTIES OF ANY KIND, either express or implied.  See the License for the specific language governing permissions and limitations under the License.
 
-#import "BPRunner.h"
-#import "BPPacker.h"
-#import "bp/src/BPUtils.h"
-#include <sys/sysctl.h>
-#include <sys/types.h>
-#include <mach/mach.h>
-#include <mach/processor_info.h>
-#include <mach/mach_host.h>
-#include <signal.h>
-#include <pwd.h>
 #import <AppKit/AppKit.h>
-#import "bp/src/BPSimulator.h"
 #import "bp/src/BPCreateSimulatorHandler.h"
+#import "bp/src/BPSimulator.h"
+#import "bp/src/BPStats.h"
 #import "bp/src/BPUtils.h"
 #import "bp/src/BPWaitTimer.h"
 #import "bp/src/SimulatorHelper.h"
-#import "bp/src/BPStats.h"
+#import "BPPacker.h"
+#import "BPRunner.h"
+
+#include <mach/mach.h>
+#include <mach/mach_host.h>
+#include <mach/processor_info.h>
+#include <pwd.h>
+#include <signal.h>
+#include <sys/sysctl.h>
+#include <sys/types.h>
 
 static int volatile interrupted = 0;
 
@@ -59,33 +59,12 @@ @implementation BPRunner
 + (instancetype)BPRunnerWithConfig:(BPConfiguration *)config
                         withBpPath:(NSString *)bpPath {
     BPRunner *runner = [[BPRunner alloc] init];
-    runner.testHostForSimUDID = [[NSMutableDictionary alloc] init];
+    runner.testHostSimTemplates = [[NSMutableDictionary alloc] init];
     runner.config = config;
-    // Find the `bp` binary.
-
-    if (bpPath) {
-        runner.bpExecutable = bpPath;
-    } else {
-        NSString *argv0 = [[[NSProcessInfo processInfo] arguments] objectAtIndex:0];
-        NSString *bp = [[argv0 stringByDeletingLastPathComponent] stringByAppendingPathComponent:@"bp"];
-        if (![[NSFileManager defaultManager] isExecutableFileAtPath:bp]) {
-            // Search the PATH for a bp executable
-            BOOL foundIt = false;
-            NSString *path = [[[NSProcessInfo processInfo] environment] objectForKey:@"PATH"];
-            for (NSString *dir in [path componentsSeparatedByString:@":"]) {
-                bp = [dir stringByAppendingPathComponent:@"bp"];
-                if ([[NSFileManager defaultManager] isExecutableFileAtPath:bp]) {
-                    foundIt = true;
-                    break;
-                }
-            }
-            if (!foundIt) {
-                fprintf(stderr, "ERROR: I couldn't find the `bp` executable anywhere.\n"
-                        "Please put it somewhere in your PATH. (Ideally next to `bluepill`.\n");
-                return nil;
-            }
-        }
-        runner.bpExecutable = bp;
+    runner.bpExecutable = bpPath ?: [BPUtils findExecutablePath:@"bp"];
+    if (!runner.bpExecutable) {
+        fprintf(stderr, "ERROR: Unable to find bp executable.\n");
+        return nil;
     }
     return runner;
 }
@@ -115,7 +94,7 @@ - (NSTask *)newTaskWithBundle:(BPXCTestFile *)bundle
         cfg.environmentVariables = bundle.environmentVariables;
     }
     if (self.config.cloneSimulator) {
-        cfg.templateSimUDID = self.testHostForSimUDID[bundle.testHostPath];
+        cfg.templateSimUDID = self.testHostSimTemplates[bundle.testHostPath];
     }
     NSError *err;
     NSString *tmpFileName = [NSString stringWithFormat:@"%@/bluepill-%u-config",
@@ -175,11 +154,10 @@ - (NSRunningApplication *)openSimulatorAppWithConfiguration:(BPConfiguration *)c
                                              NSWorkspaceLaunchAndHide;
     //launch Simulator.app without booting a simulator
     NSDictionary *configuration = @{NSWorkspaceLaunchConfigurationArguments:@[@"-StartLastDeviceOnLaunch",@"0"]};
-    NSRunningApplication *app = [[NSWorkspace sharedWorkspace]
-                                 launchApplicationAtURL:simulatorURL
-                                 options:launchOptions
-                                 configuration:configuration
-                                 error:errPtr];
+    NSRunningApplication *app = [[NSWorkspace sharedWorkspace] launchApplicationAtURL:simulatorURL
+                                                                              options:launchOptions
+                                                                        configuration:configuration
+                                                                                error:errPtr];
     if (!app) {
         [BPUtils printInfo:ERROR withString:@"Launch Simulator.app returned error: %@", [*errPtr localizedDescription]];
         return nil;
@@ -203,7 +181,6 @@ - (int)runWithBPXCTestFiles:(NSArray<BPXCTestFile *> *)xcTestFiles {
     if (sigaction(SIGHUP, &new_action, NULL) != 0) {
         [BPUtils printInfo:ERROR withString:@"Could not install SIGHUP handler: %s", strerror(errno)];
     }
-
     BPSimulator *bpSimulator = [BPSimulator simulatorWithConfiguration:self.config];
     NSUInteger numSims = [self.config.numSims intValue];
     [BPUtils printInfo:INFO withString:@"This is Bluepill %s", [BPUtils version]];
@@ -220,8 +197,8 @@ - (int)runWithBPXCTestFiles:(NSArray<BPXCTestFile *> *)xcTestFiles {
         numSims = bundles.count;
     }
     if (self.config.cloneSimulator) {
-        self.testHostForSimUDID = [bpSimulator createSimulatorAndInstallAppWithBundles:xcTestFiles];
-        if ([self.testHostForSimUDID count] == 0) {
+        self.testHostSimTemplates = [bpSimulator createSimulatorAndInstallAppWithBundles:xcTestFiles];
+        if ([self.testHostSimTemplates count] == 0) {
             return 1;
         }
     }

bluepill/src/main.m

@@ -8,15 +8,15 @@
 //  WITHOUT WARRANTIES OF ANY KIND, either express or implied.  See the License for the specific language governing permissions and limitations under the License.
 
 #import <Foundation/Foundation.h>
-#import "BPApp.h"
+#import <getopt.h>
+#import <libgen.h>
 #import "bp/src/BPConfiguration.h"
-#import "BPRunner.h"
-#import "bp/src/BPUtils.h"
 #import "bp/src/BPStats.h"
+#import "bp/src/BPUtils.h"
 #import "bp/src/BPWriter.h"
+#import "BPApp.h"
 #import "BPReportCollector.h"
-#import <getopt.h>
-#import <libgen.h>
+#import "BPRunner.h"
 
 #include <sys/ioctl.h>
 #include <string.h>
@@ -88,7 +88,7 @@ int main(int argc, char * argv[]) {
 
         NSError *err = nil;
         if (![config processOptionsWithError:&err] || ![config validateConfigWithError:&err]) {
-            fprintf(stderr, "%s: invalid configuration\n\t%s\n",
+            fprintf(stderr, "%s: Invalid configuration\n\t%s\n",
                     basename(argv[0]), [[err localizedDescription] UTF8String]);
             exit(1);
         }
@@ -111,6 +111,10 @@ int main(int argc, char * argv[]) {
         [[BPStats sharedStats] endTimer:@"Normalizing Configuration" withResult:@"INFO"];
         // start a runner and let it fly
         BPRunner *runner = [BPRunner BPRunnerWithConfig:normalizedConfig withBpPath:nil];
+        if (!runner) {
+            fprintf(stderr, "ERROR: Unable to create Bluepill Runner.\n");
+            exit(1);
+        }
         rc = [runner runWithBPXCTestFiles:app.testBundles];
         if (config.outputDirectory) {
             // write the stats

bluepill/tests/BPAppTests.m

@@ -8,12 +8,12 @@
 //  WITHOUT WARRANTIES OF ANY KIND, either express or implied.  See the License for the specific language governing permissions and limitations under the License.
 
 #import <XCTest/XCTest.h>
-#import "bp/tests/BPTestHelper.h"
-#import "bp/src/BPConfiguration.h"
 #import "bluepill/src/BPApp.h"
+#import "bp/src/BPConfiguration.h"
 #import "bp/src/BPXCTestFile.h"
 #import "bp/src/BPUtils.h"
 #import "bp/src/BPConstants.h"
+#import "bp/tests/BPTestHelper.h"
 
 @interface BPAppTests : XCTestCase
 @property (nonatomic, strong) BPConfiguration* config;
@@ -23,7 +23,7 @@ @implementation BPAppTests
 
 - (void)setUp {
     [super setUp];
-    
+
     NSString *hostApplicationPath = [BPTestHelper sampleAppPath];
     NSString *testBundlePath = [BPTestHelper sampleAppNegativeTestsBundlePath];
     self.config = [BPConfiguration new];
@@ -44,14 +44,15 @@ - (void)tearDown {
 - (void)testAppWithAppBundlePathNoError {
     NSError *error;
     self.config.testBundlePath = nil;
-    BPApp *app = [BPApp appWithConfig:self.config withError:nil];
+    self.config.appBundlePath = [BPTestHelper sampleAppPath];
+    BPApp *app = [BPApp appWithConfig:self.config withError:&error];
     XCTAssertNil(error);
     XCTAssert(app.testBundles.count > 2);
 }
 
 - (void)testAppWithOnlyTestBundlePath {
     NSError *error;
-    BPApp *app = [BPApp appWithConfig:self.config withError:nil];
+    BPApp *app = [BPApp appWithConfig:self.config withError:&error];
     XCTAssertNil(error);
     XCTAssert(app.testBundles.count == 1);
     BPXCTestFile *testBundle = app.testBundles[0];