Skip to content
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

Feat/#145: 채팅방 테스트 코드 추가 #152

Merged
merged 13 commits into from
Oct 7, 2024

Conversation

hyunn522
Copy link
Member

@hyunn522 hyunn522 commented Sep 13, 2024

📝 요약

🔖 변경 사항

repository, service, controller 단에서 ChatRoom 도메인에 대한 테스트 코드 추가

  • repository단 @DataJpaTest : 인메모리 h2 db로 테스트
  • service단 Mockito : mock 객체를 통해 필요한 의존성 주입
  • controller단 @WebMvcTest : 컨트롤러단 단위 테스트를 위해 프로젝트의 모든 빈을 등록하는 @SpringBootTest 대신 사용

✅ 리뷰 요구사항

실패 테스트가 필요한 부분들은 다른 브랜치에서 진행할 예정입니다.
컨트롤러단에서 interceptor를 통과하기 위해 JwtLoginAuthInterceptor, UserSpaceValidationInterceptor의 모킹 객체를, ArguementResolver의 반환값을 사용하기 위해 JwtLoginAuthHandlerArgumentResolver, UserSpaceIdHandlerArgumentResolver, UserSpaceAuthHandlerArgumentResolver의 모킹 객체를 추가했습니다.

📸 확인 방법 (선택)



📌 PR 진행 시 이러한 점들을 참고해 주세요

* P1 : 꼭 반영해 주세요 (Request Changes) - 이슈가 발생하거나 취약점이 발견되는 케이스 등
* P2 : 반영을 적극적으로 고려해 주시면 좋을 것 같아요 (Comment)
* P3 : 이런 방법도 있을 것 같아요~ 등의 사소한 의견입니다 (Chore)

@hyunn522 hyunn522 self-assigned this Sep 13, 2024
@hyunn522 hyunn522 linked an issue Sep 13, 2024 that may be closed by this pull request
@hyunn522 hyunn522 requested review from seongjunnoh, drbug2000 and arkchive and removed request for seongjunnoh September 13, 2024 09:32
Comment on lines +315 to +322
private ChatRoom createChatRoom(String name, Long id) {
ChatRoom chatRoom = new ChatRoom();
ReflectionTestUtils.setField(chatRoom, "id", id);
ReflectionTestUtils.setField(chatRoom, "status", "ACTIVE");
ReflectionTestUtils.setField(chatRoom, "createdAt", mockTime);
ReflectionTestUtils.setField(chatRoom, "name", name);
return chatRoom;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ReflectionTestUtils를 사용해서 객체 생성하는 방법 좋은 것 같습니다.
Entity의 Id 값을 어떻게 설정 해줄지 고민이었는데, reflection 사용하는 방법으로 저도 test 코드 짜야겠습니다

Copy link
Collaborator

@seongjunnoh seongjunnoh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

테스트 코드 작성하시느라 고생많았습니다!!
제가 이상하게 많이 만들어놓은 util들을 정리하면 의존성에서 한결 가벼운 테스트코드로 바꿀 수 있지 않을까 싶네여 하하,,
그리고 하나의 메서드가 가진 책임이 많아 유닛 테스트임에도 코드가 무거워지는것을 고민하시는것같은데, 메서드의 책임을 해당 도메인 객체로 위임하고, 그 도메인의 public 메서드에 대한 유닛테스트를 진행하면 서비스단 테스트 코드가 더 간결해지지 않을까 싶습니다.
위에 상준님께서도 언급해주신 entity의 id 값을 설정하는 부분은 저도 좀 찾아보고 고민해보겠습니다!

Copy link
Collaborator

@seongjunnoh seongjunnoh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@hyunn522
Copy link
Member Author

hyunn522 commented Sep 20, 2024

테스트 코드 작성하시느라 고생많았습니다!! 제가 이상하게 많이 만들어놓은 util들을 정리하면 의존성에서 한결 가벼운 테스트코드로 바꿀 수 있지 않을까 싶네여 하하,, 그리고 하나의 메서드가 가진 책임이 많아 유닛 테스트임에도 코드가 무거워지는것을 고민하시는것같은데, 메서드의 책임을 해당 도메인 객체로 위임하고, 그 도메인의 public 메서드에 대한 유닛테스트를 진행하면 서비스단 테스트 코드가 더 간결해지지 않을까 싶습니다. 위에 상준님께서도 언급해주신 entity의 id 값을 설정하는 부분은 저도 좀 찾아보고 고민해보겠습니다!

도메인 객체로 책임 위임하는 거 좋은 것 같아요! 다른 브랜치에서 진행해보겠습니다
id 설정하는 부분은 setter 사용을 지양하는 것이 좋다고 알고 있어서 ReflectionTestUtils를 사용했습니다 더 좋은 방식 발견하시면 공유해주세요~

@seongjunnoh
Copy link
Collaborator

테스트 코드 작성하시느라 고생많았습니다!! 제가 이상하게 많이 만들어놓은 util들을 정리하면 의존성에서 한결 가벼운 테스트코드로 바꿀 수 있지 않을까 싶네여 하하,, 그리고 하나의 메서드가 가진 책임이 많아 유닛 테스트임에도 코드가 무거워지는것을 고민하시는것같은데, 메서드의 책임을 해당 도메인 객체로 위임하고, 그 도메인의 public 메서드에 대한 유닛테스트를 진행하면 서비스단 테스트 코드가 더 간결해지지 않을까 싶습니다. 위에 상준님께서도 언급해주신 entity의 id 값을 설정하는 부분은 저도 좀 찾아보고 고민해보겠습니다!

도메인 객체로 책임 위임하는 거 좋은 것 같아요! 다른 브랜치에서 진행해보겠습니다 id 설정하는 부분은 setter 사용을 지양하는 것이 좋다고 알고 있어서 ReflectionTestUtils를 사용했습니다 더 좋은 방식 발견하시면 공유해주세요~

음 이부분은 저희가 현재 서비스단의 테스트를 영속성 계층을 Mock 처리함으로써 단위 테스트 느낌으로 서비스단을 테스트하기 때문에 발생하는 문제라고 생각합니다.
물론 이럴 경우 @SpringBootTest 를 사용하는 통합 테스트보다 테스트 속도는 빠를수 있지만, 실제 서비스단의 비즈니스 로직에서 사용하는 엔티티의 id값을 외부에서 주입해주는게 과연 의미있는 테스트인가라는 생각이 듭니다
그리고 레포지토리 테스트를 통해서 레포지토리의 동작을 보장할 수 있다고해서 이것들을 Mock 처리 하기보다는, 서비스단의 비즈니스 로직 상에서 실제 객체들이 db에 저장되고, 이 객체들을 불러와서 수행하는 비즈니스 로직을 테스트 하는 것이 더 의미있지 않을까라고 생각합니다

따라서 서비스단의 테스트 코드를 @SpringBootTest 를 이용한 통합 테스트로 바꿔보면 어떨까요? 영속성 계층을 스프링 빈으로 주입하고, given 단에 선언한 테스트 데이터들을 실제 repo에 save & find 하면 엔티티의 id를 처리하는 고민도 해결될 것 같습니다!!

관련된 키워드로 classicist vs mockist 가 있다고 합니다

괜찮은 블로그 링크도 하나 첨부하겠습니다!
https://jamesblog95.tistory.com/entry/Mockist-vs-Classicist

@hyunn522 hyunn522 merged commit 504d56d into develop Oct 7, 2024
3 checks passed
@hyunn522 hyunn522 deleted the feat/#145/채팅방-테스트-코드-추가 branch October 7, 2024 08:03
seongjunnoh pushed a commit that referenced this pull request Oct 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feat: 채팅방 테스트 코드 추가
3 participants