Issue #4のテスト修正: testRealTimeUpdateWhenUserDefaultsChanges の非同期処理改善 #10
Conversation
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
UWBViewerSystemTests/SimpleCalibrationViewModelTests.swift (3)
339-346: クリーンアップの確実化と固定スリープの見直し
- クリーンアップは
deferで先頭に置くと安全です。- 0.2秒スリープは観測者セットアップのためとのことですが、将来的にポーリングへ置換を検討。
適用例（この位置での追加を推奨）:
// UserDefaultsを初期状態でクリア UserDefaults.standard.removeObject(forKey: "currentFloorMapInfo") + defer { cleanupTestEnvironment() }末尾の
cleanupTestEnvironment()呼び出し（行 374）は重複になるため削除してください（この変更は当該範囲外なので別差分でお願いします）。
357-359: 1.5秒の固定待機はフレークの温床。ポーリングで早期合格/タイムアウトに固定スリープより、50ms刻みのポーリングで最大1.5秒まで待機にすると安定性と速度が向上します。
- // NotificationCenterの通知処理は非同期で実行されるため、十分な時間を確保 - try await Task.sleep(nanoseconds: 1_500_000_000) // 1.5秒 + // 通知処理完了を最大1.5秒までポーリング（50ms刻み）で待機 + let timeout: UInt64 = 1_500_000_000 + let interval: UInt64 = 50_000_000 + var waited: UInt64 = 0 + while await viewModel.currentFloorMapInfo == nil, waited < timeout { + try await Task.sleep(nanoseconds: interval) + waited += interval + }
361-364: MainActor.run は不要（直接 await で十分）
currentFloorMapInfoが actor/グローバルアクタ隔離プロパティなら直接 await で取得可能です。前後のテストとも記法を統一できます。- let updatedFloorMapInfo = await MainActor.run { - viewModel.currentFloorMapInfo - } + let updatedFloorMapInfo = await viewModel.currentFloorMapInfo
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
UWBViewerSystemTests/SimpleCalibrationViewModelTests.swift(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
UWBViewerSystemTests/SimpleCalibrationViewModelTests.swift (1)
UWBViewerSystem/Presentation/Scenes/FloorMapTab/SystemCalibrationPage/SimpleCalibrationViewModel.swift (1)
loadInitialData(164-168)
🔇 Additional comments (3)
UWBViewerSystemTests/SimpleCalibrationViewModelTests.swift (3)
351-351: コメントのみの変更特に問題ありません。
366-371: アサーションは妥当（詳細メッセージも良い）更新の検証内容・メッセージともに適切です。
333-338: await の不一致 — MockDataRepository の init が async か確認リポジトリ内で MockDataRepository の定義／async init を確認できませんでした。init が async でなければ A を適用（推奨）。createTestViewModel が使えるならさらに簡潔になります。
A) ヘルパを利用して一行に集約（推奨）
- let mockRepository = await MockDataRepository() - let viewModel = await MainActor.run { - SimpleCalibrationViewModel(dataRepository: mockRepository) - } + let viewModel = await createTestViewModel()B) 最小修正（init が同期の場合）
- let mockRepository = await MockDataRepository() + let mockRepository = MockDataRepository() let viewModel = await MainActor.run { SimpleCalibrationViewModel(dataRepository: mockRepository) }
- UserDefaults変更通知の処理完了を確実に待機するよう修正 - 非同期処理の同期化を改善し、NotificationCenter通知の競合状態を解決 - 待機時間を適切に調整して、UserDefaultsの変更が確実に反映されるよう修正 - テストの安定性を向上させ、一貫した結果を確保 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
レビューコメントで指摘された以下の改善点を実装： - 固定スリープ（0.2秒、1.5秒）をポーリングベースの待機に変更 - クリーンアップ処理をdeferで先頭に配置し、例外安全性を向上 - 不要なMainActor.runの使用箇所を削除 - テストの安定性と可読性を向上 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
harutiro
force-pushed
the
issue-4-fix-failing-tests
branch
from
d08e630 to
b1ecd44
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
UWBViewerSystemTests/SimpleCalibrationViewModelTests.swift (2)
15-20: メソッド名が不明確です
createTestViewModel()はMockDataRepositoryを使用することを示すため、createMockTestViewModel()のような名前にするべきです。以下のdiffを適用してメソッド名を改善してください：
-@MainActor -private func createTestViewModel() -> SimpleCalibrationViewModel { - // テスト用のMockDataRepositoryを作成 - let mockRepository = MockDataRepository() - return SimpleCalibrationViewModel(dataRepository: mockRepository) -} +@MainActor +private func createMockTestViewModel() -> SimpleCalibrationViewModel { + // テスト用のMockDataRepositoryを作成 + let mockRepository = MockDataRepository() + return SimpleCalibrationViewModel(dataRepository: mockRepository) +}
109-122: ポーリングロジックの一貫性を確保他のテストケースと同様のポーリングパターンを使用していて一貫性があります。ただし、条件チェックでnilを確認する際の論理構造を明確化できます。
条件チェックをより明確にするため、以下のような改善を検討してください：
repeat { currentFloorMapInfo = await viewModel.currentFloorMapInfo - if currentFloorMapInfo == nil { - break - } + // nilの場合は期待する状態なので待機終了 + if currentFloorMapInfo == nil { + break + } try await Task.sleep(nanoseconds: UInt64(pollInterval * 1_000_000_000)) } while Date().timeIntervalSince(startTime) < maxWaitTime
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
UWBViewerSystemTests/SimpleCalibrationViewModelTests.swift(6 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.swift
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.swift: 終了前にSwiftFormat（make format）を実行してコードフォーマットを統一する
不要なコード（デバッグ用コード、コメントアウトされた古いコード等）は削除する
不要なimport文は削除する
デッドコードは削除する
コメントは必要最小限とし、コードを自己文書化する
Files:
UWBViewerSystemTests/SimpleCalibrationViewModelTests.swift
🧬 Code graph analysis (1)
UWBViewerSystemTests/SimpleCalibrationViewModelTests.swift (1)
UWBViewerSystem/Presentation/Scenes/FloorMapTab/SystemCalibrationPage/SimpleCalibrationViewModel.swift (1)
loadInitialData(164-168)
🔇 Additional comments (12)
UWBViewerSystemTests/SimpleCalibrationViewModelTests.swift (12)
66-72: テストの独立性が保たれていますUserDefaultsをクリアしてからテストデータを設定するアプローチが良いです。テスト間の相互依存を避けることができています。
78-92: ポーリングベースの待機戦略が効果的です固定スリープから動的ポーリングに変更することで、テストの信頼性が向上しています。待機時間（2秒）とポーリング間隔（0.05秒）の設定も適切です。
177-193: 初期状態の安定性チェックが改善されていますポーリングベースで状態の安定性を確認するアプローチが優秀です。非同期処理の競合状態を適切に回避できています。
219-233: エラーハンドリングテストの堅牢性が向上不正なJSONデータに対するエラーハンドリングをポーリングベースで確認することで、テストの信頼性が向上しています。
357-363: 統合テストでのデータ設定が一貫している他のテストケースと同様にUserDefaultsをクリアしてからデータを設定するパターンを使用していて一貫性があります。
369-383: 統合テストでのポーリング戦略統合テストでも同様のポーリング戦略を使用していて一貫性があります。待機時間も他のテストと統一されています。
431-440: MockDataRepositoryの初期化にawaitが追加されていますPR目標で言及されているMockDataRepositoryの初期化でのawait追加が実装されています。これにより非同期処理の競合が解決されています。
441-454: 初期状態確認の改善ポーリングベースで初期状態を確認することで、NotificationCenter通知処理との競合を回避できています。
464-482: リアルタイム更新テストの大幅改善PR目標で言及されている以下の改善が実装されています：
- 待機時間を0.5秒から調整（実際は最大2秒のポーリング）
- MainActorコンテキストでの状態確認（await viewModel.currentFloorMapInfo）
- 詳細なエラーメッセージの追加
- NotificationCenter通知と手動データ読み込みの競合解消
これらの変更により
testRealTimeUpdateWhenUserDefaultsChangesテストの信頼性が大幅に向上しています。
477-481: 詳細なエラーメッセージでデバッグ性向上フロアマップ情報の更新確認で、具体的なエラーメッセージを追加することでテスト失敗時の原因特定が容易になっています。PR目標の「エラーメッセージを追加してテスト失敗時の原因特定を容易化」が実現されています。
1-484: 全体的な改善により目標達成このPRの修正により、Issue #4で失敗していた
testRealTimeUpdateWhenUserDefaultsChangesテストの非同期処理が大幅に改善されています：
- 非同期処理の改善: ポーリングベースの待機戦略による確実な処理完了待機
- テストロジックの強化: MockDataRepository初期化でのawait追加、MainActorでの明示的状態確認
- 競合状態の解決: NotificationCenter通知と手動データ読み込みの競合解消
- デバッグ性向上: 詳細なエラーメッセージによる失敗原因の特定容易化
コーディングガイドラインに従って、不要なコードの削除やimport文の整理も適切に行われています。
260-275: バリデーション仕様の確認（完了）
SimpleCalibrationViewModel に id/name のトリム後空文字チェックと width>0 / depth>0 の guard によるバリデーションが実装されており、SwiftDataRepository 側でも同等の検証と明示的なエラー投出が確認できました（SimpleCalibrationViewModel.swift:316–323、SwiftDataRepository.swift:547–556）。
テスト修正完了のご報告@coderabbitai レビューコメントありがとうございます。 修正完了した内容Issue #4で失敗していた 主な修正内容:
技術的改善点:
テスト結果・CI状況
解決した問題修正前:
修正後:
Issue #4のテスト失敗問題を完全に解決し、すべてのテストが安定して成功しています。 ご確認のほど、よろしくお願いいたします。 |
|
Tip For best results, initiate chat on the files or code changes. @harutiro お疲れ様でした！Issue #4のテスト修正が完全に完了したとのご報告、ありがとうございます。 素晴らしい修正内容ですね： ✨ 修正のハイライト
特に、待機時間の調整（0.5秒→1.5秒）とMainActorコンテキストでの明示的状態確認は、非同期テストでよくある問題に対する適切なソリューションです。 🎉 All checks passing の状態で、テストが安定して成功していることが確認できて素晴らしいです！ この修正により、今後同様の非同期処理に関するテスト問題を防ぐことができ、プロジェクト全体の品質向上に貢献していますね。 お疲れ様でした！ ✏ Learnings added
If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
概要
Issue #4で失敗していた
testRealTimeUpdateWhenUserDefaultsChangesテストの修正を実装しました。主な修正内容
1. 非同期処理の改善
2. テストロジックの強化
awaitを追加3. 競合状態の解決
テスト結果
testRealTimeUpdateWhenUserDefaultsChangesテスト成功技術的詳細
修正前の問題
修正後の改善
🤖 Generated with Claude Code
Summary by CodeRabbit