테스트가 실패할 때, 무엇을 고쳐야 하는가 — 4개의 실패 테스트가 알려준 것

리팩토링 과제 마지막 단계에서 만난 4개의 실패 테스트. 그중 3개는 테스트 코드를, 1개는 서비스 로직을 고쳐야 했습니다. 그 구분이 어떻게 결정되는지 정리했습니다.

들어가며

Expert 프로젝트 리팩토링의 마지막 단계는 "테스트 정합성 확보"였다. 깨져 있는 테스트 4개를 정상 상태로 만들어야 했다.

처음엔 단순한 작업으로 보였다. 테스트가 실패하면 테스트를 고치면 되니까. 그런데 케이스를 하나씩 들여다보면서, "테스트가 실패한다"는 같은 증상이 네 가지 서로 다른 원인을 갖고 있다는 걸 알게 됐다.

그중 한 케이스는 "테스트 코드가 맞고, 서비스 로직을 고쳐야 하는" 경우였다.

이 글은 그 네 가지를 어떻게 구분했는지에 대한 기록이다.

케이스 1: 인자 순서가 뒤바뀐 단순 오류

@Test
void matches_메서드가_정상적으로_동작한다() {
    String rawPassword = "raw";
    String encodedPassword = passwordEncoder.encode(rawPassword);

    boolean result = passwordEncoder.matches(encodedPassword, rawPassword);
    //                                       ^^^^^^^^^^^^^^^  ^^^^^^^^^^^
    //                                       인자 순서가 반대

    assertTrue(result);
}

PasswordEncoder.matches(rawPassword, encodedPassword) 시그니처에서 인자 순서가 뒤바뀌어 있었다.

테스트가 검증하려는 의도("encode 후 matches로 비교하면 true")는 명확하고 그 의도는 옳다. 잘못된 건 그 의도를 구현한 코드 한 줄이다.

→ 테스트 코드 수정.

가장 단순한 케이스. 의사결정할 게 거의 없다.

케이스 2: 메서드명과 실제 검증이 어긋난 경우

@Test
void manager_목록_조회_시_Todo가_없다면_NPE_에러를_던진다() {
    // ...
    assertThrows(InvalidRequestException.class, () ->
        managerService.getManagers(invalidTodoId)
    );
}

메서드명은 NPE_에러를_던진다인데 실제 검증 대상은 InvalidRequestException이다. 그런데 어느 쪽이 맞는가?

서비스 코드를 봤다. Todo를 찾지 못하면 InvalidRequestException을 던지고 있었다. 실제 동작과 메서드명이 어긋난 상태.

여기서 한 번 더 결정해야 한다.

• A) 서비스를 NPE로 던지도록 바꾼다. 도메인 예외를 죽이는 나쁜 선택.
• B) 메서드명을 실제 검증과 일치하도록 바꾼다. 올바른 선택.

→ 테스트 코드(메서드명) 수정.

@Test
void manager_목록_조회_시_Todo가_없다면_InvalidRequestException을_던진다() {
    // ...
}

여기서 배운 것: 메서드명은 테스트의 의도를 설명하는 문서다. 메서드명과 검증 내용이 다르면, 둘 중 하나가 거짓말을 하고 있는 거다. 보통은 메서드명이 옛 의도의 잔재로 남아 있는 경우가 많다 — 처음엔 정말 NPE를 던지는 코드였다가, 도메인 예외로 개선되는 과정에서 테스트 이름이 따라오지 못한 거다.

케이스 3: mock setup이 실제 흐름과 어긋난 경우

@Test
void comment_등록_중_할일을_찾지_못해_에러가_발생한다() {
    given(todoRepository.findById(any())).willReturn(Optional.empty());
    // ...
}

실제 서비스 코드는 todoRepository.findByIdWithUser() 같은 다른 메서드를 호출하고 있다면? findById에 대한 stubbing은 매칭되지 않는다. 결과적으로 mock이 기본값(빈 Optional 또는 null)을 반환하지만, 테스트가 의도한 시나리오와 다른 경로로 흘러간다.

이 경우 무엇이 맞는가?

서비스 로직이 의도적으로 findByIdWithUser를 쓰고 있다면, 그건 N+1 회피나 다른 이유로 선택된 합리적인 설계다. 테스트가 mock stubbing을 실제 호출에 맞춰 갱신해야 한다.

→ 테스트 코드 수정.

given(todoRepository.findByIdWithUser(any())).willReturn(Optional.empty());

이 케이스에서 한 번 더 곱씹어둘 만한 건: mock 기반 테스트는 "서비스가 어떤 협력자에게 어떤 메시지를 보낸다"는 가정을 그대로 박제한다는 것. 서비스 내부가 바뀌면 그 가정도 함께 깨진다. 테스트가 깨졌다는 건 "내부가 바뀌었다"는 신호이기도 하다.

케이스 4: 테스트는 맞고, 서비스 로직이 가정을 깬 경우

여기가 핵심이다.

@Test
void todo의_user가_null인_경우_예외가_발생한다() {
    Todo todo = new Todo(...);
    ReflectionTestUtils.setField(todo, "user", null);
    given(todoRepository.findById(any())).willReturn(Optional.of(todo));

    assertThrows(InvalidRequestException.class, () ->
        managerService.saveManager(authUser, todoId, request)
    );
}

이 테스트는 "Todo의 user가 null이면 도메인 예외를 던진다"를 검증한다. 의미 있는 시나리오다. Todo가 사용자 없이 존재하는 비정상 상태를 명시적으로 다루겠다는 의도.

서비스 코드를 봤다.

public ManagerResponse saveManager(AuthUser authUser, long todoId, ...) {
    Todo todo = todoRepository.findById(todoId)
        .orElseThrow(...);

    if (!ObjectUtils.nullSafeEquals(todo.getUser().getId(), authUser.getId())) {
        // ^ todo.getUser()가 null이면 여기서 NPE 발생
        throw new InvalidRequestException("담당자 권한이 없습니다.");
    }
    // ...
}

팀원이 getUser() 결과에 대한 null 체크 없이 권한 검증을 작성했다. 테스트는 이 상황을 예상하고 도메인 예외를 기대하지만, 실제로는 NPE가 발생한다.

여기서 무엇을 고칠 것인가?

• A) 테스트를 NPE 검증으로 바꾼다. 의미 있는 도메인 예외를 NPE로 후퇴시키는 나쁜 선택.
• B) 서비스에 null 체크를 추가한다. 테스트가 표현한 의도를 코드가 따라가도록.

public ManagerResponse saveManager(AuthUser authUser, long todoId, ...) {
    Todo todo = todoRepository.findById(todoId)
        .orElseThrow(...);

    if (todo.getUser() == null) {
        throw new InvalidRequestException("Todo의 작성자가 존재하지 않습니다.");
    }

    if (!ObjectUtils.nullSafeEquals(todo.getUser().getId(), authUser.getId())) {
        throw new InvalidRequestException("담당자 권한이 없습니다.");
    }
    // ...
}

→ 서비스 로직 수정.

이 PR은 다른 테스트 수정 PR들과 섞으면 안 된다. 다른 케이스는 모두 "테스트만 고치는" 변경이지만, 이건 프로덕션 코드의 동작을 바꾸는 변경이다. 리뷰어가 봐야 할 시선이 완전히 다르다.

그래서 어떻게 구분했는가

네 케이스를 모두 본 다음, 이런 질문 순서로 정리할 수 있었다.

1. 테스트의 의도가 무엇인가?
   메서드명, 검증 내용, given/when/then을 읽고 "이 테스트는 무엇이 옳다고 주장하는가"를 먼저 파악한다.

2. 그 의도가 옳은가?
   도메인 관점에서 합리적인 시나리오인지 본다.

3. 의도가 옳다면, 실패의 원인은 무엇인가?

   • 의도를 잘못 구현한 테스트 코드 → 테스트 수정 (케이스 1)
   • 메서드명이 옛 의도의 잔재 → 테스트 수정 (케이스 2)
   • mock setup이 서비스의 실제 협력 관계와 어긋남 → 테스트 수정 (케이스 3)
   • 서비스 로직이 의도된 시나리오를 처리하지 않음 → 서비스 수정 (케이스 4)

핵심은 "테스트가 실패하면 테스트를 고친다"는 반사적 반응을 멈추는 것이었다. 테스트가 가진 의도를 먼저 읽고, 그 의도가 옳은지 판단한 다음에 어느 쪽을 고칠지 결정한다.

회고

테스트의 가치는 회귀 방지에만 있는 게 아니라, "이 시스템이 어떤 시나리오를 명시적으로 다루는가"를 문서화하는 데도 있다. 케이스 4의 테스트가 없었다면 "Todo의 user가 null인 비정상 상황"이 어떻게 처리되는지 아무도 신경 쓰지 않았을 것이다. NPE는 운영 환경에서 조용히 5xx로 돌아왔을 거고, 누가 그 원인을 추적할 때쯤이면 이미 한참 뒤였을 것이다.

테스트가 실패할 때 그 의도를 먼저 읽는 습관은, 결국 도메인 시나리오를 더 신중하게 다루는 습관과 같다. "왜 이 테스트가 처음에 작성됐을까"를 묻는 것은 "이 시스템은 무엇을 보장해야 하는가"를 묻는 것과 같다.

다음에 깨진 테스트를 만나면, 빨간 줄을 없애는 가장 빠른 방법을 찾기 전에 의도부터 한 번 읽으려 한다.

---