-
Notifications
You must be signed in to change notification settings - Fork 227
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Step3 - 테스트를 통한 코드 보호 #842
base: wotjd4305
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
안녕하세요.
전체적으로 가독성 높은 테스트 좋았습니다!
가벼운 코멘트 몇 가지 남겼습니다!
함께 고민해 보면 좋을 것 같아요!
## 요구 사항 | ||
- 메뉴 그룹 | ||
- [ ] 새로운 메뉴 그룹 생성할 수 있다. | ||
- [ ] 메뉴 그룹명은 비어있으면 안된다. | ||
- [ ] 메뉴 그룹 전부 가져올 수 있다. | ||
- [X] 새로운 메뉴 그룹 생성할 수 있다. | ||
- [X] 메뉴 그룹명은 비어있으면 안된다. | ||
- [X] 메뉴 그룹 전부 가져올 수 있다. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
요구 사항 체크 박스 활용 👍
요구 사항에서 사용된 문장을 테스트에 그대로 사용해 주셨네요! 💯
class MenuGroupServiceTest { | ||
|
||
private MenuGroupRepository menuGroupRepository; | ||
private MenuGroupService menuGroupService; | ||
|
||
@BeforeEach | ||
void setUp() { | ||
menuGroupRepository = mock(MenuGroupRepository.class); | ||
menuGroupService = new MenuGroupService(menuGroupRepository); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
강의에서 다룬 fake 객체를 적용해 보는 것은 어떨까요?
모든 테스트에 적용할 필요는 없고 부분적으로 경험해 보면 좋을 것 같아 코멘트 남겨요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
menuGroup에 적용 한번 해보겠습니다!
Product product = new Product(); | ||
product.setId(productId); | ||
product.setPrice(BigDecimal.valueOf(15000)); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
테스트를 위한 반복적인 객체 생성이 있었을 것 같은데요. 이 경우는 미리 생성해 재사용해 볼 수도 있을 것 같아요. 혹은 테스트에 사용되는 객체가 상태가 변한다면 테스트마다 새로 생성하는 방법도 있을 것 같은데요. @BeforeEach
등 여러 가지 방법을 사용해 볼 수 있겠네요. 여러 가지 방법을 시도해 보시고 공유해 주셔도 좋을 것 같습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
네넵 다시 둘러보니, 미리 생성해서 사용해도 되는 객체들이 꽤 되는 것 같습니다.
주로 메뉴에서는 필수적으로 필요한 상품, 메뉴, 메뉴 상품들이 만들어서 사용하기 편하고, 다른 객체들도 필수적으로 필요한 경우에 생성될 것 같네요!
@Nested | ||
@DisplayName("메뉴 그룹 생성") | ||
class CreateMenuGroup { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nested
를 통해서 테스트 의도를 명확하게 나타내 주셨네요!
public static Menu createMenu(String name, BigDecimal price) { | ||
final var menu = new Menu(); | ||
menu.setId(UUID.randomUUID()); | ||
menu.setPrice(price); | ||
menu.setDisplayed(true); | ||
menu.setName(name); | ||
return menu; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Displayed가 기본적으로 true로 들어가 있는데요. 사용하는 입장에서 메서드명만 보고 파악하기는 어려울 것 같아요. 결국 메서드의 구현을 한 번 더 확인해야 합니다. 이럴 땐 메서드명을 조금 더 명확하게 작성해 주면 사용하는 입장에서 편하지 않을까 싶어요.
createDisplayedMenu
와 같이 작성해 볼 수도 있을 것 같아요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
넵! 픽스쳐 한번 돌아보고 수정하겠습니다
변경한 요구사항 기반으로 테스트 코드 작성해보았습니다!
확인 부탁드립니다~