Incorporating some code review comments

MobileNativeFoundation · Apr 27, 2020 · 603745f · 603745f
1 parent 8237a83
commit 603745f
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 7 deletions.
diff --git a/README.md b/README.md
@@ -58,6 +58,7 @@ A full list supported options are listed here.
 |:----------------------:|:----------------------:|-------------------------------------------------------------------------------------|:--------:|:----------------:|
 |          app           |           -a           | The path to the host application to execute (your `.app`)                           |     N    | n/a              |
 |     xctestrun-path     |                        | The path to the `.xctestrun` file that xcode leaves when you `build-for-testing`.   |     Y    | n/a              |
+|     test-plan-path.    |                        | The path of a json file which describes the test plan. It is equivalent to the .xctestrun file generated by Xcode, but it can be generated by a different build system, e.g. Bazel |     Y    | n/a              |
 |       output-dir       |           -o           | Directory where to put output log files. **(bluepill only)**                        |     Y    | n/a              |
 |         config         |           -c           | Read options from the specified configuration file instead of the command line.     |     N    | n/a              |
 |         device         |           -d           | On which device to run the app.                                                     |     N    | iPhone 8         |
@@ -87,6 +88,8 @@ A full list supported options are listed here.
 | screenshots-directory  |           n/a          | Directory where simulator screenshots for failed ui tests will be stored.           |     N    | n/a              |
 |       video-paths      |           -V           | A list of videos that will be saved in the simulators.                              |     N    | n/a              |
 |       image-paths      |           -I           | A list of images that will be saved in the simulators.                              |     N    | n/a              |
+| unsafe-skip-xcode-version-check |               | Skip Xcode version check                                                            |     N    | NO               |
+
 
 ## Exit Status
 

diff --git a/bp/src/BPConfiguration.h b/bp/src/BPConfiguration.h
@@ -89,7 +89,7 @@ typedef NS_ENUM(NSInteger, BPProgram) {
 @property (nonatomic, strong) NSString *scriptFilePath;
 @property (nonatomic) BOOL headlessMode;
 @property (nonatomic) BOOL cloneSimulator;
-@property (nonatomic) BOOL unsafeSkipXcodeCheck;
+@property (nonatomic) BOOL unsafeSkipXcodeVersionCheck;
 @property (nonatomic, strong) NSNumber *numSims;
 @property (nonatomic) BOOL listTestsOnly;
 @property (nonatomic) BOOL quiet;

diff --git a/bp/src/BPConfiguration.m b/bp/src/BPConfiguration.m
@@ -110,8 +110,6 @@ typedef NS_OPTIONS(NSUInteger, BPOptionType) {
         "Enable verbose logging"},
     {'k', "keep-individual-test-reports", BP_MASTER | BP_SLAVE, NO, NO, no_argument, "Off", BP_VALUE | BP_BOOL, "keepIndividualTestReports",
         "Keep individual test reports, in addition to the aggregated final report"},
-    {'Z', "unsafe-skip-xcode-check", BP_MASTER | BP_SLAVE, NO, NO, no_argument, "Off", BP_VALUE | BP_BOOL , "unsafeSkipXcodeCheck",
-        "Skip Xcode version check"},
 
     // options without short-options
     {349, "additional-unit-xctests", BP_MASTER | BP_SLAVE, NO, NO, required_argument, NULL, BP_LIST | BP_PATH, "additionalUnitTestBundles",
@@ -144,6 +142,8 @@ typedef NS_OPTIONS(NSUInteger, BPOptionType) {
         "A script that will be called after the simulator is booted, but before tests are run. Can be used to do any setup (e.g. installing certs). The environment will contain $BP_DEVICE_ID with the ID of the simulator and $BP_DEVICE_PATH with its full path. Exit with zero for success and non-zero for failure."},
     {364, "test-plan-path", BP_MASTER | BP_SLAVE, NO, NO, required_argument, NULL, BP_VALUE | BP_PATH, "testPlanPath",
         "The path of a json file which describes the test plan. It is equivalent to the .xctestrun file generated by Xcode, but it can be generated by a different build system, e.g. Bazel"},
+    {365, "unsafe-skip-xcode-version-check", BP_MASTER | BP_SLAVE, NO, NO, no_argument, "Off", BP_VALUE | BP_BOOL , "unsafeSkipXcodeVersionCheck",
+    "   "},
 
     {0, 0, 0, 0, 0, 0, 0}
 };
@@ -670,15 +670,15 @@ - (BOOL)validateConfigWithError:(NSError *__autoreleasing *)errPtr {
     NSString *xcodeVersion = [BPUtils runShell:@"xcodebuild -version"];
     [BPUtils printInfo:DEBUGINFO withString:@"xcode build version: %@", xcodeVersion];
 
-    if (!self.unsafeSkipXcodeCheck) {
+    if (!self.unsafeSkipXcodeVersionCheck) {
         if ([xcodeVersion rangeOfString:@BP_DEFAULT_XCODE_VERSION].location == NSNotFound) {
             BP_SET_ERROR(errPtr, @"ERROR: Invalid Xcode version:\n%s;\nOnly %s is supported\n", [xcodeVersion UTF8String], BP_DEFAULT_XCODE_VERSION);
             return NO;
         }
 
-        //Check if Bluepill compile time Xcode version is matched with Bluepill runtime Xcode version
-        //Senario to prevent: Bluepill is compiled with Xcode 8, but runs with host installed with Xcode 9
-        //Only compare major and minor version version Exg. 9.1 == 9.1
+        // Check if Bluepill compile time Xcode version is matched with Bluepill runtime Xcode version
+        // This check prevents Bluepill compiled with Xcode 8 running on host installed with Xcode 9
+        // Only compare major and minor version version Eg. 11.2 ~ 11.2.1 but 11.2 <> 11.3
         if (![[[BPUtils getXcodeRuntimeVersion] substringToIndex:4] isEqualToString:@BP_DEFAULT_XCODE_VERSION]) {
             BP_SET_ERROR(errPtr, @"ERROR: Bluepill runtime version %s and compile time version %s are mismatched\n",
                          [[[BPUtils getXcodeRuntimeVersion] substringToIndex:4] UTF8String], [@BP_DEFAULT_XCODE_VERSION UTF8String]);