Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix assert<T>(T?, T -> Bool) #3

Merged
merged 1 commit into from
Feb 27, 2015
Merged

Conversation

ishkawa
Copy link
Member

@ishkawa ishkawa commented Feb 26, 2015

Fix assert("foo", { $0.isEmpty }) passes tests.

@robrix
Copy link
Contributor

robrix commented Feb 27, 2015

Why is this change necessary? The tests on master pass as-is.

@robrix robrix self-assigned this Feb 27, 2015
@ishkawa
Copy link
Member Author

ishkawa commented Feb 27, 2015

Because assert<T>(T?, T -> Bool) does not work as expected. It does not fail even if the closure returned false. For example, assert("foo", { $0.isEmpty }) (string is not empty) and assert(string, { s in false }).

I tried to add test that expresses assert<T>(T?, T -> Bool) fails as expected, but I could not write test that passes if XCTFail is called as expected.

@robrix
Copy link
Contributor

robrix commented Feb 27, 2015

Ah, good catch—thank you! 🙇

robrix added a commit that referenced this pull request Feb 27, 2015
fix assert<T>(T?, T -> Bool)
@robrix robrix merged commit e973aae into antitypical:master Feb 27, 2015
@ishkawa
Copy link
Member Author

ishkawa commented Feb 27, 2015

👌

@robrix
Copy link
Contributor

robrix commented Feb 27, 2015

I think the only way we could reasonably test assertion failure would be to fork off xcodebuild runs. That seems a bit heavyweight for now, but it would be nice to have the assertions verified automatically.

@robrix
Copy link
Contributor

robrix commented Feb 27, 2015

(Alternatively, we might be able to construct a custom XCTest at runtime and test against its values. But I’m unsure whether that would actually work; it would depend on how XCTFail bubbles its failure state upwards.)

@ishkawa
Copy link
Member Author

ishkawa commented Feb 27, 2015

I found overriding XCTestCase.recordFailureWithDescription() gives us chance to determine if a failure is expected or not. As a result, following test code finishes with success.

final class AssertionsTests: XCTestCase {
    var expectingFailure = false
    var expectedFailureOccurred = false

    override func recordFailureWithDescription(description: String!, inFile filePath: String!, atLine lineNumber: UInt, expected: Bool) {
        if expectingFailure {
            expectedFailureOccurred = true
        } else {
            super.recordFailureWithDescription(description, inFile: filePath, atLine: lineNumber, expected: false)
        }
    }

    func expectFailure(test: Void -> Void) {
        expectingFailure = true
        expectedFailureOccurred = false
        test()
        expectingFailure = false
        XCTAssert(expectedFailureOccurred)
    }

    func testAssertFailure() {
        expectFailure {
            assert(" ", { $0.isEmpty }) // calls XCTFail but is not collected as a failure
        }
    }

    func testAssertSuccess() {
        assert("", { $0.isEmpty })
    }
}

I'm not sure that this kind of code should be used in tests of this library.

@robrix
Copy link
Contributor

robrix commented Feb 27, 2015

Cool, thank you for sharing that! That’s much simpler than I expected we could manage, and I think it’d be a great addition to the tests 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants