[레거시 코드 리팩터링 - 1단계] 다니(이다은) 미션 제출합니다. #117
Conversation
There was a problem hiding this comment.
안녕하세요 다니! 저도 만나서 반가워요!!
많이 바쁘시죠ㅠㅠ 바쁘신데도 잘 구현해 주셨네요👍🏻👍🏻
몇 가지 코멘트와 질문에 대한 답 남겼어요!
주관적인 의견이 많아서 다니가 보고 반영할지 말지 결정하면 될 것 같아요!
step1은 잘 해주셔서 머지하겠습니다! 피드백 반영은 step2와 같이 진행해주세요!
추가적으로 현재 테스트에서 프로덕션 DB를 공유하고 있는 것 같아요! 테스트 용으로 별도의 프로퍼티를 만들어 사용해보시는 건 어떤가요? 테스트 결과도 flyway 설정에 의존하고 있는 것 같아서 flyway에 변경이 있을 경우 테스트에도 문제가 생길 것 같아요!
| @@ -1,6 +1,92 @@ | |||
| # 키친포스 | |||
|
|
|||
| ## 요구 사항 | |||
| ## 요구사항 | |||
| testImplementation('org.springframework.boot:spring-boot-starter-test') { | ||
| exclude group: 'org.junit.vintage', module: 'junit-vintage-engine' | ||
| } | ||
| testImplementation 'io.rest-assured:rest-assured:4.4.0' |
There was a problem hiding this comment.
앗 처음에 인수 테스트로 작성해서 추가한 건데, 지금은 필요 없어졌네요!
삭제하겠습니다 ㅎㅎㅎㅎ
| @@ -82,6 +82,7 @@ public void ungroup(final Long tableGroupId) { | |||
|
|
|||
| for (final OrderTable orderTable : orderTables) { | |||
| orderTable.setTableGroupId(null); | |||
| // setEmpty(true)가 맞는 거 같음 | |||
There was a problem hiding this comment.
오.. 저도 이 부분 때문에 empty가 정확히 뭘 의미하는 필드인지 고민을 많이했어요.
ungroup 할 때 조건 중, 주문 상태가 COOKING이나 MEAL이 아니어야 upgroup을 할 수 있으니 결국 null이거나 COMPLETION 이어야지 ungroup을 할 수 있을텐데, 두 경우 모두 테이블은 empty 상태로 변하는게 맞는 것 같다는 생각이 드네요ㅠㅠ
|
|
||
| Menu menu = new Menu(); | ||
| menu.setName("얌 프라이"); | ||
| menu.setPrice(BigDecimal.valueOf(-1000)); |
|
|
||
| Menu menu = new Menu(); | ||
| menu.setName("얌 프라이"); | ||
| menu.setPrice(BigDecimal.valueOf(-1000)); |
| OrderTable savedOrderTable = tableService.create(orderTable); | ||
|
|
||
| OrderTable targetOrderTable = new OrderTable(); | ||
| targetOrderTable.setNumberOfGuests(0); |
| OrderTable savedOrderTable2 = tableService.create(orderTable2); | ||
|
|
||
| TableGroup tableGroup = new TableGroup(); | ||
| tableGroup.setCreatedDate(LocalDateTime.now()); |
There was a problem hiding this comment.
생성자에서 초기화하는 방법을 생각하지 못했어요..!!! ㅎㅎㅎㅎㅎ
2단계 진행하면서 수정했습니다 ~!
|
|
||
| OrderTable targetOrderTable = new OrderTable(); | ||
| targetOrderTable.setNumberOfGuests(0); | ||
| targetOrderTable.setEmpty(true); |
| OrderTable savedOrderTable = tableService.create(orderTable); | ||
|
|
||
| OrderTable targetOrderTable = new OrderTable(); | ||
| targetOrderTable.setNumberOfGuests(0); |
There was a problem hiding this comment.
바꾸려는 손님 수도 0이어서 원래 손님수와 바꾸려는 손님 수가 같아서 에러가 발생하는건가? 라는 생각이 들수도 있을 것 같아요!
| /** | ||
| * 여기도 테스트 격리가 잘 안됨 | ||
| */ | ||
| @DisplayName("테이블의 목록을 조회할 수 있다.") | ||
| @Test | ||
| void list_Valid_Success() { | ||
| // when | ||
| List<OrderTable> orderTables = tableService.list(); | ||
|
|
||
| // then | ||
| assertThat(orderTables).isNotEmpty(); | ||
| } |
There was a problem hiding this comment.
혹시 어떻게 테스트 해보셨을 때 잘 안 되신 건지 말씀해주실 수 있나요?
나름대로 테스트 해봤는데 제가 돌렸을 때는 다 잘 돌아가더라구요 ㅠㅠ
어떤 필드가 영향을 받는지 확인해보려고 아래와 같이 설정하고
- 단일 테스트만 돌려보기
- 해당 클래스 돌려보기
- 전체 테스트 돌려보기
를 진행했는데 모두 잘 돌아가더라구요..? ㅠㅠ
@DisplayName("테이블의 목록을 조회할 수 있다.")
@Test
void list_Valid_Success() {
// when
List<OrderTable> orderTables = tableService.list();
// then
assertThat(orderTables).hasSize(8);
assertThat(orderTables.get(0).isEmpty()).isEqualTo(true);
assertThat(orderTables.get(1).isEmpty()).isEqualTo(true);
assertThat(orderTables.get(2).isEmpty()).isEqualTo(true);
assertThat(orderTables.get(3).isEmpty()).isEqualTo(true);
assertThat(orderTables.get(4).isEmpty()).isEqualTo(true);
assertThat(orderTables.get(5).isEmpty()).isEqualTo(true);
assertThat(orderTables.get(6).isEmpty()).isEqualTo(true);
assertThat(orderTables.get(7).isEmpty()).isEqualTo(true);
assertThat(orderTables.get(0).getNumberOfGuests()).isEqualTo(0);
assertThat(orderTables.get(1).getNumberOfGuests()).isEqualTo(0);
assertThat(orderTables.get(2).getNumberOfGuests()).isEqualTo(0);
assertThat(orderTables.get(3).getNumberOfGuests()).isEqualTo(0);
assertThat(orderTables.get(4).getNumberOfGuests()).isEqualTo(0);
assertThat(orderTables.get(5).getNumberOfGuests()).isEqualTo(0);
assertThat(orderTables.get(6).getNumberOfGuests()).isEqualTo(0);
assertThat(orderTables.get(7).getNumberOfGuests()).isEqualTo(0);
}There was a problem hiding this comment.
운영 Flyway랑 뭔가 잘못 맞물려서 테스트 격리가 제대로 안된 것 같아요 ㅠ
2단계에서 테스트용 설정 파일을 분리하니까 문제 없이 실행됩니다!! ㅎㅎㅎㅎㅎ
같이 고민해줘서 감사합니다!!! 🙇♀️👍
|
테스트 격리 질문해주신 것에 대해 답을 드리자면 그래서 해결하기 위해서는 아래와 같은 방법이 있겠네요.
@DisplayName("주문을 등록한다.")
@Nested
class Create extends IntegrationTest {
// ...
}제가 해봤을 땐 두 방법 다 잘 동작했는데 한 번 확인해주세요! 관련 링크 남겨요!
|
|
@KJunseo |
안녕하세요, 에어 🙌
이렇게 리뷰어/리뷰이로 만나게 돼서 반가워요 ~!
요즘 정말 바쁜 것 같아요 ㅠ 하루하루 정신이 없네요 🥲
미션 요구사항대로 1) README를 정리하고 2) 테스트 코드를 작성했어요!
테스트는 @SpringBootTest를 활용해서 통합 테스트로 진행했어요.
RestAssured로 인수 테스트도 짤까 생각했는데, 우선은 필수 요구사항만 지켰습니다 ㅎㅎㅎ
테스트 코드를 작성할 때, 테스트 격리가 잘 되지 않는 현상이 몇 번 발생했는데요 🤯
이유를 잘 모르겠어요,, 혹시 왜 그런지 같이 고민해줄 수 있을까요..?!
OrderServiceIntegrationTest와 TableServiceIntegrationTest에 주석 달아뒀습니다 !
바쁜 와중에 코드 리뷰 고맙습니다 🙇♀️✨