[Koin Project][Chore] DiningRepository UseCase 단위 테스트 추가#1289
Closed
kongwoojin wants to merge 2 commits intodevelopfrom
Closed
[Koin Project][Chore] DiningRepository UseCase 단위 테스트 추가#1289kongwoojin wants to merge 2 commits intodevelopfrom
kongwoojin wants to merge 2 commits intodevelopfrom
Conversation
Add FakeDiningRepository, GetDiningUseCaseTest, and GetNotOperationFilteredDiningUseCaseTest. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a fake Dining repository for tests and two test suites covering GetDiningUseCase and GetNotOperationFilteredDiningUseCase, exercising success, empty, filtering, edge-case, and repository-failure scenarios. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
4 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
domain/src/test/java/in/koreatech/koin/domain/repository/FakeDiningRepository.kt (1)
17-19: Make the fake date-aware to prevent false-positive tests.
getDining(date)currently ignoresdate, so tests won’t catch wrong date propagation from the use case. Consider storing the requested date (or data by date) and asserting it in tests.♻️ Minimal refactor example
class FakeDiningRepository : DiningRepository { private var fakeDinings: List<Dining> = emptyList() private var shouldThrow: Boolean = false + var lastRequestedDate: String? = null + private set @@ override suspend fun getDining(date: String): List<Dining> { + lastRequestedDate = date if (shouldThrow) throw RuntimeException("Network error") return fakeDinings } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@domain/src/test/java/in/koreatech/koin/domain/repository/FakeDiningRepository.kt` around lines 17 - 19, FakeDiningRepository's getDining(date: String) ignores the date causing false-positive tests; update the FakeDiningRepository to be date-aware by either (a) storing a Map<String, List<Dining>> (e.g., fakeDiningsByDate) provided in the constructor and returning fakeDiningsByDate[date] ?: emptyList(), or (b) record the lastRequestedDate property when getDining(date) is called and return data filtered by that date; keep the existing shouldThrow behavior (throw RuntimeException when shouldThrow is true) and expose the map or lastRequestedDate so tests can assert the requested date and returned data.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@domain/src/test/java/in/koreatech/koin/domain/usecase/dining/GetNotOperationFilteredDiningUseCaseTest.kt`:
- Around line 108-117: The test "미운영이 첫 번째 항목이 아닌 경우 제외되지 않는다" contradicts the
filtering rule that any menu containing "미운영" should be excluded; update the
test in GetNotOperationFilteredDiningUseCaseTest.kt to reflect that a dining
whose menu list contains "미운영" (even when not first) is filtered out by
GetNotOperationFilteredDiningUseCase: keep using makeDining(listOf("김치찌개",
"미운영")) and diningRepository.setFakeDinings(...), call
GetNotOperationFilteredDiningUseCase(diningRepository)("2024-01-01"), then
assert the call is successful and that result.getOrThrow().size equals 0 (or
otherwise assert the dining was excluded) and optionally rename the test to
indicate exclusion when "미운영" appears anywhere in the menu.
---
Nitpick comments:
In
`@domain/src/test/java/in/koreatech/koin/domain/repository/FakeDiningRepository.kt`:
- Around line 17-19: FakeDiningRepository's getDining(date: String) ignores the
date causing false-positive tests; update the FakeDiningRepository to be
date-aware by either (a) storing a Map<String, List<Dining>> (e.g.,
fakeDiningsByDate) provided in the constructor and returning
fakeDiningsByDate[date] ?: emptyList(), or (b) record the lastRequestedDate
property when getDining(date) is called and return data filtered by that date;
keep the existing shouldThrow behavior (throw RuntimeException when shouldThrow
is true) and expose the map or lastRequestedDate so tests can assert the
requested date and returned data.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
domain/src/test/java/in/koreatech/koin/domain/repository/FakeDiningRepository.ktdomain/src/test/java/in/koreatech/koin/domain/usecase/dining/GetDiningUseCaseTest.ktdomain/src/test/java/in/koreatech/koin/domain/usecase/dining/GetNotOperationFilteredDiningUseCaseTest.kt
Comment on lines
+108
to
+117
| fun `미운영이 첫 번째 항목이 아닌 경우 제외되지 않는다`() = runTest { | ||
| // "미운영"이 첫 번째 항목이 아니면 필터링 대상이 아님 | ||
| val dining = makeDining(listOf("김치찌개", "미운영")) | ||
| diningRepository.setFakeDinings(listOf(dining)) | ||
|
|
||
| val result = GetNotOperationFilteredDiningUseCase(diningRepository)("2024-01-01") | ||
|
|
||
| assertTrue(result.isSuccess) | ||
| assertEquals(1, result.getOrThrow().size) | ||
| } |
There was a problem hiding this comment.
Test expectation conflicts with non-operational filtering rule.
This case currently expects a dining containing "미운영" (in a non-first menu position) to remain included. If "미운영" marks non-operational meals, it should be excluded regardless of position.
🛠️ Suggested test expectation fix
- fun `미운영이 첫 번째 항목이 아닌 경우 제외되지 않는다`() = runTest {
- // "미운영"이 첫 번째 항목이 아니면 필터링 대상이 아님
+ fun `메뉴에 미운영이 포함된 식단은 위치와 무관하게 제외된다`() = runTest {
val dining = makeDining(listOf("김치찌개", "미운영"))
diningRepository.setFakeDinings(listOf(dining))
val result = GetNotOperationFilteredDiningUseCase(diningRepository)("2024-01-01")
assertTrue(result.isSuccess)
- assertEquals(1, result.getOrThrow().size)
+ assertTrue(result.getOrThrow().isEmpty())
}Based on learnings: Filter non-operational dining halls by excluding menus containing "미운영" in the dining module.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| fun `미운영이 첫 번째 항목이 아닌 경우 제외되지 않는다`() = runTest { | |
| // "미운영"이 첫 번째 항목이 아니면 필터링 대상이 아님 | |
| val dining = makeDining(listOf("김치찌개", "미운영")) | |
| diningRepository.setFakeDinings(listOf(dining)) | |
| val result = GetNotOperationFilteredDiningUseCase(diningRepository)("2024-01-01") | |
| assertTrue(result.isSuccess) | |
| assertEquals(1, result.getOrThrow().size) | |
| } | |
| fun `메뉴에 미운영이 포함된 식단은 위치와 무관하게 제외된다`() = runTest { | |
| val dining = makeDining(listOf("김치찌개", "미운영")) | |
| diningRepository.setFakeDinings(listOf(dining)) | |
| val result = GetNotOperationFilteredDiningUseCase(diningRepository)("2024-01-01") | |
| assertTrue(result.isSuccess) | |
| assertTrue(result.getOrThrow().isEmpty()) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@domain/src/test/java/in/koreatech/koin/domain/usecase/dining/GetNotOperationFilteredDiningUseCaseTest.kt`
around lines 108 - 117, The test "미운영이 첫 번째 항목이 아닌 경우 제외되지 않는다" contradicts the
filtering rule that any menu containing "미운영" should be excluded; update the
test in GetNotOperationFilteredDiningUseCaseTest.kt to reflect that a dining
whose menu list contains "미운영" (even when not first) is filtered out by
GetNotOperationFilteredDiningUseCase: keep using makeDining(listOf("김치찌개",
"미운영")) and diningRepository.setFakeDinings(...), call
GetNotOperationFilteredDiningUseCase(diningRepository)("2024-01-01"), then
assert the call is successful and that result.getOrThrow().size equals 0 (or
otherwise assert the dining was excluded) and optionally rename the test to
indicate exclusion when "미운영" appears anywhere in the menu.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR 개요
이슈 번호: #1288
PR 체크리스트
작업사항
작업사항의 상세한 설명
DiningRepository 관련 UseCase 단위 테스트 추가
FakeDiningRepository추가GetDiningUseCaseTest추가 (4개 테스트)GetNotOperationFilteredDiningUseCaseTest추가 (7개 테스트)논의 사항
스크린샷
추가내용
Summary by CodeRabbit