[WTH-58] 게시판 및 댓글 테스트 코드 작성 #227
Conversation
Walkthrough테스트 리팩터링과 확장: NoticeUsecase 테스트를 Changes
Sequence Diagram(s)(생성 조건에 부합하지 않아 생략됨) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
시
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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: 1
🧹 Nitpick comments (2)
src/test/java/leets/weeth/domain/board/test/fixture/PostTestFixture.java (1)
26-38: Category 파라미터를 실제로 사용하도록 보완해주세요.
메서드 시그니처로Category category를 받고 있지만 내부에서는 언제나Category.Education으로 고정되어 호출자가 어떤 값을 넘기든 무시됩니다. 테스트 픽스처라도 시그니처와 구현이 불일치하면 추후 재사용 시 혼동을 줄 수 있으니 전달받은 값을 그대로 사용하도록 조정하는 편이 안전합니다.- .category(Category.Education) + .category(category)src/test/java/leets/weeth/domain/board/application/usecase/PostUseCaseImplTest.java (1)
146-147: 테스트 스텁에서 실제 SUT 메서드를 호출하지 않도록 정리해주세요.
given(... postUseCase.checkFileExistsByPost(...))/verify(... postUseCase.checkFileExistsByPost(...))처럼 스텁·검증 시점에 실제 구현을 호출하면, 설정 단계에서fileGetService모의 객체가findAllByPost를 두 번 이상 호출하게 되어 테스트가 내부 구현에 강하게 결합됩니다. 구현이 null을 반환하거나 호출 횟수를 검증하게 되는 순간 테스트가 쉽게 깨질 수 있으니, 먼저fileGetService에 대한 반환 값을 명시적으로 스텁한 뒤 단순한 불리언 리터럴이나 지역 변수로 매퍼 인자를 넘기는 쪽이 더 견고합니다.given(fileGetService.findAllByPost(post2.getId())).willReturn(List.of()); boolean fileExists = false; given(mapper.toAll(post2, fileExists)).willReturn(response2);이런 식으로 SUT 호출을 분리하면 검증 구문에서도 동일한
fileExists값을 재사용할 수 있습니다.Also applies to: 194-195, 209-210
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/test/java/leets/weeth/domain/board/application/usecase/NoticeUsecaseImplTest.java(5 hunks)src/test/java/leets/weeth/domain/board/application/usecase/PostUseCaseImplTest.java(1 hunks)src/test/java/leets/weeth/domain/board/domain/repository/NoticeRepositoryTest.java(3 hunks)src/test/java/leets/weeth/domain/board/domain/test/fixture/NoticeFixture.java(0 hunks)src/test/java/leets/weeth/domain/board/test/fixture/NoticeTestFixture.java(1 hunks)src/test/java/leets/weeth/domain/board/test/fixture/PostTestFixture.java(1 hunks)src/test/java/leets/weeth/domain/comment/application/usecase/NoticeCommentUsecaseImplTest.java(1 hunks)src/test/java/leets/weeth/domain/comment/test/fixture/CommentTestFixture.java(1 hunks)src/test/java/leets/weeth/domain/file/test/fixture/FileTestFixture.java(1 hunks)src/test/java/leets/weeth/domain/user/test/fixture/UserTestFixture.java(2 hunks)
💤 Files with no reviewable changes (1)
- src/test/java/leets/weeth/domain/board/domain/test/fixture/NoticeFixture.java
🧰 Additional context used
🧬 Code graph analysis (6)
src/test/java/leets/weeth/domain/board/application/usecase/PostUseCaseImplTest.java (6)
src/main/java/leets/weeth/domain/board/application/dto/PostDTO.java (1)
PostDTO(19-131)src/main/java/leets/weeth/domain/board/application/exception/CategoryAccessDeniedException.java (1)
CategoryAccessDeniedException(5-9)src/test/java/leets/weeth/domain/board/test/fixture/PostTestFixture.java (1)
PostTestFixture(14-75)src/test/java/leets/weeth/domain/file/test/fixture/FileTestFixture.java (1)
FileTestFixture(6-23)src/test/java/leets/weeth/domain/user/test/fixture/CardinalTestFixture.java (1)
CardinalTestFixture(6-45)src/test/java/leets/weeth/domain/user/test/fixture/UserTestFixture.java (1)
UserTestFixture(7-86)
src/test/java/leets/weeth/domain/comment/application/usecase/NoticeCommentUsecaseImplTest.java (5)
src/test/java/leets/weeth/domain/board/test/fixture/NoticeTestFixture.java (1)
NoticeTestFixture(8-48)src/main/java/leets/weeth/domain/comment/application/dto/CommentDTO.java (1)
CommentDTO(16-43)src/test/java/leets/weeth/domain/comment/test/fixture/CommentTestFixture.java (1)
CommentTestFixture(9-20)src/main/java/leets/weeth/domain/user/application/exception/UserNotMatchException.java (1)
UserNotMatchException(5-7)src/test/java/leets/weeth/domain/user/test/fixture/UserTestFixture.java (1)
UserTestFixture(7-86)
src/test/java/leets/weeth/domain/board/test/fixture/NoticeTestFixture.java (1)
src/test/java/leets/weeth/domain/board/domain/test/fixture/NoticeFixture.java (1)
NoticeFixture(6-23)
src/test/java/leets/weeth/domain/board/application/usecase/NoticeUsecaseImplTest.java (3)
src/test/java/leets/weeth/domain/board/test/fixture/NoticeTestFixture.java (1)
NoticeTestFixture(8-48)src/test/java/leets/weeth/domain/file/test/fixture/FileTestFixture.java (1)
FileTestFixture(6-23)src/test/java/leets/weeth/domain/user/test/fixture/UserTestFixture.java (1)
UserTestFixture(7-86)
src/test/java/leets/weeth/domain/board/test/fixture/PostTestFixture.java (1)
src/main/java/leets/weeth/domain/board/application/dto/PostDTO.java (1)
PostDTO(19-131)
src/test/java/leets/weeth/domain/board/domain/repository/NoticeRepositoryTest.java (1)
src/test/java/leets/weeth/domain/board/test/fixture/NoticeTestFixture.java (1)
NoticeTestFixture(8-48)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/test/java/leets/weeth/domain/user/test/fixture/UserTestFixture.java (1)
77-85: 일관성을 위해 파라미터 없는 버전 추가 고려다른 사용자 생성 메서드들(
createActiveUser1,createWaitingUser1등)은 모두 파라미터 없는 버전과 ID 파라미터 버전을 함께 제공합니다.createAdmin()에도 파라미터 없는 오버로드 메서드를 추가하면 픽스처의 일관성을 유지할 수 있습니다.다음과 같이 파라미터 없는 버전을 추가할 수 있습니다:
+ public static User createAdmin() { + return User.builder() + .name("적순") + .email("[email protected]") + .status(Status.ACTIVE) + .role(Role.ADMIN) + .build(); + } + public static User createAdmin(Long id) { return User.builder() .id(id) .name("적순") .email("[email protected]") .status(Status.ACTIVE) .role(Role.ADMIN) .build(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/test/java/leets/weeth/domain/user/test/fixture/UserTestFixture.java(2 hunks)
🔇 Additional comments (1)
src/test/java/leets/weeth/domain/user/test/fixture/UserTestFixture.java (1)
77-85: 이전 리뷰 이슈가 해결되었습니다!ID 설정이 올바르게 추가되었습니다 (Line 79). 이전 리뷰에서 지적된
.id(id)누락 문제가 해결되어 이제 메서드가 의도대로 동작합니다.
| Long noticeId = 1L; | ||
| Long userId = 1L; | ||
|
|
||
| User user = UserTestFixture.createActiveUser1(userId); |
There was a problem hiding this comment.
유저 Fixture 가져와서 사용해주셨네요 ! 굳👍🏻
| assertThat(notice.getTitle()).isEqualTo(dto.title()); | ||
| assertThat(notice.getContent()).isEqualTo(dto.content()); | ||
| } | ||
| } |
There was a problem hiding this comment.
given 단계에서 중복되는 객체설정은 (user, notice) , setUp() 메소드로 뺴놓아도 좋을 것 같아용
지금은 두 번 밖에 없지만! 추후에 검증이 추가된다면 고려해봐도 좋을 것 같습니다
There was a problem hiding this comment.
좋은 피드백 감사합니다! 하지만 이 테스트 클래스엔는 setUp()으로 공통 초기화 하는 것이 메소드마다 여러개의 Notice가 필요한지 단일 Notice가 필요한지가 달라서 사용하기에는 부적합한거같습니다!
| Comment comment = CommentTestFixture.createComment(commentId, dto.content(), user, notice); | ||
|
|
||
| given(userGetService.find(user2.getId())).willReturn(user2); | ||
| given(noticeFindService.find(notice.getId())).willReturn(notice); |
There was a problem hiding this comment.
이 부분은 댓글을 쓴사람과 관련이 있는 부분이라 noticeFindService가 필요한가요 ?
seokjun01
left a comment
seokjun01
left a comment
There was a problem hiding this comment.
고생하셨습니다! 전반적으로 코드가 짜임새가 좋으나 약간 길어서 😢
불필요한 verify는 제거하고, 중복을 줄이면 더 좋은 코드가 될 것 같습니다! 코멘트 확인해주세요 ~😄👍🏻
| // then | ||
| assertThat(response).isEqualTo(expectedResponse); | ||
|
|
||
| verify(noticeFindService).find(noticeId); |
There was a problem hiding this comment.
호출 여부까지는 전부 검증해줄 필요는 없을 것 같아욤
- 핵심 로직 외 verify 삭제
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/test/java/leets/weeth/domain/board/application/usecase/NoticeUsecaseImplTest.java (1)
193-230: 과도한 검증 호출을 줄이는 것을 고려해보세요.테스트가 잘 작성되었지만, 224-229번 라인의 모든 메서드 호출을 검증하는 것은 과도할 수 있습니다. 이전 리뷰에서도 "호출 여부까지는 전부 검증해줄 필요는 없을 것 같아욤"이라는 피드백이 있었습니다.
핵심 동작(파일 삭제, 저장, 업데이트)만 검증하고 나머지는 생략하는 것을 권장합니다. 예를 들어:
- verify(noticeFindService).find(noticeId); - verify(fileGetService).findAllByNotice(noticeId); verify(fileDeleteService).delete(oldFiles); - verify(fileMapper).toFileList(dto.files(), notice); verify(fileSaveService).save(newFiles); verify(noticeUpdateService).update(notice, dto);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/test/java/leets/weeth/domain/board/application/usecase/NoticeUsecaseImplTest.java(5 hunks)src/test/java/leets/weeth/domain/board/application/usecase/PostUseCaseImplTest.java(1 hunks)src/test/java/leets/weeth/domain/comment/application/usecase/NoticeCommentUsecaseImplTest.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/test/java/leets/weeth/domain/comment/application/usecase/NoticeCommentUsecaseImplTest.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/test/java/leets/weeth/domain/board/application/usecase/NoticeUsecaseImplTest.java (3)
src/test/java/leets/weeth/domain/board/test/fixture/NoticeTestFixture.java (1)
NoticeTestFixture(8-48)src/test/java/leets/weeth/domain/file/test/fixture/FileTestFixture.java (1)
FileTestFixture(6-23)src/test/java/leets/weeth/domain/user/test/fixture/UserTestFixture.java (1)
UserTestFixture(7-87)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (5)
src/test/java/leets/weeth/domain/board/application/usecase/NoticeUsecaseImplTest.java (2)
39-49: 목 주입 방식 개선이 잘 적용되었습니다.
@InjectMocks와@Mock어노테이션을 사용하여 테스트 코드가 더 명확하고 간결해졌습니다.
233-247: 엔티티 메서드 테스트가 적절합니다.Notice 엔티티의
update()메서드를 직접 테스트하여 제목과 내용이 정상적으로 변경되는지 검증하고 있습니다.src/test/java/leets/weeth/domain/board/application/usecase/PostUseCaseImplTest.java (3)
67-94: 교육 게시글 저장 테스트가 잘 작성되었습니다.given-when-then 구조가 명확하고, 필요한 검증이 적절하게 이루어지고 있습니다.
232-263: 권한 검증 로직이 잘 테스트되었습니다.사용자가 해당 기수에 속하지 않을 때 빈 리스트를 반환하는 것을 확인하고,
never()를 사용하여 불필요한 메서드 호출이 없음을 검증하고 있습니다.
265-279: 파일 존재 여부 확인 테스트가 명확합니다.간단하고 명확하게 파일 존재 여부를 검증하고 있습니다.
📌 PR 내용
🔍 PR 세부사항
📸 관련 스크린샷
x
📝 주의사항
x
✅ 체크리스트
Summary by CodeRabbit
Tests
Fixtures
참고: 내부 테스트 개선으로 사용자 기능에는 영향 없습니다.
✏️ Tip: You can customize this high-level summary in your review settings.