【Code Review Challenge】解説７問目：単体テストのベストプラクティス #try! Swift Tokyo 2024

2024-05-28

こんにちは。iOSアプリエンジニアのKiichiです。

先日開催されたtry! Swift Tokyo 2024のLINEヤフー企業ブースでは、Code Review Challengeを行いました。Code Review Challengeとは、Bad CodeをGood Codeにするための公開コードレビューです。参加者に技術への関心を持ってもらうこと、外部の方々からのレビューにより社員の学びにつなげることを目的に、過去のイベントで行っていた取り組みを今回のイベントでも実施しました。

本記事では、今回出題したCode Review Challengeの7問目の解説をします。

出題コード

// In SNSApp target
 
import Foundation
 
struct Post {
    let id: String
    let text: String
    let imageURL: URL
}
 
protocol UserRepository {
    func getCurrentUserID() -> String?
}
 
protocol PostRepository {
    func getPosts(for userID: String) async throws -> [Post]
}
 
final class PostService {
    let userRepository: UserRepository
    let postRepository: PostRepository
 
    init(userRepository: UserRepository, postRepository: PostRepository) {
        self.userRepository = userRepository
        self.postRepository = postRepository
    }
 
    func getCurrentUsersPosts() async throws -> [Post] {
        if let userID = userRepository.getCurrentUserID() {
            return try await postRepository.getPosts(for: userID)
        } else {
            throw PostServiceError.currentUserIDNotFound
        }
    }
}
 
enum PostServiceError: Error {
    case currentUserIDNotFound
}
 
// ==========================================================
// In SNSAppTests target
 
import XCTest
import SNSApp
 
final class MockUserRepository: UserRepository {
    var currentUserID: String?
    func getCurrentUserID() -> String? { currentUserID }
}
 
final class MockPostRepository: PostRepository {
    var posts: [Post]
    func getPosts(for userID: String) async throws -> [Post] { posts }
}
 
final class PostServiceTests: XCTestCase {
    var postService: PostService!
    var userRepository: MockUserRepository!
    var postRepository: MockPostRepository!
 
    override func setUp() {
        userRepository = MockUserRepository()
        postRepository = MockPostRepository()
        postService = PostService(userRepository: userRepository, postRepository: postRepository)
    }
 
    func testGettingCurrentUsersPostsFailsWhenNoUserID() async throws {
        await assertThrowsAsyncError(try await postService.getCurrentUsersPosts()) { error in
            XCTAssertEqual(error as? PostServiceError, .currentUserIDNotFound)
        }
    }
 
    func testGettingCurrentUsersPostsSuccess() async throws {
        let post = Post(id: "Foo", text: "Foo", imageURL: URL(string: "https://foo.com/image.jpg")!)
        postRepository.posts = [post]
 
        let resultPosts = try await postService.getCurrentUsersPosts()
        XCTAssertEqual(resultPosts.first, post)
    }
}
 
extension Post: Equatable {
    static func == (lhs: Post, rhs: Post) { lhs.id == rhs.id }
}
 
func assertThrowsAsyncError<T>(
    _ expression: @autoclosure () async throws -> T,
    _ errorHandler: (_ error: any Error) -> Void
) async { /* ... */ }

これは、SNSアプリのコードです。
現在のユーザーの投稿を取得するためのクラスと、そのテストが記述されています。

このコードには複数の問題点がありますが、単体テストに関する以下の点について解説していきます。

オブジェクトの保持とsetUpメソッド
テストケースの網羅性
XCTestの機能の活用

1. オブジェクトの保持とsetUpメソッド

このコードでは、インスタンスとしてオブジェクトを保持し、setUpメソッドをオーバーライドすることでそれらを初期化しています。

final class UserServiceTests: XCTestCase {
    var postService: PostService!
    var userRepository: MockUserRepository!
    var postRepository: MockPostRepository!

    override func setUp() {
        userRepository = MockUserRepository()
        postRepository = MockPostRepository()
        postService = PostService(userRepository: userRepository, postRepository: postRepository)
    }
}

これはよくあるパターンですが、基本的に複数テストケース間でオブジェクトを共有するメリットはない上、デメリットもいくつかあります。

まず、依存関係は直接プロパティに代入するのではなくコンストラクタから注入することが好まれており、今回のパターンはそれを妨げることになります。実際、今回のコードでもPostServiceではコンストラクタからの注入が採用されており、テストケース内で別のuserRepositoryを用いたい場合でも、

postService.userRepository = MockUserRepository() // Error: 'userRepository' is a 'let' constant

といった代入は禁止されているため、柔軟性が低くなっています。だからと言って、プロパティの代入を行うためにプロダクトコードのletをvarに変えてしまうのは、保守性を損なうため得策ではありません。

また、このパターンでは、関連するオブジェクトはすべてインスタンスとして記述することになりますが、テストケースによっては使用しないオブジェクトもあるはずです。この場合、何をテストしたいのか、依存として何が必要なのか、分かりにくくなります。

また、今回のコードでは触れられていませんが、オブジェクトの全体的なライフサイクルをテストするとなると、setUpと同時にtearDownメソッドも記述する必要があります。この場合、テストケースメソッドの中でテスト処理が完結しなくなるため、可読性の低下につながります。

以上を踏まえ、setUpが本当に必要な場合以外は、テストケースのスコープ内のみでオブジェクトを生成・保持することが望ましいです。この場合、MockUserRepositoryやMockPostRepositoryのプロパティはコンストラクタで設定できるようにしておくと分かりやすいでしょう。（classではなくstructにすれば、明示的にコンストラクタを定義する必要もなくなります）

例えば以下のようになります。

final class PostServiceTests: XCTestCase {
    func testGettingCurrentUsersPostsSuccess() async throws {
        let userRepository = MockUserRepository()
        let postRepository = MockPostRepository(posts: /* ... */)
        let postService = PostService(userRepository: userRepository, postRepository: postRepository)
        // ...
    }
}

もしくは、以下のようなファクトリーメソッドを用意するのも良いでしょう。

extension PostServiceTests {
    private func makePostService(currentUserID: String?, posts: [Post]) -> PostService {
        let userRepository = MockUserRepository(currentUserID: currentUserID)
        let postRepository = MockPostRepository(posts: posts)
        let postService = PostService(userRepository: userRepository, postRepository: postRepository)

        return postService
    }
}

2. テストケースの網羅性

今回のコードでは、以下の2つのケースがテストされています。

func testGettingCurrentUsersPostsFailsWhenNoUserID() async throws { /* ... */ }
func testGettingCurrentUsersPostsSuccess() async throws { /* ... */ }

しかし、プロダクトコードの方を見てみると、この2ケース以外にも、PostRepositoryのgetPostsメソッドで例外が発生する可能性があることが分かります。実際には、単体テストですべてのケースを網羅するとなるとキリがないので、重要度や発生確率などに鑑みてテストを記述することになるでしょう。今回のコードでは、サーバーエラーやネットワークエラーなどでこの例外は十分起こり得ると考えられるので、テストを書くべきだと判断します。

ただし、今のままでは、MockPostRepositoryのgetPostsメソッドから例外を発生させることはできません。そのため、モックを改善する必要があります。

例えば以下のようになります。

struct MockPostRepository {
    let getPostsHandler: (_ id: String) async throws -> [Post]

    func getPosts(for id: String) async throws -> [Post] {
        try await getPostsHandler(userID)
    }
}

これにより、以下のように例外をテストできるようになります。

final class PostServiceTests: XCTestCase {
    func testGettingCurrentUsersPostsWithPostRepositoryError() async throws {
        let userRepository = MockUserRepository()
        let postRepository = MockPostRepository(
            getPostsHandler: { _ in throw DummyError() }
        )
        let postService = PostService(userRepository: userRepository, postRepository: postRepository)
        // ...
    }
}

ただ、以下のような場合には、モックオブジェクトの実装が複雑になってしまい、テストとしては望ましくありません。

メソッドやその引数が多い
関連のないメソッドの実装を任意にしたい
structではなくclassにする必要がある（コンストラクタの定義が必要）

マクロや自動コード生成ツールを実装、もしくはそのようなサードパーティ製ツールを用いて、モックを生成すると良いでしょう。

3. XCTestの機能の活用

XCTestでは、Xcode上でテストを実行するにあたって有用な、さまざまな機能が提供されています。

例えば、各アサーションが失敗を示しても、テストは最後まで実行されます。これにより、一度のテスト実行ですべての結果を確認できます。しかし、テスト中にクラッシュが発生した場合、そこで実行が止まってしまい、テストの効率が落ちてしまいます。

今回のテストコードでは、

URL(string: "https://foo.com/image.jpg")!

のforce unwrap (!)により、URL文字列が間違っていた場合にクラッシュが発生しますが、

try XCTUnwrap(URL(string: /* ... */ ))

と書くことで、クラッシュを避けることができます。
XCTUnwrapは標準で提供されている機能なので、ぜひ使っていきましょう。

また、XCTestでは、アサーション関数のある箇所に失敗を表示してくれる機能があります。
例えばXCTFailは以下のようなインターフェースになっており、

func XCTFail(
    _ message: String = "",
    file: StaticString = #filePath,
    line: UInt = #line
)

デフォルト引数のfileやlineによって、この機能が実現されています。

自前でアサーション関数を実装する際も、この機能を活用することで、結果が見やすくなります。
今回のコードでは、

func assertThrowsAsyncError<T>(
    _ expression: @autoclosure () async throws -> T,,
    file: StaticString = #filePath,
    line: UInt = #line  
    _ errorHandler: (_ error: any Error) -> Void
) async { /* ... */ }

とすると良いでしょう。

その他の改善点

その他、以下のような改善点が考えられます。

コーディングスタイル

structのプロパティはletではなくvarで良い
getPosts(for userID: String)という引数ラベルの命名は、呼び出し側でどのような文字列を渡すべきか分かりにくくなるので、好ましくない
少なくとも今の実装では、PostServiceはclassである必要がないので、structにしたほうが良い
protocolを型として用いる場合、anyまたはsomeキーワードをつける
- 例1：let userRepository: any UserRepository
- 例2：init(userRepository: some UserRepository)
外部から使用しない変数や関数にはprivate修飾子をつける
エラーの条件分岐は、if-elseよりも、guard文を使って早期リターンをすることが望ましい

テスト特有の記述

テスト対象のモジュールをインポートする際には、以下の目的で@testableをつける
- テスト対象を明確にする
- internalなオブジェクトや関数等にアクセスできるようにする
配列をテストする際は、一部の要素だけでなく、配列全体を確認すべき
今のtestGettingCurrentUsersPostsSuccessは、currentUserIDが設定されていないので、失敗する
テストターゲット内で演算子のオーバーロードをすることはできません。また、等価を判断する際、idだけでなくすべてのプロパティを比較すべき

想定解答

上記の問題点をすべて解決した想定解答は以下です。

// In SNSApp target

import Foundation

struct Post {
    var id: String
    var text: String
    var imageURL: URL
}

protocol UserRepository {
    func getCurrentUserID() -> String?
}

protocol PostRepository {
    func getPosts(userID: String) async throws -> [Post]
}

struct PostService {
    private var userRepository: any UserRepository
    private var postRepository: any PostRepository

    init(userRepository: some UserRepository, postRepository: some PostRepository) {
        self.userRepository = userRepository
        self.postRepository = postRepository
    }

    func getCurrentUsersPosts() async throws -> [Post] {
        guard let userID = userRepository.getCurrentUserID() else {
            throw PostServiceError.currentUserIDNotFound
        }

        return try await postRepository.getPosts(userID: userID)
    }
}

enum PostServiceError: Error {
    case currentUserIDNotFound
}


// ==========================================================
// In SNSAppTests target

@testable import SNSApp
import XCTest

struct MockUserRepository: UserRepository {
    var currentUserID: String?
    func getCurrentUserID() -> String? { currentUserID }
}

struct MockPostRepository: PostRepository {
    var getPostsHandler: (_ userID: String) async throws -> [Post]

    func getPosts(userID: String) async throws -> [Post] {
        try await getPostsHandler(userID)
    }
}

final class PostServiceTests: XCTestCase {
    func testGettingCurrentUsersPostsFailsWhenNoUserID() async throws {
        let userRepository = MockUserRepository(currentUserID: nil)
        let postRepository = MockPostRepository(getPostsHandler: { _ in [] })
        let postService = PostService(userRepository: userRepository, postRepository: postRepository)
        await assertThrowsAsyncError(try await postService.getCurrentUsersPosts()) { error in
            XCTAssertEqual(error as? PostServiceError, .currentUserIDNotFound)
        }
    }

    func testGettingCurrentUsersPostsWithPostRepositoryError() async throws {
        let userRepository = MockUserRepository(currentUserID: "dummyUserID")
        let postRepository = MockPostRepository(getPostsHandler: { _ in throw DummyError.unknown })
        let postService = PostService(userRepository: userRepository, postRepository: postRepository)

        await assertThrowsAsyncError(try await postService.getCurrentUsersPosts()) { error in
            XCTAssertEqual(error as? DummyError, .unknown)
        }
    }

    func testGettingCurrentUsersPostsSuccess() async throws {
        let post = Post(id: "Foo", text: "Foo", imageURL: try XCTUnwrap(URL(string: "https://foo.com/image.jpg")))

        let userRepository = MockUserRepository(currentUserID: "dummyUserID")
        let postRepository = MockPostRepository(getPostsHandler: { _ in [post] })
        let postService = PostService(userRepository: userRepository, postRepository: postRepository)

        let resultPosts = try await postService.getCurrentUsersPosts()
        XCTAssertEqual(resultPosts.map(\.id), [post.id])
        XCTAssertEqual(resultPosts.map(\.text), [post.text])
        XCTAssertEqual(resultPosts.map(\.imageURL), [post.imageURL])
    }
}

enum DummyError: Error {
    case unknown
}

func assertThrowsAsyncError<T>(
    _ expression: @autoclosure () async throws -> T,
    file: StaticString = #filePath,
    line: UInt = #line,
    _ errorHandler: (_ error: any Error) -> Void
) async { /* ... */ }

まとめ

この記事では、Code Review Challengeの7問目の解説として、主に以下の点について説明しました。

テストクラス内でオブジェクトを保持してsetUpメソッドを使うのではなく、各テストケース内で初期化するようにする
テストケースはなるべく漏れがないようにし、例外についても確実にテストできるようにする
XCTUnwrapの使用や、自作アサーション関数のfileおよびlineなど、XCTestの機能をなるべく活用する

自分は1年前まで主に個人開発の小規模プロジェクトしか扱っていなかったのですが、入社して大規模プロジェクトを扱うようになり、単体テストの重要性を実感しています。今後も、新機能を開発した際に該当する単体テストを追加するだけでなく、既存のコードについても不足している単体テストを増やしていき、バグの少ないプロダクトを目指していきたいと思います。