코드 리뷰 훔쳐보기: 검증부(assert/expect) 는 하드코딩한다.

코드 리뷰 훔쳐보기

최근 다른 사람이 받은 코드리뷰를 훔쳐볼때마다 하나씩 배워간다.

공부하기 싫을때는 이렇게 다른식으로 배움을 얻는것도 되게 좋은것 같다.

오늘 배운건 검증부는 하드코딩한다 라는 주제이다.

 

검증부는 하드코딩한다.

요즘 테스트코드를 적용해보는건 좋지만 너무 생각없이 작성하고 있었음을 느낀다.

예를 들자면 아래같은 코드다.

문제의 테스트코드

@DisplayName("로그인 요청 시 DB 정보와 일치하면 accessToken 을 발급한다.")
@Test
void signin() throws JsonProcessingException {
    // given
    Signup signup = Signup.builder()
        .email("hyukkind@naver.com")
        .name("midcon")
        .password("1234")
        .build();
    authService.signup(signup);

    LoginRequest request = LoginRequest.builder()
        .email("hyukkind@naver.com")
        .password("1234")
        .build();

    // when
    String name = authService.signin(request);
    User user = userRepository.findByEmail("hyukkind@naver.com")
        .orElseThrow(() -> new InvalidSigninInformationException());

    // then 이 검증부가 문제 !
    assertEquals(user.getName(), name);
}

생각없이 검증부를 아래처럼 작성하고 있었다.

assertEquals(user.getName(), name);

 

---

문제점

1. 무의미한 검증

이런 검증부는 별로 의미없는 테스트라고 생각이 된다.

이처럼 도메인 로직을 사용한 소프트 코딩은 사실상 프로덕션 코드를 복사 & 붙여넣기 한것이다.

굳이 userRepository.findByEmail 일 이유도 없고 signup.getName 이어도 상관없을 것이다.

 

2. 구현코드와 강결합

이처럼 도메인 로직을 사용한 테스트는 도메인 로직과 강결합을 한다는 문제점 또한 가진다.

만약 리팩토링을 하여 user.getName 로직이 바뀐다면 테스트 또한 모두 바꿔야한다.

검증하고자 하는 로직을 변경한다면 테스트를 바꿔야하는건 당연하지만

다른 도메인 로직을 바꿨을 때도 상관없는 다른 테스트 코드를 변경해야하는건 문제가 있다.

 

따라서 검증부는 아래처럼 하드코딩하여 테스트 목적에 맞는 값을 작성하는게 옳다.

수정한 검증부

// then
assertEquals("midcon", name);

 

이와 관련한 더 자세한 내용을 원한다면 아래 참고자료에 첨부한 향로님 블로그 글을 읽어봐도 도움이 될것이다.

 

---

참고자료

검증부 (assert / expect)는 하드코딩한다