[레거시 코드 리팩터링 - 1단계] 포츈(정윤성) 미션 제출합니다. #156
Conversation
| } | ||
|
|
||
| @DisplayName("잘못된 ") | ||
| @Nested |
There was a problem hiding this comment.
중첩 클래스로 만들어서 중복되는 DisplayName을 계층화(?) 시키셨네요 👍
ParameterizedTest의 경우에는 DisplayName이 따로 출력이 안돼서 포츈이 의도하신대로 테스트 이름이 명시되지 않네요 😢
@Paramaterized 애너테이션의 name 속성을 바꾸면 이 문제점을 해결할 수 있을 것 같아요. 🤗
(이 부분은 크게 중요하다고 생각하지는 않아서 포츈의 선택으로 맡기겠습니다😃)
예시
@ParameterizedTest(name = "{displayName} {argumentsWithNames}")
There was a problem hiding this comment.
이부분을 다시 확인해보니, Intellij 속성으로 테스트를 해왔었고, 그러면 제가 원하는대로 보이긴하네요 ㅋㅋ.. 일단 그대로 가겠습니다.
| import org.springframework.test.context.jdbc.Sql; | ||
|
|
||
| @DisplayName("메뉴 기능") | ||
| @Sql({"classpath:tableInit.sql", "classpath:dataInsert.sql"}) |
There was a problem hiding this comment.
매번 이 sql 파일들을 실행함으로써 DB 초기화를 하고있군요 👍
제가 만약 인수테스트를 짰다면... 1단게는 DirtiesContext 기능 썼을 것 같은데 크.... 포츈 리스펙합니다 👍
한참 바쁜시기다보니 편한 기능의 유혹도 많았을거라 생각하는데 굳 입니다. 😃
| assertThat(response.statusCode()).isEqualTo(HttpStatus.OK.value()); | ||
| assertThat(menuGroups).hasSize(5); // defaultData 4 + insertData 1 | ||
| } | ||
|
|
There was a problem hiding this comment.
메뉴 그룹을 생성, 조회 실패 시나리오에 대한 테스트도 해볼 수 있을 것 같아요! 😃
|
|
||
| //then | ||
| assertThat(response.statusCode()).isEqualTo(HttpStatus.OK.value()); | ||
| assertThat(menuGroups).hasSize(5); // defaultData 4 + insertData 1 |
There was a problem hiding this comment.
초기 데이터를 설정해주는 sql 스크립트(이걸 뭐라고 불러야하죠....?)를 사용했을 때의 문제점인 것 같네요.
주석 없이 그냥 size가 5 라고 하면 테스트 코드를 보면서 이게 왜 5일까? 하고 헷갈릴 수 있겠네요 🤔
DirtiesContext 기능으로 Application Context를 매번 초기화해주는 것에 비해서 더 빠르게 동작하기에 빠른 피드백을 줄 수 있다는 점에서 좋은 선택지라 생각하지만 역시 마냥 장점만 존재하지는 않군요... 😢
| assertThat(response.statusCode()).isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR.value()); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Request 요청에 null 값이 포함된 경우에 대해서도 테스트 해볼 수 있을 것 같아요. 😃
| 기본_상품.setName("포츈치킨"); | ||
| 기본_상품.setPrice(BigDecimal.valueOf(7777)); | ||
| } | ||
|
|
There was a problem hiding this comment.
price가 null인 경우에 대해서도 테스트해볼 수 있을 것 같아요 😃
| .given().log().all() | ||
| .body(orderTable) | ||
| .contentType(MediaType.APPLICATION_JSON_VALUE) | ||
| .put(String.format("/api/tables/%s/number-of-guests", tableId)) |
There was a problem hiding this comment.
| .put(String.format("/api/tables/%s/number-of-guests", tableId)) | |
| .put("/api/tables/{tableId}/number-of-guests", tableId) |
String의 format 함수를 따로 쓸 필요 없이, 위와 같이 변경할 수도 있을 것 같아요 😃
There was a problem hiding this comment.
아, 이게 모종의 이유로 안됐어가지고, String.format을 썻던것으로 기억하는데, 다시 바꿔보니 이상이 없네요 ㅎㅎ
제가 뭔가 착각하고 있었나봅니다!
|
|
||
| - [x] 주문 테이블을 비우거나, 채우는 기능 (PUT) | ||
| - [x] 주문 테이블 id가 존재해야한다. | ||
| - [x] 테이블 단체 id가 NotNull 이면 안된다. |
There was a problem hiding this comment.
개인적인 의견이지만 README 파일을 읽다가 헷갈리더라구요 ㅋㅋㅋㅋ
가급적이면 테이블 단체 id가 null이 아니면 안된다 혹은 테이블 단체 id가 null이어야 한다 와 같이 풀어쓰면 풀어쓰면 어떨까 싶어요!
| } | ||
|
|
||
|
|
||
| @DisplayName("단체테이블 목록의 크기가 1이고, 단체 그룹 추가를 시도하면 500 응답을 받는다.") |
There was a problem hiding this comment.
전 개인적인 생각으로는 단체테이블 목록의 크기가 2미만인 경우, 단체 그룹 추가를 시도하면 500 응답을 받는다. 와 같은 의미가 더 이해하기 쉬운 것 같아요!! 포츈은 어떻게 생각하시나요 🤔
|
|
||
| assertThat(response.statusCode()).isEqualTo(HttpStatus.NO_CONTENT.value()); | ||
| } | ||
|
|
There was a problem hiding this comment.
단체를 해제하는 경우에 대한 예외 테스트도 고려해볼 수 있을 것 같아요 🤔
(readme 기준)
단체 id가 존재해야한다.
단체에 포함된 테이블 중에 Cooking, MEAL 상태인 주문이 있으면 안된다.
안녕하세요 로키, 다른일들이 많아 우선순위에서 밀려 제출이 몹시 늦었습니다.
서로 연관된 부분들이 많아서, 통합테스트 짜는데 굉장히 힘들었네요.
잘 부탁드립니다!