Issue: (#147) 댓글 엔티티 리팩토링 및 테스트 코드 작성 #151
Conversation
Walkthrough테스트 코드에서 Mockito를 MockK로 전환하고, 주석 및 예외 검증을 강화하며, 삭제 기능에 대한 테스트를 추가했습니다. GitHub Actions 워크플로우는 main 브랜치로의 pull request 이벤트에도 트리거되도록 수정되었습니다. Changes
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/deploy.yml(1 hunks)src/test/kotlin/gomushin/backend/schedule/domain/facade/UpsertAndDeleteCommentFacadeTest.kt(2 hunks)src/test/kotlin/gomushin/backend/schedule/domain/service/CommentServiceTest.kt(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Sarang Backend CI/CD
src/test/kotlin/gomushin/backend/schedule/domain/service/CommentServiceTest.kt
[error] 494-494: Test failure in 'getById - 존재하지 않는 id로 조회 시 BadRequestException 발생'. org.opentest4j.AssertionFailedError caused by io.mockk.MockKException at line 176.
[error] 440-440: Test failure in '업데이트 실패 - 작성자가 일치하지 않을 때'. org.opentest4j.AssertionFailedError caused by io.mockk.MockKException at line 108.
🔇 Additional comments (12)
.github/workflows/deploy.yml (1)
6-7: PR 검증을 위한 트리거 추가가 적절합니다pull request 이벤트 트리거를 추가하여 PR 검증 시에도 CI/CD 파이프라인이 실행되도록 한 것은 좋은 개선사항입니다. 이를 통해 메인 브랜치로 병합하기 전에 테스트와 빌드를 확인할 수 있습니다.
src/test/kotlin/gomushin/backend/schedule/domain/facade/UpsertAndDeleteCommentFacadeTest.kt (7)
29-29: MockK 마이그레이션이 적절하게 수행되었습니다Mockito에서 MockK로의 마이그레이션이 올바르게 적용되었습니다.
@ExtendWith(MockKExtension::class)사용이 적절합니다.
46-52: SpringContextHolder 모킹 설정이 적절합니다
@BeforeEach에서SpringContextHolder와AppEnv를 모킹하는 설정이 올바르게 구현되었습니다. 테스트 환경에서 일관된 환경 ID를 제공합니다.
54-56: customUserDetails 초기화가 적절합니다
init블록에서customUserDetails의 ID를 고정값으로 설정한 것이 테스트의 일관성을 보장합니다.
75-83: MockK의 every 블록 사용이 적절합니다
commentService.upsert메서드 호출에 대한 모킹이eq()매처와 함께 올바르게 구현되었습니다. 매개변수 검증이 정확합니다.
106-134: delete 기능 테스트가 잘 작성되었습니다새로 추가된 delete 테스트가 필요한 모든 서비스 호출을 모킹하고 검증하고 있습니다. 테스트 로직이 명확하고 적절합니다.
136-162: 권한 검증 테스트가 적절합니다작성자가 아닌 사용자의 삭제 시도를 테스트하는 로직이 올바르게 구현되었습니다. 예외 메시지 검증도 포함되어 있습니다.
164-190: letterId 불일치 검증 테스트가 적절합니다편지 ID가 일치하지 않는 경우의 예외 처리 테스트가 올바르게 구현되었습니다. 에러 메시지 검증이 포함되어 있어 좋습니다.
src/test/kotlin/gomushin/backend/schedule/domain/service/CommentServiceTest.kt (4)
25-26: MockK 마이그레이션이 적절하게 수행되었습니다Mockito에서 MockK로의 마이그레이션이 올바르게 적용되었습니다.
@ExtendWith(MockKExtension::class)사용이 적절합니다.
34-40: SpringContextHolder 모킹 설정이 적절합니다
@BeforeEach에서SpringContextHolder와AppEnv를 모킹하는 설정이 다른 테스트 파일과 일관되게 구현되었습니다.
183-199: findAllByLetter 테스트가 적절합니다Letter 엔티티를 모킹하고 repository 호출을 검증하는 테스트가 올바르게 구현되었습니다.
201-244: 삭제 관련 테스트들이 적절합니다
delete,deleteAllByMemberId,deleteAllByLetterId메서드들에 대한 테스트가 모두 적절하게 구현되어 있습니다. repository 호출을 올바르게 검증하고 있습니다.
| @Test | ||
| fun update_failed_authorId_not_match() { | ||
| // given | ||
| val id = 1L | ||
| val comment = mockk<Comment>(relaxUnitFun = true) { | ||
| every { this@mockk.id } returns id | ||
| every { authorId } returns 1L | ||
| every { content } returns "기존 내용" | ||
| } | ||
| every { commentRepository.findByIdOrNull(id) } returns comment | ||
| every { commentService.getById(id) } returns comment | ||
|
|
||
| @DisplayName("댓글 생성 성공") | ||
| @Test | ||
| fun insert_success() { | ||
| // given | ||
| val letterId = 1L | ||
| val authorId = 1L | ||
| val nickname = "닉네임" | ||
| val upsertCommentRequest = UpsertCommentRequest( | ||
| commentId = null, | ||
| content = "내용" | ||
| // when | ||
| val exception = assertThrows<BadRequestException> { | ||
| commentService.upsert( | ||
| id = id, | ||
| letterId = 1L, | ||
| authorId = 2L, | ||
| nickname = "작성자", | ||
| upsertCommentRequest = mockk { every { content } returns "새 내용" } | ||
| ) | ||
| val mockComment = mock(Comment::class.java) | ||
| } | ||
| val errorMessage = exception.error.element.message.resolved | ||
|
|
||
| // then | ||
| assertEquals("댓글은 작성자만 삭제하거나 업데이트 할 수 있어요.", errorMessage) | ||
|
|
||
| } |
There was a problem hiding this comment.
테스트 실패 원인: MockK 설정 문제 해결 필요
파이프라인에서 이 테스트가 실패하고 있습니다. commentService.getById(id)를 모킹하는 것이 문제의 원인일 수 있습니다.
다음과 같이 수정하세요:
- every { commentService.getById(id) } returns comment이 라인을 제거하고 repository 모킹만 유지하세요. 서비스 클래스의 내부 메서드를 모킹하는 것은 좋지 않은 패턴입니다.
📝 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.
| @Test | |
| fun update_failed_authorId_not_match() { | |
| // given | |
| val id = 1L | |
| val comment = mockk<Comment>(relaxUnitFun = true) { | |
| every { this@mockk.id } returns id | |
| every { authorId } returns 1L | |
| every { content } returns "기존 내용" | |
| } | |
| every { commentRepository.findByIdOrNull(id) } returns comment | |
| every { commentService.getById(id) } returns comment | |
| @DisplayName("댓글 생성 성공") | |
| @Test | |
| fun insert_success() { | |
| // given | |
| val letterId = 1L | |
| val authorId = 1L | |
| val nickname = "닉네임" | |
| val upsertCommentRequest = UpsertCommentRequest( | |
| commentId = null, | |
| content = "내용" | |
| // when | |
| val exception = assertThrows<BadRequestException> { | |
| commentService.upsert( | |
| id = id, | |
| letterId = 1L, | |
| authorId = 2L, | |
| nickname = "작성자", | |
| upsertCommentRequest = mockk { every { content } returns "새 내용" } | |
| ) | |
| val mockComment = mock(Comment::class.java) | |
| } | |
| val errorMessage = exception.error.element.message.resolved | |
| // then | |
| assertEquals("댓글은 작성자만 삭제하거나 업데이트 할 수 있어요.", errorMessage) | |
| } | |
| @Test | |
| fun update_failed_authorId_not_match() { | |
| // given | |
| val id = 1L | |
| val comment = mockk<Comment>(relaxUnitFun = true) { | |
| every { this@mockk.id } returns id | |
| every { authorId } returns 1L | |
| every { content } returns "기존 내용" | |
| } | |
| every { commentRepository.findByIdOrNull(id) } returns comment | |
| // when | |
| val exception = assertThrows<BadRequestException> { | |
| commentService.upsert( | |
| id = id, | |
| letterId = 1L, | |
| authorId = 2L, | |
| nickname = "작성자", | |
| upsertCommentRequest = mockk { every { content } returns "새 내용" } | |
| ) | |
| } | |
| val errorMessage = exception.error.element.message.resolved | |
| // then | |
| assertEquals("댓글은 작성자만 삭제하거나 업데이트 할 수 있어요.", errorMessage) | |
| } |
🤖 Prompt for AI Agents
In
src/test/kotlin/gomushin/backend/schedule/domain/service/CommentServiceTest.kt
between lines 94 and 121, the test fails because commentService.getById(id) is
being mocked, which is an internal service method and should not be mocked.
Remove the mocking line for commentService.getById(id) and keep only the mocking
of commentRepository.findByIdOrNull(id) to fix the test.
| val comment = mockk<Comment>(relaxUnitFun = true) { | ||
| every { this@mockk.id } returns id | ||
| every { authorId } returns 1L | ||
| every { content } returns "기존 내용" | ||
| } | ||
| every { commentRepository.findByIdOrNull(id) } returns comment | ||
| every { commentService.getById(id) } returns comment | ||
|
|
||
| // when | ||
| `when`(commentRepository.findById(id)).thenReturn(Optional.of(mockComment)) | ||
| commentService.upsert(id, letterId, authorId, nickname, upsertCommentRequest) | ||
| // when | ||
| commentService.upsert( | ||
| id = id, | ||
| letterId = 1L, | ||
| authorId = 1L, | ||
| nickname = "작성자", | ||
| upsertCommentRequest = mockk { every { content } returns "새 내용" } | ||
| ) | ||
|
|
||
| // then | ||
| verify(mockComment).content = upsertCommentRequest.content | ||
| // then | ||
| verify { comment.content = "새 내용" } | ||
| } |
There was a problem hiding this comment.
MockK의 relaxUnitFun 사용과 모킹 순환 참조 문제
update_success 테스트에서 commentService.getById(id)를 모킹하고 있는데, 이는 테스트 대상 클래스 내부의 메서드를 모킹하는 것입니다. 이로 인해 순환 참조나 예상치 못한 동작이 발생할 수 있습니다.
다음과 같이 수정하는 것을 권장합니다:
- every { commentService.getById(id) } returns comment대신 commentRepository.findByIdOrNull(id)만 모킹하고, 실제 getById 메서드의 로직을 테스트하도록 변경하세요.
📝 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.
| val comment = mockk<Comment>(relaxUnitFun = true) { | |
| every { this@mockk.id } returns id | |
| every { authorId } returns 1L | |
| every { content } returns "기존 내용" | |
| } | |
| every { commentRepository.findByIdOrNull(id) } returns comment | |
| every { commentService.getById(id) } returns comment | |
| // when | |
| `when`(commentRepository.findById(id)).thenReturn(Optional.of(mockComment)) | |
| commentService.upsert(id, letterId, authorId, nickname, upsertCommentRequest) | |
| // when | |
| commentService.upsert( | |
| id = id, | |
| letterId = 1L, | |
| authorId = 1L, | |
| nickname = "작성자", | |
| upsertCommentRequest = mockk { every { content } returns "새 내용" } | |
| ) | |
| // then | |
| verify(mockComment).content = upsertCommentRequest.content | |
| // then | |
| verify { comment.content = "새 내용" } | |
| } | |
| val comment = mockk<Comment>(relaxUnitFun = true) { | |
| every { this@mockk.id } returns id | |
| every { authorId } returns 1L | |
| every { content } returns "기존 내용" | |
| } | |
| every { commentRepository.findByIdOrNull(id) } returns comment | |
| // when | |
| commentService.upsert( | |
| id = id, | |
| letterId = 1L, | |
| authorId = 1L, | |
| nickname = "작성자", | |
| upsertCommentRequest = mockk { every { content } returns "새 내용" } | |
| ) | |
| // then | |
| verify { comment.content = "새 내용" } | |
| } |
🤖 Prompt for AI Agents
In
src/test/kotlin/gomushin/backend/schedule/domain/service/CommentServiceTest.kt
around lines 72 to 91, remove the mocking of commentService.getById(id) to avoid
circular reference issues. Instead, only mock
commentRepository.findByIdOrNull(id) and let the actual getById method execute
during the test to properly verify the logic without unintended side effects.
| @DisplayName("실패 - getById - 존재하지 않는 id로 조회 시 BadRequestException 발생") | ||
| @Test | ||
| fun getById_notFound() { | ||
| // given | ||
| val id = 1L | ||
| every { commentRepository.findByIdOrNull(id) } returns null | ||
|
|
||
| // when & then | ||
| val exception = assertThrows<BadRequestException> { | ||
| commentService.getById(id) | ||
| } | ||
| val errorMessage = exception.error.element.message.resolved | ||
|
|
||
| assertEquals("댓글을 찾을 수 없어요.", errorMessage) | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
테스트 실패 원인: getById 메서드 테스트 수정 필요
파이프라인에서 이 테스트가 실패하고 있습니다. MockKException이 발생하는 것으로 보아 모킹 설정에 문제가 있을 수 있습니다.
테스트가 실제로 getById 메서드를 호출하도록 하고, 예외가 올바르게 발생하는지 확인하세요. 현재 설정으로는 MockK가 예외를 올바르게 처리하지 못할 수 있습니다.
다음과 같이 검증 스크립트를 실행해서 실제 실패 원인을 파악하세요:
🏁 Script executed:
#!/bin/bash
# Description: MockK 관련 테스트 실패 원인 분석을 위해 CommentService 구현체 확인
# CommentService의 getById 메서드 구현 확인
ast-grep --pattern 'fun getById($PARAM): Comment {
$$$
}'
# findByIdOrNull 사용 패턴 확인
rg -A 5 "findByIdOrNull" --type kotlinLength of output: 19581
🏁 Script executed:
#!/bin/bash
# Inspect CommentService.kt implementation around getById
rg -n "class CommentService" -A30 -B5 --type kotlin src/main/kotlinLength of output: 4002
🏁 Script executed:
#!/bin/bash
# CommentService의 getById 메서드 구현 확인
rg -n "fun getById" -C3 --type kotlin src/main/kotlin/gomushin/backend/schedule/domain/service/CommentService.ktLength of output: 311
🏁 Script executed:
#!/bin/bash
# Show top of CommentServiceTest.kt
sed -n '1,30p' src/test/kotlin/gomushin/backend/schedule/domain/service/CommentServiceTest.ktLength of output: 1214
테스트 실패 원인: Spring Data Extension 함수 findByIdOrNull 모킹 오류
commentRepository.findByIdOrNull(id)는 Kotlin 확장 함수이기 때문에 MockK가 기본적으로 인터셉트하지 못합니다.
따라서 every { commentRepository.findByIdOrNull(id) } … 로 스텁을 걸어도 실제 호출 시 예외(MockKException)가 발생합니다.
해결 방안:
- 테스트 클래스 상단 또는
@BeforeEach에mockkStatic("org.springframework.data.repository.findByIdOrNull")추가@BeforeEach fun setUp() { mockkStatic("org.springframework.data.repository.findByIdOrNull") }
- 스텁 구문을 확장 함수 호출 형태로 변경
every { findByIdOrNull(commentRepository, id) } returns mockComment - 또는 확장 함수가 내부에서 호출하는 원본 메서드(
findById)를 직접 스텁every { commentRepository.findById(id) } returns Optional.of(mockComment) // 성공 케이스 every { commentRepository.findById(id) } returns Optional.empty() // 실패 케이스
추가 확인:
commentService가@InjectMockKs로 제대로 주입되고 있는지 검증
[src/test/kotlin/gomushin/backend/schedule/domain/service/CommentServiceTest.kt]
- 상단에
mockkStatic추가 - 기존
every { commentRepository.findByIdOrNull(id) }→위 제안 중 하나로 교체
🤖 Prompt for AI Agents
In
src/test/kotlin/gomushin/backend/schedule/domain/service/CommentServiceTest.kt
around lines 167 to 181, the test mocks the Kotlin extension function
findByIdOrNull on commentRepository, which MockK cannot intercept by default,
causing MockKException. To fix this, add
mockkStatic("org.springframework.data.repository.findByIdOrNull") in a
@BeforeEach setup method, then replace the stub from every {
commentRepository.findByIdOrNull(id) } to either every {
findByIdOrNull(commentRepository, id) } or mock the underlying
repository.findById method directly with every { commentRepository.findById(id)
} returning Optional.empty() for the failure case. Also verify that
commentService is properly injected with @InjectMockKs.
✅ PR 유형
어떤 변경 사항이 있었나요?
📝 작업 내용
✏️ 관련 이슈
🎸 기타 사항 or 추가 코멘트
Summary by CodeRabbit
테스트
Chores