既存コードへのテスト追加とリファクタリングの実践

Transcript

1. 2024-12-19 pixiv App Night / makun 既存コードへのテスト追加と リファクタリングの実践
2. 自己紹介： makun コミック事業部 Palcy部 エンジニア 2018年新卒入社 7年目 新卒エンジニア採用リーダー 趣味：バスケ、麻雀、アコギなど
3. Palcy - パルシィ 講談社の少女・女性マンガを 毎日無料で読めるマンガアプリ 講談社とピクシブで協業 2018年に正式リリース
4. テスト対象の機能
5. テスト対象の機能 • マンガが更新された際にユーザーのスマホに通知する機能 • マンガは1日に数十作品の更新 がある • どの更新について通知するか選別する必要 がある
6. テスト対象の機能 • 通知するマンガは、どういった条件で決める？ • 通知は、いつ、何件まで送信する？ • そもそも更新の定義って何？ • etc…
7. テスト対象の機能 • 実装した処理が仕様通り正しく動作しているか不安 • 実機での動作確認も日が暮れるような作業が必要 • 通知時間の調整 • 複数パターンの組み合わせの再現 • 本番環境で稼働、手が加えづらい
8. テストコードの追加 ※テストの紹介は一部のみ
9. class TestWorker( private val context: Context, workerParams: WorkerParameters ) : CoroutineWorker(context, workerParams) { private suspend fun filterEpisodes(...): Result<List<...>> { … val filteredEpisodes = updatedEpisodes .filter { ... } .filter { ... } .distinctBy { ... } return Result.success(filteredEpisodes) } } WorkManagerの CoroutineWorkerに実装され たfilterEpisodesのロジックを テストする。 テスト対象クラス：TestWorker テスト対象関数 ：filterEpisodes
10. CoroutineWorker を扱うため src/androidTest 以下にテストファイルを作成
11. class TestWorkerTest { @Test fun 作品のベルマークがoffの場合はその作品の通知を除く() {} @Test fun ひとつの作品で複数のエピソードの更新があった場合は一番優先度の高いnotificationTypeの通知だけを残す() {} @Test fun すでに読了しているエピソードの通知を除く() {} @Test fun ユーザーのコイン通知設定がoffの場合はnotificationTypeがNEW_COINの通知を除く() {} @Test fun ユーザーのチケットで読める話の追加の通知設定がoffの場合はnotificationTypeがNEW_TICKETとCOIN_TO_TICKETの通知を除く() {} @Test fun ひとつの作品で同じnotificationTypeが複数あった場合はエピソードIDが一番大きいものを残す() {} }
12. @Ignore("サーバー側でフィルタリングされる") @Test fun ユーザーのコイン通知設定がoffの場合はnotificationTypeがNEW_COINの通知を除く() {} @Ignore("サーバー側でフィルタリングされる") @Test fun ユーザーのチケットで読める話の追加の通知設定がoffの場合はnotificationTypeがNEW_TICKETとCOIN_TO_TICKETの通知を除く() {} @Ignore("サーバー側でフィルタリングされる") @Test fun ひとつの作品で同じnotificationTypeが複数あった場合はエピソードIDが一番大きいものを残す() {}
13. class TestWorkerTest { @Test fun 作品のベルマークがoffの場合はその作品の通知を除く() {} @Test fun ひとつの作品で複数のエピソードの更新があった場合は一番優先度の高いnotificationTypeの通知だけを残す() {} @Test fun すでに読了しているエピソードの通知を除く() {} … }
14. class TestWorker( private val context: Context, workerParams: WorkerParameters ) : CoroutineWorker(context, workerParams) { private suspend fun filterEpisodes(...): Result<List<...>> { … val filteredEpisodes = updatedEpisodes .filter { ... } .filter { ... } .distinctBy { ... } return Result.success(filteredEpisodes) } } WorkManagerの CoroutineWorkerに実装され たfilterEpisodesのロジックを テストする。 androidx.work:work-testing WorkManagerのWorkerをテ ストするために必要
15. val context = ApplicationProvider.getApplicationContext<Context>() val worker = TestListenableWorkerBuilder<TestWorker>(context).build() val inputData = listOf<UpdatedEpisodeCache>() worker.filterEpisodes(inputData)
16. private suspend fun filterEpisodes(...): Result<List<...>> {...} ↓ @VisibleForTesting internal suspend fun filterEpisodes(...): Result<List<...>> {...}
17. private suspend fun filterEpisodes(...): Result<List<...>> {...} ↓ @VisibleForTesting internal suspend fun filterEpisodes(...): Result<List<...>> {...}
18. @Test fun 作品のベルマークがoffの場合はその作品の通知を除く() = runTest { val context = ApplicationProvider.getApplicationContext<Context>() val worker = TestListenableWorkerBuilder<TestWorker>(context).build() val inputData = listOf<UpdatedEpisodeCache>() val result = worker.filterEpisodes(inputData) assertTrue(result.isSuccess) }
19. @Test fun 作品のベルマークがoffの場合はその作品の通知を除く() = runTest { val context = ApplicationProvider.getApplicationContext<Context>() val worker = TestListenableWorkerBuilder<TestWorker>(context).build() val inputData = listOf<UpdatedEpisodeCache>() val result = worker.filterEpisodes(inputData) assertTrue(result.isSuccess) }
21. val inputData = listOf( UpdatedEpisodeCache( episodeId = "12", comicId = "7"), UpdatedEpisodeCache(...), ... ) val expectedData = listOf( UpdatedEpisodeCache(...), UpdatedEpisodeCache(...), … ) val result = worker.filterEpisodes(inputData) assertTrue(result.isSuccess)
22. val inputData = listOf( UpdatedEpisodeCache( episodeId = "12", comicId = "7"), UpdatedEpisodeCache(...), ... ) val expectedData = listOf( UpdatedEpisodeCache(...), UpdatedEpisodeCache(...), … ) val result = worker.filterEpisodes(inputData) assertTrue(result.isSuccess)
24. val inputData = listOf(...) val expectedData = listOf(...) val result = worker.filterEpisodes(inputData) // assertTrue(result.isSuccess) assertContentEquals( expected = expectedData, actual = result.getOrThrow() )
26. class TestWorkerTest : KoinTest { @BeforeTest fun setup() { startKoin {} } @AfterTest fun tearDown() { stopKoin() } } Koinの公式ドキュメントに従っ てテストコードに KoinTest, startKoin, stopKoin を追記
28. @Test fun 作品のベルマークがoffの場合はその作品の通知を除く() = runTest { loadKoinModules(module { single<NotificationRepository> { object : NotificationRepository by mock() {} } })
29. @Test fun 作品のベルマークがoffの場合はその作品の通知を除く() = runTest { loadKoinModules(module { single<NotificationRepository> { object : NotificationRepository by mock() {} } })
31. val filteredEpisodes = updatedEpisodes .filter { episode -> try { notificationRepository.isComicNotificationEnabled(episode.comicId) } catch (e: Exception) { return kotlin.Result.failure(e) } }
32. @Test fun 作品のベルマークがoffの場合はその作品の通知を除く() = runTest { loadKoinModules(module { single<NotificationRepository> { object : NotificationRepository by mock() {} } }) 関数の実装を委譲
33. object : NotificationRepository by mock() { override suspend fun isComicNotificationEnabled(comicId: String): Boolean { return false } } isComicNotificationのStubを作成
34. テストが失敗している
35. 期待の値とリストの長さが異なる
36. val filteredEpisodes = updatedEpisodes .filter { episode -> try { // ここで常にfalseを返している状態 notificationRepository.isComicNotificationEnabled(episode.comicId) } catch (e: Exception) { return kotlin.Result.failure(e) }
37. object : NotificationRepository by mock() { override suspend fun isComicNotificationEnabled(comicId: String): Boolean { // return false return true } }
39. val inputData = listOf( UpdatedEpisodeCache(), UpdatedEpisodeCache( episodeId = "32", comicId = "8", ), UpdatedEpisodeCache( episodeId = "28", comicId = "9", ), ... ) val expectedData = listOf( UpdatedEpisodeCache(...), UpdatedEpisodeCache( episodeId = "28", comicId = "9", ), … comicId = “8” のデータは無い comicId = “8”のデータは ベルマークがoffのため 通知対象から除かれている テストで利用するデータの前提条件
40. object : NotificationRepository by mock() { override suspend fun isComicNotificationEnabled(comicId: String): Boolean { return true } } comicId に関わらず true 前提条件を完全に無視
41. object : NotificationRepository by mock() { override suspend fun isComicNotificationEnabled(comicId: String): Boolean { return when (comicId) { “8”, “10” -> false else -> true } } } 作品のベルマークの状態 についての データを返す関数 予め知っていれば テストの作成が楽になる
42. interface NotificationRepository { /** * 対象作品の通知設定の状態を返す * * @param comicId 対象の作品ID * @return 対象作品の通知がONの場合はtrue、OFFの場合はfalse */ suspend fun isComicNotificationEnabled(comicId: String): Boolean KDoc形式で説明されている
44. single<ComicsRepository> { object : ComicsRepository by mock() {} }
45. .filter { episode -> try { val timestamp = comicsRepository.getFinishReadEpisodeTimestamp( comicId = episode.comicId, episodeId = episode.episodeId, ) if (timestamp == null) true else timestamp < currentTime } catch (e: Exception) { return kotlin.Result.failure(e) } }
46. single<ComicsRepository> { object : ComicsRepository by mock() { override suspend fun getFinishReadEpisodeTimestamp( comicId: String, episodeId: String ): Long? {...}
47. single<ComicsRepository> { object : ComicsRepository by mock() { // 何も実装せずにテストを実行する }
49. この後の対応 • テスト対象の関数をWorkManagerから剥がしローカルテストに変更 • androidTestをRobolectricを使ってローカルテストに変更 • WorkManagerの通知以外の処理を分離しローカルテストを作成 • etc…
50. 時間があれば、テストの目的について • テストの目的「バグを補足すること」 • 同等に重要な目的「変化を可能にする能力を備えておく」 • プロダクト開発において「変化」は常に発生する • 変化はソフトウェア開発の本質的特徴 • 変化の速度を望むほど高速にテストを行う必要性が増す