테스트 코드 그리고 리팩토링

들어가기

 

테스트하기 쉽다고 좋은 함수, 좋은 설계는 아니지만 좋은 함수, 좋은 설계는 테스트하기 쉽다.
 

 

여기저기서 많이 들어볼 수 있는 말이다.

 

아직 테스트 코드를 많이 작성해보지 못했고 공부하는 과정이라 저 말이 잘 와닿지는 않는다.

 

아주 조금씩 이 말을 깨달아가고 있고 그 과정을 남겨보려 한다.

 

어려운 테스트, 좋지 못한 설계

진행 중인 프로젝트에서 회원의 관심분야를 등록, 수정, 삭제할 요구사항이 생겼다.

 

처음엔 아래와 같이 Service 레이어를 구현했다.

  @Transactional
    public void saveInterests(InterestsRequest interestsRequest, UserDetails userdetails) {
        User user = userRepository.findByLoginId(userdetails.getUsername())
                .orElseThrow(() -> new UserException(USER_NOT_FOUND));

        List<Interests> oldInterests = user.getInterests();
        List<InterestsType> newInterests = interestsRequest.interests();

        if (oldInterests.isEmpty()) {
            saveNewInterests(newInterests, user);
        } else {
            updateInterests(oldInterests, newInterests);
        }
    }

    private void updateInterests(List<Interests> oldInterests, List<InterestsType> newInterests) {
        oldInterests.removeAll(
                oldInterests
                        .stream()
                        .filter(oldType -> !newInterests.contains(oldType.getInterestsType()))
                        .toList()
        );
        oldInterests.addAll(
                newInterests
                        .stream()
                        .filter(newType -> !interestsMapper.toInterestsType(oldInterests).contains(newType))
                        .map(Interests::new)
                        .toList()
        );
    }

    private void saveNewInterests(List<InterestsType> newInterests, User user) {
        user.getInterests().addAll(
                newInterests
                        .stream()
                        .map(Interests::new)
                        .toList()
        );
    }

 

saveInterests 에서 관심 분야를 파라미터로 받고 이전에 등록한 적이 없다면 saveNewInterests메서드를 호출해 새롭게 컬렉션에 추가하고, 이전에 등록했었다면  updateInterests를 호출해 변경사항만 제거하거나 추가하는 로직이다.

 

처음엔 굉장히 아름다워 보였지만,, 테스트 코드를 작성하면서 뭔가 잘못된 것 같은 느낌을 받았다.

 

UserService를 테스트하기 위해서 함께 물려있는 많은 의존성을 목 객체로 차단해야 했다.

@ExtendWith(MockitoExtension.class)
class UserServiceTest {
    @Mock
    UserRepository userRepository;
    @Mock
    UserMapper userMapper;
    @Mock
    InterestsMapper interestsMapper;
    @Mock
    UserDetails userDetails;
    @InjectMocks
    UserService userService;

    User user;
    List<Interests> oldTypes = new ArrayList<>();
    List<InterestsType> newTypes = new ArrayList<>();

    @BeforeEach
    void setUp() {
        oldTypes.add(new Interests(SPRING));
        oldTypes.add(new Interests(DJANGO));
        oldTypes.add(new Interests(REACT));

        user = User.builder()
                .loginId("bukak2019")
                .password("jeieaojef")
                .nickname("song")
                .interests(oldTypes)
                .build();

        newTypes.add(SPRING);
        newTypes.add(DJANGO);
        newTypes.add(VUE);
    }

    @Test
    @DisplayName("새로운 관심분야에 없는 관심분야는 기존 관심분야에서 제거된다.")
    void Remove_Interests() {
        InterestsRequest newRequest = new InterestsRequest(newTypes);
        when(userRepository.findByLoginId(any())).thenReturn(Optional.ofNullable(this.user));
        when(interestsMapper.toInterestsType(user.getInterests())).thenReturn(user.getInterests().stream().map(Interests::getInterestsType).toList());
        userService.saveInterests(newRequest, userDetails);
        List<InterestsType> updatedInterests = user.getInterests().stream().map(Interests::getInterestsType).toList();
        assertThat(updatedInterests).containsExactlyElementsOf(newTypes);
    }
}

 

서비스 레이어는 위에 보이듯 UserRepository, InterestsMapper, UserMapper, UserDetails 등 많은 의존성들이 물려있다.

 

처음엔 테스트가 쉬웠다. 그도 당연한 것이 의존성을 모두 모킹으로 둘렀기 때문이다.

 

예전에 읽어봤던 이동욱 개발자님의 포스팅이 떠올랐다.

@SpyBean @MockBean 의도적으로 사용하지 않기

모킹은 개발자가 의존성을 쉽게 제어할 수 있게 해주고 테스트를 쉽게 만들어준다.

 

 

위 포스팅의 핵심은 테스트를 쉽게 만든다는 사실이 잘못된 설계(코드 스멜)을 지나치게 만들 수 있다는 것이다.

 

내가 테스트하고 싶었던 핵심적인 기능은 아래와 같다.

if (oldInterests.isEmpty()) {
            saveNewInterests(newInterests, user);
        } else {
         	// 이 곳 !
            updateInterests(oldInterests, newInterests);
        }

 

관심분야가 의도한 대로 삭제,추가한 뒤 컬렉션의 상태가 궁금했던 것인데

 

곰곰이 생각해 보면 그 상태를 검증하는데 UserRepository, InterestsMapper, UserMapper와 같은 의존성은 불필요하다.

 

이동욱 님의 포스팅을 보면 별도의 클래스로 분리해 의존성을 제거하셨다.

 

하지만 나의 경우 굳이 별도의 클래스까지 만들 필요 없이 회원 도메인에 로직을 추가하는 것이 더 적절하다고 판단했다.

 

관심 분야는 회원의 상태이기 때문이다.

 

아래는 User라는 도메인 객체에 로직을 추가한 모습이고

public void updateInterests(List<InterestsType> newTypes) {
    this.interests
            .stream()
            .filter(isDifferentWithOldTypes(newTypes))
            .forEach(oldType -> this.interests.remove(oldType));
    newTypes
            .stream()
            .filter(isDifferentWithNewTypes())
            .forEach(newType -> this.interests.add(new Interests(newType)));
}

private Predicate<InterestsType> isDifferentWithNewTypes() {
    return newType -> !this.interests
            .stream()
            .map(Interests::getInterestsType)
            .toList()
            .contains(newType);
}

private Predicate<Interests> isDifferentWithOldTypes(List<InterestsType> newTypes) {
    return oldType -> !newTypes.contains(oldType.getInterestsType());
}

 

서비스 레이어의 볼륨도 많이 줄었고 가독성도 올라갔다.

@Transactional
public void saveInterests(InterestsRequest interestsRequest, UserDetails userdetails) {
    User user = userRepository.findByLoginId(userdetails.getUsername())
            .orElseThrow(() -> new UserException(USER_NOT_FOUND));

    List<InterestsType> newInterests = interestsRequest.interests();
    List<Interests> oldInterests = user.getInterests();

    if (oldInterests.isEmpty()) {
        oldInterests.addAll(newInterests.stream().map(Interests::new).toList());
    } else {
        user.updateInterests(newInterests);
    }
}

 

테스트 코드는 수많은 모킹 없이 해당 기능을 테스트할 수 있게 된다.

@Test
@DisplayName("새로운 관심분야에 없는 관심분야는 기존 관심분야에서 제거된다.")
void test() {
    // given
    List<Interests> originInterests = new ArrayList<>(List.of(new Interests(SPRING), new Interests(DJANGO), new Interests(VUE)));
    
    List<InterestsType> newInterests = Arrays.asList(SPRING, DJANGO, REACT);
    User user = User.builder()
            .interests(originInterests)
            .build();

    // when
    user.updateInterests(newInterests);

    // then
    List<InterestsType> originTypes = originInterests.stream().map(Interests::getInterestsType).toList();
    assertThat(originTypes).containsExactlyElementsOf(newInterests);
}