Refactoring for testability
Tags: testing, ios, swift
Published: 2020-07-07 11:21:42

A lot of the time I will build a component and in the rush  to make the component work I don't write tests. One thing that writing tests exposes is how badly your object is made for testing. To paraphrase a famous sailor "If it's difficult to test, you haven't written it right". 

The case in hand is a file I wrote to request App Store reviews.

```swift
import Foundation
import StoreKit

class ReviewRequester {
    private let requestRateKey = "lastReviewRequestDate"

    @discardableResult public func requestReviewIfCriteriaMatching() -> Bool {
        if reviewCriteriaSatisfied() {
            markDate()
            SKStoreReviewController.requestReview()
            return true
        }
        return false
    }
    
    private func reviewCriteriaSatisfied() -> Bool {
        guard dateCriteriaPassed() else {
            return false
        }

        if someOtherBizCriteria {
            return true
        }
        return false
    }

    private func dateCriteriaPassed() -> Bool {
        let date = UserDefaults.standard.double(forKey: requestRateKey)
        let now = Date().timeIntervalSince1970
        let sevenDays = 3600.0 * 24 * 7
        return (now - date) > sevenDays
    }

    private func markDate() {
        let now = Date().timeIntervalSince1970
        UserDefaults.standard.setValue(now, forKey: requestRateKey)
    }

    func clearMarkers() {
        UserDefaults.standard.removeObject(forKey: requestRateKey)
        //...
    }

}
```

The object checks to see if it has been seven days since the last request and if other business criteria have been matched then allows a call to `SKStoreReviewController.requestReview()`. `SKStoreReviewController` has its own internal rules for presentation frequency but that's out of my hands. What I want to check is that my logic is correct. 

The first thing to notice is that the elapsed time in `dateCriteriaPassed()` is dependant on `Date` which we have no control over. This is impossible to test without manipulating the system clock right?

## Making a Time Machine.

You need a Time Machine. The testing requirement is that you can manipulate the date to perform the following tests. 

- If it has been less than 7 days since last request then no review request is made.
- If it has been more than 7 days since last request then a review request is made. 

To build a time machine im going to use a design pattern known as [Strategy](https://en.wikipedia.org/wiki/Strategy_pattern). This pattern allows you replace an algorithim with one of your choice. 

```swift
protocol TimeStrategy {
    var now: Date { get }
}

class RealTime: TimeStrategy {
    var now: Date {
        return Date()
    }
}
```

I now have a date generator that conforms to `TimeStrategy`.

I add an `init` to optionally supply any `TimeStrategy` at instantiation. 

```swift
var timeStrategy: TimeStrategy

init(timeStrategy: TimeStrategy = RealTime()) {
    self.timeStrategy = timeStrategy
}
```

I can replace all instances of `Date()` with `timeStrategy.now`.

```swift
private func dateCriteriaPassed() -> Bool {
    let date = UserDefaults.standard.double(forKey: requestRateKey)
    let now = timeStrategy.now.timeIntervalSince1970
    let sevenDays = 3600.0 * 24 * 7
    return (now - date) > sevenDays
}

private func markDate() {
    let now = timeStrategy.now.timeIntervalSince1970
    UserDefaults.standard.setValue(now, forKey: requestRateKey)
}
```

## Manipulating Time but not Space

Now im ready to write those tests. First step is to create  a `TimeStrategy` that can be manipulated. 

```swift
class TestTime: TimeStrategy {
    private var relativeDate = Date().timeIntervalSince1970
    var now: Date {
        return Date(timeIntervalSince1970: relativeDate)
    }

    func advance(_ interval: TimeInterval) {
        relativeDate += interval
    }
}
```

Then I can use that time machine in my tests. In **Quick/Nimble** syntax:

```swift
import Quick
import Nimble

class ReviewRequesterTests: QuickSpec {

    override func spec() {

        it("less than 7 days since last request then no review request is made") {
            let timeMachine = TestTime()
            let requestor = ReviewRequester(timeStrategy: timeMachine)
            requestor.clearMarkers()

            expect(requestor.requestReviewIfCriteriaMatching()).to(beTrue())
            expect(requestor.requestReviewIfCriteriaMatching()).to(beFalse())
            timeMachine.advance(3600.0 * 24 * 7 - 1) //7 days - 1 sec
            expect(requestor.requestReviewIfCriteriaMatching()).to(beFalse())
        }

        it("more than 7 days since last request then a review request is made") {
            let timeMachine = TestTime()
            let requestor = ReviewRequester(timeStrategy: timeMachine)
            requestor.clearMarkers()

            expect(requestor.requestReviewIfCriteriaMatching()).to(beTrue())
            timeMachine.advance(3600.0 * 24 * 7 + 1) //7 days + 1 sec
            expect(requestor.requestReviewIfCriteriaMatching()).to(beTrue())
        }
    }
}
```

The negative test advances the date by not enough while the positive test does the opposite proving to me that the  component works under changing conditions.

## Summary 

`ReviewRequester` was modified from an untestable component to an object that allows you to inject an abstracted time function. Production calls are unaffected at the call site but at a test level you know have power of the decisions that can be made. 

- Identify your indeterminate API. 
- Replace that API with a shim layer under your control. 
- Validate your assumptions about the object.