This is an extremely overcomplicated solution to a relatively straightforward problem.
The waitForExpectationsWithTimeout method should effectively only be used assure that a particular asynchronous expectation was fulfilled.
So, in the case where we are testing whether or not the user was successfully logged in, it'd be tempting to write something along the lines of:
func testLogin() {
let loginExpectation = expectationWithDescription("User successfully logged in")
LoginManager.doLogin { success in
if success {
loginExpectation.fulfill()
}
}
waitForExpectationsWithTimeout(10, handler: nil)
}
But problematically, this test can fail for two reasons:
- The user wasn't successfully logged in.
- The expectation wasn't fulfilled in the time limit.
If the test fails, we can't be sure whether it would have passed given a longer time limit or if it returned very quickly but wasn't successful.
So, we need to modify our test to look more like this:
func testLogin() {
let networkExpectation = expectationWithDescription("Network request completed")
LoginManager.doLogin { success in
XCTAssertTrue(success)
networkExpectation.fulfill()
}
waitForExpectationsWithTimeout(10, handler: nil)
}
Now, the test determines whether or not the asynchronous code executed, and if so, we verified whether or not the login was a success (or whatever the success variable represents here).
Now, you're looking for the opposite case. You want to know not whether or not the user logged in, but whether or not the user was prevented from logging in. And that's as simple as inverting an XCTAssertTrue to an XCTAssertFalse.
func testLogin() {
let networkExpectation = expectationWithDescription("Network request completed")
LoginManager.doLogin { success in
XCTAssertFalse(success)
networkExpectation.fulfill()
}
waitForExpectationsWithTimeout(10, handler: nil)
}
Importantly, this pattern doesn't require any complicated code (that we would have to of course write tests around ourselves). Our tests are using stuff purely out of the Xcode test suite box--exactly what any one looking at our code base would expect to see. It's very straight forward what is going on here.
And we are able to distinguish between whether the asynchronous call back ever happened or if it did happen but we got a bad result. There's a huge difference here between a test failing because the asynchronous code didn't run before our time out and it did run but it gave an incorrect result. We need to clearly distinguish between these results, and doing so doesn't require writing any complicated code whatsoever.
An important note here, make sure that fulfilling the expectation is the very last thing you do in the asynchronous callbacks, or you may set up a race condition.
And finally, your tests should certainly be far more robust than this. Ultimately, we should be testing with mocked network responses and we should know, for each test, exactly what network response we will get. We should test that whatever code is doing the login returns back the correct information to us. There's a difference between the server allowing the user to log in successfully and denying the user with invalid credentials. There's also a difference between invalid credentials and getting any number of various server errors (most commonly, errors in the 5xx range). Getting a 503 and getting an invalid credential response are far from the same thing, and your tests should validate you're getting what you expect to get.
So, I've basically just told you to completely scrap all the code you've written. And I'll stand by that. There's no reason for the code you've written to exist. But, we can still talk about some of the issues your existing code does have.
This function name has a weird name: waitFalseExpectationsUntilTimeout. It's hard for me to tell by name alone what to expect this method to do. It looks similar to the existing waitForExpectationsWithTimeout(timeout: NSTimeInterval, handler: XCWaitCompletionHandler?) method, but it has this weird array of expectations I'm supposed to pass in. The name needs to be better. Something like assertUnfulfilledExpectationsWithTimeout.
Even with the better name, we're in extraordinary trouble here. As far as I can see, there's no public means of checking whether or not an expectation has been previously fulfilled. Your code doesn't really do anything beyond fulfilling all of the expectations in the last moment before the test completes. It doesn't assure that they weren't fulfilled earlier.
let smaller = timeout < 0 ? 0 : timeout.advancedBy(-0.5)
This variable deserves a better name, and this logic either deserves a comment or it should be eliminated altogether. I'd opt for eliminating it altogether and throwing an exception if the user has passed a value less than 0.5. And finally, this logic is ultimately problematic, because you can end up with negative time (if timeout is less than 0.5).
You should probably opt for some logic that looks more like this:
if timeout < 0.5 {
// fulfill expectation immediately
}
else {
// set up timer with timeout - 0.5 to fulfill expectations
}
Finally, there are several issues with the nested class:
class CompleteSelector {
@objc func fullFillExpectations(timer: NSTimer) {
let expectations = timer.userInfo as! [XCTestExpectation]
for expc in expectations {
expc.fulfill()
}
}
}
First, we don't need to mark this as an @objc function. We can instead mark it as dynamic.
Second, we're force-casting, but we're not Jedi. This would not pass my personal set of swiftlint rules where force-casting is considered an error and refuses to compile.
Third, expc is a meaningless variable name, and there's no real value in abbreviating here.
Fourth, you've misspelled fulfill even though it's spelled correctly several other times in this context by virtue of autocompleting method names.
Fifth, why don't we just add a method to the extension to handle this instead of over complicating with a nested class with a weird name?
enum ExpectationArgumentError: ErrorType {
case InvalidTimeInterval, NoExpectations
}
extension XCTestCase {
func assertUnfulfilledExpectations(timeout: NSTimeInterval, expectations: [XCTestExpectation], handler: XCWaitCompletionHandler?) throws {
if expectations.empty {
throw ExpectationArgumentError.NoExpectations
}
let fulfillTime = timeout - 0.5
if fulfillTime < 0 {
throw ExpectationArgumentError.InvalidTimeInterval
}
NSTimer.scheduledTimerWithTimeInterval(fulfillTime, target: self, selector: "fulfillExpectations:", userInfo: expectations, repeats: false)
waitForExpectationsWithTimeout(timeout, handler: handler)
}
private dynamic func fulfillExpectations(timer: NSTimer) {
if let expectations = timer.userInfo as? [XCTestExpectation] {
for expectation in expectations {
expectation.fulfill
}
}
}
}
So, that's a significantly cleaned up and safer version of your code... but again, it doesn't actually assert that these expectations were not fulfilled before you fulfilled them (as far as I know that's impossible). And ultimately, it's a quite a lot of code to implement, test, maintain, and verify that it works... and that is especially so when we compare that to the simply expectation+assertion combo I outlined at the top of this answer.
I wanted to add an addendum in case the issue was that there is a particular block of code that is only executed in the case of a successful login. If this is the scenario, there are only two possibilities.
The first is that there are separate success and failure blocks. Something along these lines:
LoginManager.doLogin("username", password: "password",
success: {
// login was successful
},
failure: {
// login failed
}
)
In this case, the gist is roughly the same as what I've outlined at the top. We do want to fulfill the expectation in both blocks, but in the success block, we'll just add an XCTFail("User was allowed to log in") so that we still wait around for this block (or the failure block) to execute, but if it takes this path, we fail for the specific reason that the wrong branch was entered (as opposed to simply taking the right path but not taking it quickly enough).
The other case is that there is only a callback for the success path:
LoginManager.doLogin {
// if logged in, this is called... otherwise, nothing
}
If THIS is the design, I don't have a clue how your app manages to give any feedback to the user at all that something went wrong during the login process... I know you can't modify the code, but you need to be telling the people who can that they need to fix this problem.
