-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Test : CommentServiceTest & CommentLikeServiceTest 추가 #87
The head ref may contain hidden characters: "feature/#54_Test_Code_for_Comment(\uB313\uAE00)_Domain"
Test : CommentServiceTest & CommentLikeServiceTest 추가 #87
Conversation
- 부모댓글 검증하지 않고 댓글 작성되던 현상에 대한 예외처리 추가.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고하셨습니다.
엄청 꼼꼼하게 나눠서 작성해주셨네요 ㄷㄷ
리뷰 남긴 부분들 수정 + 몇가지 협의해야 할 내용 참고해주시면 감사하겠습니다.
orury-client/src/main/java/org/fastcampus/oruryclient/comment/error/CommentErrorCode.java
Outdated
Show resolved
Hide resolved
...-client/src/test/java/org/fastcampus/oruryclient/comment/service/CommentLikeServiceTest.java
Outdated
Show resolved
Hide resolved
...-client/src/test/java/org/fastcampus/oruryclient/comment/service/CommentLikeServiceTest.java
Show resolved
Hide resolved
...-client/src/test/java/org/fastcampus/oruryclient/comment/service/CommentLikeServiceTest.java
Show resolved
Hide resolved
orury-client/src/test/java/org/fastcampus/oruryclient/comment/service/CommentServiceTest.java
Outdated
Show resolved
Hide resolved
orury-client/src/test/java/org/fastcampus/oruryclient/comment/service/CommentServiceTest.java
Show resolved
Hide resolved
orury-client/src/test/java/org/fastcampus/oruryclient/comment/service/CommentServiceTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any() 사용하여 테스트 하셔도 되었는데 하나씩 고생하셨네요 수고하셨습니다~
@@ -95,6 +92,15 @@ public void isValidate(CommentLikeDto commentLikeDto) { | |||
throw new BusinessException(CommentErrorCode.FORBIDDEN); | |||
} | |||
|
|||
@Transactional(readOnly = true) | |||
public void validateParentComment(Long parentCommentId) { | |||
if (!parentCommentId.equals(NumberConstants.PARENT_COMMENT)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기에서 NumberConstants.PARENT_COMMENT가 현재 0L로 선언되어 있어, 비교했을 때 같지 않으면 대댓글이라는 것이니까 대댓글일 경우 아래의 로직을 수행한다고 이해했는데 맞나요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
넵넵 맞습니다.
해당 메서드에서는 부모댓글의 id를 받고 그것이 유효한 값인지를 판단해야 하는 건데,
0L이라면, 자녀댓글이 아닌, 부모댓글로 생성한다는 뜻이기 때문에 이해하신 바가 맞습니다!
@Mock | ||
private CommentRepository commentRepository; | ||
@InjectMocks | ||
private CommentLikeService commentLikeService; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
형준님이 작성한 테스트 코드에서는 @beforeeach로 초기 setup() 메소드 생성해서 mock 을 만들었는데, 민협님은 setup() 메소드 없이 어노테이션으로 만드셨군요,, 두 가지 mock 사용 방법 알아갑니다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
위 링크를 참고하시면, 수동으로 의존성을 주입한 이유가 조금 더 명확해 보입니다!
이 코드에서는 어떤 방법을 쓰는지 상관 없습니다. 그러나 모든 걸 Mocking하지 않고 스프링 컨테이너에 있는 객체를 그대로 주입하고 싶은 경우에는 @Autowired를 사용해야 하는데, 그러면 @Injectmocks가 인식하지 못합니다.
따라서 개인적인 의견으로는, 수동으로 명시하는 것(형준님 작성 방식)이 추후의 다른 코드와도 통일성 있는 나은 방법이라고 생각합니다!
mock 이해가 드디어 된 것 같네요 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM :)
개요
Test : CommentServiceTest & CommentLikeServiceTest 추가
Fix : validateParentComment 메서드 추가
Chore : Common 모듈에 java 패키지 추가
closed #54
closed #84
PR 유형
어떤 변경 사항이 있나요?
PR Checklist
PR이 다음 요구 사항을 충족하는지 확인하세요.