[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)
}

[modocache]

Yes, very good points about the API. I'm fairly certain Apple's Objective-C implementation of XCTest involves using exceptions, and while methods that throw an NSException do not need to be annotated in any way, I guess Swift requires syntax like try.

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

I'm not sure I follow you here. I think maybe what I had said in a previous comment was confusing. I had said "XCTFail() throws an InterruptMe exception", but the actual mechanisms Apple's Objective-C implementation of XCTest uses are a bit more convoluted. Like swift-corelibs-xctest, all of Apple's implementations of the XCTAssert() family of macros message -[XCTestCase recordFailureWithDescription:inFile:atLine:expected:]. Within this method, if continueAfterFailure is false, an InterruptMe exception is raised. Apple's Objective-C XCTest already runs each test method within a @try/@catch, in order to report uncaught exceptions as failures. Except when this outer @try/@catch catches an InterruptMe exception, it doesn't fail the test with an uncaught InterruptMe exception, it just ends it as it would otherwise normally do. So the XCTFail() would have failed the test, and then the exception would have prevented the rest of the test from executing.

I think maybe there was some confusion because what I originally wrote made it seem like XCTFail() would always throw an exception, but that's not the case. So the following example would print "1", but not "2":

func testFoo() {
    self.continueAfterFailure = true
    XCTFail()
    print("1")
    self.continueAfterFailure = false
    XCTFail()
    print("2")
}

I guess the question is: can we use Swift errors without forcing users to call try XCTFail()? If not, is there some other way to implement continueAfterFailure? If not, should we assert if a user attempts to set it to false?

[swift-ci]

Comment by Tiago Martinho (JIRA)

That was my first idea: prevent the rest of the test from executing in recordFailure, only if continueAfterFailure is set to false. Since all XCTAssert methods use _XCTEvaluateAssertion, which in case of a test failure calls the method recordFailure.

The only way to stop the normal flow of execution in the current test in Swift would be to throw an Error, correct?
If we throw an error in recordFailure it propagates to _XCTEvaluateAssertion which in turn propagates to the XCTAssert methods and so the API of XCTest would have to change.

"can we use Swift errors without forcing users to call try XCTFail()?"

I don't see a solution for this, can someone help me?

"If not, is there some other way to implement continueAfterFailure?"

Maybe we could stop the assertions and measurements? But let the remaining test code run?

"If not, should we assert if a user attempts to set it to false?"

Yes, I would somehow alert the user of the framework that this property doesn't work as defaults to true.

"but the actual mechanisms Apple's Objective-C implementation of XCTest uses are a bit more convoluted"

It's possible to see the current implementation of Apple's Objective-C XCTest?

[modocache]

Yeah, first of all let me apologize for labeling this as a "StarterBug". It's definitely tricky. A perfect solution might not even exist.

Maybe we could stop the assertions and measurements? But let the remaining test code run?

I'm against this idea, personally. When I think of continueAfterFailure = false, I imagine several use cases:

1. The tests take a long time to run. If they're going to fail, users want them to fail and be done quickly. So continuing to run the tests would probably be the opposite of what they want.

2. The tests do something sensitive, and users use continueAfterFailure = false like they would use precondition() – that is, as a way to prevent their tests from getting into a bad state and doing something catastrophic. Here again, continuing to run would be the opposite of the desired behavior.

This is why, I think, fatalError() is currently used.

It's possible to see the current implementation of Apple's Objective-C XCTest?

This would be great, but I don't think so. Apple typically has not open-sourced their SDK frameworks, like UIKit and XCTest, in the past. So I doubt they will in the future. However, you can attach a debugger like lldb to an XCTest bundle as it is being run to learn a lot about it. Some developers also use disassembler programs in order to learn more about how these frameworks work.

Anyway, maybe there's no good solution here. It'd be nice to have a test for the current behavior, though. Right now we don't have any tests for what happens when continueAfterFailure = false.

[swift-ci]

Comment by Tiago Martinho (JIRA)

Yeah, first of all let me apologize for labeling this as a "StarterBug". It's definitely tricky. A perfect solution might not even exist.

No problem! I learned a lot 🙂

For me one solution is changing the API so the assertions are marked with try and the test annotated with throws. I made a simple PoC in the following repository.

I'm also against the idea to continue the test after a failure if continueAfterFailure is set to false.

Anyway, maybe there's no good solution here. It'd be nice to have a test for the current behavior, though. Right now we don't have any tests for what happens when continueAfterFailure = false.

I can then add the test I wrote for the continueAfterFailure and we can revise that?

[swift-ci]

Comment by Basem Emara (JIRA)

Is this still being worked on? I'm forced to use nested `guards` in my tests instead of just using `continueAfterFailure`. Although I realize there are complexities involved, I really wish the default could be the opposite.. when an assertion fails, it seems intuitive that I wouldn't want to continue on anyway just to see how far the broken car will travel.

Btw not sure if this is known but `continueAfterFailure` only works when running the single test. However, when running unit tests in batches, `continueAfterFailure` is no longer respected.