[SR-1944] [corelibs-xctest] Replace xctest-checker with FileCheck

[ec882e32-f2b6-4d2a-849c-98d6c7df0cfb]

Previous ID	SR-1944
Radar	None
Original Reporter	@modocache
Type	Improvement

Additional Detail from JIRA

| | | |------------------|-----------------| |Votes | 0 | |Component/s | XCTest | |Labels | Improvement | |Assignee | None | |Priority | Medium | md5: 961cab0c1de40e4880f026d33ac12047

Issue Description:

xctest-checker was originally added in https://github.com/apple/swift-corelibs-xctest/pull/20, as a replacement for FileCheck. At that time, it was easy to build swift-corelibs-xctest without first building apple/swift from source. As we continued to develop xctest-checker into a more fully-featured FileCheck replacement, @briancroom wondered whether it would make sense to switch to FileCheck completely: https://github.com/apple/swift-corelibs-xctest/pull/94#issuecomment-207684042. I argued against this because it would mean completely coupling the corelibs-xctest build to the overall apple/swift build.

However, as its dependencies on other projects such as swift-corelibs-foundation have expanded, swift-corelibs-xctest has been encouraging contributors to use the Swift build script to build and test the project for a long while now:

apple/swift-corelibs-xctest $ ../swift/utils/build-script --preset corelibs-xctest

The Swift build script has access to the LLVM bin directory, and as such it knows where to find FileCheck.

Is it time to replace xctest-checker with FileCheck? I think the answer is yes. We can migrate using the following steps:

1. XCTest's build_script.py test subcommand should take an optional path to a FileCheck executable to use when testing. At first, the option will do nothing.

2. The apple/swift build script should be modified to pass the path to FileCheck to the build script.

3. XCTest's lit.cfg should provide a %{FileCheck} substitution to the tests.

4. Tests should migrate over from using the %{xctest_checker} substitution to the %{FileCheck} substitution.

5. Once all tests have been migrated, xctest-checker should be deleted.

[belkadan]

@ddunbar has also talked about distributing some of the LLVM tools in the development snapshot packages.

[ddunbar]

Yes, I even have a patch for putting FileCheck in, but its gotten buried under other stuff unfortunately.

[ec882e32-f2b6-4d2a-849c-98d6c7df0cfb]

Yeah, that'd be great, too. I wouldn't mind doing this as an intermediary step. Once FileCheck is included in the Swift toolchain, we can use its location as a default value for the swift-corelibs-xctest/build_script.py --filecheck argument.

[briancroom]

Thanks for writing this up, @modocache! xctest-checker has served us well, but I think we will end up appreciating the lightened maintenance burden of using the standard tooling.

One further consideration before doing away with xctest-checker is that it does have a small amount of extra Xcode integration which we wouldn't get from FileCheck as far as I know. Discussion here. I would love to see us put together a patch adding this to FileCheck! Still, I wouldn't consider that a blocker for moving ahead with the migration proposed here.

@ddunbar is your patch available in a PR or branch somewhere? Is it just a matter of someone having time to follow up on getting it merged?

[ec882e32-f2b6-4d2a-849c-98d6c7df0cfb]

For reference on the topic of Xcode-friendly output in FileCheck, here's FileCheck's current output:

/path/to/FileName.m:13:16: error: expected string not found in input
// CHECK-NEXT: Tsdasdaest Case '-[FailureTestCase test_fails]' started.
               ^
<stdin>:4:1: note: scanning from here
Test Case '-[FailureTestCase test_fails]' started.
^

To get this to display properly in Xcode, I think we'd need something like this:

/path/to/FileName.m:13:16: error: expected string not found in input
Actual: Test Case '-[FailureTestCase test_fails]' started.
Expected: // CHECK-NEXT: Tsdasdaest Case '-[FailureTestCase test_fails]' started.

This seems like a big change – should FileCheck take a -format argument, so we can opt-in to an Xcode friendly format, without changing the format for everyone else?

[ec882e32-f2b6-4d2a-849c-98d6c7df0cfb]

@ddunbar: Does your patch include the LLVM utility not? We don't currently test the exit code of any of our XCTest programs; instead we write the following:

// RUN: %{swiftc} %s -o %T/SingleFailingTestCase
// NOTE: The following line prevents us from verifying the `SingleFailingTestCase` executable's exit code.
// RUN: %T/SingleFailingTestCase > %t || true
// RUN: %{xctest_checker} %t %s

Using not, we could write the following for only executables that fail:

// RUN: %{swiftc} %s -o %T/SingleFailingTestCase
// RUN: not %T/SingleFailingTestCase | %{FileCheck} %s