3단계 - 테스트를 통한 코드 보호 #264

dacapolife87 · 2022-03-18T16:17:46Z

안녕하세요
리뷰어님
이번에는 리뷰요청이 많이 늦었네요.
이번에 요구사항분석부분을 전체적으로 다시 작성해보았습니다.
일단 최대한 처음보는 사람도 이해가 가능하도록 재 작성을 해보았는데
요구사항작성을 하고난뒤 step3인 테스트코드를 작성해보니 테스트코드를 어떻게 작성해야할지가 눈에 들어오네요
지난 수업을 듣고나서 가짜객체를 이용하여 테스트코드를 작성해보았는데.
생각보다 생각할 점이 많았습니다.
특히 Order부분은 다른 서비스영역과 겹치는것이 많아서 테스트 데이터를 만드는것부터가 생각이 많아졌습니다.
테스트 샘플 데이터를 중복을 최소화하면서 어떻게하면 해당 테스트가 어떠한 목적을 가진 테스트인지 쉽게 보이도록 해보았는데... 다른 사람의 눈에는 어떻게 보이는지가 모르겠네요

또 다른 고민은 테스트객체를 생성하기위해 static메소드를 사용해요 다른 테스트에서 다른서비스영역의 데이터가 필요할때 생성할수있도록 작성해 보았습니다.
이 과정에서 static메소드가 많이 늘어나게되었는데요
이렇게 테스트코드가 작성되도 상관이없는지 잘 판단이 서질않네요
Menu관련은 MenuServiceTest쪽에서 생성해서 값을 받아 사용하고 Order관련은 Order쪽에서 생성해서 값을 받아 사용하고... 이런식으로 하다보니 필요한 데이터는 해당 서비스테스트코드에서 메소드를 사용하여 가져오면된다고 생각 하였는데
이 역시 다른사람눈에는 어떻게 보이는지가 의문입니다.

이번에 테스트관련해서 많은 배움을 얻고 싶습니다.
좋은 피드백부탁드리겠습니다
매번 감사드립니다.

- 지난번 요구사항정리 관련 피드백 보충

- Repository관련 가짜 객체 생성

- PurgomalumClient관련 가짜객체 생성하여 로직수정

- Repository 테스트를 위하여 가짜객체용 interface와 실사용 interface분리

- 테스트용 객체 생성을 위한 로직 추가

- 각 기능별 테스트 로직 추가

wotjd243

테스트 코드를 잘 작성해 주셨습니다. 👍
테스트 코드를 작성하는 이유 중에는 '테스트 코드를 통해 요구 사항을 이해할 수 있다'와 '클라이언트가 객체를 사용하는 방법에 관한 적절한 코드 예제를 제공하는 것'이 있습니다.
그 외에도 몇 가지 의견을 남겨 놓았으니 충분히 고민하고 도전해 보세요!
진행 과정에서 궁금한 점은 댓글이나 Slack을 통해 물어보세요.

wotjd243 · 2022-04-02T05:33:41Z

src/test/java/kitchenpos/application/MenuGroupServiceTest.java

+
+        List<MenuGroup> findedMenuGroup = menuGroupService.findAll();
+
+        assertThat(findedMenuGroup).hasSize(menuGroupSize);


변수와 필드 대신 실제 값을 사용하여 단언문을 작성해 보세요.

Suggested change

assertThat(findedMenuGroup).hasSize(menuGroupSize);

assertThat(findedMenuGroup).hasSize(2);

실제 테스트시 적재한 갯수를 기준으로 변경하였습니다.

wotjd243 · 2022-04-02T07:07:00Z

README.md

-    - [ ] 빈테이블은 손님수를 변경할수 없다.
-  - [ ] 전체 테이블의 목록을 조회할수 있어야한다.
+  - [X] 사용자는 메뉴를 등록할 수 있다.
+    - [X] 메뉴의 가격은 0원 이상이여야한다.


오타 발견!

Suggested change

- [X] 메뉴의 가격은 0원 이상이여야한다.

- [X] 메뉴의 가격은 0원 이상이어야 한다.

오타 수정하였습니다 ^^;;;

wotjd243 · 2022-04-02T07:07:17Z

README.md

+  - [X] 사용자는 주문 테이블을 등록할 수 있다.
+    - [X] 주문테이블의 이름은 필수로 지정해야한다.
+    - [X] 주문테이블 생성시 손님수는 0명에 빈테이블로 설정된다.
+  - [X] 주문테이블의 상태를 착성으로 변경 할 수 있다.


오타 발견!

Suggested change

- [X] 주문테이블의 상태를 착성으로 변경 할 수 있다.

- [X] 주문테이블의 상태를 착석으로 변경할 수 있다.

오타 수정하였습니다 ^^;;;

wotjd243 · 2022-04-02T07:08:28Z

src/main/java/kitchenpos/domain/MenuGroupRepository.java

+
+}
+
+interface JpaMenuGroupRepository extends MenuGroupRepository, JpaRepository<MenuGroup, UUID> {


아래의 Google Java Style Guide를 참고해 주세요.

Each top-level class resides in a source file of its own.

기본적인건데 이부분을 분리를 안했었네요
모든 Repository를 분리하였습니다.

wotjd243 · 2022-04-02T07:08:40Z

src/main/java/kitchenpos/domain/MenuRepository.java

+
+    List<Menu> findAll();
+
+    void deleteDataForTest();


테스트를 위한 메서드는 과감하게 지우는 것이 좋겠어요.

이 부분을 하면서도 과연 이게 맞는건가하는 의문이 많이 들었습니다.
테스트코드의 전면수정을 통하여 해당 코드없이 테스트가 돌아가도록 하였습니다.

wotjd243 · 2022-04-02T07:34:19Z

src/test/java/kitchenpos/application/OrderTableServiceTest.java

+
+    @DisplayName("주문테이블의 이름은 필수로 지정해야한다.")
+    @ParameterizedTest
+    @MethodSource("invalidOrderTableName")


@MethodSource 대신 @NullAndEmptySource를 사용하면 어떨까요?

@NullAndEmptySource 를 깜빡하고 뭐 좋은 방법이 없을까 찾아보다가 MethodSource를 찾아서 해보았는데;;
생각해보니 @NullAndEmptySource가 있었네요;;;;

wotjd243 · 2022-04-02T07:34:33Z

src/test/java/kitchenpos/application/OrderTableServiceTest.java

+
+        OrderTable orderTable = orderTableService.clear(savedOrderTable.getId());
+
+        assertThat(orderTable.isEmpty()).isTrue();


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

단언문을 추가하였습니다.

wotjd243 · 2022-04-02T07:34:56Z

src/test/java/kitchenpos/domain/InMemoryMenuGroupRepository.java

+
+import java.util.*;
+
+public class InMemoryMenuGroupRepository implements MenuGroupRepository {


가짜 객체 👍

wotjd243 · 2022-04-02T07:35:01Z

src/test/java/kitchenpos/domain/InMemoryMenuGroupRepository.java

+    @Override
+    public MenuGroup save(MenuGroup menuGroup) {
+        UUID uuid = UUID.randomUUID();
+        menuGroup.setId(uuid);


다른 개발자도 예상할 수 있는 기능일까요?

ID를 저장소에서 생성하여 할당한다고 생각하고 save안에서 지정해 보앗었습니다.
하지만 이렇게 하다보니 테스트코드의 복잡도가 올라가게 되고
리뷰어님 말씀하신대로 다른 개발자들은 예상할수 없을듯하네요
이런 정해지지않은 랜덤적인 요소는 외부로 빼서 테스트를 하는게 맞을거 같습니다.

wotjd243 · 2022-04-02T07:35:15Z

src/test/java/kitchenpos/domain/InMemoryOrderRepository.java

+                .filter(order -> Objects.equals(order.getOrderTableId(), orderTable.getId()))
+                .filter(order -> !Objects.equals(order.getStatus(), status))
+                .findAny()
+                .isPresent();


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

Suggested change

.filter(order -> Objects.equals(order.getOrderTableId(), orderTable.getId()))

.filter(order -> !Objects.equals(order.getStatus(), status))

.findAny()

.isPresent();

.anyMatch(order -> order.getOrderTableId().equals(orderTable.getId()) && order.getStatus() != status);

anyMatch를 사용하니 좀더 간결해 지네요
피드백주신 내용으로 개선해보았습니다.

- repository 코드 분리

- 오타수정

- 테스트목적의 불필요 코드 제거

- test를 위한 fixture 생성

- inMemoryRepository save로직 수정

- 불필요파일 제거

- 테스트 전면수정, 테스트별로 repository 공유가 아닌 독립적으로 사용하도록 변경

- InMemoryOrderRepository의 existsByOrderTableAndStatusNot 로직 수정

- 테스트 케이스 추가

- isNotNull 체크 케이스 외부로 추출

dacapolife87 · 2022-04-13T10:05:13Z

안녕하세요 리뷰어님
정말 오랜만에 리뷰요청을 드리게되었습니다.
회사업무도 바빴지만 그보다 이번에는 좀더 생각을 많이 해보느랴 수정도 좀 늦었고 이리저리 변경해보느랴 시간도 오래걸렸던거같네요
테스트코드를 작성한다는게 매번 느끼지만 쉽지가 않네요
한번 작성해두면 마음의평화?를 가져다 주는데 실제로 이부분을 지속적으로 사용하려면 이해하기 쉽고 파악이 쉽고 간결하게 읽혀야 그 테스트코드는 오래동안 사용되고 시스템을 파악하는데도 더 도움이 되는거같습니다.
이 부분에 대해 고민을 정말 많이 해보는 미션이 되엇네요

특히 테스트코드를 간편하게해보고자 공통으로 사용하여 만들어 쓰려던것이 오히려 독이되어 테스트코드를 망치게되었던거 같습니다.
그래서 이번에 전체부분에서 많이 수정을 하게되었고 특히 테스트를 진행하면서 테스트를 위한 데이터를 저장할때 어디까지 실제 데이터를 저장해야하고 어느 범위부터는 임의의 값으로 테스트를 진행해도 되는가에 대해 정말 많은 깨달음을 느꼈습니다.

이번에도 좋은 리뷰와 피드백 부탁드리겠습니다
감사합니다.

dacapolife87 added 6 commits March 19, 2022 01:02

docs(키친포스) : 3단계 - 테스트를 통한 코드 보호

175e14c

- 지난번 요구사항정리 관련 피드백 보충

docs(키친포스) : 3단계 - 테스트를 통한 코드 보호

dc83dcb

- Repository관련 가짜 객체 생성

docs(키친포스) : 3단계 - 테스트를 통한 코드 보호

39bc073

- PurgomalumClient관련 가짜객체 생성하여 로직수정

docs(키친포스) : 3단계 - 테스트를 통한 코드 보호

0a03f8f

- Repository 테스트를 위하여 가짜객체용 interface와 실사용 interface분리

docs(키친포스) : 3단계 - 테스트를 통한 코드 보호

7b9c31c

- 테스트용 객체 생성을 위한 로직 추가

docs(키친포스) : 3단계 - 테스트를 통한 코드 보호

e3ae3e5

- 각 기능별 테스트 로직 추가

wotjd243 requested changes Apr 2, 2022

View reviewed changes

dacapolife87 added 10 commits April 12, 2022 23:12

docs(키친포스) : 3단계 - 테스트를 통한 코드 보호

a06859d

- repository 코드 분리

docs(키친포스) : 3단계 - 테스트를 통한 코드 보호

21f4541

- 오타수정

docs(키친포스) : 3단계 - 테스트를 통한 코드 보호

4d778ba

- 테스트목적의 불필요 코드 제거

test(키친포스) : 3단계 - 테스트를 통한 코드 보호

1da68c1

- test를 위한 fixture 생성

test(키친포스) : 3단계 - 테스트를 통한 코드 보호

76fc081

- inMemoryRepository save로직 수정

test(키친포스) : 3단계 - 테스트를 통한 코드 보호

56b4bc5

- 불필요파일 제거

test(키친포스) : 3단계 - 테스트를 통한 코드 보호

b8eda57

- 테스트 전면수정, 테스트별로 repository 공유가 아닌 독립적으로 사용하도록 변경

test(키친포스) : 3단계 - 테스트를 통한 코드 보호

b218919

- InMemoryOrderRepository의 existsByOrderTableAndStatusNot 로직 수정

test(키친포스) : 3단계 - 테스트를 통한 코드 보호

6d5a66d

- 테스트 케이스 추가

test(키친포스) : 3단계 - 테스트를 통한 코드 보호

d8611e3

- isNotNull 체크 케이스 외부로 추출

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

3단계 - 테스트를 통한 코드 보호 #264

3단계 - 테스트를 통한 코드 보호 #264

dacapolife87 commented Mar 18, 2022

wotjd243 left a comment

wotjd243 Apr 2, 2022

dacapolife87 Apr 13, 2022

wotjd243 Apr 2, 2022

dacapolife87 Apr 13, 2022

wotjd243 Apr 2, 2022

dacapolife87 Apr 13, 2022

wotjd243 Apr 2, 2022

dacapolife87 Apr 13, 2022

wotjd243 Apr 2, 2022

dacapolife87 Apr 13, 2022

wotjd243 Apr 2, 2022

dacapolife87 Apr 13, 2022

wotjd243 Apr 2, 2022

dacapolife87 Apr 13, 2022

wotjd243 Apr 2, 2022

wotjd243 Apr 2, 2022

dacapolife87 Apr 13, 2022

wotjd243 Apr 2, 2022

dacapolife87 Apr 13, 2022

dacapolife87 commented Apr 13, 2022


		List<MenuGroup> findedMenuGroup = menuGroupService.findAll();

		assertThat(findedMenuGroup).hasSize(menuGroupSize);

	assertThat(findedMenuGroup).hasSize(menuGroupSize);
	assertThat(findedMenuGroup).hasSize(2);

	- [X] 메뉴의 가격은 0원 이상이여야한다.
	- [X] 메뉴의 가격은 0원 이상이어야 한다.

	- [X] 주문테이블의 상태를 착성으로 변경 할 수 있다.
	- [X] 주문테이블의 상태를 착석으로 변경할 수 있다.


		}

		interface JpaMenuGroupRepository extends MenuGroupRepository, JpaRepository<MenuGroup, UUID> {


		OrderTable orderTable = orderTableService.clear(savedOrderTable.getId());

		assertThat(orderTable.isEmpty()).isTrue();


		import java.util.*;

		public class InMemoryMenuGroupRepository implements MenuGroupRepository {

3단계 - 테스트를 통한 코드 보호 #264

Are you sure you want to change the base?

3단계 - 테스트를 통한 코드 보호 #264

Conversation

dacapolife87 commented Mar 18, 2022

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

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

dacapolife87 commented Apr 13, 2022