Add lit tests #14

modocache · 2015-12-11T06:52:53Z

Another attempt at #10! See the discussion on that pull request, plus the commit messages on this one, for details on the approach I'm taking. 🌂

As @ddunbar suggested in the comments for the last pull request, I tried using lit instead of a custom test runner script. Unfortunately, I think I might be doing something wrong, since I ended up adding two dependencies in the process:

Obviously lit itself, which could be installed via https://pypi.python.org/pypi/lit
FileCheck, which is included in the LLVM built products directory bin

It seems like nearly all of the lit tests in the LLVM and Swift projects use FileCheck. Unfortunately, it's not distributed outside of LLVM. As a result, these tests require that users have built Swift prior to running the test suite.

I also couldn't find a nice way to pass the LLVM bin path to the Xcode project. It now assumes that LLVM is built in a specific location relative to this project's source directory:

lit -v $SRCROOT/Tests --param native_llvm_tools_path="$SRCROOT/../build/Ninja-DebugAssert/llvm-macosx-x86_64/bin"

Ideally, we could just reuse the lit.cfg from the Swift project, which would not only get us the LLVM bin path, but also neat substitutions like %target-swiftc_driver. Those would allow us to replace the OS X-specific steps from the "adding tests" section of the README (also added in this pull request). But it turns out the Swift lit.cfg is very tightly coupled to the Swift build script and the environment variables it exports--it'd be a lot more work to separate those two.

I'm not really sure how to proceed, so feedback would be appreciated. I'd love to add some regression tests to this project.

Adds a test runner (lit) that compares annotations in the source code of an XCTest file to actual output when that source code is compiled and run. This acts as a regression test suite for the project. Adds two new Xcode targets: 1. SingleFailingTestCase: An executable that runs `XCTMain()` with a failing test case. It is built and installed to the Tests/Fixtures/Products/ directory. 2. SwiftXCTestTests: An aggregate target that builds SingleFailingTestCase (and, in the future, all test fixtures), then runs the lit test runner, passing the path the `FileCheck`, another LLVM tool that is used to test the output.

By passing `--test` to the build script, the lit regression tests are run. This allows the test suite to be run on Linux. Require the path to `FileCheck` be passed along as well.

modocache · 2015-12-11T07:11:46Z

In hindsight, it might be nicer to ask developers to simply add FileCheck to their PATH, rather than have the Linux and OS X build systems require additional arguments. Thoughts? 🙌

mike-ferris · 2015-12-11T14:43:52Z

I do think that the amount of work required on the OS X side for adding new tests is unfortunate.

The more I think about this, the more I think that I would really prefer that the tests actually be written as XCTests. I have been working on broadening the framework implementation to allow this and I am hoping to have a patch ready in the near future that will get us to that point.

If we had support for XCTestObservation in the framework, I think it would go a long way to providing the testability we need. There might still be use for the kind of black box testing we could get from a tool like lit, though. For example, the test in your pull request could be written as an XCTest and verify that the test ran, had the expected failures, etc… but would not be able to verify the exact console output.

I would suggest that:
- we try to find a way to make adding the lit tests on OS X as automatic as on Linux.
- we rename the directory to something more specific (LitTests, maybe)

I’d also be interested in whether Daniel has any suggestions on simplifying the requirements here. Your additions to the ReadMe don’t mention it, but does running these tests require getting and installing lit on the system separately?

Mike

On Dec 10, 2015, at 10:52 PM, Brian Gesiak [email protected] wrote:

Another attempt at #10 #10! See the discussion on that pull request, plus the commit messages on this one, for details on the approach I'm taking.

As @ddunbar https://github.com/ddunbar suggested in the comments for the last pull request, I tried using lit instead of a custom test runner script. Unfortunately, I think I might be doing something wrong, since I ended up adding two dependencies in the process:

Obviously lit itself, which could be installed via https://pypi.python.org/pypi/lit https://pypi.python.org/pypi/lit
FileCheck, which is included in the LLVM built products directory bin
It seems like nearly all of the lit tests in the LLVM and Swift projects use FileCheck. Unfortunately, it's not distributed outside of LLVM. As a result, these tests require that users have built Swift prior to running the test suite.

I also couldn't find a nice way to pass the LLVM bin path to the Xcode project. It now assumes that LLVM is built in a specific location relative to this project's source directory:

lit -v $SRCROOT/Tests --param native_llvm_tools_path="$SRCROOT/../build/Ninja-DebugAssert/llvm-macosx-x86_64/bin"
Ideally, we could just reuse the lit.cfg from the Swift project, which would not only get us the LLVM bin path, but also neat substitutions like %target-swiftc_driver. Those would allow us to replace the OS X-specific steps from the "adding tests" section of the README (also added in this pull request). But it turns out the Swift lit.cfg is very tightly coupled to the Swift build script and the environment variables it exports--it'd be a lot more work to separate those two.

I'm not really sure how to proceed, so feedback would be appreciated. I'd love to add some regression tests to this project.

You can view, comment on, or merge this pull request online at:

#14 #14
Commit Summary

[Tests] Add functional lit tests for OS X
[build_script] Add --test option to test on Linux
[README] Add instructions on testing
File Changes

M .gitignore https://github.com/apple/swift-corelibs-xctest/pull/14/files#diff-0 (5)
M README.md https://github.com/apple/swift-corelibs-xctest/pull/14/files#diff-1 (23)
A Tests/Functional/Sources/SingleFailingTestCase/main.swift https://github.com/apple/swift-corelibs-xctest/pull/14/files#diff-2 (29)
A Tests/lit.cfg https://github.com/apple/swift-corelibs-xctest/pull/14/files#diff-3 (13)
M XCTest.xcodeproj/project.pbxproj https://github.com/apple/swift-corelibs-xctest/pull/14/files#diff-4 (207)
M build_script.py https://github.com/apple/swift-corelibs-xctest/pull/14/files#diff-5 (49)
Patch Links:

https://github.com/apple/swift-corelibs-xctest/pull/14.patch https://github.com/apple/swift-corelibs-xctest/pull/14.patch
https://github.com/apple/swift-corelibs-xctest/pull/14.diff https://github.com/apple/swift-corelibs-xctest/pull/14.diff
—
Reply to this email directly or view it on GitHub #14.

modocache · 2015-12-11T16:27:36Z

Your additions to the ReadMe don’t mention it, but does running these tests require getting and installing lit on the system separately?

Ack, you're right! I had thought it was placed on a user's PATH when building LLVM/Swift, but I think it may just be there on my system from a previous installation.

I do think that the amount of work required on the OS X side for adding new tests is unfortunate.

I think the majority of the work here is to add executable targets and add them as dependencies to the aggregate target. If I knew how to compile SingleFailingTestCase/main.swift and link it to SwiftXCTest.framework on the OS X command-line, adding tests would be equally simple on all platforms.

I can't tell if I'm missing something, or if it's actually pretty tough to do this. It might be simpler if we adapted build_script.py to work on OS X as well, since then we could run that script as part of the test suite, having it take parameters like paths to --lit and --file-check. I think that's the simplest way to implement this test suite, and I think making the script cross-platform would be an improvement anyway--thoughts?

I have been working on broadening the framework implementation to allow this and I am hoping to have a patch ready in the near future that will get us to that point. [...] If we had support for XCTestObservation in the framework, I think it would go a long way to providing the testability we need.

Awesome!! Can't wait! 😍

Let me know if there's anything I can help with. I think we discussed methods like -[XCTestObservation testBundleWillStart:] on the mailing list--are you planning on simply never sending that message to observers?

mike-ferris · 2015-12-11T16:56:55Z

On Dec 11, 2015, at 8:27 AM, Brian Gesiak [email protected] wrote:

Your additions to the ReadMe don’t mention it, but does running these tests require getting and installing lit on the system separately?

Ack, you're right! I had thought it was placed on a user's PATH when building LLVM/Swift, but I think it may just be there on my system from a previous installation.

I do think that the amount of work required on the OS X side for adding new tests is unfortunate.

I think the majority of the work here is to add executable-targets and add them as dependencies to the aggregate target. If I knew how to compile SingleFailingTestCase/main.swift and link it to SwiftXCTest.framework on the OS X command-line, adding tests would be equally simple on all platforms.

Well, this should be relatively simple… you can get an idea of what’s needed by looking at the transcript for one of your targets. Then I guess we could have a single target that had a shell script phase that simply iterated the lit test directories and built them similarly to how they are built on linux… Some differences to note: to invoke swiftc you’ll want to use “xcrun swiftc …”. Since XCTest on OS X is a framework, not a bare shared library, you’ll want to replace the appropriate -I and -L options that locate the module and library locations with a -F that locates the framework probably -F ${PRODUCT_BUILD_DIR}.
I can't tell if I'm missing something, or if it's actually pretty tough to do this. It might be simpler if we adapted build_script.py to work on OS X as well, since then we could run that script as part of the test suite, having it take parameters like paths to --lit and --file-check. I think that's the simplest way to implement this test suite, and I think making the script cross-platform would be an improvement anyway--thoughts?

This might be a bit tricky, or at least more work than might be worth while since we’d have to teach the script how to build a framework. Not super complicated, but replicating Xcode build system logic. I would also expect most folks working on OS X will be working in Xcode, so having this work in the Xcode project is desirable.
I have been working on broadening the framework implementation to allow this and I am hoping to have a patch ready in the near future that will get us to that point. [...] If we had support for XCTestObservation in the framework, I think it would go a long way to providing the testability we need.

Awesome!! Can't wait!

Let me know if there's anything I can help with. I think we discussed methods like -[XCTestObservation testBundleWillStart:] on the mailing list--are you planning on simply never sending that message to observers?

Yeah, the bundle stuff I am just leaving out (this whole thing is still very much a subset of Xcode’s XCTest, so the protocol will also be a subset.) The interesting challenge with this is turning out to be that CoreLibs XCTest has not quite the right model for XCTestCase. In Xcode’s XCTest, an XCTestCase instance only runs a single test (though its class may contain many test methods, an instance is configured to run one of them and an instance is created for each test method.) The XCTestObservation protocol kind of depends on that because the finest grain notifications are the testCase… ones. But CoreLibs XCTest right now uses a single XCTestCase instance to run all the test methods the class contains.

So most of my work so far has been around fixing that by implementing enough of the surrounding model of XCTest to support that.

Mike

—
Reply to this email directly or view it on GitHub #14 (comment).

ddunbar · 2015-12-11T17:21:06Z

Ok, first off a couple comments on the implementation:

Ideally I think the test cases should be built as part of the tests. Conceptually, the end-to-end process of building the test with XCTest and running it is a part of the test case, I dislike splitting it across the build and test process.

The best way to do this is to write a helper tool which performs the boilerplate (building the test appropriately for each platform).

For integration into Xcode, it is possible to integrate the lit tests directly with XCTest, so that you can just "Cmd-U" and get all the tests to run. I can help out with this piece. There are occasionally glitches, but this would be motivation to improve XCTest's support for dynamic test methods in the project itself. :)

Obviously before we pursue these things more we should decide on what our exact approach is going to be.

As far as the dependencies question, if we did go this direction I would suggest:

The dependency on lit itself can easily be automatically managed, using the Python packaging tools and installing into a user local directory. This can be scripted so that it all automatically "just works" whether or not a user has lit installed.
1. The dependency on FileCheck maybe is best being dropped. While it is true that many LLVM/Clang/Swift tests depend on this tool, it is also a little obscure and may not be all that valuable for XCTest. What we could do installed was include our own support tools for being able to easily write the kinds of tests that make sense for XCTest. This approach in general is a little more robust than just the FileCheck style of checking for patterns.

modocache · 2015-12-15T22:04:39Z

Oops, sorry for the slow turnaround! 🐌

Then I guess we could have a single target that had a shell script phase that simply iterated the lit test directories and built them similarly to how they are built on Linux...

@mike-ferris-apple This seems reasonable to me.

In Xcode’s XCTest, an XCTestCase instance only runs a single test (though its class may contain many test methods, an instance is configured to run one of them and an instance is created for each test method).

@mike-ferris-apple Yeah, I looked into this a while back; this has been the model since OCUnit. Have you spoken to the Objective-C XCTest team? I bet they have a ton of context on the problem space.

Conceptually, the end-to-end process of building the test with XCTest and running it is a part of the test case, I dislike splitting it across the build and test process. [...] The best way to do this is to write a helper tool which performs the boilerplate (building the test appropriately for each platform).

@ddunbar I was thinking, and based on @mike-ferris-apple's comments above, that this helper tool should have two incarnations: a script used for Linux, and an Xcode project target for OS X. Does that sound reasonable?

For integration into Xcode, it is possible to integrate the lit tests directly with XCTest, so that you can just "Cmd-U" and get all the tests to run.

@ddunbar This pull request adds a script execution phase to the Xcode project that runs lit. I can probably add a python ../llvm/utils/lit/setup.py install to that script to ensure lit is installed, preferably to a specific directory that doesn't muck with the user's PATH. Or do you have something else in mind here?

The dependency on FileCheck maybe is best being dropped. [...] What we could do [instead is] include our own support tools for being able to easily write the kinds of tests that make sense for XCTest.

This makes sense to me. Should this be a Python module in the swift-corelibs-xctest repository?

Based on the above, I think the direction I'll take here is as follows:

swift-corelibs-xctest/
    XCTest.xcodeproj/ # Updated with a FunctionalTests target, which builds the Functional/Sources/ and runs lit
    XCTest/
    Tests/            # Tests directory for this project.
        Functional/   # Functional tests that use lit. They're in their own sub-directory to differentiate from the unit tests we'll be adding in the future
            Sources/  # A collection of main.swift files that run XCTMain()
            Products/ # The built products from Tests/Fixtures/Sources/ go here. They'll be .gitignore'd
            lit.cfg   # The lit config for these tests. Adds xctest-checker to the Python path
    xctest-checker/   # A Python module for verifying the output of XCTest runs
    build_script.py   # Updated with a --test parameter, to build and run the tests on Linux

Adding a test will involve two steps:

Add a directory with the name of your test to the Tests/Functional/Sources/ directory. For example, if your test is named "TwoFailingTestCases", make a directory named Tests/Functional/Sources/TwoFailingTestCases.
Add a main.swift file to the directory you added above. The file should contain comments, beginning with //. The test runner will verify that the output of main.swift matches your comments exactly, using the xctest-checker tool.

Does all that sound reasonable? 🙌

mike-ferris · 2015-12-16T18:02:21Z

On Dec 15, 2015, at 2:24 PM, Brian Gesiak [email protected] wrote:

Oops, sorry for the slow turnaround!

Then I guess we could have a single target that had a shell script phase that simply iterated the lit test directories and built them similarly to how they are built on Linux...

@mike-ferris-apple https://github.com/mike-ferris-apple This seems reasonable to me.

In Xcode’s XCTest, an XCTestCase instance only runs a single test (though its class may contain many test methods, an instance is configured to run one of them and an instance is created for each test method).

@mike-ferris-apple https://github.com/mike-ferris-apple Yeah, I looked into this a while back http://modocache.io/sentestingkit-how-does-it-even; this has been the model since OCUnit https://github.com/modocache/OCUnit/blob/f609940845d48c23b4629ae990c3395cd0540364/OCUnit/SourceCode/SenTestingKit/SenTestSuite.m#L151-L156. Have you spoken to the Objective-C XCTest team? I bet they have a ton of context on the problem space.

Conceptually, the end-to-end process of building the test with XCTest and running it is a part of the test case, I dislike splitting it across the build and test process. [...] The best way to do this is to write a helper tool which performs the boilerplate (building the test appropriately for each platform).

@ddunbar https://github.com/ddunbar I was thinking, and based on @mike-ferris-apple https://github.com/mike-ferris-apple's comments above, that this helper tool should have two incarnations: a script used for Linux, and an Xcode project target for OS X. Does that sound reasonable?

For integration into Xcode, it is possible to integrate the lit tests directly with XCTest, so that you can just "Cmd-U" and get all the tests to run.

@ddunbar https://github.com/ddunbar This pull request adds a script execution phase to the Xcode project that runs lit. I can probably add a python ../llvm/utils/lit/setup.py install to that script to ensure lit is installed, preferably to a specific directory that doesn't muck with the user's PATH. Or do you have something else in mind here?

The dependency on FileCheck maybe is best being dropped. [...] What we could do [instead is] include our own support tools for being able to easily write the kinds of tests that make sense for XCTest.

This makes sense to me. Should this be a Python module in the swift-corelibs-xctest repository?

Based on the above, I think the direction I'll take here is as follows:

swift-corelibs-xctest/
XCTest.xcodeproj/ # Updated with a FunctionalTests target, which builds the Functional/Sources/ and runs lit
XCTest/
Tests/ # Tests directory for this project.
Functional/ # Functional tests that use lit. They're in their own sub-directory to differentiate from the unit tests we'll be adding in the future
Sources/ # A collection of main.swift files that run XCTMain()
Products/ # The built products from Tests/Fixtures/Sources/ go here. They'll be .gitignore’d
Why put these here rather than in the same build-directory that is specified by the build script (or the $(BUILT_PRODUCTS_DIR) in the Xcode case?)
lit.cfg # The lit config for these tests. Adds xctest-checker to the Python path
xctest-checker/ # A Python module for verifying the output of XCTest runs
Would this be used outside the functional tests, do you think? If not, perhaps putting it in the Functional directory would scope it better.
build_script.py # Updated with a --test parameter, to build and run the tests on Linux
Adding a test will involve two steps:

Add a directory with the name of your test to the Tests/Functional/Sources/ directory. For example, if your test is named "TwoFailingTestCases", make a directory named Tests/Functional/Sources/TwoFailingTestCases.
Add a main.swift file to the directory you added above. The file should contain comments, beginning with //. The test runner will verify that the output of main.swift matches your comments exactly, using the xctest-checker tool.
Does all that sound reasonable?

This definitely makes it easier to add tests and be sure that they’ll run on both platforms. It also removes the FileCheck dependency as well, which I think is a good thing.

—
Reply to this email directly or view it on GitHub #14 (comment).

modocache · 2015-12-16T18:08:02Z

Why put these here rather than in the same build-directory that is specified by the build script (or the $(BUILT_PRODUCTS_DIR) in the Xcode case?)

Good point, will do! 👍

Would this be used outside the functional tests, do you think? If not, perhaps putting it in the Functional directory would scope it better.

Another great point!

Based on the above, here's the structure I'm envisioning:

swift-corelibs-xctest/
    XCTest.xcodeproj/ # Updated with a FunctionalTests target, which builds the Functional/Sources/ and runs lit
    XCTest/
    Tests/            # Tests directory for this project.
        Functional/   # Functional tests that use lit. They're in their own sub-directory to differentiate from the unit tests we'll be adding in the future
            xctest-checker/   # A Python module for verifying the output of XCTest runs
            lit.cfg   # The lit config for these tests. Adds xctest-checker to the Python path
    build_script.py   # Updated with a --test parameter, to build and run the tests on Linux

When adding a test named "TwoFailingTestCases", developers will add a file named Tests/Functional/TwoFailingTestCases/main.swift to the project.

modocache · 2015-12-21T04:20:30Z

Closing in favor of #20. Thanks for your feedback, everyone!! 😃

modocache added 3 commits December 11, 2015 02:03

[build_script] Add --test option to test on Linux

2ab7b8d

By passing `--test` to the build script, the lit regression tests are run. This allows the test suite to be run on Linux. Require the path to `FileCheck` be passed along as well.

[README] Add instructions on testing

b974f32

modocache force-pushed the add-lit-tests branch from 27e2bfb to b974f32 Compare December 11, 2015 07:06

modocache mentioned this pull request Dec 21, 2015

[Tests] Add functional tests #20

Merged

modocache closed this Dec 21, 2015

briancroom mentioned this pull request Apr 9, 2016

[xctest_checker] Inline expectations #94

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add lit tests #14

Add lit tests #14

Uh oh!

modocache commented Dec 11, 2015

Uh oh!

modocache commented Dec 11, 2015

Uh oh!

mike-ferris commented Dec 11, 2015

Uh oh!

modocache commented Dec 11, 2015

Uh oh!

mike-ferris commented Dec 11, 2015

Uh oh!

ddunbar commented Dec 11, 2015

Uh oh!

modocache commented Dec 15, 2015

Uh oh!

mike-ferris commented Dec 16, 2015

Uh oh!

modocache commented Dec 16, 2015

Uh oh!

modocache commented Dec 21, 2015

Uh oh!

Uh oh!

Add lit tests #14

Add lit tests #14

Uh oh!

Conversation

modocache commented Dec 11, 2015

Uh oh!

modocache commented Dec 11, 2015

Uh oh!

mike-ferris commented Dec 11, 2015

Uh oh!

modocache commented Dec 11, 2015

Uh oh!

mike-ferris commented Dec 11, 2015

Uh oh!

ddunbar commented Dec 11, 2015

Uh oh!

modocache commented Dec 15, 2015

Uh oh!

mike-ferris commented Dec 16, 2015

Uh oh!

modocache commented Dec 16, 2015

Uh oh!

modocache commented Dec 21, 2015

Uh oh!

Uh oh!