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: 단위 테스트 코드 추가 #133

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

Gyuhyeok99
Copy link

@Gyuhyeok99 Gyuhyeok99 commented Dec 27, 2024

관련 이슈

작업 내용

테스트코드

단위 테스트 코드 추가

  • 대학 추천 서비스
  1. 일반 추천 목록 반환 테스트
  2. 개인화 추천 목록 반환 테스트
  3. 개인화 추천이 섞이는지 검증
  4. 사용자 존재하지 않을 시 예외 처리 테스트
  5. 개인화 추천이 부족할 경우 일반 추천으로 보충하는 로직 검증
  • 대학 좋아요 서비스
  1. 특정 대학에 대해 사용자가 좋아요 상태인지 확인하는 테스트
  2. 좋아요 상태일 경우 True 반환, 좋아요 상태가 아닐 경우 False 반환 테스트
  3. 사용자가 대학 좋아요를 추가/취소하는 로직 검증
  4. 좋아요 추가 시 성공 메시지 반환, 좋아요 취소 시 취소 메시지 반환 테스트
  5. 존재하지 않는 대학 ID로 요청 시 예외 처리 테스트
  • 대학 조회 및 검색
  1. 대학 상세 조회 테스트
  2. 대학 검색 조건 테스트
  • 카카오 로그인 서비스
  1. 기존 회원 로그인 시 로그인 정보 반환 테스트
  2. 신규 회원 로그인 시 회원가입 정보 반환 테스트
  3. 탈퇴한 회원 로그인 시 탈퇴일 초기화 및 로그인 정보 반환 테스트
  4. 인증 코드 만료 시, 리다이렉트 URI 불일치 시, 카카오 사용자 정보 조회 실패 시 예외 반환 테스트
  • 회원가입 서비스
  1. 정상 회원가입 시 회원가입 성공 후 토큰 반환 테스트
  2. 중복된 닉네임/이메일로 회원가입 시 예외 반환 테스트
  3. 만료되거나 이미 사용된 카카오 토큰으로 회원가입 시 예외 반환 테스트
  • 인증 서비스
  1. 로그아웃 요청 시 리프레시 토큰 무효화 테스트
  2. 회원탈퇴 요청 시 탈퇴일자 설정 테스트
  3. 존재하지 않는 이메일로 회원탈퇴 시 예외 반환 테스트
  4. 유효한 리프레시 토큰으로 재발급 요청 시 새 엑세스 토큰 반환 테스트
  5. 만료된 리프레시 토큰으로 재발급 요청 시 예외 반환 테스트
  • 리팩토링
    1 .기존에는 service와 repository 폴더에 모든 테스트 코드가 혼재되어 있었으나, 이를 도메인별로 정리

특이 사항

  1. 대학 추천 서비스에서 개인화 추천 검증 로직이 셔플된 결과로 인해 간헐적으로 테스트가 실패할 수 있는 이슈가 있습니다.(셔플 후에도 순서가 동일한 경우)

리뷰 요구사항 (선택)

  1. 리팩토링된 테스트 코드 구조에 대한 개선 사항이 있다면 제안 부탁드립니다.
  2. 작성된 단위 테스트가 서비스 로직을 충분히 검증하고 있는지 확인 부탁드립니다.
  3. 단위 테스트 작성 방식에서 보완해야 할 점이 있다면 피드백 부탁드립니다.
  4. 현재 서비스 로직 관련 단위테스트만 작성했으며, Controller 및 Repository 레이어의 단위 테스트가 필요한지 궁금합니다.
  5. InterestedCountyRepository의 스펠링 오타를 발견하였는데 아직 수정하지 않았습니다.
사진

- 일반 추천 목록 반환 테스트 추가
- 개인화 추천 목록 반환 테스트 추가
  - 개인화 추천이 섞이는지 검증
- 사용자 존재하지 않을 시 예외 처리 테스트 추가
- 개인화 추천이 부족할 경우 일반 추천으로 보충하는 로직 검증 테스트 추가
- 특정 대학에 대해 사용자가 좋아요 상태인지 확인하는 테스트 추가
  - 좋아요 상태일 경우 True 반환 테스트
  - 좋아요 상태가 아닐 경우 False 반환 테스트
- 사용자가 대학 좋아요를 추가/취소하는 로직 검증 테스트 추가
  - 좋아요 추가 시 성공 메시지 반환 테스트
  - 좋아요 취소 시 취소 메시지 반환 테스트
- 존재하지 않는 대학 ID로 요청 시 예외 처리 테스트 추가
- 대학 상세 조회 테스트
  - 유효한 ID로 조회 시 대학 상세 정보 반환
  - 존재하지 않는 ID로 조회 시 예외 반환
- 대학 검색 조건 테스트
  - 조건 없이 검색 시 전체 대학 목록 반환
  - 특정 조건(region, keyword) 만족 시 필터링된 대학 목록 반환
  - 모든 조건(region, keyword, testType, testScore) 만족 시 필터링된 대학 목록 및 언어 요구 사항 확인
  - 조건 불충족 시 빈 결과 반환
- 서비스 및 레포지토리 테스트 코드를 도메인별로 분리하여 응집력 강화
  - 기존에는 service 및 repository 폴더에 모든 테스트 코드가 혼재
  - 도메인별로 테스트 코드를 정리하여 관련성 높은 코드끼리 묶음
- 기존 회원 로그인 시 로그인 정보 반환
- 신규 회원 로그인 시 회원가입 정보 반환
- 탈퇴한 회원 로그인 시 탈퇴일 초기화 및 로그인 정보 반환
- 인증 코드 만료 시 예외 반환
- 리다이렉트 URI 불일치 시 예외 반환
- 카카오 사용자 정보 조회 실패 시 예외 반환
- 정상 회원가입 시 회원가입 성공 후 토큰 반환
- 중복된 닉네임/이메일로 회원가입 시 예외 반환
- 만료되거나 이미 사용된 카카오 토큰으로 회원가입 시 예외 반환
- 로그아웃 요청 시 리프레시 토큰 무효화
- 회원탈퇴 요청 시 탈퇴일자 설정
- 존재하지 않는 이메일로 회원탈퇴 시 예외 반환
- 유효한 리프레시 토큰으로 재발급 요청시 새 엑세스 토큰 반환
- 만료된 리프레시 토큰으로 재발급 요청시 예외 반환
Copy link
Collaborator

@nayonsoso nayonsoso left a 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일지?"도 생각해보셨으면 좋겠습니다 ㅎ.ㅎ

Comment on lines +61 to +62
@Test
@DisplayName("로그아웃_요청시_리프레시_토큰을_무효화한다()")
Copy link
Collaborator

Choose a reason for hiding this comment

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

기존 코드 컨벤션을 지키지 않고 DisplayName 어노테이션을 사용한 이유가 있나요?

Comment on lines +71 to +77
// then & verify
verify(valueOperations).set(
eq(refreshTokenKey),
eq(SIGN_OUT_VALUE),
eq(604800000L),
eq(TimeUnit.MILLISECONDS)
);
Copy link
Collaborator

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 [세부 구현에 의존적인 테스트] 부분

Comment on lines +91 to +92
assertThat(testUser.getQuitedAt()).isNotNull();
assertThat(testUser.getQuitedAt()).isEqualTo(LocalDate.now().plusDays(1));
Copy link
Collaborator

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번 라인은 지우는게 좋겠습니다.

테스트 코드에서 가독성은 굉장히 중요한 요소라고 생각합니다. 테스트하고자 하는 것이 무엇인지, 그것을 검증하기 위해서 반드시 필요한 것들만 있는지 다시 생각해보시면 더 좋을 것 같아요.

Comment on lines +94 to +95
// verify
verify(siteUserRepository).getByEmail(testUser.getEmail());
Copy link
Collaborator

Choose a reason for hiding this comment

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

대부분의 테스트 코드에 verify 를 사용하신 이유가 무엇인지 듣고 싶네요!

Comment on lines +27 to +29
import static org.assertj.core.api.Assertions.*;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.Mockito.*;
Copy link
Collaborator

Choose a reason for hiding this comment

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

와일드카드 *는 사용하지 않는게 좋을 것 같네요!
인텔리제이에서 설정하는 방법은 링크를 참고하시면 됩니다.

https://giantdwarf.tistory.com/58

Comment on lines +162 to +164
);
}
}
Copy link
Collaborator

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/

Comment on lines +66 to +74
@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);
Copy link
Collaborator

Choose a reason for hiding this comment

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

BDD mockito 가 아니라 그냥 mockito 를 선호하시는 것 같은데, 특별한 이유가 있나요?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

단위 테스트가 작성되지 않은 곳에 테스트 코드 작성
2 participants