[SR-906] Allow XCTestCase.continueAfterFailure to be set

[modocache]

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

Additional Detail from JIRA

Votes	2
Component/s	XCTest
Labels	Improvement
Assignee	None
Priority	Medium

md5: 8a68ba9a31eeee0ca919922b7bddbeee

Issue Description:

XCTestCase.continueAfterFailure currently always returns true, no matter what: https://github.com/apple/swift-corelibs-xctest/blob/d28cbd679e407bda760ce7a862c062de4b6912c8/Sources/XCTest/XCTestCase.swift#L65-L72

However, as of #40 there is no reason for this attribute to always return true--if a user sets it to false, it should currently work (grep the codebase for "continueAfterFailure" to see for yourself).

1. Remove the custom setter/getter from continueAfterFailure in order to allow it to be set to true/false by the user.
2. Add functional tests that confirm the test suite stops when continueAfterFailure is false. This might be tricky--a false value for continueAfterFailure causes the test to throw a fatalError, and none of the functional tests currently test this sort of behavior. Still, I don't think this will require any massive changes to the testing architecture in the repo. Start by adding a simple test case that uses continueAfterFailure and check the output. It should be similar to other functional test output.

[swift-ci]

Comment by Veronica Ray (JIRA)

Hey Brian - I'd like to take on this task. Currently following your tips on SR-907 and the README to get my environment set up.

I'm having issues building the project.
When I run utils/build-script -r -t from the swift folder I get a lot of errors and at the end:
fatal error: too many errors emitted, stopping now [-ferror-limit=]
1 warning and 20 errors generated.
ninja: build stopped: subcommand failed.
utils/build-script: command terminated with a non-zero exit status 1, aborting

The errors are all in llvm project.

An example of the kinds of errors I’m getting:
/Users/vray/Documents/ios/llvm/include/llvm/ADT/DenseMap.h:295:22: error: no member named 'isEqual' in 'llvm::DenseMapInfo<std::__1::basic_string<char> >'
if (!KeyInfoT::isEqual(B->getFirst(), EmptyKey) &&

I followed the directions in the README to get the Swift source via HTTPS. I downloaded cmake and ninja via Homebew.
I downloaded the the toolchain you built and put it in my user’s Library folder. In the latest xcode beta I switched the toolchain to be that one. I did not roll back the foundation repo to the last commit before 3/16 like Kyle did.

[modocache]

Hey mathonsunday (JIRA User)! It sounds like you're having trouble building the Swift project itself, specifically because Swift source code is attempting to use interfaces in LLVM that don't exist. Your version of LLVM must be out of sync with your version of Swift. To keep these in sync, there's a script called utils/update-checkout from within the Swift directory. Running that should solve your problems. Specifically, the script goes into each of the following directories to run git pull --rebase origin master:

your-directory-where-you-keep-these-projects/
    swift/
    swift-corelibs-xctest/
    swift-corelibs-foundation/
    swift-corelibs-libdispatch/
    swiftpm/
    llbuild/

The script also goes into the following directories to run git pull --rebase origin stable:

your-directory-where-you-keep-these-projects/
    llvm/
    clang/

The idea is that the tip of all of these branches should always work together. In general, if you ever get build errors when attempting to build Swift from source, try pulling down the latest changes from llvm, clang, and swift.

[swift-ci]

Comment by Veronica Ray (JIRA)

It worked! Thank you.

[swift-ci]

Comment by Veronica Ray (JIRA)

I'm running into an issue that seems like it should be simple to solve. I removed the custom setter/getter from continueAfterFailure. Since extensions may not contain stored properties I moved it out of the extension.
Now the code looks like this:

    public var name: String
    public var continueAfterFailure: Bool

    public required init() {
        name = "\(self.dynamicType).<unknown>"
        continueAfterFailure = true
    }

I decided that the FailingTestSuite functional test would be the easiest place to try this out. But this code:

class PassingTestCase: XCTestCase {
    static var allTests: [(String, PassingTestCase -> () throws -> Void)] {
        return [
            ("test_passes", test_passes),
        ]
    }

    continueAfterFailure = false

is giving me an "Expected declaration" error.

[modocache]

Awesome! Hmm, expected declaration... I think the problem is that, within your test code, you're attempting to override an instance variable from within a class scope. The following should work:

class PassingTestCase: XCTestCase {
    static var allTests: [(String, PassingTestCase -> () throws -> Void)] {
        return [
            ("test_passes", test_passes),
        ]
    }

    override func setUp() {
        super.setUp()
        continueAfterFailure = false
    }

[modocache]

By the way, #86 was merged a few days ago, and changed a bunch of stuff in corelibs-xctest. Based on your code snippets, it looks like you're using an older revision without those changes. You should pull down the latest version and take a look.

#86 was focused on providing an API identical to Apple XCTest. As such, it already declares continueAfterFailure as a mutable instance variable. It still needs tests, though (step 2 from this task's original description)!

[swift-ci]

Comment by Veronica Ray (JIRA)

Thanks for the heads up! However, I don't see in the PR how continueAfterFailure is declared as a mutable instance variable. In the latest code we still have this:

extension XCTestCase {
    
    public var continueAfterFailure: Bool {
        get {
            return true
        }
        set {
            // TODO: When using the Objective-C runtime, XCTest is able to throw an exception from an assert and then catch it at the frame above the test method.
            //      This enables the framework to effectively stop all execution in the current test.
            //      There is no such facility in Swift. Until we figure out how to get a compatible behavior,
            //      we have decided to hard-code the value of 'true' for continue after failure.
        }
    }

[modocache]

Whoops! You're absolutely right. My bad, I must've been mixed up.

[swift-ci]

Comment by Chris Williams (JIRA)

I was taking a look at this earlier. Is there any way to make this functionality work in Swift without having all asserts throw?

[modocache]

I think the asserts throw due to the proposal in https://lists.swift.org/pipermail/swift-evolution/Week-of-Mon-20160104/006091.html, not necessarily in order to implement continueAfterFailure. The commit that modified the asserts to throw is here: e5cf2f4

[swift-ci]

Comment by Veronica Ray (JIRA)

I haven't been able to work on this due to an upcoming product launch. Anyone else who wants to take this on is welcome to.

[swift-ci]

Comment by Tiago Martinho (JIRA)

Hello @modocache!

I would like to tackle this issue if you believe it's still relevant.

If I understood correctly it would involve:

1. define 'continueAfterFailure' as read/write property

2. write a regression test that verifies this behavior

However the following things are not clear to me.

The following comment in the code says:

"Apple XCTest does not throw a fatal error and crash the test process, it merely prevents the remainder of a testClosure from expecting after it's been determined that it has already failed. The following behavior is incorrect."

It's not a main objective of swift-corelibs-xctest to match the behavior of XCTest? Shouldn't the remaining tests be executed and the test suite results be collected after a test fails?

Example: I used mainly this property in UI tests where if a test fails I still want the remaining test in the suite to run and gather all the results (this is the current behavior in Xcode).

[modocache]

Thanks, t_martinho (JIRA User)! Yes, it's still relevant. You're right that when continueAfterFailure = true, then even after a test fails, the test suite should continue to execute. For example, the following test will print "Hello" when using Apple XCTest:

func testFoo() {
  self.continueAfterFailure = true
  XCTFail()
  print("Hello")
}

By default XCTest runs with continueAfterFailure = true, and so the following also prints "Hello":

func testFoo() {
  XCTFail()
  print("Hello")
}

The primary objective this task is tracking is the continueAfterFailure = false case. That is, in Apple XCTest, the following test does not print "Hello":

func testFoo() {
  self.continueAfterFailure = false
  XCTFail()
  print("Hello")
}

How can swift-corelibs-xctest accomplish this? The FIXME you pointed out was meant to explain that (a) swift-corelibs-xctest current does not implement this feature. swift-corelibs-xctest does declare a XCTestCase.continueAfterFailure property, but it doesn't yet work for continueAfterFailure = false.

continueAfterFailure = true is easy to implement: swift-corelibs-xctest doesn't have to do anything special, it just reports the failure and doesn't crash the program, so print("Hello") is eventually executed and "Hello" is printed.

But what about the continueAfterFailure = false case? In that case, we'd need to prevent print("Hello") from ever being executed. I thought of one way to do that: crash the program! If the program crashes, it certainly won't execute print("Hello"). That's the explanation for the FIXME you read.

But this, of course, doesn't meet our needs. That's because, in Apple XCTest, the following set of tests prints "Hola", but not "Hello":

func test_00_foo() {
  self.continueAfterFailure = false
  XCTFail()
  print("Hello")
}

func test_01_bar() {
  print("Hola")
}

In other words, Apple XCTest prevents the remainder of test_00_foo() from being executed once that test fails, but it doesn't crash the test suite program, and so test_01_bar has a chance to execute, and so "Hola" is printed (but not "Hello").

So, how to implement this? Apple XCTest is implemented in Objective-C, and accomplishes this using NSException. When a test failure occurs and continueAfterFailure = false, then XCTFail() throws an InterruptMe exception. The internal code of XCTestCase uses a @try/@catch to catch that exception and handle it without the program crashing. And exceptions are sort of like goto statements – they circumvent the normal execution of instructions. So by throwing an exception, Apple's Objective-C XCTest is able to implement continueAfterFailure = false and prevent "Hello" from being printed in the example above.

How can we accomplish this using just Swift, in swift-corelibs-xctest? I'm not sure! I think Swift errors are promising. Their control flow mechanics are similar to Objective-C exceptions. Maybe we can throw an error, and have the XCTestCase catch it? I think this could work, but I'm not certain.

[swift-ci]

Comment by Tiago Martinho (JIRA)

Understood!

I was able to write a test that verifies that behavior.

I will try to follow your suggestion and throw an error when a test fails and stop the execution of that single test, but still execute the remaining tests and gather the test suite results.

Thanks for the clarification!

[swift-ci]

Comment by Tiago Martinho (JIRA)

Hello @modocache!

I'm not sure if throwing an exception in XCTFail can be a solution, please correct me if I'm wrong.

For what I understand the behavior should be more generic, in XCTest the continueAfterFailure property applies to all kind of failures in tests.

Example (this is the functional test I imagined):

func testDoesNotContinueAfterFailure() {
    continueAfterFailure = false
    print("First Log in DoesNotContinueAfterFailure test")
    XCTAssert(false)
    print("Second Log in DoesNotContinueAfterFailure test")
}

func testContinueAfterFailure() {
    continueAfterFailure = true
    print("First Log in ContinueAfterFailure test")
    XCTAssert(false)
    print("Second Log in ContinueAfterFailure test")
}

Prints in XCTest and should print in SwiftXCTest:

First Log in DoesNotContinueAfterFailure test
First Log in ContinueAfterFailure test
Second Log in ContinueAfterFailure test

Wouldn't throwing an error change significantly the API? And force users to annotate the testing methods with throw and the assertions with try?

Example:

func testExample() throws {
    try XCTAssert(false)
}