-
Notifications
You must be signed in to change notification settings - Fork 1
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: 단위 테스트 코드 추가 #133
base: main
Are you sure you want to change the base?
feat: 단위 테스트 코드 추가 #133
Conversation
- 일반 추천 목록 반환 테스트 추가 - 개인화 추천 목록 반환 테스트 추가 - 개인화 추천이 섞이는지 검증 - 사용자 존재하지 않을 시 예외 처리 테스트 추가 - 개인화 추천이 부족할 경우 일반 추천으로 보충하는 로직 검증 테스트 추가
- 특정 대학에 대해 사용자가 좋아요 상태인지 확인하는 테스트 추가 - 좋아요 상태일 경우 True 반환 테스트 - 좋아요 상태가 아닐 경우 False 반환 테스트 - 사용자가 대학 좋아요를 추가/취소하는 로직 검증 테스트 추가 - 좋아요 추가 시 성공 메시지 반환 테스트 - 좋아요 취소 시 취소 메시지 반환 테스트 - 존재하지 않는 대학 ID로 요청 시 예외 처리 테스트 추가
- 대학 상세 조회 테스트 - 유효한 ID로 조회 시 대학 상세 정보 반환 - 존재하지 않는 ID로 조회 시 예외 반환 - 대학 검색 조건 테스트 - 조건 없이 검색 시 전체 대학 목록 반환 - 특정 조건(region, keyword) 만족 시 필터링된 대학 목록 반환 - 모든 조건(region, keyword, testType, testScore) 만족 시 필터링된 대학 목록 및 언어 요구 사항 확인 - 조건 불충족 시 빈 결과 반환
- 서비스 및 레포지토리 테스트 코드를 도메인별로 분리하여 응집력 강화 - 기존에는 service 및 repository 폴더에 모든 테스트 코드가 혼재 - 도메인별로 테스트 코드를 정리하여 관련성 높은 코드끼리 묶음
- 기존 회원 로그인 시 로그인 정보 반환 - 신규 회원 로그인 시 회원가입 정보 반환 - 탈퇴한 회원 로그인 시 탈퇴일 초기화 및 로그인 정보 반환 - 인증 코드 만료 시 예외 반환 - 리다이렉트 URI 불일치 시 예외 반환 - 카카오 사용자 정보 조회 실패 시 예외 반환
- 정상 회원가입 시 회원가입 성공 후 토큰 반환 - 중복된 닉네임/이메일로 회원가입 시 예외 반환 - 만료되거나 이미 사용된 카카오 토큰으로 회원가입 시 예외 반환
- 로그아웃 요청 시 리프레시 토큰 무효화 - 회원탈퇴 요청 시 탈퇴일자 설정 - 존재하지 않는 이메일로 회원탈퇴 시 예외 반환 - 유효한 리프레시 토큰으로 재발급 요청시 새 엑세스 토큰 반환 - 만료된 리프레시 토큰으로 재발급 요청시 예외 반환
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.
우선 1차로 리뷰 드립니다! 궁금한 부분들에 대해서 질문 남겨놨습니다. 규혁님이 코드리뷰와 협업을 하고싶다 하셔서, '무엇이 좋은 테스트 코드일까?'를 물어보는 위주의 질문을 하게되었습니다. 그리고 이 PR에서만 이야기하기엔 부족할 것 같아서, 디스커션을 따로 만들었습니다. 이 디스커션들에 더 이야기해봐도 좋을 것 같습니다!
그리고 "1300줄의 PR이 좋은 PR일지?"도 생각해보셨으면 좋겠습니다 ㅎ.ㅎ
@Test | ||
@DisplayName("로그아웃_요청시_리프레시_토큰을_무효화한다()") |
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.
기존 코드 컨벤션을 지키지 않고 DisplayName 어노테이션을 사용한 이유가 있나요?
// then & verify | ||
verify(valueOperations).set( | ||
eq(refreshTokenKey), | ||
eq(SIGN_OUT_VALUE), | ||
eq(604800000L), | ||
eq(TimeUnit.MILLISECONDS) | ||
); |
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.
이 테스트 코드는 결과가 아니라 행위를 검증하고 있는 것 같네요, 테스트 코드는 세부 구현을 담으면 위험합니다. 예를 들어 '값을 저장하는 시간'을 604800000L 에서 604800001L로 변경한다면 위 테스트 코드는 깨질 것입니다. 저라면 리프레시 토큰이 무효화되었다는 결과만 검증할 것 같습니다. 관련 글을 첨부합니다!
https://ojt90902.tistory.com/1364 [테스트로 유출된 도메인 지식] 부분
https://www.youtube.com/watch?v=R7spoJFfQ7U [세부 구현에 의존적인 테스트] 부분
assertThat(testUser.getQuitedAt()).isNotNull(); | ||
assertThat(testUser.getQuitedAt()).isEqualTo(LocalDate.now().plusDays(1)); |
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.
중복 검증을 하고 있는 것 같습니다😔
assertThat(testUser.getQuitedAt()).isEqualTo(LocalDate.now().plusDays(1));
가 곧 not null 이라는 것을 포함하고 있지 않나요? 91번 라인은 지우는게 좋겠습니다.
테스트 코드에서 가독성은 굉장히 중요한 요소라고 생각합니다. 테스트하고자 하는 것이 무엇인지, 그것을 검증하기 위해서 반드시 필요한 것들만 있는지 다시 생각해보시면 더 좋을 것 같아요.
// verify | ||
verify(siteUserRepository).getByEmail(testUser.getEmail()); |
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.
대부분의 테스트 코드에 verify 를 사용하신 이유가 무엇인지 듣고 싶네요!
import static org.assertj.core.api.Assertions.*; | ||
import static org.junit.jupiter.api.Assertions.assertThrows; | ||
import static org.mockito.Mockito.*; |
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.
와일드카드 *
는 사용하지 않는게 좋을 것 같네요!
인텔리제이에서 설정하는 방법은 링크를 참고하시면 됩니다.
); | ||
} | ||
} |
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.
파일 끝 개행은 왜 해야하는걸까요?
https://hyeon9mak.github.io/github-no-newline-at-a-end-of-file/
@Test | ||
@DisplayName("기존_회원이_로그인하면_로그인_정보를_반환한다()") | ||
void shouldReturnSignInInfoWhenUserAlreadyRegistered() { | ||
// given | ||
when(kakaoOAuthClient.processOauth(VALID_CODE)).thenReturn(testKakaoUserInfo); | ||
when(siteUserRepository.existsByEmail(testUser.getEmail())).thenReturn(true); | ||
when(siteUserRepository.getByEmail(testUser.getEmail())).thenReturn(testUser); | ||
when(tokenService.generateToken(testUser.getEmail(), TokenType.ACCESS)).thenReturn(TEST_ACCESS_TOKEN); | ||
when(tokenService.generateToken(testUser.getEmail(), TokenType.REFRESH)).thenReturn(TEST_REFRESH_TOKEN); |
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.
BDD mockito 가 아니라 그냥 mockito 를 선호하시는 것 같은데, 특별한 이유가 있나요?
관련 이슈
작업 내용
단위 테스트 코드 추가
1 .기존에는 service와 repository 폴더에 모든 테스트 코드가 혼재되어 있었으나, 이를 도메인별로 정리
특이 사항
리뷰 요구사항 (선택)