[Tests] Add functional tests

[modocache]

What's in this pull request?

Yet another attempt at #10, this pull request adds functional tests that verify the output of programs that use XCTest. This pull request is based off of the discussion on #14, and uses lit as a test runner.

A developer can kick off the tests by either running build_script.py --test or by building and running the SwiftXCTestFunctionalTests Xcode target.

Here's a breakdown of what happens when a user kicks off the tests:

1. ../llvm/utils/lit/lit.py Tests/Functional is run, which kicks off the configuration in Tests/Functional/lit.cfg.
2. lit.cfg uses environment variables to provide substitutions, such as %swiftc, to the tests. On Linux, these environment variables are set as part of build_script.py. On OS X, these environment variables are provided by xcodebuild.
3. lit finds every Swift source file that contains a RUN: command and executes those commands.
4. The RUN: commands in SingleFailingTestCase/main.swift compiles the source into an executable in a temporary directory, runs that executable, and checks the output. Output verification is done via xctest_checker, a Python script.
5. The checker raises an exception and produces a failing exit code for the test runner if any tests fail.

Why merge this pull request?

• It adds a functional test suite to this project. These tests will allow us to prevent regressions and verify the behavior of XCTest. For example, I noticed the typo in [XCTestCase] Fix missing open paren for unexpected #18 as a result of functional testing.
• Adding tests is as easy as adding a single file: Tests/Functional/MyNewTest/main.swift. Unlike [Tests] Add regression test suite for OS X and Linux #10 and Add lit tests #14, new tests don't need to be configured in the Xcode project to work.

What downsides are there to merging this pull request?

• The test runner, because it uses lit, is dependent upon the llvm codebase being installed in ../llvm. Alternatively, it could check for pip or easy_install being installed on the host system, and if available use them to install https://pypi.python.org/pypi/lit. Either way, the test suite is not completely independent: it relies on either llvm being cloned ahead of time, or on an Internet connection to install lit from the Python Package Index.
• The test runner in [Tests] Add regression test suite for OS X and Linux #10 could verify the exit code of the program. lit is unable to do so. The tests verify the output, but not whether the exit code emitted by XCTest indicated a failure. @ddunbar, is it possible to test this using lit?
• Functional tests only test the system as a whole. Ideally, these would be complemented with a unit test suite that could verify the behavior of smaller components in XCTest. I'd certainly like to pursue this idea in the future, but I think these tests are beneficial in their own way. They're not meant to be the only tests for this project.

[modocache]

FileCheck uses a special syntax for regular expressions. xctest_checker parses everything as a regular expression. As a result, developers need to take extra care to escape certain characters when writing their checks--for example, 1 failure \(0 unexpected\).

This is suboptimal, and could be improved in the future, but xctest_checker remains a simple program as a result of this decision.

[mike-ferris]

Thanks, Brian!

As I mentioned last week, I am out of the office traveling until after New Years but I will take a closer look as soon as I get back.

Mike

On Dec 20, 2015, at 11:20 PM, Brian Gesiak notifications@github.com wrote:

What's in this pull request?

Yet another attempt at #10, this pull request adds functional tests that verify the output of programs that use XCTest. This pull request is based off of the discussion on #14, and uses lit as a test runner.

A developer can kick off the tests by either running build_script.py --test or by building and running the SwiftXCTestFunctionalTests Xcode target.

Here's a breakdown of what happens when a user kicks off the tests:

../llvm/utils/lit/lit.py Tests/Functional is run, which kicks off the configuration in Tests/Functional/lit.cfg.
lit.cfg uses environment variables to provide substitutions, such as %swiftc, to the tests. On Linux, these environment variables are set as part of build_script.py. On OS X, these environment variables are provided by xcodebuild.
lit finds every Swift source file that contains a RUN: command and executes those commands.
The RUN: commands in SingleFailingTestCase/main.swift compiles the source into an executable in a temporary directory, runs that executable, and checks the output. Output verification is done via xctest_checker, a Python script.
The checker raises an exception and produces a failing exit code for the test runner if any tests fail.
Why merge this pull request?

It adds a functional test suite to this project. These tests will allow us to prevent regressions and verify the behavior of XCTest. For example, I noticed the typo in #18 as a result of functional testing.
Adding tests is as easy as adding a single file: Tests/Functional/MyNewTest/main.swift. Unlike #10 and #14, new tests don't need to be configured in the Xcode project to work.
What downsides are there to merging this pull request?

The test runner, because it uses lit, is dependent upon the llvm codebase being installed in ../llvm. Alternatively, it could check for pip or easy_install being installed on the host system, and if available use them to install https://pypi.python.org/pypi/lit. Either way, the test suite is not completely independent: it relies on either llvm being cloned ahead of time, or on an Internet connection to install lit from the Python Package Index.
The test runner in #10 could verify the exit code of the program. lit is unable to do so. The tests verify the output, but not whether the exit code emitted by XCTest indicated a failure. @ddunbar, is it possible to test this using lit?
Functional tests only test the system as a whole. Ideally, these would be complemented with a unit test suite that could verify the behavior of smaller components in XCTest. I'd certainly like to pursue this idea in the future, but I think these tests are beneficial in their own way. They're not meant to be the only tests for this project.
You can view, comment on, or merge this pull request online at:

#20

Commit Summary

[Tests] Add functional tests
[README] Explain how to run functional tests
File Changes

M README.md (17)
A Tests/Functional/SingleFailingTestCase/main.swift (28)
A Tests/Functional/lit.cfg (73)
A Tests/Functional/xctest_checker/.gitignore (26)
A Tests/Functional/xctest_checker/setup.py (43)
A Tests/Functional/xctest_checker/tests/init.py (0)
A Tests/Functional/xctest_checker/tests/test_compare.py (29)
A Tests/Functional/xctest_checker/xctest_checker.py (6)
A Tests/Functional/xctest_checker/xctest_checker/init.py (9)
A Tests/Functional/xctest_checker/xctest_checker/compare.py (38)
A Tests/Functional/xctest_checker/xctest_checker/main.py (28)
M XCTest.xcodeproj/project.pbxproj (72)
M build_script.py (22)
Patch Links:

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

[modocache]

Yeah, thanks for the heads-up on the mailing list. Enjoy the holidays! 🎄

[modocache]

I rebased on top of #23. The tests still pass! I also added a blurb to the README: in order for the tests to run, you must launch Xcode using the open-source Swift toolchain:

$ xcrun launch-with-toolchain /Library/Developer/Toolchains/swift-latest.xctoolchain

This is because the tests require the SWIFT_EXEC environment variable to be set, but this is not set when running Xcode with its default toolchain.

[modocache]

I rebased on top of #22; the tests verify the new output from @briancroom's updates.

[mike-ferris]

I asked a couple questions in the diff about the workings of xctest_checker. I also noticed the following.

• The SwiftXCTestFunctionalTests target should have a build setting like the following in order to allow it to find swiftc in the active toolchain.
  • SWIFT_EXEC = $(TOOLCHAIN_DIR)/usr/bin/swiftc
• With the merging of Print additional details when emitting assertion failure messages #22 another tweak is needed to the CHECK stuff in the test case. I added " XCTAssertTrue failed - $" to the end of the error line (the $ to avoid overzealous editor trimming of trailing whitespace...)

[mike-ferris]

I would also suggest adding the Tests folder to the Xcode project (as groups, with no target membership) just to make it easier to work on the tests for us OS X folks.

[ddunbar]

Overall, this looks like a good lit integration for XCTest. I added a couple of specific comments on the lit.cfg to the initial commit.

There a couple of follow ons we could do to make things even more convenient:

1. We can make the Xcode project automatically install lit (in the user-local installation area), if necessary, while using lit from the LLVM source tree available. This makes things "just work" more often. llbuild does this, see for example:
   https://github.com/apple/swift-llbuild/blob/master/utils/Xcode/install-user-lit.sh
   https://github.com/apple/swift-llbuild/blob/master/utils/Xcode/create-lit-site-cfg.sh
2. We can provide an adaptor to adapt the Lit tests so they can be run as XCTests. This allows Cmd-U in Xcode to run all of the lit tests (and then you can rerun individual ones). llbuild does this too, see:
   https://github.com/apple/swift-llbuild/blob/master/utils/Xcode/LitXCTestAdaptor/LitTests.m

[ddunbar]

One more thing to mention is that I also have a "reverse-adaptor" which adapts XCTest to lit. Since lit has the ability to run multiple test suites as part of a single invocation, then if we get to the point where the XCTest project has a lot of XCTest based tests in it, we can also provide an integration so that the lit-based invocation will run all of the tests (both the lit and the XCTest ones).

[modocache]

Oomph, turns out that when you comment on an individual commit, and that commit is amended (such that its hash changes), the comments no longer appear alongside the pull request.

Here's the commit that includes review comments: modocache@eed9cdb

If you're using the GitHub web interface, try commenting on the "Files changed" tab instead:

[modocache]

@mike-ferris-apple I've addressed all of your comments--thanks!

What happens if the number of lines in the actual vs. expected differs?

Thanks for pointing this out. I've amended the XCTest checker such that the number of actual and expected lines must match. I've also added Python tests for this behavior.

The SwiftXCTestFunctionalTests target should have a build setting like the following in order to allow it to find swiftc in the active toolchain.

Awesome, thanks to this suggestion developers no longer need to use xcrun launch-with-toolchain /Library/Developer/Toolchains/swift-latest.xctoolchain when launching Xcode--the tests can be run using stock Xcode.

---

I'm still working on @ddunbar's comments.

[mike-ferris]

On Jan 8, 2016, at 11:17 AM, Brian Gesiak notifications@github.com wrote:

@mike-ferris-apple https://github.com/mike-ferris-apple I've addressed all of your comments--thanks!

What happens if the number of lines in the actual vs. expected differs?

Thanks for pointing this out. I've amended the XCTest checker such that the number of actual and expected lines must match. I've also added Python tests for this behavior.

The SwiftXCTestFunctionalTests target should have a build setting like the following in order to allow it to find swiftc in the active toolchain.

Awesome, thanks to this suggestion developers no longer need to use xcrun launch-with-toolchain /Library/Developer/Toolchains/swift-latest.xctoolchain when launching Xcode--the tests can be run using stock Xcode.

Cool. And presumably if they do have a custom toolchain active, that should work as well with this build setting, I think.

Mike

I'm still working on @ddunbar https://github.com/ddunbar's comments.

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

[modocache]

@ddunbar I've addressed all of your comments, except for one:

lit has a builtin substitution for the output directory (%t), why not use that?
I should add, if the goal here is to ensure things are kept outside the source tree, then what you can do is set "config.test_exec_root" to a temporary directory. That will cause lit to produce a shadow tree mirroring the tree in the source root and use that directory for test outputs.

That was indeed my intention! As per your suggestions I tried the following:

diff --git a/Tests/Functional/SingleFailingTestCase/main.swift b/Tests/Functional/SingleFailingTestCase/main.swift
index 2cffe97..a4f4802 100644
--- a/Tests/Functional/SingleFailingTestCase/main.swift
+++ b/Tests/Functional/SingleFailingTestCase/main.swift
@@ -1,6 +1,6 @@
 // RUN: %{swiftc} %s -o %{built_tests_dir}/SingleFailingTestCase
-// RUN: %{built_tests_dir}/SingleFailingTestCase > %{test_output} || true
-// RUN: %{xctest_checker} %{test_output} %s
+// RUN: %{built_tests_dir}/SingleFailingTestCase > %t || true
+// RUN: %{xctest_checker} %t %s
 // CHECK: Test Case 'SingleFailingTestCase.test_fails' started.
 // CHECK: .*/Tests/Functional/SingleFailingTestCase/main.swift:24: error: SingleFailingTestCase.test_fails : XCTAssertTrue failed -
 // CHECK: Test Case 'SingleFailingTestCase.test_fails' failed \(\d+\.\d+ seconds\).
diff --git a/Tests/Functional/lit.cfg b/Tests/Functional/lit.cfg
index 6f9aafc..71d22cd 100644
--- a/Tests/Functional/lit.cfg
+++ b/Tests/Functional/lit.cfg
@@ -8,6 +8,7 @@ import lit
 config.name = 'SwiftXCTestFunctionalTests'
 config.test_format = lit.formats.ShTest(execute_external=False)
 config.suffixes = ['.swift']
+config.text_exec_root = tempfile.mkdtemp()

 # Set up the substitutions used by the functional test suite.

But running the tests still results in Tests/Functional/SingleFailingTestCase/Output/main.swift.tmp being created in the source tree. I'm currently checking lit for perhaps some other setting I could use, but any tips would be appreciated! 🙇

[mike-ferris]

This looks good to me. We can iterate with another pull request if the %t thing @ddunbar mentions gets resolved.

[modocache]

Awesome, thanks @mike-ferris-apple! Very happy to see this in. 👍

[briancroom]

Excellent! This will greatly assist in further work on this project.