Conversation
📝 Walkthrough개요라운드 연장 결정 가능 기간을 D-2..D-1에서 D-3..D-2로 확대하는 유효성 검사 로직 수정입니다. 이는 알림 당일에 연장이 불가능한 버그를 해결하며, 서비스와 테스트 코드가 함께 업데이트되었습니다. 변경사항
코드 리뷰 노력 예측🎯 3 (중간 복잡도) | ⏱️ ~25분 관련 가능성 있는 PR
시
💡 개선 제안테스트 커버리지 관점에서의 고려사항:
코드 품질 개선:
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/test/java/com/hrr/backend/domain/round/RoundFlowUnitTest.java (1)
395-436:⚠️ Potential issue | 🟡 Minor드랍 테스트 — UNDECIDED 케이스 누락 확인 필요
테스트 이름은
dropsStopAndUndecided이지만, 실제 테스트 데이터에는STOP만 포함되어 있고UNDECIDEDRoundRecord가 없습니다. UNDECIDED도 드랍 대상이라면 테스트에 추가하여 이름과 검증 범위를 일치시켜야 합니다.💡 UNDECIDED 케이스 추가 예시
RoundRecord rrStop = mock(RoundRecord.class); when(rrStop.getNextRoundIntent()).thenReturn(NextRoundIntent.STOP); + RoundRecord rrUndecided = mock(RoundRecord.class); + when(rrUndecided.getNextRoundIntent()).thenReturn(NextRoundIntent.UNDECIDED); UserChallenge ucStop = mock(UserChallenge.class); when(rrStop.getUserChallenge()).thenReturn(ucStop); when(ucStop.getStatus()).thenReturn(ChallengeJoinStatus.JOINED); + UserChallenge ucUndecided = mock(UserChallenge.class); + when(rrUndecided.getUserChallenge()).thenReturn(ucUndecided); + when(ucUndecided.getStatus()).thenReturn(ChallengeJoinStatus.JOINED); + when(roundRecordRepository.findAllByRoundWithUserAndSetting(endedRound, ChallengeJoinStatus.JOINED)) - .thenReturn(List.of(rrContinue, rrStop)); + .thenReturn(List.of(rrContinue, rrStop, rrUndecided)); // when roundDropService.dropNonContinuersAt(endDate); // then verify(ucStop).updateStatus(ChallengeJoinStatus.DROPPED); + verify(ucUndecided).updateStatus(ChallengeJoinStatus.DROPPED); - verify(challengeRepository, times(1)).decreaseCurrentParticipantCount(999L); + verify(challengeRepository, times(2)).decreaseCurrentParticipantCount(999L);
🧹 Nitpick comments (3)
src/main/java/com/hrr/backend/domain/round/service/RoundDecisionServiceImpl.java (1)
94-120: 결정 기간 로직 변경 — 핵심 수정이 올바르게 적용되었습니다 👍D-3 ~ D-2 윈도우로의 변경이 PR 목적에 부합하며,
isBefore/isAfter조합으로 양 끝 날짜를 포함(inclusive)하는 처리가 정확합니다.다만, Line 116의 주석에서
!today.isBefore(decisionCloseDate) → today.isAfter(decisionCloseDate)라고 동치(equivalence)로 설명하고 있는데, 이 둘은 동치가 아닙니다.
!isBefore(X)=X이후 또는 같은 날 (>=)isAfter(X)=X이후 (strictly>)실제 코드는
isAfter를 사용하므로 D-2 당일을 포함시키는 의도에 부합하지만, 주석이 잘못된 동치 관계를 서술하고 있어 향후 유지보수 시 혼란을 줄 수 있습니다. 주석을 수정하는 것을 권장합니다.📝 주석 수정 제안
- // D-2를 넘어가면 기간 만료 (D-2 23:59까지 가능) - // !today.isBefore(decisionCloseDate) → today.isAfter(decisionCloseDate) - // 이유: D-2 당일까지 포함시키기 위함 + // D-2를 넘어가면 기간 만료 (D-2 당일까지 가능) + // isAfter(decisionCloseDate) → D-2 당일은 통과, D-2 다음 날부터 차단 if (today.isAfter(decisionCloseDate)) {src/test/java/com/hrr/backend/domain/round/RoundFlowUnitTest.java (2)
72-266: 테스트 경계값 검증이 잘 구성되어 있습니다 🎯D-4(실패), D-3(성공), D-2(성공), D-1(실패) — 네 가지 경계 시나리오를 빠짐없이 커버하고 있어 좋습니다.
한 가지 주의점: 테스트에서는
LocalDate.now()를 사용하지만, 프로덕션 코드(RoundDecisionServiceImpl)에서는LocalDate.now(ZoneId.of("Asia/Seoul"))을 사용합니다. UTC 기준 자정 전후에 CI가 실행되면 두 값이 다른 날짜를 반환할 수 있어 테스트가 간헐적으로(flaky) 실패할 수 있습니다.권장 개선:
Clock을RoundDecisionServiceImpl에 주입하여 테스트에서 고정된 시각을 사용하면 이 문제를 근본적으로 해결할 수 있습니다. (Java Clock 공식 문서 참고)💡 Clock 주입 예시 (프로덕션 코드)
`@Service` `@RequiredArgsConstructor` public class RoundDecisionServiceImpl implements RoundDecisionService { + private final Clock clock; // ... private void validateDecisionPeriod(Round currentRound) { - LocalDate today = LocalDate.now(ZoneId.of("Asia/Seoul")); + LocalDate today = LocalDate.now(clock);Bean 등록:
`@Bean` public Clock clock() { return Clock.system(ZoneId.of("Asia/Seoul")); }테스트에서:
Clock fixedClock = Clock.fixed( LocalDate.of(2026, 2, 7).atStartOfDay(ZoneId.of("Asia/Seoul")).toInstant(), ZoneId.of("Asia/Seoul") );
222-266:realScenario_day18_afterNotification는D_minus_3_shouldSucceed와 사실상 동일합니다두 테스트 모두
endDate = today.plusDays(3), intent =CONTINUE로 동일한 조건을 검증합니다. 테스트 이름만 다르고 로직이 중복되어 유지보수 부담이 늘어납니다.차별화가 필요하다면 (예: 특정 시각 기반 시나리오), 위에서 제안한
Clock주입 후 시각을 고정하여 "오전 9시 알림 직후" 같은 시간 조건을 실제로 반영하는 것이 좋겠습니다. 그렇지 않다면 하나로 통합하는 것을 고려해 주세요.
2 participants
#️⃣ 연관된 이슈
✨ 작업 내용 (Summary)
✅ 변경 사항 체크리스트
🧪 테스트 결과
📸 스크린샷
💬 리뷰 요구사항
📎 참고 자료
Summary by CodeRabbit
버그 수정