3단계 - 테스트를 통한 코드 보호 #661
3단계 - 테스트를 통한 코드 보호 #661
Conversation
| private void mockingMenuGroupRepositoryForCreate(Menu menu) { | ||
| Mockito.when(menuGroupRepository.findById(Mockito.any())) | ||
| .thenReturn(Optional.of(MenuFixture.extractMenuGroupFrom(menu))); | ||
| } | ||
|
|
||
| private void mockingProductRepositoryForCreate(Menu menu) { | ||
| Mockito.when(productRepository.findAllByIdIn(Mockito.any())) | ||
| .thenReturn(MenuFixture.extractProductsFrom(menu)); | ||
| Mockito.when(productRepository.findById(Mockito.any())) | ||
| .thenReturn(Optional.of(MenuFixture.extractProductFrom(menu))); | ||
| } | ||
|
|
||
| private void mockingPurgomalumClientForCreate(boolean result) { | ||
| Mockito.when(purgomalumClient.containsProfanity(Mockito.any())) | ||
| .thenReturn(result); | ||
| } | ||
|
|
||
| private void mockingMenuRepositoryForCreate(Menu menu) { | ||
| Mockito.when(menuRepository.save(Mockito.any())) | ||
| .thenReturn(menu); | ||
| } |
There was a problem hiding this comment.
mocking용 메서드를 분리하여 재활용 할수 있게 선언해주신 부분이 인상적이네요 👍
|
|
||
| MenuGroup result = menuGroupService.create(한식); | ||
|
|
||
| Assertions.assertThat(result.getName()).isEqualTo(한식.getName()); |
There was a problem hiding this comment.
생성이 정상적으로 되었다면 UUID를 가진 객체가 리턴되었는지 검증하는것도 좋아보이네요 👍
| @Test | ||
| @DisplayName("메뉴의 가격은 상품 포함된 상품의 총 가격 보다 클 수 없다.") | ||
| void create_exception_price_difference() { | ||
| Menu 상품_가격보다_큰_메뉴 = menuFixture.상품_가격보다_큰_메뉴; |
There was a problem hiding this comment.
Fixture 사용 잘해주셨네요 👍
Fixture를 정의하는 방법은 정해진것이 없지만 저는 가독성을 위주로 Fixture를 작성하곤 합니다.
상품_가격보다_큰_메뉴로 정의 해주신것도 좋지만 메뉴의 가격이나 상품의 가격도 알수 있게 나타내보는건 어떨까요 ?
There was a problem hiding this comment.
네이밍에 대한 고민을 많이 했는데 말씀하신 것처럼 가격을 아예 명시하는 것이 가격도 알 수 있고 어떤 케이스인지 알 수 있어 좋은 것 같습니다!
| Menu 메뉴_B = menuFixture.메뉴_B; | ||
| Menu 메뉴_A = menuFixture.메뉴_A; |
There was a problem hiding this comment.
두 메뉴의 가격을 테스트 코드만 보고 알수 있을까요 ?
Fixture는 자주 사용하거나, 의미를 가지는 테스트 객체를 정의하는 용도로 사용합니다.
가격에 관련한 테스트가 많이 보이는데, 가격을 입력값으로 전달 받는 Fixture 함수를 정의하는건 어떨까요 ?
There was a problem hiding this comment.
좋은 말씀이신 것 같습니다.
그런데 가격을 입력 값으로 전달 받는 Fixture 함수라는 것은 생성된 Fixture에 값을 변경하는 메서드와 값만을 인자 값으로 가지는 생성자 메서드 중에서 어떤 것을 말씀하시는 것인지 알 수 있을까요 ?
There was a problem hiding this comment.
Fixture에서 정의한 create 함수를 Test 코드에서도 사용하는것 입니다 😄
가격과 필수값들을 인자로 받는 함수를 정의하여 재활용해보는것도 좋을것 같아요 👍
| for (Product 상품 : 상품_목록) { | ||
| Assertions.assertThatThrownBy( | ||
| () -> productService.create(상품) | ||
| ).isInstanceOf(IllegalArgumentException.class); | ||
| } |
There was a problem hiding this comment.
하나 이상의 단언문을 테스트 할땐 AssertAll함수를 사용해보세요 😄
|
|
||
| @Test | ||
| @DisplayName("해당 상품으로 구성된 메뉴의 가격이 변경된 상품의 가격 총합보다 크다면 메뉴를 노출하지 않는다.") | ||
| void changePrice_exception_price() { |
There was a problem hiding this comment.
마찬가지로 Fixture를 사용한 테스트 코드만 봐서는 어떤 값이 준비되어 있는지 알기 힘들다고 생각합니다 😄
모든 테스트를 Fixture를 사용하기 보단 의미를 가지는 테스트 객체만 Fixture로 정의해보느건 어떨까요 ?
There was a problem hiding this comment.
제가 Fixture를 많이 안다뤄봐서 사용이 미숙했던 것 같습니다..
말씀하신 것처럼 기본 더미 값, 자주 사용 되는 값, 특별한 의미를 가지는 값에 대해서 Fixture로 정의하는 것이 좋은 것 같습니다.
| } | ||
|
|
||
| @Test | ||
| @DisplayName("손님은 식당에서 음식을 주문할 수 있다.") |
There was a problem hiding this comment.
@Nested기능을 활용하여 주문 상태별로 테스트 코드를 분리해보는건 어떨까요 ?
| } | ||
| } | ||
|
|
||
| private enum RepositoryMethod { |
There was a problem hiding this comment.
RepositoryMethod를 활용하여 모킹을하는 기법도 인상적이네요 😄
| Order result = orderService.create(매장_주문); | ||
| Assertions.assertThat(result.getOrderTable()).isEqualTo(주문_테이블); |
There was a problem hiding this comment.
| Order result = orderService.create(매장_주문); | |
| Assertions.assertThat(result.getOrderTable()).isEqualTo(주문_테이블); | |
| Order result = orderService.create(매장_주문); | |
| Assertions.assertThat(result.getOrderTable()).isEqualTo(주문_테이블); |
공백 라인을 활용하여 문단을 나누어 가독성을 높혀보세요 😄
|
동철님 안녕하세요~ 피드백 주신 것 확인하였으며 나머지 테스트 코드 작성했습니다. 테스트 Fixture 리팩터링만 남은 상태인데 질문 답변 확인 후에 추가 Push 진행하도록 하겠습니다. 감사합니다. |
| @DisplayName("손님은 포장 주문을 할 수 있다.") | ||
| void create_takeOut() { | ||
| Order 포장_주문 = orderFixture.포장_주문_A; | ||
| @Nested |
| Menu 메뉴_B = menuFixture.메뉴_B; | ||
| Menu 메뉴_A = menuFixture.메뉴_A; |
There was a problem hiding this comment.
Fixture에서 정의한 create 함수를 Test 코드에서도 사용하는것 입니다 😄
가격과 필수값들을 인자로 받는 함수를 정의하여 재활용해보는것도 좋을것 같아요 👍
| for (Order exceptionOrder : exceptionOrders) | ||
| { | ||
| Assertions.assertThatThrownBy( | ||
| () -> orderService.create(exceptionOrder) | ||
| ).isInstanceOf(IllegalArgumentException.class); | ||
| } |
There was a problem hiding this comment.
AssertAll 과 AssertThat을 여러번 사용하는것의 차이점을 확인해보세요 😄
동철님 안녕하세요, step3 진행하여 PR 생성합니다.
처음에 Fixture를 static으로 선언하여 사용했는데 상태 값을 변경하는 테스트 코드가 있어서 static -> non-static 수정하였습니다. DM으로 말씀드렸던 것처럼 피드백 해주시면 남은 부분 마저 진행하도록 하겠습니다. 이번 미션에서 mocking 및 fixture 사용에 대한 피드백도 많이 해주시면 감사하겠습니다.
감사합니다.