Usecase 들 UnitTest #346
Open
Usecase 들 UnitTest #346
Conversation
This file contains 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
HamBP
reviewed
Dec 19, 2023
Comment on lines
+11
to
+23
| fun <T> Flow<T>.throttleFirst(windowDuration: Long): Flow<T> { | ||
| var job: Job = Job().apply { complete() } | ||
|
|
||
| return onCompletion { job.cancel() }.run { | ||
| flow { | ||
| coroutineScope { | ||
| collect { value -> | ||
| if (!job.isActive) { | ||
| emit(value) | ||
| job = launch { delay(windowDuration) } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
바뀐 코드가 의도를 이해하기 어려운 것 같아.
테스트 코드 때문이라면, 테스트를 작성하기 위해 기존보다 어려운 코드를 작성하는 게 옳은 일인지 생각해봐야 할 것 같아.
HamBP
reviewed
Dec 19, 2023
|
|
||
| `when`("windowDuration 을 400 만큼 주면") { | ||
| val result = mutableListOf<Int>() | ||
| runTest { |
HamBP
reviewed
Dec 19, 2023
| val result = mutableListOf<Int>() | ||
| runTest { | ||
| testFlow.throttleFirst(400) | ||
| .onEach { result.add(it) } |
There was a problem hiding this comment.
testFlow가 cold flow라 List로 변환하는 방법으로도 테스트할 수 있을 것 같아
HamBP
reviewed
Dec 19, 2023
There was a problem hiding this comment.
비즈니스 로직에 대한 테스트의 given, when, then은 비개발자도 이해할 수 있게 작성하면 좋을 것 같아.
"accessToken가 존재한다", "true를 반환한다"를 비즈니스 로직에 가까운 말로 바꾸면, 로그인 기록이 존재한다, 자동 로그인에 성공한다와 같이 변경할 수 있지 않을까?
HamBP
reviewed
Dec 19, 2023
Comment on lines
+29
to
+36
| `when`("accessToken이 존재 한다면") { | ||
| val validToken = "Bearer ~~" | ||
| coEvery { userTokenRepository.getAccessToken() } returns validToken | ||
| coEvery { authRepository.verifyToken(validToken) } returns true | ||
| val result = automaticallyLoginUseCase.invoke() | ||
|
|
||
| then("true 를 반환 한다") { result shouldBe true } | ||
| } |
There was a problem hiding this comment.
accessToken이 존재하면 유효하지 않은 토큰이라도 로그인에 성공하는 걸까?
HamBP
reviewed
Dec 19, 2023
Comment on lines
+14
to
+37
| given("GetPlaylistUsecase 를 호출 하는 상황에서") { | ||
| val playlistRepository: PlaylistRepository = mockk() | ||
| val getPlaylistUseCase = GetPlaylistUseCase(playlistRepository) | ||
| val dummyRecentPlaylist = listOf( | ||
| Music( | ||
| id = "odio", | ||
| title = "dis", | ||
| artist = "epicurei", | ||
| imageUrl = "https://duckduckgo.com/?q=dolorum", | ||
| musicUrl = "https://search.yahoo.com/search?p=volutpat" | ||
| ) | ||
| ) | ||
|
|
||
| val dummyNormalPlaylist = listOf( | ||
| Music( | ||
| id = "feugiat", | ||
| title = "fabellas", | ||
| artist = "adolescens", | ||
| imageUrl = "https://www.google.com/#q=sollicitudin", | ||
| musicUrl = "http://www.bing.com/search?q=invenire" | ||
| ) | ||
| ) | ||
|
|
||
| `when`("playlistId이 RECENT_PLAYLIST_ID 이라면") { |
There was a problem hiding this comment.
given과 when이 바뀐 것 같아. given, when, then은 AAA패턴과 거의 유사해.
given(arrange) : 무엇이 주어졌을 때
when(act) : 어떤 행동을 하면
then(assert) : 결과
지금은 given에서 행동을 정의하고, when에서 값을 주는 거 같아
HamBP
reviewed
Dec 19, 2023
| emit(dummyNormalPlaylist) | ||
| } | ||
| val result = getPlaylistUseCase.invoke(playlistId = testPlaylistId).first() | ||
| then("getPlaylist 를 호출 한다") { result shouldBe dummyNormalPlaylist } |
There was a problem hiding this comment.
"getPlaylist를 호출한다"
- 화이트 박스 테스트임을 암시하고 있어. 블랙 박스 테스트로는 불가능할까?
- 설명하는 내용은 함수의 호출인데, 실제로는 반환값을 검증하고 있어.
HamBP
reviewed
Dec 19, 2023
| ) | ||
|
|
||
| then("recentMusics 들로 새로운 Playlist를 만들고 합치고, id로 정렬한 새로운 Playlist를 반환한다.") { | ||
| result.first() shouldBe excepted |
HamBP
reviewed
Dec 19, 2023
Comment on lines
+49
to
+61
| `when`("playlistId이 RECENT_PLAYLIST_ID 이라면") { | ||
| every { playlistRepository.getPlaylists() } returns flow { emit(dummyPlaylists) } | ||
| every { playlistRepository.getRecentPlaylist() } returns flow { emit(dummyRecentMusics) } | ||
| val result = getPlaylistsUseCase.invoke().first() | ||
| val excepted = Playlist( | ||
| id = RECENT_PLAYLIST_ID, | ||
| title = "최근 재생 목록", | ||
| thumbnailUrl = "https://duckduckgo.com/?q=dolorum", | ||
| trackSize = 2, | ||
| ) | ||
|
|
||
| then("recentMusics 들로 새로운 Playlist를 만들고 합치고, id로 정렬한 새로운 Playlist를 반환한다.") { | ||
| result.first() shouldBe excepted |
There was a problem hiding this comment.
재생 목록과 다른 재생목록들을 합쳐서 반환하는지를 검증하고 싶은 것 같은데, 최근 재생 목록을 검증하는 이유가 있을까?
HamBP
requested changes
Dec 19, 2023
|
좋은 의견 감사합니다~! 좀 더 개선해보겠습니다~ |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Issue
Overview
throttleFirstUnitTestAutomaticallyLoginUseCaseUnitTestGetPlaylistUseCaseUnitTestGetPlaylistsUseCaseUnitTest