[우WOO] 1단계 - 점진적인 리펙터링 미션 제출합니다.

[hwookim]

안녕하세요!
미션 중 몇가지 궁금한 점이 생겨서 리뷰 요청과 함께 질문드립니다.

미션 초반에는 @ExtendWith(MockitoExtension.class)를 이용한 단위 테스트로 Service 테스트를 작성했는데요.

    @DisplayName("주문 추가")
    @Test
    void create() {
        OrderLineItem orderLineItem = OrderLineItem.builder()
            .menuId(MENU1.getId())
            .quantity(1)
            .build();

        Order order = Order.builder()
            .orderTableId(NOT_EMPTY_TABLE.getId())
            .orderLineItems(Arrays.asList(orderLineItem))
            .orderedTime(LocalDateTime.now())
            .build();

        List<OrderLineItem> orderLineItems = order.getOrderLineItems();
        List<Long> menuIds = orderLineItems.stream()
            .map(OrderLineItem::getMenuId)
            .collect(Collectors.toList());
        given(menuDao.countByIdIn(menuIds)).willReturn(1L);
        given(tableDao.findById(order.getOrderTableId())).willReturn(Optional.of(NOT_EMPTY_TABLE));

        Order savedOrder = Order.builder()
            .id(1L)
            .orderTableId(NOT_EMPTY_TABLE.getId())
            .orderStatus(OrderStatus.COOKING.name())
            .build();
        given(orderDao.save(order)).willReturn(savedOrder);
        given(orderLineItemDao.save(orderLineItem)).willReturn(ORDER_LINE_ITEM);

        Order createdOrder = orderService.create(order);

        assertAll(
            () -> assertThat(createdOrder.getId()).isNotNull(),
            () -> assertThat(createdOrder.getOrderStatus()).isEqualTo(OrderStatus.COOKING.name()),
            () -> assertThat(createdOrder.getOrderLineItems().get(0).getSeq()).isNotNull()
        );
    }

어느새 보니 제가 테스트를 이렇게 짜고 있더라고요....😅 (다행히 지금은 테스트 구조를 완전히 바꿔 조금은 나아졌습니다.)

제가 짰던 테스트가 길어진 이유는 given().willReturn() 내부 메소드의 매개변수를 any()로 채우지 않고 거의 다 실제로 로직이 작동할 때 예상하는 중간값들을 넣던 것이라고 보는데요.
위 코드에서 savedOrder와 같은 중간값을 계산하기 위한 로직이 테스트에 튀어나오게 되면서 테스트가 난잡해진 것 같습니다.
builder패턴도 한 몫 했지만...

given(func(any()))의 방식으로 테스트 코드를 작성했을 때, 테스트 대상 메소드의 중간 로직에 따라 any()에 들어갈 값이 바뀐다면 테스트가 더 이상 해당 메소드를 보장하지 못한다고 생각했었습니다.
any()를 사용하지 않는다면 given 메소드에 특정 값이 들어가지 않을 때 에러가 발생할 테니, 중간 결과값을 확인하는 효과도 볼 수 있겠네요.

이런 생각과 동시에 이게 맞는건가...? 하는 생각이 자꾸만 듭니다.😭

이에 대해 킹뽀대님의 의견을 들어보고 싶어요.
mock 테스트는 어떤 경우에 써야하고, 어떤 방식으로 진행해야하는지 머릿속에 잘 확립되질 않습니다. 😥

말이 약간 횡설수설 하네요....
나중에 다시 읽어보고 가능한 깔끔하게 정리해보도록 하겠습니다.ㅠㅠ

리뷰 잘부탁드립니다!

[kingbbode]

안녕하세요.
질문 주신 내용에 대해 장황하게 글을 남긴 것 같네요.
더 궁금한 내용은 DM 주셔도 좋습니다!
피드백도 작성해두었습니다.

[kingbbode]

테스트의 setUp에서는 테스트의 목적을 달성하는데에 직접적인 연관이 없는 테스트의 기반을 구성할 것을 추천드립니다.

대부분의 테스트는 조건과 행위, 예측 으로 구성이 되는데, 현재 이곳에서 세팅하고 있는 값들은 어떠한 테스트에서는 조건으로 사용되는 값들이 세팅이 되고 있네요. 직접적인 연관이 있는 값들이 세팅되고 있다는 이야기입니다.

setUp 에서 테스트의 조건이 세팅되었을 때,

첫번째로는 테스트의 가독을 심하게 방해하게 됩니다. 테스트 메서드 안에서 수행되는 조건이 명확히 드러나지 않을 때 조건을 확인하기 위해 이 setUp 선언을 반복하여 확인하게 될 것 입니다. 또한 테스트 안에서의 order 가 이 테스트 안에서 선언한 것인지, setUp 에서 선언한 것인지를 계속 확인하게 되겠지요. 또한 테스트에서 사용하고 있지 않은 다른 선언들로 인해 테스트를 작성할 때 마다 setUp의 선언을 확인해야할까요

두번째로도 가독과 관련된 이야기이지만 일관성에 대한 고민을 하게 될 것이고, 이러한 고민이 테스트를 작성하는 것을 더 어렵게 할 것 입니다. 현재 setUp 에서 선언한 order 의 기준은 가장 정상적인 케이스를 선언한 것이라고 생각이 되는데요. 이러한 상황에서 테스트를 추가해야하는데, setUp 에서 선언한 order 와 대부분의 구성이 같지만, 하나의 값이 다른 케이스를 검증해야 합니다. 아마 고민을 하게 되시겠지요. order2 를 만들 것인가, 이 테스트에서는 저 긴 선언들을 테스트 안에 작성할 것인가. order2 가 작성된다면 전체적인 테스트 가독은 더더욱 심각하게 어려워질 것이고, 테스트 메서드에 작성하기에는 일관성 측면에서 찝찝하게 됩니다. 그리고 이러한 고민으로 테스트가 추가될 때마다 고통받겠지요.

이미 위와 같은 일들을 경험한 선배 개발자로서 조언을 드리자면, 테스트를 구성하는 요소들은 테스트 안에서 작성되는게 가장 좋은 테스트가 될 것이라고 말씀드립니다.

그렇다면 저 복잡한 선언들을 모든 테스트마다 작성해줘야할까라는 고민이 생길 수 있는데요.
첫번째로는 모든 테스트마다 작성해줘도 됩니다. 테스트의 가독을 방해할 바에는 여러 중복을 가져가는 것이 더 관리하기 쉬운 테스트가 될 것입니다.
두번째로는 우리는 이미 중복을 제거하는 방법을 알고 있을 것 입니다. 메서드로 분리하는 방식입니다. 예를 들면 createOrder(조건1, 조건2) 와 같이 테스트에 필요한 서포트 메서드를 생성하여 중복을 제거하면서도 테스트 안에서 조건이 잘 드러나도록 작성해볼 수 있을거에요.

[hwookim]

확실히 테스트 코드를 다시 읽어보려니 테스트의 전체 흐름이 눈에 안담기네요.
리팩토링 과정에도 객체 생성 시 어떤 데이터가 필요한지 setUp을 반복적으로 확인하게 되더라고요.😥

테스트에 영향을 끼치는 객체의 생성을 각 테스트 내부에서 하도록 변경했습니다.
여러 테스트에 걸쳐 중복되는 코드가 생겨 TestObjectFactory 클래스에서 테스트를 위한 객체 생성 코드를 관리하도록 했습니다. 클래스 네이밍이 올바른지 긴가민가 하네요.😅

[kingbbode]

테스트 간 서로 영향을 주지 않기 위해 위 작업을 하고 있군요 👍
deleteAll 작업은 테스트 전보다 테스트 후에 진행되는게 올바른 테스트 사이클이 될 것 같습니다 :)

[hwookim]

@Sql을 통해 데이터를 삭제하는 이유는 말씀하신 테스트 격리 뿐만 아니라 기본 데이터의 삭제 효과도 겸하기 위해서였습니다.
만일 테스트 후에 데이터 정리 작업을 하게 된다면, 기본 데이터의 관리는 어떻게 해야할까요?
조회 테스트에 기본 데이터를 고려해 테스트하고, @Dirtiescontext를 이용해 기본 데이터를 유지한 채로 테스트를 진행해야 하나요?

테스트 전에 데이터를 정리하는 것과 후에 관리하는 것이 올바른 테스트 사이클에 어떤 영향을 주게 되는지도 궁금합니다.🤔

[kingbbode]

사용하신 어노테이션에 지원하는 기능이 있는 것 같은데요! 확인 해보시면 좋겠네요 :)

테스트 전에 데이터를 정리하는 것과 후에 관리하는 것이 올바른 테스트 사이클에 어떤 영향을 주게 되는지도 궁금합니다.

결론부터 이야기하자면, 내가 싼 X은 내가 치운다 와 같은 개념입니다 ^^;

첫번째로 모두 똑같이 위 SQL 을 사용할 수 있는 환경이라면 우려가 없을 수도 있겠지만, 복잡한 비지니스를 가지면 가질수록 BEFORE 에서 더 다양한 셋업 케이스들이 생길거에요. 우리는 테스트는 서로 상호독립적이라는 전재가 있기 때문에 테스트의 순서는 고려하지 않습니다. 100개의 테스트 클래스가 있는 프로젝트에서 새로운 테스트를 작성할 때 BEFORE 에서 데이터를 클랜징한다면, 이 전 테스트에서 어떤 동작을 했는지 알아야겠지요? 이건 이미 상호독립적인 테스트라는 전재가 깨진 상태가 됩니다. 데이터를 사용한 테스트에서 지워준다면 다른 테스트에서 이전 테스트를 신경쓸 일이 없어지겠지요.

두번째로 항상 저장소를 100% 클랜징해주면 문제가 없다고 생각할 수도 있습니다. 맞는 이야기이지만, 최적화 측면에서 본다면 비효율적인 테스트가 될 확률이 높을거에요. 테스트는 빠른 피드백을 줄 수 있어야 합니다. 테스트가 한사이클 도는데 몇시간이 걸려버린다면 CI를 하는 의미가 없겠지요? (실제로 현업에서도 테스트 시간을 단축시키기 위한 리펙토링을 계속 진행하고 있습니다.)

[kingbbode]

질문 주신 내용은 이쪽에 작성하겠습니다 :)

일단 mock 을 사용한 테스트에는 상당히 많은 논란과 갈리는 의견이 있는 점도 참고해주시면 좋겠네요. 제가 말씀드리는 의견도 그 의견 중 일부입니다. 그리고 저는 mock 사용을 굉장히 엄격하게 사용하고자 하는 쪽인 점도 미리 말씀드려요.

Mockito 의 기능은 우리가 테스트하는 것을 꽤나 편리하게 해줍니다. 내가 테스트하고자 하는 부분을 제외하고는 다른 부분을 내가 기대한 동작을 하도록 만들 수 있기 때문입니다. 그러나 저는 부분별한 Mockito 사용이 설계에 있어 가장 중요한 부분을 훼손시키고 있다고 생각을 하고 있습니다.

질문을 주셨던 내용 중 그 핵심 내용이 들어있기도 하군요.

given(func(any()))의 방식으로 짜게된다면 테스트 대상 메소드의 중간 로직이 바뀌어 any()에 들어갈 값이 바뀐다면 테스트가 더 이상 해당 메소드를 보장하지 못한다고 생각했었습니다.

우리가 테스트하는 것은 설계이며, 이러한 설계의 기본 원칙 중 Interface 를 이야기해보겠습니다. Interface 는 어플리케이션의 비지니스 흐름을 완성시키는 가장 기본적인 단위이며, 우리가 Interface 를 사용하는 것은 Interface 를 통한 설계로 하여금 그 구현이 바뀌어도 Interface 에서 선언한 메서드들의 목적이 바뀌지 않는 이상 정상 동작한다는 것 입니다. 쉽게 말하면 Interface 는 구현을 알지 못하고, 그렇기 때문에 우리는 자유롭게 리펙토링을 할 수 있고, 구현체를 개선 혹은 변경할 수 있는 유연한 시스템을 만들 수 있게 되는 것 입니다.

우리가 하는 테스트는 어떨까요? 우리가 하는 테스트는 Interface 를 테스트하는 것과 같습니다. 내부의 구현이 어찌되었건, 조건, 행위로 하여금 내가 기대한 결과를 테스트하는 것이지요. 이러한 이유로 테스트가 탄탄한 프로젝트를 리펙토링하는 것은 꽤나 즐거운 일이 될 수 있습니다. 테스트가 구현이 어떻게 바뀌어도 그 기대값을 탄탄히 테스트해줄 수 있기 때문입니다.

그럼 다시 위에 작성하신 내용을 살펴보겠습니다. 테스트 대상 메소드의 중간 로직이 바뀌어 any()에 들어갈 값이 바뀐다면 테스트가 더 이상 해당 메소드를 보장하지 못한다고 생각 은 제 기준에는 아주 올바른 생각입니다. 위 말을 다르게 표현한다면 Mockito 를 사용한 Mock 선언으로 우리는 이미 구현이 어떻게 되고 있는지를 알고 있게 되며, 구현을 알고 있는 테스트는 리펙토링이나 구현의 변경으로부터 자유로울 수 없다는 풀이가 되겠지요.

method gogo() {
   a.a();
   b();
   c();
}

그렇다면 위와 같은 테스트에서 b 영역만 테스트할 때는 어떻게 해야하는 것인가? 결국 이 이야기는 private 메서드가 어떻게 테스트되어야 하는가와 같은 이야기가 됩니다. 이 때 우리가 b 라는 private 영역에 대해서 별도의 테스트가 필요하다고 느꼈다면, 그것은 사실 pirvate 으로 작성되야할 것이 아닌 별도 Interface 혹은 class 로 추출되어 의미있는 응집을 구성했어야 할 가능성이 굉장히 큽니다.

이 이야기는 약간 번외이기는 합니다만, 응집의 중심이 되는 단위를 테스트하는 것을 우리는 흔히 유닛 테스트라고 말하며, 이러한 여러가지 응집의 단위를 혼합하여 하는 테스트를 통합 테스트라고 말합니다. b() 를 테스트하는 것은 유닛테스트, 위의 gogo() 를 테스트하는 것이 통합테스트가 되겠지요. 이러한 응집의 단위는 설계의 구성마다 다르고 대부분 대규모 어플리케이션은 작은 응집들의 깊이 상당히 깊으므로, 사실 어떤 것을 유닛테스트다 통합테스트다 라고 명확히 말할 수 있는 시스템은 많지 않을 것 입니다. 그렇기에 설계자의 관점에서 유닛테스트 를 설계 단위에서 진행하고, 반드시 통합테스트도 이루어져야 완벽히 커버하는 테스트가 될 것 입니다. 일반적 스프링 어플리케이션에서 응집의 단위를 상당히 크게 잡는다면 Controller 테스트를 통합테스트, Service 테스트를 유닛테스트라고 말할 수 있기도 합니다만, 나누는 큰 의미는 없을 것 입니다.

본론으로 돌아와서 구현을 알게 되는 테스트를 원하지 않기 때문에 저는 Mockito 혹은 Mock 사용을 굉장히 엄격하게 사용합니다. 여기에서 엄격하게 사용한다는 것은 사용을 안한다는 의미는 아니겠지요 ㅎㅎ 그럼 저는 어떤 경우에 Mockito 를 활용하고 있는지 말씀드리겠습니다.

Mockito 에 대한 제 첫번째 기준은 제어할 수 없는 영역에서 사용하는 것 입니다.
제가 2019 스프링캠프에서 연사발표했던 장표를 첨부합니다. (https://www.slideshare.net/ssuser59a869/ksug-2019)
45페이지의 내용인데요. 저는 • Random, Shuffle, LocalDate.now() • 외부 세계 • HTTP • 외부 저장소 를 제어할 수 없는 영역이라고 말합니다. 랜덤의 성격을 띄고 있는 함수나, LocalDate.now() 처럼 계속 흘러가는 시간의 순간, 그리고 외부 요청에 대한 영역, 그리고 외부에 존재하여 내가 제어를 할 수 없는 외부 서버, 외부 저장소 등이 입니다. 이러한 영역에서는 테스트가 가능하도록 설계를 변경하거나, Mockito를 사용하고 있습니다.

두번째 기준은 로컬 구성이 가능한 저장소는 사용하지 않는다는 것 입니다. 저장소의 종류를 떠나서도 저장소의 구현체는 굉장히 큽니다. 그리고 대부분의 저장소는 각자 갖는 제약조건이 무수히 많습니다.(따로 설명안해도 되겠지요?) 그렇기때문에 저장소를 Mock 처리하는 것은 굉장히 많은 것을 포기하여, 거의 신뢰할 수 없는 테스트가 됩니다. 저장소를 테스트할 수 있게 h2, embedded redis 등 다양한 로컬 셋업환경이 있습니다. 이러한 환경을 설치하는 것이 비용도 굉장히 작기 때문에 사용하지 않을 이유가 없습니다. 현업에서는 h2 도 신뢰하지 못하여 운영에서 사용하는 mysql 버전과 동일한 버전으로 테스트 환경에서 docker 를 사용하여 테스트를 하는 곳도 있을 정도 입니다 :)

세번째 기준은 레이어링의 경계에서 사용하는 것 입니다.
대부분의 대규모 어플리케이션은 (아마도?) 단방향으로 흐르는 레이어 계층을 설계를 갖습니다. 예를 들면 응용 계층 - 도메인 계층 - 인프라스트럭처 계층 와 같이 말이죠. 이 레이어는 시스템의 설계마다 전부 다릅니다. 제가 현재 담당하는 시스템은 응용 계층 - 어플리케이션 비지니스 계층 - 도메인 계층 - 데이터 모델 계층 - 인프라스트럭처 계층 과 같이 복잡하게 레이어가 펼쳐져 있기도 합니다.
이러한 각 계층을 테스트할 때 저는 Mockito를 사용합니다. 이건 어플리케이션의 규모가 대규모이기 때문이기도 한데요. 상당히 깊은 depth 의 레이어를 갖는 설계에서 응용 계층 의 테스트를 작성하기 위해 하위 계층들의 테스트 셋업이 지나치게 방대해질 수 있습니다. 그렇기 때문에 이러한 레이어링을 갖는 설계에선는 레이어의 경계 영역에서 Mockito 를 사용하고 있습니다.

설명을 장황하게 해보았지만, 이해하기 상당히 어려운 내용일 것 같네요. 정답은 사실 없습니다. :) 본인이 생각하는 좋은 테스트에 대해 앞으로도 고민이 필요할 것 입니다.ㅎㅎ

[hwookim]

정성스러운 답변 감사드립니다! 🥰
확실히 mock을 이용한 테스트는 메소드의 구현 내용을 다 알고 있다는 전제하에에 사용하니 TDD 기반의 테스트 코드에서는 작성할 일이 거의 없겠네요. 구현을 확실히 알고 있는 상태에서만 제한적으로 사용해야 한다는 느낌이 많이 듭니다.

첨부해주신 장표 겸해서 유튜브에 발표하신 게 올라와 있길래 재밌게 봤습니다! Test Double 외에도 많은 도움이 되었습니다.
발표를 듣고 나니 테스트는 구현보다는 인터페이스에 대한 검증이 필요하다는 말이 더 와닿네요.
아직 완벽하게 이해하진 못했지만, 앞으로 테스트를 작성할 때 좋은 지표가 될 수 있을 것 같아요. 다시 한번 좋은 리뷰감사드립니다! 😄

[woowahan-pjs]

안녕하세요.
질문 주신 내용에 대해 장황하게 글을 남긴 것 같네요.
더 궁금한 내용은 DM 주셔도 좋습니다!
피드백도 작성해두었습니다.

[kingbbode]

수고하셨습니다.
👍 이만 머지할게요!

[kingbbode]

확인이 좀 늦었네요. 혹시 제가 너무 소식이 없다싶으면 DM 주시면 감사하겠습니다 ㅜㅜ