[1주차] 3단계 - 테스트를 통한 코드 보호 (5) #92

naheenosaur · 2020-02-19T14:51:04Z

안녕하세요. 김나희입니다.
3단계 테스트 코드 추가했습니다.

controller에 대한 테스트 코드에 대해서 한 가지 궁금한 것이 있는데
controller에 대한 테스트는

서비스 로직에서 정확한 결과가 리턴되는 것 까지를 테스트 하기 위함인지,
올바른 request로 200, 204와 같은 올바른 response 응답값이 나오는것에 대한 controller 의 기본 역할을 테스트 하기 위함인지 정확하게 알고 싶습니다.

1번의 경우라면 역할 위임하는 단계에서 단순하게 역할을 위임하기 위해 포장한 메서드의 경우는 테스트 하지 않아도 된다고 들었던 기억이 있는데, 실제 서비스 기능에 대한 것을 테스트 하는 것 역시 동일하게 접근하면 하지 않아도 될 것 같아서요.
2번의 경우라면 controller에 대한 테스트는 get, post, put, delete에 대한 테스트만 되어도 되지 않을 까 생각을 했는데 controller 에 대한 테스트가 필요한 이유가 혹시 따로 있는지 궁급합니다.
감사합니다.

naheenosaur · 2020-02-19T14:57:25Z

src/test/java/kitchenpos/bo/TableGroupBoTest.java

+    }
+
+    @Test
+    @DisplayName("테이블 그룹은 2개 이상의 빈 테이블로 생성된다.")


테스트 코드 작성을 요구사항에 있는 항목 별 하나로 만들었는데,
그러다 보니 한 요구사항에 여러가지 조건이 같이 들어있는 경우가 있었습니다
이 요구사항 역시 2개 이상의 테이블 / 빈 테이블 이라는 두개의 요구사항으로 나누어 졌는데요,
그래서 한 테스트 코드에 두개의 검증이 생겼네요.

요구사항을 세밀하게 나눌 것인가, 테스트 코드를 나누어서 작성할 것인가 고민하다가
같은 내용에 대한 검증이라는 생각에 요구사항을 나누지 않고, 한 테스트 코드 안에 요구사항에 해당하는 검증을 모으는 것이 더 가독성이 있지 않을까 생각이 들어 이렇게 작성해봤습니다.

일반적으로 테스트마다 하나의 논리적 개념을 테스트하는 것이 좋아요. 아래의 글이 도움 되면 좋겠어요.
https://softwareengineering.stackexchange.com/questions/7823/is-it-ok-to-have-multiple-asserts-in-a-single-unit-test

naheenosaur · 2020-02-19T15:04:59Z

src/test/java/kitchenpos/bo/TableGroupBoTest.java

+        tableGroup = tableGroupBo.create(tableGroup);
+
+        tableGroup.getOrderTables()
+                .forEach(orderTable -> assertThat(orderTable.isEmpty()).isFalse());


assert를 foreach 안에 사용했는데, assertAll처럼 함꼐 확인할 수 있는 좋은 방법이 없을까요..? 😢

ObjectEnumerableAssert#allSatisfy(Consumer)을 찾아보고 그 기능을 사용해 보면 어떨까요?

- Number -> PositiveNumber로 변경 - 객체내에서 멤버볌수 직접 접근

wotjd243

테스트 코드를 잘 작성해 주셨습니다. 👍
질문에 대한 답변은 코멘트로 남겼어요.
그 외에도 몇 가지 의견을 남겨 놓았으니 충분히 고민하고 도전해 보세요!
진행 과정에서 궁금한 점은 댓글이나 Slack을 통해 물어보세요.

wotjd243 · 2020-02-24T00:16:19Z

src/main/java/calculator/PositiveNumber.java

    private static final Pattern NUMERIC_PATTERN = Pattern.compile("[+]?\\d+");
    private int value;

-    Number(List<String> strings) {
+    PositiveNumber(List<String> strings) {


생성자 대신 정적 팩토리 메서드를 사용하면 어떨까요? 정적 팩토리 메서드는 이름만 잘 지으면 반환될 객체의 특성을 쉽게 묘사할 수 있어요. 아래의 글이 도움 되면 좋겠어요.
https://www.slipp.net/wiki/pages/viewpage.action?pageId=30771479

wotjd243 · 2020-02-24T01:06:30Z

src/main/java/kitchenpos/dao/DefaultMenuDao.java

+import java.util.List;
+import java.util.Optional;
+
+public interface DefaultMenuDao {


접두사 'Default'는 주로 구상 클래스(concrete class) 이름에 사용해요.

Suggested change

public interface DefaultMenuDao {

public interface MenuDao {

wotjd243 · 2020-02-24T01:08:45Z

src/main/java/kitchenpos/dao/MenuDao.java

@@ -16,7 +16,7 @@
 import java.util.Optional;

 @Repository
-public class MenuDao {
+public class MenuDao implements DefaultMenuDao {


클래스 이름은 JdbcTemplateMenuDao가 적절해 보여요.

Suggested change

public class MenuDao implements DefaultMenuDao {

public class JdbcTemplateMenuDao implements DefaultMenuDao {

wotjd243 · 2020-02-24T01:11:29Z

src/main/java/kitchenpos/dao/OrderTableDao.java

@@ -70,7 +75,8 @@ public OrderTable save(final OrderTable entity) {
        return jdbcTemplate.query(sql, parameters, (resultSet, rowNumber) -> toEntity(resultSet));
    }

-    private OrderTable select(final Long id) {
+    @Override
+    public OrderTable select(final Long id) {


OrderTableDao 외에 사용하는 곳이 없어 보이네요. public이 아닌 private으로 바꾸면 어떨까요?

wotjd243 · 2020-02-24T01:19:30Z

src/test/java/kitchenpos/bo/MenuBoTest.java

+    @DisplayName("메뉴는 추가될 수 있다.")
+    void createTest() {
+        Menu menu = MenuTest.ofHalfAndHalf();
+        assertThat(menuBo.create(menu));


더 많은 단언문이 필요해 보여요.

wotjd243 · 2020-02-24T01:30:10Z

src/test/java/kitchenpos/bo/MenuBoTest.java

+    @DisplayName("모든 메뉴 리스트를 조회할 수 있다.")
+    void readAllMenuListTest() {
+        Menu menu = MenuTest.ofHalfAndHalf();
+        menuBo.create(menu);


menuBo.list()를 테스트하기 위해서 menuBo.create(menu)에 대한 테스트가 꼭 통과해야 할까요?

wotjd243 · 2020-02-24T01:55:37Z

src/test/java/kitchenpos/bo/TableGroupBoTest.java

+    }
+
+    @Test
+    @DisplayName("테이블 그룹은 2개 이상의 빈 테이블로 생성된다.")


일반적으로 테스트마다 하나의 논리적 개념을 테스트하는 것이 좋아요. 아래의 글이 도움 되면 좋겠어요.
https://softwareengineering.stackexchange.com/questions/7823/is-it-ok-to-have-multiple-asserts-in-a-single-unit-test

wotjd243 · 2020-02-24T01:57:23Z

src/test/java/kitchenpos/bo/TableGroupBoTest.java

+        tableGroup = tableGroupBo.create(tableGroup);
+
+        tableGroup.getOrderTables()
+                .forEach(orderTable -> assertThat(orderTable.isEmpty()).isFalse());


ObjectEnumerableAssert#allSatisfy(Consumer)을 찾아보고 그 기능을 사용해 보면 어떨까요?

wotjd243 · 2020-02-24T01:57:46Z

src/test/java/kitchenpos/dao/InMemoryMenuDao.java

+
+import java.util.*;
+
+public class InMemoryMenuDao implements DefaultMenuDao {


가짜 객체 👍

- Class, Interface 이름 수정

- assertion 분리 - parameterizedTest 사용

- allSatisfy로 foreach 대체

- create에 대한 test에 속성값 확인

wotjd243

피드백을 잘 반영해 주셨습니다. 👍
추가로 고민해 보면 좋을 것 같은 의견을 남겨 놓았으니 충분히 고민하고 도전해 보세요!
진행 과정에서 궁금한 점은 댓글이나 Slack을 통해 물어보세요.

wotjd243 · 2020-02-28T02:28:02Z

README.md

-    - [ ]   메뉴는 0개 이상의 항목를 포함한다.
+    - [X]   메뉴는 한 가지 메뉴그룹에 속해 있어야 한다.
+    - [X]   메뉴의 가격은 0원 이상이다.
+    - [X]   메뉴는 0개 이상의 항목를 포함한다.


오타 발견!

Suggested change

- [X] 메뉴는 0개 이상의 항목를 포함한다.

- [X] 메뉴는 0개 이상의 항목을 포함한다.

wotjd243 · 2020-02-28T02:28:27Z

src/test/java/kitchenpos/bo/MenuBoTest.java

+        Menu menuResult;
+        assertThat(menuResult = menuBo.create(menu));


아래의 코드로 개선해 보면 어떨까요?

Suggested change

Menu menuResult;

assertThat(menuResult = menuBo.create(menu));

Menu menuResult = menuBo.create(menu);

wotjd243 · 2020-02-28T02:28:37Z

src/test/java/kitchenpos/bo/MenuBoTest.java

+    }
+
+    @Test
+    @DisplayName("메뉴는 0개 이상의 항목를 포함한다.")


오타 발견!

Suggested change

@DisplayName("메뉴는 0개 이상의 항목를 포함한다.")

@DisplayName("메뉴는 0개 이상의 항목을 포함한다.")

wotjd243 · 2020-02-28T05:30:17Z

src/test/java/kitchenpos/bo/OrderBoTest.java

+        Order order = OrderTest.ofOneHalfAndHalfInSingleTable();
+        order.setOrderStatus(OrderStatus.COMPLETION.toString());
+
+        assertThrows(IllegalArgumentException.class,
+                () -> orderBo.changeOrderStatus(order.getId(), order));


의도한 테스트와는 다른 테스트 코드입니다.

wotjd243 · 2020-02-28T05:34:35Z

src/test/java/kitchenpos/bo/TableBoTest.java

+        OrderTable orderTable = OrderTableTest.ofSingle();
+        orderTable = orderTableDao.save(orderTable);
+
+        orderTable.setEmpty(true);
+        tableBo.changeEmpty(orderTable.getId(), orderTable);
+
+        assertThat(orderTable.isEmpty()).isTrue();


의도한 테스트와는 다른 테스트 코드입니다.

wotjd243 · 2020-02-28T05:38:04Z

src/test/java/kitchenpos/model/TableGroupTest.java

+        tableGroup.setCreatedDate(LocalDateTime.now());
+        tableGroup.setId(EMPTY_TABLE_GROUP_ID);
+        tableGroup.setOrderTables(
+                Arrays.asList(OrderTableTest.ofEmpty(), OrderTableTest.ofAnotherEmpty())


'단체 지정된 빈 테이블'은 실제로 존재할 수 있을까요?

naheenosaur added 6 commits February 18, 2020 22:51

test : interface 분리, menu 테스트 코드 작성

307a68f

test : menuGroup 테스트 코드 작성

72de7c4

test : order, product 테스트 코드 작성

53c641e

test : table 테스트 코드 작성, 기본 model 데이터 추출

cf2aa32

test : tableGroup 테스트 코드 작성

3da5f67

refactor : 코드정리

9915d0d

naheenosaur commented Feb 19, 2020

View reviewed changes

refactor : 피드백 반영

2091097

- Number -> PositiveNumber로 변경 - 객체내에서 멤버볌수 직접 접근

wotjd243 requested changes Feb 24, 2020

View reviewed changes

naheenosaur added 5 commits February 24, 2020 21:30

refactor : 피드백 반영

e4879b6

- Class, Interface 이름 수정

refactor : 피드백 반영

990bb6a

- assertion 분리 - parameterizedTest 사용

refactor : 피드백 반영

206582a

- allSatisfy로 foreach 대체

refactor : 피드백 반영

d65b03c

- create에 대한 test에 속성값 확인

refactor : Number Class 정적팩토리로 표현, 메모리에 저장

b8c5830

naheenosaur force-pushed the feature/step3 branch from 0a8fbea to b8c5830 Compare February 24, 2020 13:45

naheenosaur changed the title ~~[1주차] 3단계 - 테스트를 통한 코드 보호 (3)~~ [1주차] 3단계 - 테스트를 통한 코드 보호 (4) Feb 25, 2020

wotjd243 requested changes Feb 28, 2020

View reviewed changes

refactor : 피드백 반영, 기본 데이터를 유효한 데이터로 수정

7bfd18b

naheenosaur changed the title ~~[1주차] 3단계 - 테스트를 통한 코드 보호 (4)~~ [1주차] 3단계 - 테스트를 통한 코드 보호 (5) Mar 19, 2020

refactor : 코드 정리

7668dd6

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[1주차] 3단계 - 테스트를 통한 코드 보호 (5) #92

[1주차] 3단계 - 테스트를 통한 코드 보호 (5) #92

naheenosaur commented Feb 19, 2020 •

edited

Loading

naheenosaur Feb 19, 2020

wotjd243 Feb 24, 2020

naheenosaur Feb 19, 2020

wotjd243 Feb 24, 2020

wotjd243 left a comment

wotjd243 Feb 24, 2020

wotjd243 Feb 24, 2020

wotjd243 Feb 24, 2020

wotjd243 Feb 24, 2020

wotjd243 Feb 24, 2020

wotjd243 Feb 24, 2020

wotjd243 Feb 24, 2020

wotjd243 Feb 24, 2020

wotjd243 Feb 24, 2020

wotjd243 left a comment

wotjd243 Feb 28, 2020

wotjd243 Feb 28, 2020

wotjd243 Feb 28, 2020

wotjd243 Feb 28, 2020

wotjd243 Feb 28, 2020

wotjd243 Feb 28, 2020

	public class MenuDao implements DefaultMenuDao {
	public class JdbcTemplateMenuDao implements DefaultMenuDao {


		import java.util.*;

		public class InMemoryMenuDao implements DefaultMenuDao {

	- [X] 메뉴는 0개 이상의 항목를 포함한다.
	- [X] 메뉴는 0개 이상의 항목을 포함한다.

		Menu menuResult;
		assertThat(menuResult = menuBo.create(menu));

	Menu menuResult;
	assertThat(menuResult = menuBo.create(menu));
	Menu menuResult = menuBo.create(menu);

	@DisplayName("메뉴는 0개 이상의 항목를 포함한다.")
	@DisplayName("메뉴는 0개 이상의 항목을 포함한다.")

[1주차] 3단계 - 테스트를 통한 코드 보호 (5) #92

Are you sure you want to change the base?

[1주차] 3단계 - 테스트를 통한 코드 보호 (5) #92

Conversation

naheenosaur commented Feb 19, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wotjd243 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wotjd243 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

naheenosaur commented Feb 19, 2020 •

edited

Loading