[단위테스트] Unit Test 를 통해 함수를 Refactoring 해보자

Spring Boot 를 통해 앱을 개발하는 중에 기존 코드들을 단위테스트를 진행하면서 Refactoring 을 할 수 있었는데, 사실 Refactoring 을 하겠다고 생각하면서 굉장히 NAIVE 하게 짰던 기존 코드긴 하지만, Refactoring 에 큰 도움을 준다는 점을 다시 한 번 알게 되어 글을 작성해 보게 되었다. 바로 코드로 들어가보자. 

 

 

 

 

우선 이 코드는 특정 Group 에 Member 가 가입하는 요청을 보내서, Group 에 가입을 시켜주는 흔한 로직이다. 다음과 같이 Controller 에서 Service 단으로 요청이 넘어온 상태부터 시작한다.

 

 

public GroupMember joinGroup(RequestDto reqDto, Member member){

    // 해당 그룹이 있는지 먼저 확인한다
    Group groupPs = groupRepository.findWithJoinRuleById(reqDto.getGroupId()).orElseThrow(~);
    
    // 가입한 이력이 있는 유저인지 확인한다
    Optional<GroupMember> groupMemberOp = groupMemberRepository.findByMemberAndGroupId(member.getId, reqDto.getGroupId());
    
    // 가입한 이력이 있다면 재가입 가능한지 확인한다 
    groupMemberOp.ifPresent(GroupMember::checkRejoinAvailable);
    
    // 그룹의 가입 조건에 유저가 부합하는지 확인한다
    groupPs.getGroupJoinRule().judgeByRule(member); 
    
    
    GroupMember groupMember; // 생성 Object 준비 
    
    if(groupMemberOp.isPresent()){
        groupMember = groupMemberOp.get(); // 가입한 이력 객체를 불러온다
        groupMember.changeState(JOINED); // 가입 상태로 바꾼다
    } else {
        groupMember = new GroupMember(~,~);
    }
    
    return groupMember; 
}

 

 

상당히 복잡해 보이긴 하지만, 주석들과 함께 읽어보면 일반적인 가입 로직에서 크게 벗어나진 않은 것을 알 수 있다.  그리고 이 상태 그대로 딱히 문제점이라고 느끼지 않는 분들도 장담하건데 상당수일 것이다. 

 

 

뭐가 문제일까?? 뭐가 문제인지를 파악해보기에 가장 좋은 방법이, 역시 Test 를 짜보는 것이다. 그 이유에 대해서 먼저 설명해보고, 실제로 그런지를 살펴보도록 하자

 

 

1. 단위테스트는 철저히 외부 의존성을 배제하여야 한다. 외부 의존성을 잘라보면서, [내가 Test 하려는 함수가 굳이 하지 않아도 될 일] 에 대해서 더 파악해볼 수 있다. 

2.  Fail Test Case 를 작성해보면서 단위 테스트 입장에서 놓친 분기를 파악할 수 있다. 

3.  함수의 흐름을 전체적으로 다시 보게 함으로써, 중복 혹은 불필요한 영역을 확인할 수 있다.

 

 

우선 NAIVE 하게 작성된 위 코드에 대한 단위 테스트를 작성해보자. 성공 Case 를 우선적으로 작성할 것이고, Mocking Library 를 사용하여 외부 Object 들은 Mocking 하여 진행하였다. 

 

 

@Test
void joinGroup_shouldProcess_whenGroupMemberAlreadyExists(){

    // given
    RequestDto requestDto = mock;
    Member member = mock;
    Group group = mock;
    GroupMember preExistingGroupMember = mock;
    
    // given - stub
    when(groupRepository.findWithJoinRuleById(any()).thenReturn(Optional.ofNullable(group));
    when(groupMemberRepository.findByMemberAndGroupId(any(), any()).thenReturn(Optional.ofNullabe(preExistingGroupMember));
    doNothing().when(preExistingGroupMember).checkRejoinAvailable(); // 통과하도록 한다
    doNothing().when(preExistingGroupMember).changeState(any()); // 무슨 일을 하든 상관 없다
    
    // when
    // then 
    assertDoesNotThrow(() -> groupMemberService.joinMoim(requestDto, member)); // 아무 문제 없이 수행된다

}

 

 

실제 GroupMember Object 의 값들이 어떻게 바뀌는지는 changeState() 함수에서 수행하므로 그닥 관심을 주지 않았고, 사실 그룹에 가입한다는 행위도 요구사항에 따라 return value 가 꼭 필요하지 않을 수도 있기 때문에 verify 관점에서 test 를 진행 하였다 (changeState 등이 호출되는지 확인해도 된다). 

 

 

우선 위 Test 를 실행하자마자 Exception 발생으로 인한 Test Fail 이 일어났다. Group 에 JoinRule 이 없어서 Null 값이 들어간 상태였는데, 이를 확인하지 않고 Group 의 JoinRule 을 가져와서  judgeByRule(member) 함수를 수행했기 때문이였다. 이게 바로 문제인 부분이였다. 

 

 

내가 Test 하려는 Class (GroupMemberService) 가 아닌 곳 (JoinRule) 에서 예외가 발생하여 Test Fail 한다

 

 

단위 테스트의 목적과 아예 부합하지 않는 부분이다. 하지만 단위 테스트는 해줄 수 있는 mocking 은 다 해준 상태였다. 물론 JoinRule 을 또 만들어서 넣어줄 수 있고,  null checking 을 추가 해주는 것도 맞다. 하지만 이 단위 테스트가 자신이 할 일 외 외부 Object 일에 관여하고 있다는 점은 자명하게 확인할 수 있다.  

 

 

가입 조건에 대한 모든 것은 아예 Group 도메인에서 진행하면 어떨까? 그룹에 조건이 있으면 그 조건 안에서 판단하는 로직으로 변경해보기 위해  Group Class 내의 함수를 추가하였다. 

 

 

// Group.java

public void judgeMemberJoinByRule(GroupMember groupMember, Member member) {
    
    if(this.groupJoinRule != null){ // 있으면 조건 판별을 거친다
        this.groupJoinRule.judgeByRule(member);
    }
    
    // 가입 조건이 없으면 바로 가입시켜준다
    if(groupMember != null){
        groupMember.changeState(JOINED);
    } else {
        GroupMember.memberJoinGroup(~,~);
    }
}

 

 

이렇게 하면 Group 에 실질적으로 가입하는 핵심 로직은 Group 도메인에 위임하게 된다. 이에 따라 다음과 같이 Test 대상 함수를 줄여볼 수 있다. (If Branch 감소, GroupJoinRule 과 연관된 일에 대해 전혀 관여하지 않음) 

 

 

public GroupMember joinGroup(RequestDto reqDto, Member member){
    
    Group groupPs = groupRepository.findWithJoinRuleById(reqDto.getGroupId()).orElseThrow(~~);
    
    Optional<GroupMember> groupMemberOp = groupMemberRepository.findByMemberAndGroupId(member.getId(), reqDto.getGroupId());
    
    GroupMember groupMember = null; 
     
    if(groupMemberOp.isPresent()){
        groupMember = groupMemberOp.get();
        groupMember.checkRejoinAvailable();
    }
    
    groupPs.judgeMemberJoinByRule(groupMember, member);
    
    return groupMember;
}

 

 

하는 일을 조금 분리하긴 했지만, 아직도 조금 이상한게 보였다.

 

 

1. Optional<GroupMember> 를 가져온 이후 null 을 할당하는 GroupMember 변수를 굳이 만들고, 나중에 있으면 이 둘을 합친다

2. GroupMember 가 기존에 있다는 if 분기를 한 번 거치는데도, Group.java 안에 새로 만들어 준 함수에도 GroupMember 에 대한 존재 if 분기가 들어간다. 

 

 

현재 joinGroup 함수 내에서 Group 도메인에게 judgeMemberJoinByRule() 함수 수행을 요청할 때, null 에 대한 판별을 굳이 해주지 않아도, Group 도메인 내에서 해주기 때문에 이를 모두 제거해 줄 수 있는 부분이다. 이에 맞춰 Group.java 내의 함수와 Test 대상 함수를 다음과 같이 보정할 수 있었다.   

 

 

// Group.java

public void judgeMemberJoinByRule(GroupMember groupMember, Member member) {
    
    if(this.groupJoinRule != null){ // 있으면 조건 판별을 거친다
        this.groupJoinRule.judgeByRule(member);
    }
    
    // 가입 조건이 없으면 바로 가입시켜준다
    if(groupMember != null){
        groupMember.checkRejoinAvailable(); 
        groupMember.changeState(JOINED); // 나중엔 이 두 함수도 결국 합쳐졌다
    } else {
        GroupMember.memberJoinGroup(~,~);
    }
}

 

 

public void joinGroup(RequestDto reqDto, Member member) {

    // 해당 모임이 있는지 확인한다
    Group groupPs = groupRepository.findWithJoinRuleById(reqDto.getGroupId()).orElseThrow(~);

    // 둘 관계가 맺어진 적이 있는지 확인한다 // 없으면 null 로 채운다
    // null 이 전달되든 말든 신경쓰지 않는다
    GroupMember groupMemberPs = groupMemberRepository.findByMemberAndGroupId(member.getId(), reqDto.getGroupId()).orElse(null);
    groupMemberPs.judgeMemberJoinByRule(groupMemberPs, member);

}

 

 

Group 과 GroupJoinRule 도메인에 [가입] 관련하여서 수행할 일들을 분배해 준 결과, 내가 Test 하려는 함수는 훨씬 깨끗해졌음을 알 수 있다. 여기서 "어차피 그럼 그 옮겨진 도메인에서 Test 해야 하는 것 아닌가?" 라는 생각이 들 수 있다. 맞는 말이다.

 

 

하지만 단위 테스트의 목적대로, Test 대상 함수와 외부는 철저히 분리되는게 맞으며, stubbing 하는게 많을 수록 이 모습이 맞는지 정말 의심할 수 있는 것이다. Test 대상 함수인 joinGroup 이 정말 Group 도메인이 join 을 시도할 수 있도록 호출만 해주는 모습을 통해 역할이 훨씬 잘 분배된 모습으로 바뀔 수 있었다. 이를 토대로 성공 Test Case 를 다음과 같이 수정할 수 있었다. 

 

 

// 최종 TC
@Test
void joinGroup_shouldProcess_whenGroupMemberAlreadyExists() {
    
    // given
    RequestDto reqDto = mock;
    Member member = mock;
    Group group = mock;
    
    // given - stub
    when(groupRepository.findWithJoinRuleById(any()).thenReturn(Optional.ofNullable(group));
    
    // when
    groupMemberService.joinGroup(reqDto, member);
    
    // then
    verify(group, times(1)).judgeMemberJoinByRule(any(), any());
    
}

 

 

참고로 groupMemberRepostory() 도 stubbing 을 해줘야 하는 것 아니냐고 물어볼 수 있다. 하지만, 이 repository 는 null 을 반환하든, 실제 객체를 찾아주든 아무 상관이 없는 상황이기 때문에 아무 Stubbing 을 지정하지 않았다. Stubbing 지정이 없으면 null 반환이 원칙이기 때문이다. 

 

 

중요한 것은 null, 특정 객체의 반환을 stubbing 해주는 것이 아니라, stubbing 을 하지 않은채로 Test 를 짠 행위이다. 그래야 타 개발자가 TC만 봤을 때 "상관 없구나" 라는 점을 알 수 있기 때문이다.

물론 위에서 requestDto 와 member 에 대한 null checking 분기는 필수적으로 진행이 되어야 한다. Application 관점에서 단위테스트를 진행하면 안되기 때문이다. Application 상 null 이 올 수 없다고 하더라도, 그건 개발자만 아는거고, Application 을 모르는 타 개발자가 보면 null checking 이 안되어 있으면 "null 이여도 상관 없구나" 라는 뜻이기 때문이다. 

 

 

 

 

 

Unit Test 를 함으로써 얻을 수 있었던 결과물에 대해서 정리해보자

 

 

• Service Layer 가 간략하게 바뀌어서 Test 분기가 많이 줄었고, Service Layer TC의 유지 보수가 훨씬 유리해짐
  
  
• Domain Layer 에서 할 일이 늘긴 하지만, Service Layer 에서만 이루어졌던 Test 분기들을 여러 Domain 으로 분배해줄 수 있으므로, 구조적으로 훨씬 효율적이며 SRP 에도 더 부합한다
  
  
• joinGroup 함수도 복잡했던 로직에서 3줄짜리 로직으로 줄었고, Test Case 도 훨씬 간단하게 작성되었다

 

 

지금까지 한 Refactoring 의 효과를 정리해보면 다음과 같다. 그리고 중요한 점은 이 모든 것들이 [단위 테스트]를 진행하면서 찾아낼 수 있었던 점들인 것이다.  

 

 

이렇게 단위 테스트를 통해서 성공적으로 Refactoring 을 했다고 생각해서 경험을 적어보았다. DDD 에 대해서 공부한다면 더 부드러운 로직들과 TC 들을 만들 수 있을 것 같은데, 아직도 공부할게 많은 것 같다. 앞으로도 Unit Test 에 대해서 정리하고 싶은 부분은 더 정리해보고 싶다