[REFACTOR] 리뷰 생성 및 컨트롤러 테스트 리팩토링 #254
Conversation
| @NotNull | ||
| private Page<Review> getReviews(ReviewSearchCondition condition, BooleanExpression predicate) { | ||
| private Page<Review> getReviews(ReviewSearchCondition condition, BooleanExpression exp) { | ||
| List<Review> content = queryFactory.select(review) |
There was a problem hiding this comment.
조회에 사용되는 쿼리 같은데 Review의 모든 컬럼이 필요한건지 여쭤보고 싶습니다
만약 아니라면 필요한 컬럼만 추출해서 응답시간 단축, 자원 절약을 시도해보는게 어떨까요
쿼리 최적화를 위한 6가지팁
There was a problem hiding this comment.
모두 필요합니다 리스트 조회에 대한 거라 필요합니다.
| public void setup() { | ||
| this.mockMvc = MockMvcBuilders.webAppContextSetup(ctx) | ||
| .addFilters(new CharacterEncodingFilter("UTF-8", true)) | ||
| .alwaysDo(print()) |
There was a problem hiding this comment.
테스트 코드에서 print의 사용은 지양하는게 좋은거 같은데 어떻게 생각하시나요
검증과 무관하기도 하고 프로덕션에서의 디버깅에는 도움이 되지만
자동화된 검증이라는 테스트의 의미에는 반하는 것 같습니다
그리고 print의 리소스 사용량이 생각보다 커서 테스트 개수가 많아짐에 따라 cicd 시간이 늘어날 것 같다는 의견입니다
필요하다면 테스트가 지속적으로 깨질때에만 개인적인 디버깅 용도로 사용하고 지우는게 어떨까요
There was a problem hiding this comment.
@7zrv 컨트롤러 테스트에서 print를 전부 사용해왔기에, 한곳으로 모으는 과정이지 않나요?
저는 사실 아직.. 컨트롤러 테스트 진행 과정을 완벽히 이해를 못 해서 여쭤보고 싶습니다!
There was a problem hiding this comment.
제 의견은 print 자체가 테스트에 필요하지 않다는 의견이었습니다!
테스트의 목적중 하나가 컴퓨터를 이용한 검증으로 휴먼에러를 없애는것이고
만약 print로 직접 내용을 확인하면서 테스트, 프로덕션을 수정한다면 그 과정에서 휴먼에러의 발생으로
기존의 설계, 테스트 자체가 오염될 가능성이 높아집니다
때문에 TDD에서는 객관적인 성공, 실패로만 검증 여부를 판단 하고있고 print 사용시 사람의 주관이 들어가
이러한 원칙을 위배하게 됩니다.
그리고 print는 사람이 확인하기 위한것이라 빌드과정과 cicd에 포함될 이유가 더더욱 없기때문에
마지막줄에 개인적인 디버깅 용도로만 사용을 하자는 의견을 남겼습니다
첫번째 문단은 테스트 강의를 하신 박우빈님의 의견인데 공감이 되어 적어봤습니다!
빌드, cicd 과정에 대한 의견은 제 개인적인 의견입니다.
There was a problem hiding this comment.
코드 작성시에는 로그 확인을 위해 print했지만
실제 운영 환경이라면 디버깅이 필요 없을때는 주석처리 하거나 제거, log()를 사용하라는 말을 들었었는데
여러 예제들을 찾아보니 제거하라는 말도 하나의 의견일뿐이라는 생각도 드네요
지금 생각은 개발 단계인 저희는 그대로 뒀다가 개발 완료후에 제거해도 될 거 같다는 의견으로 기울었습니다...
그래서 일단 범수님이 작성하신것 처럼 하나로 모아두는게 좋은거 같은데 내일 얘기나눠보면 좋을거 같습니다
m-a-king
left a comment
m-a-king
left a comment
There was a problem hiding this comment.
수고하셨습니다~
- static import는 원래 최상단으로 가는 게 올바른 것인지 여쭤보고 싶습니다.
- if문으로 처리하는 것과 삼항연산으로 처리하는 것의 가독성 차이에 대해서도 여쭤보고 싶습니다.
리팩토링 PR이라서 여쭤보는 거라 편하게 답해주세요~~
| .andExpect(jsonPath("$.data.center_id").value(centerId.toString())) // center_id로 수정 | ||
| .andExpect(jsonPath("$.data.name").value("Test Center")) | ||
| .andExpect(jsonPath("$.data.contact_number").value("010-1234-5678")) // contact_number로 수정 | ||
| .andExpect(jsonPath("$.data.img_url").value("http://example.com/image.jpg")) // img_url로 수정 | ||
| .andExpect(jsonPath("$.data.contact_number").value( | ||
| "010-1234-5678")) // contact_number로 수정 | ||
| .andExpect(jsonPath("$.data.img_url").value( | ||
| "http://example.com/image.jpg")) // img_url로 수정 | ||
| .andExpect(jsonPath("$.data.introduce").value("This is a test center.")) | ||
| .andExpect(jsonPath("$.data.homepage_link").value("http://example.com")) // homepage_link로 수정 | ||
| .andExpect(jsonPath("$.data.homepage_link").value( | ||
| "http://example.com")) // homepage_link로 수정 | ||
| .andExpect(jsonPath("$.data.prefer_items").isArray()); // prefer_items로 수정 |
There was a problem hiding this comment.
이거 기존 코드라 제가 작성한 부분이 아니에요
| public void setup() { | ||
| this.mockMvc = MockMvcBuilders.webAppContextSetup(ctx) | ||
| .addFilters(new CharacterEncodingFilter("UTF-8", true)) | ||
| .alwaysDo(print()) |
There was a problem hiding this comment.
@7zrv 컨트롤러 테스트에서 print를 전부 사용해왔기에, 한곳으로 모으는 과정이지 않나요?
저는 사실 아직.. 컨트롤러 테스트 진행 과정을 완벽히 이해를 못 해서 여쭤보고 싶습니다!
leebs0521
force-pushed
the
refactor/253-modify-review-and-controller
branch
from
e549671 to
27c6f8c
Compare
- 컨트롤러 테스트시 print() 일괄 적용 - print()시 한글 깨짐 현상 해결
- 컨트롤러 테스트시 print() 일괄 적용 - print()시 한글 깨짐 현상 해결
leebs0521
force-pushed
the
refactor/253-modify-review-and-controller
branch
from
27c6f8c to
e00f475
Compare
|
resolved :
📌 과제 설명
리뷰 생성 및 컨트롤러 테스트 리팩토링
👩💻 요구 사항과 구현 내용
✅ PR 포인트 & 궁금한 점
위 두 개가 중점적이긴한데 이거에 대해 괜찮은지 의견 부탁드릴게요