Accidentally Synchronous Tests

Asynchronous code is hard and it’s very easy to make subtle mistakes. I look at a real life influenced example where we accidentally made our unit tests synchronous, therefore reducing the tests effectiveness at catching bugs.

Background

It’s worth repeating that asynchronous code is hard and it’s very easy to make subtle mistakes.

The real life issue manifested in considerably more interesting ways than what follows but the example here is just to highlight the root cause of the problem. One more disclaimer - the code that caused the issue is pretty poorly designed and is a hang over from an old Objective-C code base, which means the following code is in no way idiomatic and one could argue that more modern coding styles would never have allowed this bug.

Apologies out of the way…

We have two functions:

• populate(random:) - is a function that will take a Random (a reference type class) and destructively set its number value.
• process() - is a synchronous function that creates a Random, feeds it to populate(random:) and then returns the result.

(Yes I know this is a terrible example but please stick with me)

The current tests for this set up look something like this:

class Random {
    var number = Int(0)
}

class RandomNumberGenerator {
    func populate(random: Random) {
        random.number = 42
    }
}

func process(randomNumberGenerator: RandomNumberGenerator = .init()) -> Int {
    let random = Random()
    randomNumberGenerator.populate(random: random)
    return random.number
}

final class Step1: XCTestCase {
    func testProcess() {
        XCTAssertEqual(42, process())
    }
}

Everything is working great and we continue working through our backlog. A year later a new requirement comes in that we should use a fancy new random number generating web service. We integrate this new service and add the infrastructure to make it possible to stub it out in our tests. The end result is not too dissimilar. First we start by updating the RandomNumberGenerator to utilise our new service (in this case I’m just going to call the completion after a short wait).

class RandomNumberGenerator {
    var remoteNext: (@escaping (Int) -> Void) -> Void = { completion in
        DispatchQueue.main.asyncAfter(deadline: DispatchTime.now() + 3) {
            completion(42)
        }
    }

    func populate(random: Random) {
        remoteNext { number in
            random.number = number
        }
    }
}

The above provides a default implementation of remoteNext but allows us to override it in our tests because it’s just a var. The next thing to do is update our tests to provide a stub implementation of remoteNext that we control for testing.

final class Step2: XCTestCase {
    func testProcess() {
        let randomNumberGenerator = RandomNumberGenerator()

        randomNumberGenerator.remoteNext = { completion in
            completion(42)
        }

        XCTAssertEqual(42, process(randomNumberGenerator: randomNumberGenerator))
    }
}

The test passed so we move on to our next task.

NB: Unit tests are not acceptance tests and in real life we should run the automated/manual acceptance tests before moving on.

Where did it all go wrong?

Some people will have been internally screaming whilst reading the above changes. We’ve made the error of not taking a step back from our implementation and sense checking it. The process function is synchronous but the RandomNumberGenerator.populate(random:) function was updated to be asynchronous - this is not going to work. The issue has been completely masked by the fact that our unit tests were accidentally making the asynchronous RandomNumberGenerator.populate(random:) synchronous.

What does it mean to make “the asynchronous RandomNumberGenerator.populate(random:) synchronous”? Let’s demonstrate by changing the tests to be truly async again:

1
2
3
4
5
6
7
8
9
10
11
12
13
final class Step3: XCTestCase {
    func testProcess() {
        let randomNumberGenerator = RandomNumberGenerator()

        randomNumberGenerator.remoteNext = { completion in
            DispatchQueue.main.async {
                completion(42)
            }
        }

        XCTAssertEqual(42, process(randomNumberGenerator: randomNumberGenerator))
    }
}

In lines 6-8 I’m executing the completion within a DispatchQueue.async instead of calling it immediately. With this change we now get a failing test XCTAssertEqual failed: ("42") is not equal to ("0") -, which is what we expect.

Now that we have a failing test to guide us we have a few options to resolve this. Let’s update the test first to what we believe the api should now be in a fully asynchronous world:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
final class Step4: XCTestCase {
    func testProcess() {
        let randomNumberGenerator = RandomNumberGenerator()

        randomNumberGenerator.remoteNext = { completion in
            DispatchQueue.main.async {
                completion(42)
            }
        }

        weak var completionCalled = expectation(description: "Completion was called.")

        process(randomNumberGenerator: randomNumberGenerator) { number in
            XCTAssertEqual(42, number)
            completionCalled?.fulfill()
        }

        wait(for: [ completionCalled! ], timeout: 0.1)
    }
}

• Lines 11, 15 and 18 are added to take advantage of the asynchronous testing features of XCTest.
• Line 13 represents how the new API will look when it has been made asynchronous.

Finally we need to update the production code to make this test pass:

class RandomNumberGenerator {
    var remoteNext: (@escaping (Int) -> Void) -> Void = { completion in
        DispatchQueue.main.asyncAfter(deadline: DispatchTime.now() + 3) {
            completion(42)
        }
    }

    func populate(random: Random, completion: @escaping (Random) -> Void) {
        remoteNext { number in
            random.number = number
            completion(random)
        }
    }
}

func process(randomNumberGenerator: RandomNumberGenerator = .init(), completion: @escaping (Int) -> Void) {
    let random = Random()
    randomNumberGenerator.populate(random: random) { random in
        completion(random.number)
    }
}

The source transformation here boils down to adding completion handlers to RandomNumberGenerator.populate and process and wiring everything up. With this in place we have passing unit tests.

Conclusion

By accidentally making our asynchronous code synchronous in our tests we are not validating our code in a realistic situation. If remoteNext is performing networking then these tests are actually testing an impossible situation as we would never expect an immediate response.

In the example above we changed the implicit contract of our implementation by accident, which lead to incorrect behaviour. It could have been equally possible that we deliberately made our tests synchronous to make them easier to write. Whilst not adding the expectation boilerplate is nicer it also leads to potentially fragile tests or weakened acceptance criteria. A DispatchQueue.async may not model the production code exactly but it’s better than changing the functions implicit contract and will not add much overhead to a test run.