Tests in xctestrun SkipTestIdentifiers not skipped

[krze]

I got the latest release of bluepill 1.1.1, and ran bluepill with the new xctestrun-path option pointing to a valid xctestrun file generated by a build-for-testing compile of my project. It ran the correct test scheme, but it did not skip any of the tests specified in SkipTestIdentifiers.

This is with Xcode 8.3.2 and Bluepill 1.1.1 release

[krze]

I think I may have found the issue and I'm investigating a possible fix. On master, if you launch with an xctestrun file, when you hit this line in main.m, normalizedConfig is missing any excluded tests:
https://github.com/linkedin/bluepill/blob/ce998c3d762942cd07840a57261eeded3009adbb/Bluepill-runner/Bluepill-runner/main.m#L100

(lldb) po normalizedConfig
{
  "runtime" : "iOS 10.3",
  "delete-timeout" : 300,
  "error-retries" : 4,
  "list-tests" : 0,
  "test-timeout" : 300,
  "create-timeout" : 300,
  "max-sim-launch-attempts" : 2,
  "stuck-timeout" : 300,
  "output-dir" : "\/Users\/krze\/develop\/xcodebuild-test\/output",
  "plain-output" : 1,
  "repeat-count" : 1,
  "headless" : 0,
  "verbose" : 0,
  "only-retry-failed" : 0,
  "xcode-path" : "\/Applications\/Xcode.app\/Contents\/Developer",
  "num-sims" : 4,
  "junit-output" : 1,
  "reuse-simulator" : 0,
  "keep-simulator" : 0,
  "quiet" : 0,
  "max-sim-create-attempts" : 2,
  "max-sim-install-attempts" : 2,
  "launch-timeout" : 300,
  "xctestrun-path" : "\/Users\/krze\/develop\/xcodebuild-test\/Build\/Products\/SmokeTests_iphonesimulator10.3-x86_64.xctestrun",
  "json-output" : 1,
  "exclude" : [

  ],
  "device" : "iPhone 7",
  "failure-tolerance" : 0
}

BPRunner passes this config with an empty exclude section here into BPPacker:
https://github.com/linkedin/bluepill/blob/ce998c3d762942cd07840a57261eeded3009adbb/Bluepill-runner/Bluepill-runner/BPRunner.m#L152

Once in BPPacker, the array is not nil, but it is empty, so it passes the conditional on BPPacker.m:42, but the next line that attempts to subtract the empty set of testCasesToSkip from bundleTestsToRun, which of course results in no change:
https://github.com/linkedin/bluepill/blob/ce998c3d762942cd07840a57261eeded3009adbb/Bluepill-runner/Bluepill-runner/BPPacker.m#L43

It's not until BPRunner.m:207 that the skip tests get passed in:
https://github.com/linkedin/bluepill/blob/ce998c3d762942cd07840a57261eeded3009adbb/Bluepill-runner/Bluepill-runner/BPRunner.m#L207

In BPRunner.newTaskWithBundle (BPRunner.m:92), the bundle.skipTestIdentifiers are finally assigned correctly:
https://github.com/linkedin/bluepill/blob/ce998c3d762942cd07840a57261eeded3009adbb/Bluepill-runner/Bluepill-runner/BPRunner.m#L92

(lldb) po bundle.skipTestIdentifiers
<__NSArrayM 0x100311d20>(
DebugTests/testOneSkipTest,
DebugTests/testTwoSkipTest
)

(note: these are fake test names, modified because I'm testing against a real product, but the format is identical)

I'm not quire sure why the skipped tests assigned to the task's config in newTaskWithBundle are not respected, but when launching with the default params (4 sims), I see the assignment happen 4 times, one for each sim, but I see tests that were supposed to be skipped being performed across all 4 simulators.

[krze]

Still can't quite figure out what's going on. Interesting to note: if you specify --num-sims 1, no skip tests are passed in from the .xctestrun file.

[lyndsey-ferguson]

@krze you can fork the repo and create a PR from your fork to linkedin's.

[krze]

Thanks. Still investigating as #162 anticipates getting the skip tests from the xctestrun file so I'm not quite sure if I'm on the right path here:
master...krze:krze/skip-tests-from-xctestrun-file-issue-165

[lyndsey-ferguson]

One comment (I cannot comment on the code until the PR is created), but I think that you want to make the second if an else if as the config skipped tests should override the xctestrun settings.

[krze]

Thanks for clarifying, I wasn't sure which should have precedence or if they should all be accounted for. I'll make sure those override when I submit the PR

[krze]

I'm trying to add tests, or at least not break the ones that exist, and it appears that there are a lot of test failures on master already. One issue in particular that's affecting the BPConfiguration validation is that [[NSFileManager defaultManager] fileExistsAtPath:path isDirectory:&isdir] often fails the first time it tries to validate the file's existence, but succeeds the second time. In fact the documentation for fileExistsAtPath warns you not to rely on it.

I don't want to add retry logic to these configuration classes just for the sake of passing tests, and these failures have nothing to do with the code I've added (again, I've checked against master and the result is the same). Is there a workaround for this issue?

[lyndsey-ferguson]

I've found that I have to build the BPSampleApp scheme first so that the tests can run with sample data. Have you done that?

[oliverhu]

Sorry about this, I just ran the testTwoBPInstancesWithXCTestRunFile test and put a break point at line 118, skipped tests seem not respected. I'll check this and add a test to guard against this. Have you guys got any PR/pre-work I can leverage?

[oliverhu]

@lyndsey-ferguson @krze can you double check if this would fix this issue? #166

[krze]

#166 fixes the issue, thanks!

[oliverhu]

Cool, I'll merge the PR and bump the release.

[lyndsey-ferguson]

Sorry, I haven't been able to look at this due to my schedule. I'm glad that this was resolved.