Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[예지니어스] 1단계 - 점진적인 리팩터링 미션 제출합니다. #12

Merged
merged 24 commits into from
Oct 21, 2020

Conversation

YejiAhn
Copy link

@YejiAhn YejiAhn commented Oct 11, 2020

안녕하세요, 킹뽀대!
예지니어스입니다.
마지막 미션이네요.
잘 부탁드립니다 😊

Copy link

@kingbbode kingbbode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요 :)
한번 고민해보셨으면 하는 점을 피드백으로 작성해두었습니다.
확인 부탁드려요!

- 예외 사항
- 방문한 손님의 수가 0 미만일 경우 예외를 발생한다.
- 주문 테이블이 없는 경우 예외를 발생한다.
- 주문 테이블이 비어있는 경우 예외를 발생한다.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요 :)
꼼꼼하게 잘 짜여진 테스트 시나리오와 테스트들을 잘 보았습니다.

각 서비스에서 저장소를 모두 Mock 처리 하셨네요! 이 부분에 대하여 트레이드 오프를 생각해보셨으면 하여 아래 내용 작성합니다.

일단 현재 구조에서 갖는 문제점을 말씀드립니다.

첫번째로 Mock 처리에 대한 내용입니다. 현재의 서비스 테스트는 저장소의 인터페이스를 Mock 처리하고 있습니다. 이런 인터페이스의 Mock 처리는 하위 레이어의 구현에 관계없이 독립적으로 테스트를 할 수 있다는 점에서 변경과 리펙토링에 유연한 테스트가 가능해져 좋은 테스트라고 말할 수 있습니다.
단 현재의 Mock 처리는 완벽하지 않습니다. 하위의 인터페이스를 구현했지만, 이미 이 인터페이스의 어떤 메서드를 사용할지 알고 있는 테스트가 되고 있습니다. 위에서 변경과 리펙토링에 유연한 테스트가 될 수 있다고 했지만, 이러한 Mock 처리는 위의 장점을 전혀 흡수할 수 없습니다.

  1. findById 라는 메서드를 사용하던 구현이 findByIdAnd~ 라는 메서드를 사용하는 구현으로 변경됐을 때
  2. findById 라는 메서드만 사용하던 구현이 findBy~~ 라는 메서드를 추가 사용하는 구현으로 변경됐을 때

등등 결국 테스트는 구현의 변경에 자유롭지 못하게 구현을 따라가는 테스트더블 생성이 계속 필요하게 됩니다. 즉 Mock 처리를 한다면 하위 인터페이스를 완전히 구현하는 테스트더블 생성이 필요합니다. 그래야만 하위 레이어의 구현에 상관없이 유연한 테스트가 가능해집니다.

두번째로 위 Mock 처리를 완벽히 하셨다고 할지라도 테스트는 보완이 필요해보입니다. 저장소에 대한 테스트입니다. 저장소는 수많은 제약조건을 갖습니다. Dao 에서 구현하신 쿼리에 대한 내용도 반드시 테스트가 필요하며, 발생할 수 있는 케이스에 대해서도 테스트가 필요합니다. Dao에 대한 테스트를 하지 않는 것은 굉장히 많은 예외케이스를 확인하지 못하는 것이니, 어떠한 규모의 시스템에서도 거의 신뢰할 수 없는 테스트이자 시스템이라고 판단하게 됩니다.

위 내용은 현재 저장소를 Mock 처리하면서 발생한 문제들에 대한 피드백이였습니다.

저는 저장소를 Mock 처리하는 것은 굉장히 많은 것을 포기하게 된다는 것을 미리 말씀드렸습니다. 저장소에 대한 Mock에 대한 제 생각을 말씀드립니다.

첫번째로 제가 경험하고 있는 대부분의 현업에서는 저장소를 Mock 처리하지 않습니다. 저장소를 테스트할 수 있게 h2, embedded redis 등 다양한 로컬 셋업환경이 지원되며, 이러한 환경을 설치하는 것이 비용도 굉장히 작기 때문에 사용하지 않을 이유가 없습니다. 현업에서는 h2 도 신뢰하지 못하여 운영에서 사용하는 mysql 버전과 동일한 버전으로 테스트 환경에서 docker 를 사용하여 테스트를 하는 곳도 있을 정도 입니다 :)

두번째로 어떠한 시스템의 어떠한 레이어 설계라 할지라도 시스템에서 사용되는 종단의 데이터는 모든 레이어를 관통하는 설계의 일부분이라고 생각을 하고 있습니다. 어떤 복잡한 시스템을 설계할 때도, 우리는 그에 대한 저장소 구조를 먼저 설계하고 그 위에 어플리케이션 아키텍처를 쌓고 있기도 하며, 한번 결정된 저장소에 대한 설계는 시스템 전체를 통틀어서 가장 변하지 않는 계층으로 분류할 수 있습니다. 어떠한 레이어의 어떠한 구현이라 할지라도, 저장소에 대한 사용성이 공개되는 것은 아키텍처적으로 지나친 공개라고 생각하지 않으며, 그렇기 때문에 상위 레이어에서 저장소에 대한 테스트 조건 생성 또한 가능할 것으로 생각됩니다.

세번째로 저장소에 대한 공개를 통해 모든 레이어에서 자유로운 변경과 리펙토링을 이룰 수 있다고 생각합니다. 저는 저장소 인터페이스를 모두 구현한 테스트더블을 작성하는 것과 실제로 저장소 환경에 테스트 데이터를 주입하는 것 중 후자가 훨씬 더 적은 비용이 든다고 생각하며, 후자의 환경은 전자와 마찬가지로 하위 레이어의 구현에 관계없이 자유롭게 변경, 리펙토링이 가능하도록 해줄 수 있습니다.


위의 두 가지 사용성을 이야기했는데요. 장.단점을 비교를 해보자면

  1. Mock 을 사용한 테스트
    장점:
  • 테스트가 빠르다
    단점:
  • 구현을 알게 되어 어플리케이션 코드 변경에 자유로울 수 없다.
  • 실제 DB를 통해 발생하는 문제를 캐치할 수 없다.
  1. 저장소를 사용한 테스트
    장점 :
  • 실제 DB를 통해 발생하는 문제를 캐치할 수 있다.
    단점 :
  • 저장소 설계를 알게 되므로 데이터베이스 설계 변경에 자유로울 수 없다.
    1. 에 비해 테스트가 느리다.

훨씬 더 많은 장단이 존재하지만, 현재로서는 위와 같이 되겠네요 :) 장단을 비교해보시고, 어떤 것을 버리고 어떤 것을 얻을지 생각해보시면 좋을 것 같아요.


추가로 몇가지 더 궁금해질만한 것을 이야기드리면,

Q. 그럼 항상 컨텍스트를 사용한 테스트를 해야하는 것인가?
A. 아닙니다. 컨텍스트 의존없이 쉽게 테스트더블을 사용할 수 있는 경우(혹은 경계)가 있습니다. 현재의 어플리케이션 규모에서는 아마 거의 없을 것 같네요. 컨텍스트를 무조건 적으로 사용하는 자세보다는 컨텍스트를 꼭 사용해야만 하는 테스트인지, 없이도 가능한 테스트인지를 항상 고민하는 자세가 필요합니다.

Copy link

@kingbbode kingbbode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

잘 작성해주셨네요 :)
코멘트도 남겨두었으니 확인 부탁드려요!
이만 머지하겠습니다.

Comment on lines +40 to +41
MenuGroup savedMenuGroup = menuGroupService.create(menuGroup);
List<MenuGroup> menuGroups = menuGroupService.list();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

createMenu() 테스트에서도 menuGroupService.create 을 테스트하고 있는데,
리스트를 조회하기 위한 사전 데이터 생성을 위해 다시 menuGroupService.create 을 실행하고 있네요 :)

createMenu 테스트가 실패한다면 list 테스트도 실패하게 될테니, 데이터를 공유하는 의존관계는 아니지만, 테스트 자체가 의존이 생긴 것으로 보이네요.

테스트의 조건은 저장소에 어떤 데이터가 어떻게 들어있을 때 로 시작되는 것이므로, 같은 레이어상에 있는 create 를 사용하기 보다는 하위 레이어의 인터페이스를 직접 사용하여 직접 저장소에 데이터를 셋팅하는 것이 어떨까요?

@kingbbode kingbbode merged commit cfea9c9 into woowacourse:yejiahn Oct 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants