今まで書いたダメなXCTestから良いテスト、悪いテストを考える

概要

iOSアプリ開発に携わる中でXCTestを使って色んなUnitTestを書いてきました。

その中でテストの基本を守っていなかったり、
覚えたての内容で無茶なテスト書いてしまったりと色々な失敗をしてきました。

変な失敗パターンも多いですが、クリスマス前に懺悔の意味も込めて、

• 実際に書いてしまったダメなUnitTestのパターン
• 何がダメだったのか
• どうなったら良くなるのか

を個人の考えとして言語化していきます。

対象読者

以下の経験があることを前提としています。

• iOSアプリの開発をしたことがある
• XcodeでXCTestを使ってUnitTestを書いたことがある
• MockやStubを使って依存性の注入をして、UnitTestを書いたことがある

ダメなテストその1: 出力結果が常に変わるテスト

いままで私がよくないテストやコードを書いてしまう場面の多くは、
Dateに関するコードを書く時でした。

例えばDateをメソッド内部で生成して、現在から十分後のTimeIntervalを返すメソッドを書いたとします。

/// 十分後のTimeInterval形式で返す
func tenMinutesLater() -> TimeInterval {
    let now = Date()
    let tenMinutes: TimeInterval = 10 * 60
    return now.timeIntervalSince1970 + tenMinutes
}

このコードをテストする場合コードは以下のようになります。

func testTenMinutesLater() {
    // When
    let result = tenMinutesLater()
    // Then
    let tenMinutesLater = Date().timeIntervalSince1970 + 10 * 60
    // ❗️テストコードとテスト対象でDateを生成するタイミングが違うのでEqualで判定できない
    XCTAssertGreaterThanOrEqual(result, tenMinutesLater - 1)
    XCTAssertGreaterThanOrEqual(tenMinutesLater + 1, result)
}

本来ならXCTAssertEqualを使って「ぴったり10分後」になっているかどうかを確認したいのですが、
テストコードとテスト対象でDateを生成するタイミングがわずかに違うためEqualで比較することができません。

実際にXCTAssertEqualを使って、テスト結果と期待値を比較するとテストが失敗します。

なので「指定の範囲に収まっている」テストを書かざる得なくなり、
テストしたい内容と若干意味が変わってしまいます。

ダメな理由

• テスト対象のメソッドのfunc tenMintesLater()の戻り値を固定する手段がない

というのがダメな理由だと考えています。

実行するたびにfunc tenMintesLater()の戻り値が変わるので、
前述したようにテストで「10分後」というのを正確に確認することができません。

さらには実行するマシンのスペックが悪いと、テスト内とfunc tenMintesLater()の間でDateを生成するタイミングの差の開きが大きくなります。
差がひらいてくると、たまにテストが失敗するようになったりと不安定なテストになります。

たまに失敗するテストを確認するのは、調査コストなどの面で地味に辛いことが多かったです。

より良いコードにするには？

今回の例の場合は「現在の時間を固定できる手段を提供する」のが良さそうです。

func tenMinutesLater()に引数を作ってDateを渡せるようにして、戻り値を固定できるようにしてみます。

引数にはデフォルト引数を設定すれば、func tenMinutesLater()を使っている箇所への影響範囲も少なく、
利用者側がメソッドの挙動を選択できるようになるので簡単に直すことができます。

/// 十分後のTimeInterval形式で返す
func tenMinutesLater(from referenceDate: Date = Date()) -> TimeInterval {
    let tenMinutes: TimeInterval = 10 * 60
    return referenceDate.timeIntervalSince1970 + tenMinutes
}

そうするとテストコードは以下のように書けます。

func testTenMinutesLater() {
    // Given
    let timestamp: TimeInterval = 1607710972
    let now = Date(timeIntervalSince1970: timestamp)
    // When
    // ❗️引数の値で戻り値は一意に決まるので、テスト側で戻り値をコントロールできる
    let result = tenMinutesLater(from: now)
    // Then
    let tenMinutesLater = timestamp + 10 * 60
    XCTAssertEqual(tenMinutesLater, result)
}

テスト側でサンプル用の値を引数に渡すことで、func tenMinutesLater(from: Date)は常に同じ値を返すようになります。
これで不安定なテストではなくなりました。

デフォルト引数を本当に使うべきかどうかは一考するべきかもしれませんが、
既存の状態よりは良くなったのではないかなと思います。

ダメなテストその2: テストのためにprivateを削除する

private以外にも、protocol経由でのアクセスなど別の手段でメソッドへのアクセスを制御する方法があります。
それを使えばクラス側にはprivateは必要ないと思っていた時期がありました。

すべてのメソッドを公開すれば、テストカバレッジ100%が簡単に手に入ります。
なので一時期は全てのメソッドやプロパティに対して、テストコードを書いていました

ダメな理由

• 壊れやすいテストになる

メソッドに引数を追加したい場合、
引数を追加したメソッドがテストから参照されていたらテストがコンパイルエラーになります。

struct FragileTestTarget {
    /// privateにしたいメソッド
    /// ❗️引数の追加や、戻り値を変えるとテストコードにも変更が必要になる。
    func thisMethodShouldBePrivate() -> Int {
       ...
    }
}

実装を変更する頻度はpublicのメソッドよりprivateなメソッドのほうが機会が多いと思います。

テストコードが本来privateにすべきメソッドを無理に参照した場合、
テスト対象のクラスの実装を変更するたびにコンパイルエラーになってしまう壊れやすいテストになります。

クラスの内部実装を少し変えたいだけなのに、毎回テストコードのメンテナンスが必要になるので大変です。

より良いコードにするには？

どうしてもprivateを削除して内部実装をテストしたいということは、
テスト対象のクラスの中に多くのことを詰め込みすぎているのかもしれません。

そのクラスに「多くのことが詰め込まれている」ということは「多くの役割を持っている」可能性があります。

なのでどうしてもテストしたいprivateメソッドやメソッドの一部を別のクラスに移動して、
役割を別のクラスに分けてあげると良さそうです

struct FragileTestTarget {
    /// privateなプロパティに切り出したので、外部からアクセスされずにいままでと同じ処理ができる
    private let composite: Composite

    private func thisMethodShouldBePrivate() -> Int {
       ...
       // ❗️切り出したメソッドの一部の内容を呼び出す
       composite.invokePartOfMethod()
    }
}

/// FragileTestTargetから、クラスの一部の役割を切り出したクラス
struct Composite {
    /// どうしてもテストしたい箇所は別クラスのメソッドに切り出して、別でテストを書く
    func invokePartOfMethod() -> Int {
       ...
    }
}

テストしたい内部実装を別クラスに公開メソッドとして移動させてテストを書くと、
役割の分割とprivateでのアクセス制限、テストのしやすさが保てます。

privateなメソッドをテストしたくなったら、
そのクラスが多くのことをやりすぎてないか？やっていることを別のクラスに分割できないか？
を検討する機運かもしれません。

ダメなテストその3: テスト内で非同期処理を使う

iOS開発で非同期処理をするコードを書くことはよくあると思いますが、
私はテストコードでも非同期処理を使って検証をするコードをよく書いていました。

例えば以下のようなクラスがあったとします。

/// AsynchronousTestTargetが利用するインターフェース
protocol AsynchronousTestTargetOutput {
    func loadFromSomeWhere(completion: @escaping (() -> Void))
}

final class AsynchronousTestTarget {
    enum Status {
        case initialized
        case loading
        case didLoad
    }
    /// リクエストの状態
    private(set) var status: Status = .initialized
    private let output: AsynchronousTestTargetOutput

    init(output: AsynchronousTestTargetOutput) {
        self.output = output
    }

    /// ❗️テストをしたいメソッド
    func load() {
        // ロード開始でステータスを変更
        status = .loading
        output.loadFromSomeWhere { [weak self] in
            // ロード完了でステータスを変更
            self?.status = .didLoad
        }
    }
}

AsynchronousTestTargetのstatusの状態遷移をテストしたいと思っていた時に、
以前の私は以下のようなコードを書いて検証をしていました。

/// Mockクラス
final class MockOutput: AsynchronousTestTargetOutput {
    /// loadFromSomeWhereが呼ばれた際に実行される
    var invokedLoadFromSomeWhere: (() -> Void)?
    func loadFromSomeWhere(completion: @escaping (() -> Void)) {
        // リクエスト後に即時completionを実行
        // ❗️AsynchronousTestTargetのstatus更新を確認したいので、遅延実行で処理を遅らせる
        DispatchQueue.global().asyncAfter(deadline: .now() + 1) { [weak self] in
            completion()
            self?.invokedLoadFromSomeWhereCompletion?()
        }
    }
}

final class AsynchronousTest: XCTestCase {
    func testLoad() {
        // Given
        let expectation = self.expectation(description: "loadFromSomeWhereが呼び出されること")
        let mock = MockAsynchronousTestTargetOutput()
        let target = AsynchronousTestTarget(output: mock)
        mock.invokedLoadFromSomeWhereCompletion = {
            // Then
            expectation.fulfill()
        }
        // When
        target.load()
        // Then
        wait(for: [expectation], timeout: 1)
        XCTAssertEqual(target.status, .didLoad, "ステータスがロード完了になること")
    }
}

テストをしたいクラスのプロパティが書き換わっていることを検証をするために、
Mockクラスの中でDispatchQueueを使って遅延を発生させてプロパティの状態遷移を確認するテストになっています。

ダメな理由

DispatchQueueなどを使って遅延実行や非同期処理をしていた場合は

• 遅延実行を待つ分、テストの実行時間が長くなる
• 非同期処理の実行タイミングはマシンの状態に左右される

などに悩まされることになります。
例えばマシンのスペックが悪く、非同期処理がなかなか処理されずにタイムアウトでテストが失敗することはよくあります。

レビュワー: 「このPRテスト通ってないですね」
私: 「非同期実行の箇所でたまに失敗するテストですね。テスト再実行しておきます」

のような他の人に不毛なやりとりを頻繁にさせてしまうことが多々ありました
テストではなるべくマシンの状態に依存しないように、非同期処理を回避したほうが良さそうです。

より良いコードにするには？

先ほどのテストコードを同期的に実行できるように修正できると良さそうです。
同期的に検証ができるようにテストを修正してみました。

/// Mockクラス
final class MockOutput: AsynchronousTestTargetOutput {
    var invokedLoadFromSomeWhereCount = 0
    /// ❗️引数のcompletionをプロパティとして保持する
    var invokedLoadFromSomeWhereCompletionParameter: (() -> Void)?
    func loadFromSomeWhere(completion: @escaping (() -> Void)) {
        // 呼び出された回数を記録するためインクリメントする
        invokedLoadFromSomeWhereCount += 1
        // 引数のcompletionをキャプチャ
        invokedLoadFromSomeWhereCompletionParameter = completion
    }
}

final class AsynchronousTest: XCTestCase {
    func testLoad() {
        // Given
        let mock = MockAsynchronousTestTargetOutput()
        let target = AsynchronousTestTarget(output: mock)
        // When
        target.load()
        // Then
        XCTAssertEqual(mock.invokedLoadFromSomeWhere, 1, "1回だけloadが呼ばれること")
        XCTassertEqual(target.status, .loading, "loadが呼ばれてリクエストが完了する前は、loadingにstatusが変わっていること")
        // When
        mock.invokedLoadFromSomeWhereCompletionParameter?()
        // Then
        XCTAssertEqual(target.status, .didLoad, "completionが呼ばれたらステータスがロード完了になること")
    }
}

完了コールバックのcompletionをMockのプロパティとして保持することで、
任意のタイミングでコールバックを発火できるようになりました。
これでDispatchQueueを使わなくても状態遷移のテストをすることができます。

この書き方を始めてから、テスト結果がマシンの状態に左右されることもなくなりました。

完全に非同期処理を無くすのは難しいですが、
非同期処理が必要ない箇所を見直して同期処理をするコードにできると安定したテストになりそうです。

ダメなテストその4: ViewControllerのライフサイクルのテスト

XCTestでViewControllerのテストを書く時に、
ライフサイクルに沿ってViewControllerが動作をしているかを確認をしたいことがあると思います。

XCTestの中でもUIApplicationが呼び出せるので、
UIApplicationにViewControllerを表示するコードを書けば一連のライフサイクルを再現することができます。

例えば以下のViewControllerがあったとします。

protocol TargetViewControllerOutput {
    /// ViewControllerで表示された時に一度だけ呼び出したい処理
    func wantToInvokeOnlyOnce()
}

final class TargetViewController: UIViewController {

    let output: TargetViewControllerOutput
    private var didAppear = false

    init(output: TargetViewControllerOutput) {
        self.output = output
        super.init(nibName: nil, bundle: nil)
    }

    override func viewDidAppear(_ animated: Bool) {
        super.viewDidAppear(animated)
        // ❗️viewControllerを表示した時一回だけ処理する
        if !didAppear {
            output.wantToInvokeOnlyOnce()
        }
        didAppear = true
    }
}

以下の様にUIApplicationを使ってViewControllerの一連のライフサイクルを再現できます。

final class ViewControllerLifecycleTests: XCTestCase {

    func testLifecycleAntiPattern() {
        // Given
        let output = MockTargetViewControllerOutput()
        let target = TargetViewController(output: output)
        let otherViewController = UIViewController()
        let expectation = self.expectation(description: "dispatch queueが実行されること")
        DispatchQueue.main.async {
            // When
            UIApplication.shared.windows.first?.rootViewController?.present(target, animated: false) {
                // Then
                XCTAssertEqual(output.invokedWantToInvokeOnlyOnceCount, 1, "最初に表示されたときは呼び出されること")
                target.present(otherViewController, animated: false) {
                    otherViewController.dismiss(animated: false, completion: {
                        // Then
                        // ❗️別の画面からの戻りの場合を再現して、2回目のviewDidAppearを実行する
                        expectation.fulfill()
                        XCTAssertEqual(output.invokedWantToInvokeOnlyOnceCount, 1, "別画面からの戻りでは呼び出されないこと")
                    })
                }
            }
        }
        wait(for: [expectation], timeout: 5)
    }

これでライフサイクル通りにTargetViewControllerのviewDidLoadやviewDidAppearなどが呼ばれるようになります。

一回目の表示ではwantToInvokeOnlyOnceのメソッドの呼び出され、
二回目の表示ではwantToInvokeOnlyOnceが呼び出されないことを確認できるようになりました。

ダメな理由

• テストコードのコールバックが多くて複雑
• 各テストケースでUIApplicationを共有するので、テストケース同士が影響しあってしまう
• テストの目的が多い

テストコードのコールバックが多くて複雑

まず目につくのがコールバックが多いことです。
テストコードが複雑なのでメンテナンスするのは大変です。(実際大変でした)

テストケース同士が影響しあってしまう

別の画面のViewControllerのテストが追加されると、
テスト対象のViewControllerの数が増えるにつれて不安定なテストになっていきます。

UIApplicationがシングルトンだったり画面遷移をメインスレッドで処理をしていたため、
ViewControllerのテスト完了を待たずに別のViewControllerのテストが実行されることがあったからです。

レビュワー：「このPR、テストが通ってないですね。」
自分: 「いつものViewControllerのテストですね、テスト再実行しておきます。」

という既視感のあるやりとりをまたも繰り返していて、すごく無駄な時間を使わせてしまい申し訳なかったなと思っています...

テストの目的が多い

UniTestでやっている検証がTargetViewControllerの検証と、
iOSが実行するUIViewControllerのライフサイクルの検証になっています。

やっていることが複数あるのでテストコードがわかりづらくなります。

クラスのテストとUIViewControllerのライフサイクルの検証は分けた方が良さそうです。

より良いコードにするには？

「各テストケースでオブジェクトを共有しない」かつ「TargetViewControllerのテストにフォーカスする」のが良さそうです。
UIViewControllerのライフサイクルのメソッドはテストから直接呼び出せます。

    func testLifecycleGoodPattern() {
        XCTContext.runActivity(named: "viewDidAppearが呼ばれた場合") { _ in
            // Given
            let output = MockTargetViewControllerOutput()
            let target = TargetViewController(output: output)
            // When
            target.viewDidAppear(false)
            // Then
            XCTAssertEqual(output.invokedWantToInvokeOnlyOnceCount, 1)
        }
        XCTContext.runActivity(named: "2回目のviewDidAppearが呼ばれた場合") { _ in
            // Given
            let output = MockTargetViewControllerOutput()
            let target = TargetViewController(output: output)
            // When
            target.viewDidAppear(false)
            target.viewDidAppear(false)
            // Then
            XCTAssertEqual(output.invokedWantToInvokeOnlyOnceCount, 1, "2回呼び出しても1回しか呼び出されないこと")
        }
    }

さらに複雑なviewDidLoad -> viewDidAppearの順番で呼ばれた場合のテストも、この方法でならば問題なくエミュレートできます。
テスト内でUIApplicationを呼び出すよりもこちらのほうが見通しも良くて断然おすすめです

ダメなUnitTestから見えてくる、良いUnitTest

いままで挙げたダメなUnitTestから、以下の5つがダメなUnitTestの主な特徴になりそうです。

• 環境などテスト外の要因によってテストが成功したり、失敗したりする
• 実行完了まで時間がかかる
• テストコードが複雑に書かれている
• UnitTestでテスト対象のクラスの挙動以外もテストしている
• privateなメソッドやプロパティを無理やりテストする

特に「環境などテスト外の要因によってテストが成功したり、失敗したりする」ことが常態化してしまうと、
不毛な調査の時間だけでなく、テストが失敗することが当たり前になってしまいます。

失敗が当たり前になるとテストの信頼度が下がり、
テストで検知できた不具合も見逃しやすくなるので早めに直すように心がけていきたいです。

これらの失敗パターンの逆から、自分は以下を意識するべきだと考えています。

• テストケースが何回実行しても同じ結果になるようにする
• 実行完了までを早くする
• テストコードはなるべく単純に書く
• UnitTestではテスト対象のクラスの挙動だけに絞ってテストする
• 検証に必要な最低限のメソッドとプロパティだけテストをする

まだまだあると思いますが、特に重要なのは上の5つだと思っています。

最後に

「UnitTestにだいぶ慣れてきたけど...次はどうしよう？」というときに、
慣れてくると少し凝ったことをしてみたいという欲求から、いろんな方法でUnitTestを書いてきました。

ですがどれもメンテナンス性の観点などで失敗することが多かったです。

ソフトウェア開発の書籍を読み、勉強会でLTを聞き、
自分のやってきたことを振り返るとメンテナンス性や可読性などでかなり難があることが多く反省することが多々ありました。

いまでは普通に書いて、わかりやすいのが一番だなと思っています。

良いUnitTestが書ける状況は多いとは言えませんが、
凝った方法は最後の手段で使わないようにしていきたいです。