-
Notifications
You must be signed in to change notification settings - Fork 5
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
[Feature] - 레디스 관련 테스트 코드 작성 #637
Conversation
Test Results 30 files 30 suites 1m 7s ⏱️ Results for commit 4b7c3d6. ♻️ This comment has been updated with latest results. |
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.
수고하셨습니다~ 리비!
한 가지 리뷰 보다가 생각난 부분에 대해서 의견 달아뒀는데 확인 부탁드려요 ^_^
// given | ||
TravelogueSearchRequest searchRequest = new TravelogueSearchRequest(null, null); | ||
TravelogueFilterRequest filterRequest = new TravelogueFilterRequest(null, null); | ||
Pageable pageRequest = PageRequest.of(pageNumber, 5, Sort.by("id")); |
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.
테스트코드 보다 생각난건데 저희 목적은 좋아요 수 캐싱이었는데, 프로덕션에서도 굳이 이걸 제한해두지 않고 테스트도 id
기반 Sort.by로 테스트가 진행되네요! 괜찮을까요?
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.
헉.. 놓치고 있었던 부분이네요.
최신순이든, 좋아요 순이든 상위 4개의 페이지를 캐싱하는 것이 현재 정책이 되겠군요.
지금 적용되어 있는 정책도 나쁘지 않아보이는데요 우려되는 지점은 역시 레디스 메모리 용량과 가용성이군요
저는 레디스 모니터링의 필요성이 느껴지는 것 같아요
현재 적용되어 있는 정책을 그대로 두고 레디스를 모니터링 하며 임계점을 찾아보는 것이 어떨지 제안드립니다
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.
그라파나로 레디스를 모니터링 하는 방법을 소개하는 블로그들이 다수 있군요. 우선은 새로운 이슈로 발간해놓겠습니다. #638
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.
레디스 모니터링은 저도 필요하다고 생각합니다유
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.
제가 모니터링 추가해보았습니다. redis 대시보드에서 redis 프로메테우스 선택하시면 대시보드 확인하실 수 있습니다~
#638
해당 이슈 살펴봐주세요~
@@ -112,6 +126,96 @@ void findTravelogues() { | |||
assertThat(result).containsAll(expect); | |||
} | |||
|
|||
@DisplayName("여행기 컨텐츠 페이징 응답 시 페이지 번호가 4이하이고 필터 조건과 검색 조건이 없으면 응답을 캐싱한다.") | |||
@ParameterizedTest | |||
@ValueSource(ints = {0, 1, 2, 3, 4}) |
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.
경계값 테스트 좋네용~ 👍
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.
경계값 테스트까지 꼼꼼하게 하신 것 확인했습니다 리비!
클로버가 단 리뷰 외에는 다 잘 하신 것 같아서 바로 어프루브 때리겠습니다~
Long ttl = redisTemplate.getExpire(key, TimeUnit.MINUTES); | ||
Assertions.assertAll( | ||
() -> assertThat(ttl).isGreaterThan(0), | ||
() -> assertThat(ttl).isLessThanOrEqualTo(30) |
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.
누군가 실수로 TTL을 3분으로 설정해도 잡지 못하는 문제가 생길 수 있을 것 같긴 한데 찾아봐도 더 좋은 방법이 딱히 없네요..
범위로 테스트를 하자니 시간에 따라 변하는 값이라 안정성이 보장되지 않을 것 같아서 좋은 방법은 아닌 것 같습니다.
혹시 더 좋은 방법 생각나신 분 있으시면 알려주세요 🙇
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.
와우 생각지도 못했군요.. 검증의미가 약한 테스트 코드 같은데.. 머지하기 전까지 개인적으로 좀 생각해보고 공유드려보겠습니다.
다른 분들도 생각나는 거 있으시면 알파카 말씀처럼 언급해주시면 감사하겠습니다.
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.
29분에서 30분 사이로 조금 더 엄격하게 테스트 하도록 로직 수정했어요. 테스트가 1분 넘게 실행될 일은 없으니 이러한 처리도 나쁘지 않을 것 같더군요. 이대로 진행하겠습니다~
레디스 모니터링 관련 작업이 추가로 수행되었고 TTL 검증 테스트가 0-30 에서 29-30으로 엄격해졌습니다~ 해당 변경사항 전파드렸고 PR 머지하겠습니다. |
✅ 작업 내용
🙈 참고 사항
레디스 동작도 애플리케이션 동작으로서 검증되어야 한다고 판단하여 테스트 코드 작성했습니다.