Step3 - 테스트를 통한 코드 보호

[bgpark82]

이번주 미션도 잘 부탁드립니다.😀

이번에는 인수 테스트와 단위 테스트를 작성하여 미션을 진행해보았습니다.
일단은 중간 정도 하다가 궁금한게 생겨서 PR 먼저 보냅니다.
주로 Menu 도메인 관련하여 인수테스트를 봐주셨으면 합니다.
관련하여 몇가지 질문이 있습니다.

1. 인수 테스트 데이터베이스 분리 및 데이터 입력

   • 인수 테스트 시, 테스트의 독립성을 위해 데이터베이스를 분리하고 데이터가 없는 상태를 선호합니다.
   • 그러다 보니 given에 복잡한 관계의 엔티티를 생성하고 DB에 넣어주는 작업이 많이 걸립니다.
   • 만약에 데이터를 저장하는 인수 테스트가 있으면 재사용하여 Repository 대신 저장요청을 합니다
   • 하지만 그렇지 못하는 상황이라면 테스트에 필요한 데이터를 일일이 입력을 하는데요
   • 이런 반복적인 작업의 복잡도를 어떻게 낮출 수 있을까요?😢 (특히, MenuAcceptanceTest)

2. 서비스 테스트

   • 서비스 테스트 시, 메소드의 depth가 깊다 보니까 테스트가 갈수록 복잡해진다고 생각합니다
   • 예를들어, MenuServiceTest의 두번째 테스트는 첫번째를 통과하는 조건을 given 절에 넣어줬습니다
   • 이렇게 가다보니 마지막 테스트는 조건을 거의 모두 만족하도록 하기위해 더 복잡해졌다고 생각합니다
   • Service에 많은 책임이 있어서 Domain으로 책임이 분리되면 서비스 테스트가 덜 복잡해질 것 같습니다.
   • 하지만 현재 단계에서는 이렇게 구현체를 꼼꼼하게 테스트하는게 맞는가라는 의심이 들기도 하네요
   • 아니면 제가 테스트를 잘못하고 있는지 의문도 듭니다
   • 리뷰어님의 생각을 듣고 싶습니다.

3. ReflectionTestUtils

   • 도메인 객체 생성 할 때, 테스트를 위해 운영 코드를 건드리면 안된다고 생각합니다.
   • 그러다 보니 기본 생성자만 있는 객체는 ReflectionTestUtils을 사용합니다.
   • 리뷰어님은 이런 상황에 어떻게 객체를 생성하는지 궁금합니다.🤔

4. 빌더 패턴? 정적 팩토리 메소드? 일반 생성자?

   • 운영 코드에서 객체를 생성할 수 있으면 위 세가지중에 어느걸 더 선호하시나요?
   • 개인적으로는 파라미터가 많아지면 빌더패턴이 좋다고 생각합니다
   • 또한 정적 팩토리 메소드가 주는 장점도 많다고 생각합니다
   • 트레이드 오프 관계인 것 같기도 한데 어떤 것을 사용하시는지 궁금합니다.

질문이 많이 길어져서 죄송합니다😥.
평소에 회사 코드가 저런 상태인데 인수 테스트를 작성하며 고민해왔던 내용입니다.
어디 물어볼데가 없어서 의견을 한번 듣고 싶습니다.

[sah3122]

테스트 코드 작성해주신다고 고생 많으셨습니다 (_ _)
우선 질문에 대한 답변을 남겨드릴게요.

1. 인수 테스트 데이터베이스 분리 및 데이터 입력

이번 강의에서도 이야기 해주신것 처럼 미리 데이터 셋을 정의 하거나, TestFixture를 사용해볼수 있을것 같아요.
그렇다고 해서 필요한 데이터가 많이 줄어들것 같지는 않을것 같네요 😢
말씀해주신것 처럼 복잡한 관계의 엔티티 때문에 필요한 데이터셋이 많아지니 이것을 단순하게 변경해보는것은 어떨까요 ?
강의 내용에 포함되지만 정리해드릴게요 😄
일반적으로 JPA와 같은 ORM을 사용하다 보면 엔티티간의 연관관계를 무의식적으로 선언하는 경우가 있어요.
반드시 필요한 연관관계 인지 판단하고 엔티티를 설계 해보세요. DDD의 개념으로는 동일한 Context에 포함되어 있는지 등의 고민이 필요해요.
만약 동일한 Context가 아니라면 ID를 통한 간접 참조를 사용하여 결합도를 낮춘다면 필요한 데이터 셋이 확 줄어들거라고 생각합니다.

2. 서비스 테스트

현재 단계에서 꼼꼼하게 테스트 잘해주고 계십니다 😄
알고 계시다 시피 이번 미션의 레거시 코드에서는 서비스 레이어에 모든 비즈니스 로직이 포함되어있어 복잡한 테스트 코드가 발생하게 되요.
추후에 도메인으로 책임이 분산된다면 테스트 코드는 더 간결해질수 있을것 같아요.

3. ReflectionTestUtils

다른 코멘트에서도 많이 언급한것 처럼 ReflectionTestUtils을 거의 사용하지 않는것 같아요.
자주 사용하는 객체를 TestFixture로 정의하여 사용하다보니 ReflectionTestUtils의 필요성을 못느끼는것 같습니다. 😄

4. 빌더 패턴? 정적 팩토리 메소드? 일반 생성자?

해당 질문은 개인적인 답변과 팀에서 사용하는 방법 두가지로 드릴수 있을것 같아요.
우선 개인적으로는 정적 팩토리 메소드 > 빌더 > 생성자 순으로 선호 하는것 같아요.
만약 파라미터가 적다면 생성자 > 정적 팩토리 메소드 > 빌더 입니다 😄
정적 팩토리 메소드는 객체를 생성하는 방법중에 하나지만 파라미터가 적고 많고를 기준으로 나누기 보다는 의미를 가지는 객체를 생성하는 방법으로 더 많이 사용하곤 해요. 정적 팩토리 메소드 내부에서도 빌더 패턴을 사용할 수 있죠 ㅎㅎ
저희 팀에서는 빌더 > 정적 팩토리 메소드 > 생성자 순으로 더 많이 사용하는것 같네요 !

이번 리뷰 요청은 수정 요청 드릴게요.
단위 테스트 위주로 작성, TestFixture 적용, 테스트의 의도를 확실하게 나타내도록 구현 부탁 드립니다. 👍

[sah3122]

테스트 코드 구현 잘해주셨습니다. 💯
몇가지 코멘트를 남겨두었는데 확인부탁드려요 🙏

[bgpark82]

안녕하세요 동철님 리뷰 부탁드립니다🙏
실례가 안된다면 시간이 걸리더라도 조금 꼼꼼히 리뷰 부탁드려도 될까요?
이 부분이 요즘 고민하는 부분이라 같은 고민을 해왔던 분께 의견을 들었으면 합니다.😅

말씀하신 조언대로 Fixture의 네이밍을 해보았는데요 (OrderTable과 Order 테스트 부터 적용했습니다)
조합을 하다보니 구체적인 역할을 하는 Fixture가 보이기 시작하더라구요.
무슨 말씀이었는지 이해가 되는 것 같습니다

그리고 작성하다보니 질문이 하나 있습니다.
조금 바보 같은 질문일 수도 있는데, Fixture를 중복으로 만드는 경우가 있나요?
예를들어 MenuFixture는 순수하게 Menu에 관한 Fixture만 모아 놓고 다른 테스트에서 Menu 픽스처가 필요하면 MenuFixture에서 가져와서 사용하는게 좋다고 생각하는데
막상 Menu 이외의 테스트에서 예를들면 Order 테스트에서 구체적인 내용의 Menu 픽스처를 생성해서 OrderFixture에 모아놓고 사용하게 되더라구요.
여러 사람이 함께 작업하면 비슷한 내용의 Fixture들이 여기저기 산재될 것 같은데,
한 종류의 Fixture를 모두 한곳에 관리한다 vs 상황에 따라 다른 Fixture에서도 관리하게 한다
이 부분에 대해서는 어떻게 생각하시는지 궁금하네요🤔

[sah3122]

피드백 반영 너무 잘해주셨네요 👍
질문에 대한 제 생각 남겨드릴게요 ㅎㅎ

우선 Fixture를 의도적으로 중복생성 하지는 않을것 같아요.
Fixture의 존재를 모르고 있거나 생성된 Fixture를 잊고 새로 만드는 경우는 있을것 같아요.
다른 코멘트에서 남겨두었듯이 저는 도메인 별로 Fixture를 모아두는편을 선호해요.
Order 테스트에서 Menu Fixture가 필요하다면 MenuFixture 클래스에서 적절한 Fixture가 있는지 찾아보고, 없다면 생성할것 같습니다.
Fixture의 사용이 익숙해지고 코드가 축적 된다면 새로운 객체를 생성하는 일이 많이 줄어들수 있을거에요 🙏

[sah3122]

피드백 반영 잘해주셨습니다 ㅎㅎ
몇가지 생각거리를 남겨두었는데 확인 부탁드려요 🙏

[bgpark82]

안녕하세요. 꼼꼼히 리뷰 해주셔서 감사합니다😄

Fixture에 신경이 팔려서 꼼꼼하지 못했네요ㅠ

덕분에 많이 배웠습니다. 감사합니다!

[sah3122]

피드백 반영 너무 잘해주셧네요 👍

이대로 미션을 끝내도 될거 같지만 한가지만 더 부탁드려볼게요.
2주차 강의에서 학습한 Fake객체를 이용하여 테스트 코드를 작성해보는것이 도움이 되실거 같아요.
Mockito를 걷어내고 Repository를 Fake 객체로 만들어 테스트 코드를 작성해보세요.
아마 경험해보시지 못했을껀데 다양한 테스트 코드 작성 방법을 경험하는것도 중요하다고 생각합니다.
이미 만들어져 있는 테스트 코드를 기반으로 FakeRepository로만 변경하면 될것 같아요.
급하게 구현하실 필요는 없으시니 여유를 가지고 도전해보세요 !

[sah3122]

피드백 반영 너무 잘해주셨네요 💯
이대로 끝내긴 아쉬워 추가 미션을 남겨두엇는데 확인 부탁드려요 😄

[sah3122]

assertThatExceptionOfType 을 사용해보는것도 좋을것 같아요 😄
assertThatThrownBy과 동일한 기능이지만 조금더 자연스럽게 표현 가능하다고 되어 있어요 !

This method is more or less the same of assertThatThrownBy(ThrowableAssert.ThrowingCallable) but in a more natural way