DEV Community

Warren Burton
Warren Burton

Posted on

Refactoring for testability

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.

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. This pattern allows you replace an algorithim with one of your choice.

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.

var timeStrategy: TimeStrategy

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

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

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.

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:

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.

Top comments (0)