Unit Testing 11장 : 단위 테스트 안티패턴

들어가기 전

이 글은 단위 테스트 11장, 단위 테스트 안티 패턴을 공부하며 작성한 글입니다.

 

11. 단위 테스트 안티패턴

안티패턴은 '겉으로는 적절한 것처럼 보이지만' 나중에 더 큰 문제로 돌아올 패턴들을 의미한다. 이 장에서는 그것들을 작성해보고자 한다.

---

11.1 비공개 메서드 단위 테스트

결론부터 말하면 비공개 메서드 단위 테스트는 하지 않는 것이 맞다. 왜냐하면 비공개 메서드는 구현 세부 사항이기 때문이다. 그런데 꼭 테스트를 해야하는 상황이라면 비공개 메서드는 잘못 작성된 비공개 메서드다. 자세한 내용은 아래에서 볼 수 있다.

 

---

11.1.1 비공개 메서드와 테스트 취약성

단위 테스트는 식별할 수 있는 동작만 테스트 한다. '식별할 수 있는 동작'과 '공개 API'가 일치해야 좋은 코드라고 이야기를 했었다. 비공개 메서드는 구현 세부 사항이며, 테스트가 구현 세부 사항과 결합하면 '리팩토링 내성'이 떨어진다. 따라서 비공개 메서드는 테스트를 하지 않는 것이 맞다.

---

11.1.2 비공개 메서드와 불필요한 커버리지

비공개 메서드가 너무 복잡한 경우가 있다. 이런 이유 때문에 식별할 수 있는 동작을 테스트 하는 것만으로는 충분하지 않은 경우가 있다. 만약 식별할 수 있는 동작이 충분히 합리적인 테스트 커버리지를 제공하는 상황이라면, 비공개 메서드는 다음 두 가지 상황 중 하나다.

• 비공개 메서드 중 일부 코드는 죽은 코드다. 테스트에서 벗어난 코드가 어디에서도 사용되지 않는다면 리팩토링 후에 남은 관계없는 코드일 수 있다. 이런 코드는 삭제하자.
• 비공개 메서드에서 일부 추상화가 누락되어있다. 비공개 메서드가 너무 복잡하고, 이것 때문에 공개 API의 테스트가 어렵다면 추상화가 누락되었다는 것이다. 필요한 비즈니스 로직을 클래스로 추출해서 추상화하자.

예를 들어 아래 코드를 고려해보자. 

공개된 API는 generateDescription()이다. 그런데 이것만으로는 식별할 수 있는 동작을 모두 테스트 할 수 없다. 왜냐하면 비공개 메서드인 getPrice()에 비즈니스 로직이 포함 되어있기 때문이다. 이것은 추상화가 부족한 것이기 때문에 비공개 메서드를 추상화 시켜서 비즈니스 로직을 분리한다. 

public class MyOrder {

    private MyCustomer customer;
    private List<MyProduct> productList;
    
    
    public String generateDescription() {
        return String.format(
                "Customer Name : %s \n" +
                        "Total number of Products : %d \n" +
                        "Total Price : %d",
                customer.getName(),
                productList.size(),
                getPrice()
        );
    }
    
    private double getPrice() {
        /*
        double basePrice = 0;
        double discounts = 0;
        double taxes = 0;
        return basePrice - discounts + taxes;
         */
    }
    
}

위에서 계산하는 부분은 주요한 비즈니스 로직이었다. 이 부분이 비공개 메서드로 되어있기 때문에 테스트를 하는데 문제가 있었던 것이다. 이것의 문제점은 추상화의 부족이었다. 

아래 코드에서는 계산하는 비즈니스 로직을 추상화 한  'Calculator' 클래스를 생성했다. 테스트 코드는 더욱 작성하기 쉬워졌다. 개발자는 굳이 비공개 API를 테스트 하지 않고, Calculator 클래스의 calculate() 메서드만 단위 테스트하면 된다.

public class MyOrderRefactor {

    private MyCustomer customer;
    private List<MyProduct> productList;
    private Calculator calculator;

    public String generateDescription() {
        return String.format(
                "Customer Name : %s \n" +
                        "Total number of Products : %d \n" +
                        "Total Price : %d",
                customer.getName(),
                productList.size(),
                calculator.calculate(customer, productList));
    }

    
    class Calculator{

        public double calculate(MyCustomer myCustomer, List<MyProduct> productList) {
            /* MyCustomer , MyProduct 기반 계산
            double basePrice = 0;
            double discounts = 0;
            double taxes = 0;
            return basePrice - discounts + taxes;
         */
        }
        
        
    }
    
}

---

11.2 비공개 상태 노출

단위 테스트를 목적으로만 비공개 상태를 노출하는 것이다. 비공개 상태는 '구현 세부 사항'이기 때문에 단위 테스트에서 이것을 검증할 필요는 없다. 오히려 리팩토링 내성을 떨어뜨려 나쁜 테스트를 만들게 된다. 

테스트를 실행하고 검증할 때는 반드시 다음과 같이 동작해야한다.

제품 코드와 정확히 같은 방식으로 테스트 대상과 상호작용해야하며, 특별한 권한이 있으면 안된다. 즉, 제품 코드에서는 getter가 필요없는데 굳이 getter를 만들어서 상태를 노출하지 않아야 한다는 것이다. 

식별할 수 있는 동작으로만 검증하되, 비공개 상태는 '테스트만을 위한 노출'하는 것을 극도로 경계해야한다. 

---

11.3 테스트로 유출된 도메인 지식

도메인 지식이 테스트로 유출되는 것은 또 하나의 안티패턴이다. 이것은 도메인 지식이 바뀔 때 마다 코드를 수정해야하며, 이는 리팩토링 내성의 저하를 불러일으킨다. 도메인 지식이 유출되는 경우는 다음과 같다. 

• Calculator는 n1 + n2를 알고리즘으로 사용한다.
• 테스트 코드에서는 기대값을 설정할 때, n1 + n2로 사용했다. (도메인 지식 노출)

알고리즘이 매번 바뀔 때 마다 이런 테스트 코드들은 수정된 알고리즘을 다시 붙여넣는 것 밖에 방법이 없다. 리팩토링 내성이 떨어지고, 의미없는 테스트가 된다. 따라서 이런 경우에 기대값은 반드시 '하드코딩'하는 것을 추천한다. 

public class MyCalculator {

    public int calculate(int n1, int n2) {
        return n1 + n2;
    }
    
    @Test
    void test() {
        // given
        MyCalculator myCalculator = new MyCalculator();
        int n1 = 1;
        int n2 = 2;
        int expected = n1 + n2; // 도메인 지식 유출
        
        // when
        int calculate = myCalculator.calculate(n1, n2);

        // then

        assertThat(calculate).isEqualTo(expected);
    }
}

---

11.4 코드 오염

또 다른 안티패턴은 코드 오염이다. 코드 오염은 다음을 의미한다.

테스트에서만 사용하는 코드가 제품 코드에 포함되는 것이다

이것은 왜 문제가 있을까? 각 클래스에 제품 코드와 테스트에 필요한 코드가 혼재하게 된다. 클라이언트 입장에서는 어떤 코드를 사용해야하는지 혼란스럽다. 테스트 코드 작성하는 사람은 어떤 코드를 테스트 해야하는지 혼란스럽다. 테스트 할 것을 테스트 할 것을 테스트 하는 무한 재귀에 빠지게 된다. 

따라서 테스트에서만 사용하는 코드는 제품 코드에 포함시켜서는 안된다. 추천할만한 방법은 '코드 오염이 덜하도록 오염시키는 것'이다. 이것은 다음을 의미한다. 

A 클래스는 구현체만 존재하는 경우를 가정하자. 이 때 A 클래스를 테스트 하기 위해서 특정 메서드가 필요하다고 가정해보자. 그렇다면 이 때 A 클래스를 위한 A` 인터페이스를 만든다. 그리고 테스트 코드에서 사용할 메서드를 가진 B 클래스를 생성하고, B클래스는 A` 인터페이스를 구현하도록 한다. 

테스트 코드가 제품 코드에 반영되어 일부는 오염되지만, 제품 코드와 테스트 코드가 한 공간에서 공존하는 것이 아니라, 인터페이스를 기반으로 분리되기 때문에 최소화 될 수 있다. 

---

11.5 구체 클래스를 목으로 처리하기

Mock이라는 것은 외부 프로세스와 통신하는 녀석들에게만 사용한다. 그런데 Mcok을 이용해서 구체 클래스의 모든 기능은 유지한 채, 일부 기능만 stub() 하는 경우가 있다. (Mockito에서는 이것을 Spy라고 한다.) 그런데 이 경우는 단일책임원칙(SRP)를 위반한 것이다. 

왜 단일책임원칙을 위반했다고 하는 것일까? 

• Mock은 외부 프로세스와 통신하는 경우에만 사용한다. 
• 그런데 Mock에서 구체 클래스의 기본 기능을 살린다는 것은 Mock의 비즈니스 로직이 필요한 경우다. 즉, Mock으로 사용된 객체는 비즈니스 로직 + 외부 프로세스 통신에 대한 책임을 모두 가진다는 것을 의미한다. 
• 즉, Mock의 대상이 되는 클래스는 두 가지 책임을 모두 가지고 있기 때문에 단일책임원칙을 위반했다는 것이다. 

예를 들면 아래와 같은 코드에는 문제가 있을 수 있다. 

public class MyCalculator {

    private final JdbcTemplate jdbcTemplate;


    public MyCalculator(JdbcTemplate jdbcTemplate) {
        this.jdbcTemplate = jdbcTemplate;
    }

    public int calculate(String itemName) {
        List<MyItem> myItemFromDB = getMyItemFromDB(itemName);
        return myItemFromDB.stream()
                .reduce((myItem, myItem2) -> myItem.getCost() + myItem2.getCost());
    }

    public List<MyItem> getMyItemFromDB(String itemName) {
        return jdbcTemplate.execute("SELECT * FROM MY_ITEM");
    }
    
}

위 코드는 Calculator 클래스를 보여준다. Calculator 클래스는 DB에서 조회도 하고 계산도 직접한다. 따라서 이건 분리가 필요한 상태로 볼 수 있다.

• Calculator는 DB에서 직접 데이터를 가져온다. 
• 가져온 데이터에 있는 값을 계산해서 반환한다. 

만약 이 녀석을 사용하는 곳에서 테스트 코드를 작성하려면 이런 형태가 되어야 한다.

@Test
void test1() {


    MyCalculator myCalculator = Mockito.spy(MyCalculator.class);
    when(myCalculator.getMyItemFromDB("hello")).return(List.of());

    // ...

}

Spy 객체는 stub 하지 않는다면 기존 클래스와 동일한 형태로 동작한다. 위에서는 DB와 통신하는 부분만 Stub하고 나머지 메서드 (calculate())는 그대로 사용하도록 했다. 즉, Spy()를 사용한다는 것 자체가 구체 클래스를 그대로 사용하되 외부 프로세스만 Mocking 한다는 것을 의미한다. 이것은 단일책임원칙을 위반하는 것이다.

이런 부분은 험블 객체 패턴으로 분리한다. 험블 객체 패턴으로 분리하면 비즈니스 로직 / 외부 프로세스와 통신하는 부분이 나뉘게 된다. 따라서 구현체를 Spy()로 생성해서 일부는 구체 메서드를 그대로 사용하고, 일부는 Stub할 필요가 없다. 

 

---

11.6 시간 처리하기

많은 어플리케이션에는 시간에 따라 달라지는 기능이 존재할 수 있다. 시간에 따라 달라지는 기능을 테스트하면 거짓 양성이 발생할 가능성이 크다. 따라서 이 부분을 테스트 하기 위해서는 프로덕트 코드 역시 수정되어야 하고, 테스트 코드 역시 수정되어야 한다. 

명시적 의존성 형태로 시간을 주입하기

LocalDateTime.now()와 같은 형식으로 생성된 시간은 테스트 할 때 거짓 양성이 나오기 쉽다. 왜냐하면 그 값은 매번 바뀌는 것이고 테스트에서 예측할 수도 없기 때문이다. 따라서 이것을 해결 하기 위해서는 시간에 의존적인 클래스에게 '시간 값을 가진 객체'를 주입해주는 방법으로 처리할 수 있다. 아래에 예시가 있다.

public class TimeController {

    private MyTimer timer;

    // 시간을 값 객체로 주입받는다.
    public TimeController(MyTimer timer) {
        this.timer = timer;
    }

    public void toInquery() {

        LocalDateTime time = timer.getTime();

    }
}

그리고 시간 객체에는 LocaldateTime.now()를 가지고 있도록 한다.  

public class MyTimer {

    private LocalDateTime time = LocalDateTime.now();


    public LocalDateTime getTime() {
        return time;
    }
}

위와 같이 시간 값을 가진 객체를 시간을 사용하는 클래스에 주입해주면 테스트 할 때 편리하게 할 수 있다. 굳이 MyTimer 같은 객체를 생성해서 넣어주지 않고, LocalDateTime 객체를 넣어주는 것만으로도 충분하다. 

 

---

요약

• 단위 테스트를 가능하게 하고자 비공개 메서드를 노출하게 되면 테스트가 세부 구현 사항에 결합되게 된다. 그 결과 리팩토링 내성이 떨어진다. 
• 비공개 메서드가 너무 복잡해서 공개 API로 테스트를 할 수 없다면, 추상화가 누락되었다는 뜻이다. 비공개 메서드를 공개로 하지 말고, 복잡한 녀석을 별도의 클래스로 추출해서 추상화한다
• 비공개 상태를 단위 테스트를 위해 노출하지 마라. 테스트 코드는 제품 코드와 같은 코드가 사용되는 방식을 이용해서 검증해야한다. 이것은 또한 코드 오염에 해당되기도 한다.
• 테스트를 작성할 때 특정 구현을 암시하지 마라. 이것은 도메인 로직이 테스트 코드에 유출된 것이고, 리팩토링 내성을 떨어뜨린다. 
• 코드 오염은 테스트에만 필요한 코드를 제품 코드에 포함하는 것을 의미한다. 제품 코드 내에 다양한 코드가 산재되기 때문에 유지보수 비용이 증가한다. 만약 테스트를 위한 코드가 반드시 포함되어야하면 인터페이스를 하나 추가하고, 그 인터페이스의 구현체를 생성하는 것으로 코드 오염을 최소화한다. 
• 기능을 지키기 위해 구체 클래스를 Mock으로 처리해야 한다면, 이는 단일책임원칙(SRP)를 위반한 결과다. Mock으로 처리한다는 것은 외부 프로세스와 통신하는 역할을 하기 때문이다. 만약 이런 문제가 있다면, 외부 프로세스 통신과 도메인 로직이 있는 클래스를 분할하라.
• 시간에 의존적인 테스트가 있다면, 제품 코드에서 시간값 객체를 주입받아 동작하는 형태로 리팩토링 한 후 테스트 코드를 작성한다.