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

Dongock · 2025-02-06T14:37:03Z

개요

안녕하세요. 리뷰어님. 3단계는 유독 시간이 많이 걸렸네요. 고민도 고민이지만 일단 물리적인 양이 많았어서 약간 압도당하는 기분이었습니다.

테스트 코드를 작성하면서 느꼈던 부분이 굉장히 공통적인 내용이 많이 나온다는 점이었는데요. 그래서 생략해도 되지 않을까란 생각을 했는데, 다시 생각해보니 언제 어떻게 비즈니스가 변할지 모르는데 지금 공통적인 내용이 있다고 생략해버리면 문제가 될 거 같아서 최대한 모두 작성하려고 했습니다.
질문도 따로 드렸었지만 생성자에 대한 리팩토링 여부가 고민이 있긴했었는데요. 교재를 보니 Text Fixture가 있어서 이걸 이용해서 따로 생성자 리팩토링 없이 진행을 했습니다.
가격 검증을 하는 부분이 좀 있었던 거 같은데 조금 이해가 안됐던게 메뉴의 가격이 상품들의 가격의 합보다 크면 에러가 발생하는데 사실 메뉴의 가격이 상품들의 가격의 합보다 커야되지 않나라는게 제 생각인데 이 부분은 어떻게 생각하실까요?

이번 과제는 유독 오래걸렸지만서도 오히려 다른 과제들보다 부족한게 많다는 생각이 드는데요. 세세해도 좋으니 최대한 자세한 피드백 부탁드리겠습니다. 시간이 좀 걸려도 잘 마무리하고 싶단 생각이 드네요.

- 구분자 연속으로 올 시 사이 빈 값은 0으로 처리 - 구분자 기능 완성 및 요구사항 추가

- 커스텀 구분자 기능 완성 및 요구사항 추가

- 음수 예외처리 기능 완성 및 요구사항 추가

…() & makeTestDeliveryOrder() 생성

…() 생성

Hyeon9mak

안녕하세요 동옥님! 마지막 단계 고생 많으셨습니다!
88개 테스트 작성에 많은 시간이 드셨을거 같아요.
하지만 덕분에 동료 개발자들이 믿고 리팩터링 할 수 있는 환경이 구성된거 같습니다 👍

테스트 코드를 작성하면서 느꼈던 부분이 굉장히 공통적인 내용이 많이 나온다는 점이었는데요. 그래서 생략해도 되지 않을까란 생각을 했는데, 다시 생각해보니 언제 어떻게 비즈니스가 변할지 모르는데 지금 공통적인 내용이 있다고 생략해버리면 문제가 될 거 같아서 최대한 모두 작성하려고 했습니다.

매우 건강하고 멋지다고 생각합니다!
비즈니스가 어떻게 변경될지 모르는 복잡한 환경에서는 "귀찮음" 과 "어려움" 중에 "귀찮음" 을 희생하는게 맞죠 👍
잘 아시겠지만, 반대로 비즈니스 어떻게 변경될지 뻔히 예측되는 환경에서는 과감하게 이를 생략하고,
빠르게 퇴근해서 가족들과 시간을 보내는 것도 방법인거 같습니다 😄

가격 검증을 하는 부분이 좀 있었던 거 같은데 조금 이해가 안됐던게 메뉴의 가격이 상품들의 가격의 합보다 크면 에러가 발생하는데 사실 메뉴의 가격이 상품들의 가격의 합보다 커야되지 않나라는게 제 생각인데 이 부분은 어떻게 생각하실까요?

코드를 작성하시면서 정책적으로 말이 안되는 부분을 발견하는 것도 중요하죠 👍
(기획자도 사람이라 충분히 실수 할 수 있으니까요!)
우선 해당 부분을 기억해두시고, 동옥님께서 생각하시기에 올바른 정책 방향으로 "실패하는 테스트" 를 만드신 다음 이후 테스트가 성공할 수 있도록 리팩터링을 해주시면 될 것 같습니다!
모두가 알고 있는 정책 대비 변경사항이 생긴것이므로, 함께 미션을 진행하는 리뷰어분께 설명은 필수겠네요!

아주 간단한 코멘트 몇 가지 남겼습니다.
궁금하신 점 편하게 질문주세요~!

Hyeon9mak · 2025-02-08T04:04:16Z

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

+    @DisplayName("메뉴 모음명은 비어있다면 에러를 발생시킨다.")
+    @ParameterizedTest
+    @NullSource
+    void nullName(String name) {
+        // given
+        MenuGroup menuGroup = makeTestMenuGroup(name);
+        when(menuGroupRepository.save(any(MenuGroup.class))).thenReturn(menuGroup);
+
+        // then
+        assertThatThrownBy(() -> menuGroupService.create(menuGroup)).isInstanceOf(IllegalArgumentException.class);
+    }


null 은 테스트가 된 것 같은데, empty, blank 는 테스트가 되었을지 궁금하네요!

원래 NullAndEmptySoruce를 사용했었는데요. 코드를 보니까 empty일때는 정상으로 넘어가서 Null만 막게 되더라구요. 그래서 NullSource로 변경해서 작성했는데, 말씀하신대로 생각해보니까 이름에 emtpy인 것도 문제가 될 거 같습니다. 테스트를 바꿀게 아니라 기존 코드를 바꾸는게 맞았던 거 같아요

메뉴그룹은 애초에 Emtpy도 잡고 있었네요 ㅎㅎ 다른데서 착각한 거 같습니다. 감사합니다~

Hyeon9mak · 2025-02-08T04:04:33Z

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

+    @Test
+    void findAll() {
+    }


불필요해진 테스트는 제거해주시면 빌드 속도가 훨씬 더 빨라질거 같아요 😄

Hyeon9mak · 2025-02-08T04:04:46Z

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

+    @Test
+    void findAll() {
+    }
+}


https://hyeon9mak.github.io/github-no-newline-at-a-end-of-file/
😉

바로 설정 완료 했습니다!

Hyeon9mak · 2025-02-08T04:05:21Z

src/test/java/kitchenpos/application/MenuServiceTest.java

+    @DisplayName("메뉴 가격은 0원보다 작다면 에러를 발생시킨다.")
+    @Test
+    void minusPrice() {
+        // given
+        Product product = makeTestProduct("짜장면", BigDecimal.valueOf(5000));
+        MenuProduct menuProduct = makeTestMenuProduct(product);
+        MenuGroup menuGroup = makeTestMenuGroup("추천메뉴");
+        Menu menu = makeTestMenu("중식", BigDecimal.valueOf(-5000), menuGroup, menuProduct);
+
+        // then
+        assertThatThrownBy(() -> menuService.create(menu)).isInstanceOf(IllegalArgumentException.class);
+    }


👍 경계값을 이용해 테스트한다면 훨씬 더 명확하겠네요!

습관이 들어야하는데 무심코 데이터를 넣어서 했네요 감사합니다

Hyeon9mak · 2025-02-08T04:11:58Z

src/test/java/kitchenpos/fixture/TestFixture.java

+public class TestFixture {
+
+    public static Product makeTestProduct(String name, BigDecimal price) {
+        Product product = new Product();
+        product.setId(UUID.randomUUID());
+        product.setName(name);
+        product.setPrice(price);
+        return product;
+    }


fixture 를 만들어두면 모든 테스트에서 활용하며 큰 편의를 얻을 수 있죠 😄 👍

Hyeon9mak · 2025-02-08T04:13:44Z

src/test/java/kitchenpos/application/MenuServiceTest.java

+    @DisplayName("메뉴명에 비속어가 들어있다면 에러를 발생시킨다.")
+    @Test
+    void hasProfanity() {
+        // given
+        Product product = makeTestProduct("짜장면", BigDecimal.valueOf(5000));
+        MenuProduct menuProduct = makeTestMenuProduct(product);
+        MenuGroup menuGroup = makeTestMenuGroup("추천메뉴");
+        Menu menu = makeTestMenu("중식", BigDecimal.valueOf(5000), menuGroup, menuProduct);
+
+        // when


아주 간단하게 PurgomalumClient 에 대해서만 fake object 를 만들어서 다뤄보는 건 어떨까요?
내부적으로 PurgomalumClient 에 대한 이해도 높이고, 테스트 코드도 훨씬 쉽고 빠르게 이해할 수 있게 될 것 같아요!

fake object에 대해 설명 들었을때, 이게 mock이랑 다른게 뭐지? 어차피 가짜 값이 아닌가 했는데 말씀하신대로 오히려 이해도를 높이기에 도움이 될 거란 생각이 드네요. 외부 시스템이라 굳이 테스트가 필요한가 했는데 이해도에 도움이 될 것으로 보입니다

…mClient.java 가짜객체 생성

Dongock and others added 30 commits January 21, 2025 21:59

feat(racingcar): carNameRuleTest01-이름5글자

b031e18

feat(racingcar) : stop&movingTest

c3c20fd

feat(racingcar) : movingStrategyTest

4406d32

feat(cal): nullCheckTest

840ad93

refactor(cal): beforeEach add

b204e91

feat(cal): onlyNumberCheck

64a8142

docs(cal) : README 요구사항 1번 정리

c146750

docs(cal) : README 요구사항 1번 정리(내용추가)

db97850

refactor(cal) : StringUtils 제거

ffb2f89

refactor(cal) : NullAndEmptySource로 변경

f99307c

feat(cal) : 숫자로 변환 가능하지 않은 문자 입력 시 예외 발생

1cc4d8c

doc(cal) : 예외사항 추가

199cd05

test(cal) : 구분자 검증 테스트 코드 추가

3fa268c

feat(cal) : 구분자를 컴마(,) 이외에 콜론(:)을 사용할 수 있다.

89fa384

- 구분자 연속으로 올 시 사이 빈 값은 0으로 처리 - 구분자 기능 완성 및 요구사항 추가

feat(cal) : "//"와 "\n" 문자 사이에 커스텀 구분자를 지정할 수 있다.

881ba69

- 커스텀 구분자 기능 완성 및 요구사항 추가

feat(cal) : 음수를 전달할 경우 RuntimeException 예외가 발생해야 한다.

504f6b2

- 음수 예외처리 기능 완성 및 요구사항 추가

refactor(cal) : 패턴 재사용 가능하게 축출 및 요구사항 정리

9e4a0d5

refactor(cal) : Empty 체크 리팩토링

75b5ff4

doc(cal) : 리팩토링을 위한 필요 정리 추가

15f8e09

doc(cal) : 리팩토링을 위한 필요 정리 추가(Front Controller)

572c43b

feat(cal) : EmptyCalculator 클래스 및 Test 추가

dc7a993

feat(cal) : CustomCalculator 클래스 및 Test 추가

9d80278

feat(cal) : DefaultCalculator 클래스 및 Test 추가

3800b42

refactor(cal) : frontController 패턴 적용

c1b0273

refactor(cal) : 클래스 이름 변경

eb118f0

refactor(cal) : FeedBack 받은 부분 수정

3da5a4f

doc(force) : Product 요구사항 등록

bacb407

doc(force) : MenuGroup 요구사항 등록

c6aefa6

doc(force) : Menu 요구사항 등록

b51a7e6

doc(force) : Order 요구사항 등록

71cf69e

Dongock added 24 commits February 6, 2025 20:28

test(pos) : TextFixture - makeTestOrderTable() 생성

0915537

refactor(pos) : TextFixture - name 매개변수로 변경

169dbe5

test(pos) : OrderTableServiceTest - create() 생성

0e8783e

test(pos) : OrderTableServiceTest - nullName() 생성

4058c3d

test(pos) : OrderTableServiceTest - sit() 생성

788b51c

test(pos) : OrderTableServiceTest - clear() 생성

969689c

refactor(pos) : TextFixture - makeTestOrderTable() 매개변수에 고객수 추가

926b39f

test(pos) : OrderTableServiceTest - changeNumberOfGuests() 생성

47e6451

test(pos) : TestFixture - makeTestEatInOrder() & makeTestTakeOutOrder…

d02cb1a

…() & makeTestDeliveryOrder() 생성

test(pos) : TestFixture - makeTestOrderLineItem() 생성

fa4e993

test(pos) : OrderServiceTest - OrderServiceTest 테스트 및createEatInOrder…

f1fcef2

…() 생성

test(pos) : OrderServiceTest - createTakeOutOrder() 생성

4252fe8

test(pos) : OrderServiceTest - createDeliveryOrder() 생성

17ec2b7

test(pos) : OrderServiceTest - acceptEatInOrder() 생성

678f914

test(pos) : OrderServiceTest - acceptTakeOutOrder() 생성

a6339fd

test(pos) : OrderServiceTest - acceptDeliveryOrder() 생성

01e69f7

test(pos) : OrderServiceTest - serveEatInOrder() 생성

fd55e81

test(pos) : OrderServiceTest - serveTakeOutOrder() 생성

363a8a5

test(pos) : OrderServiceTest - serveDeliveryOrder() 생성

2fb4b30

test(pos) : OrderServiceTest - startDelivery() 생성

ad32cdf

test(pos) : OrderServiceTest - completeDelivery() 생성

b5517eb

test(pos) : OrderServiceTest - completeEatInOrder() 생성

afb1747

test(pos) : OrderServiceTest - completeTakeOutOrder() 생성

cec4f6e

test(pos) : OrderServiceTest - completeDeliveryOrder() 생성

5563b36

Dongock changed the base branch from master to dongock February 6, 2025 14:38

Hyeon9mak requested changes Feb 8, 2025

View reviewed changes

Dongock added 4 commits February 9, 2025 17:54

refactor(pos) : MenuServiceTest - minusPrice() 경계값 테스트

772cc02

refactor(pos) : MenuGroupServiceTest - nullName() Emtpy 추가

d91afb6

refactor(pos) : MenuGroupServiceTest - findAll 제거

644d6c5

refactor(pos) : MenuGroupServiceTest - hasProfanity() / FakePurgomalu…

621183d

…mClient.java 가짜객체 생성

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

Dongock commented Feb 6, 2025

Hyeon9mak left a comment

Hyeon9mak Feb 8, 2025

Dongock Feb 9, 2025

Dongock Feb 9, 2025

Hyeon9mak Feb 8, 2025

Hyeon9mak Feb 8, 2025

Dongock Feb 9, 2025

Hyeon9mak Feb 8, 2025

Dongock Feb 9, 2025

Hyeon9mak Feb 8, 2025

Hyeon9mak Feb 8, 2025

Dongock Feb 9, 2025

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

Are you sure you want to change the base?

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

Conversation

Dongock commented Feb 6, 2025

개요

Hyeon9mak 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