Conversation
- 메트릭이 이미 존재한다면 metric.update()를 호출하는지 검증합니다. - 일일 활성 사용자수를 정확히 계산하는지 검증합니다. - MetricFixture를 통해 ClubMetric을 mock객체로 정의하여 수정불가능한 값의 반환값을 직접 정의합니다.
- 일부 Club 정보가 누락되어도 null 포함하여 정상 동작해야 합니다. - 반드시 1개 이상의 동아리를 조회해야 합니다.
- 하루 미만의 사용자수 요청시 빈배열을 반환합니다. - ip 중복을 제거하여 카운트 합니다
- 하루 미만의 사용자수 요청시 빈배열을 반환합니다. - ip 중복을 제거하여 카운트 합니다
- 같은 동아리에 대해 기존 스케줄이 존재하면 취소합니다.
- 모집 상태 업데이트를 성공하여 repository를 통해 저장합니다. - 존재하지 않는 클럽 id에 대해 업데이트를 요청할시 예외를 발생합니다.
Walkthrough이 변경 사항은 주로 서비스와 테스트 코드에 대한 추가 및 개선으로 구성되어 있습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as ClubMetricServiceTest
participant Service as ClubMetricService
participant Repo as ClubMetricRepository
participant ClubRepo as ClubRepository
Test->>Service: getDailyRanking(n)
alt n <= 0
Service-->>Test: 빈 리스트 반환
else
Service->>Repo: find metrics by date
Service->>ClubRepo: find clubs by id
Service-->>Test: 랭킹 리스트 반환
end
sequenceDiagram
participant Test as RecruitmentSchedulerTest
participant Scheduler as RecruitmentScheduler
participant TaskScheduler as TaskScheduler
participant ClubRepo as ClubRepository
Test->>Scheduler: scheduleRecruitment(clubId, start, end)
Scheduler->>TaskScheduler: schedule(start)
Scheduler->>TaskScheduler: schedule(end)
Scheduler-->>Test: 예약된 작업 반환
Test->>Scheduler: cancelScheduledTask(clubId)
Scheduler-->>Test: 작업 취소 및 삭제
Test->>Scheduler: updateRecruitmentStatus(clubId, status)
Scheduler->>ClubRepo: findById(clubId)
alt club 존재
Scheduler-->>Test: 상태 업데이트
else
Scheduler-->>Test: 예외 발생
end
Suggested labels
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR improves test coverage and stability by adding extensive tests for ClubMetricService and RecruitmentScheduler while enhancing input validation and error handling. Key changes include:
- New unit tests for ClubMetricService and RecruitmentScheduler along with fixture classes.
- Enhanced input validation in ClubMetricService to handle edge cases.
- A getter added in RecruitmentScheduler for accessing scheduled tasks during testing.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| backend/src/test/java/moadong/user/service/UserCommandServiceTest.java | Adds a basic test for user registration. |
| backend/src/test/java/moadong/club/service/RecruitmentSchedulerTest.java | Implements scheduling, cancellation, and status update tests for recruitment tasks. |
| backend/src/test/java/moadong/club/service/ClubMetricServiceTest.java | Provides comprehensive tests for daily, weekly, and monthly metric calculations. |
| backend/src/test/java/moadong/club/fixture/MetricFixture.java | Introduces fixture methods for creating club metric mocks. |
| backend/src/test/java/moadong/club/fixture/ClubFixture.java | Introduces a fixture for creating club mocks. |
| backend/src/main/java/moadong/user/service/UserCommandService.java | Cleans up duplicate import in the service code. |
| backend/src/main/java/moadong/club/service/RecruitmentScheduler.java | Adds a getter for scheduledTasks to enable state verification via tests. |
| backend/src/main/java/moadong/club/service/ClubMetricService.java | Adds n ≤ 0 validations to return empty collections for ranking and active user queries. |
Comments suppressed due to low confidence (1)
backend/src/main/java/moadong/club/service/RecruitmentScheduler.java:66
- Exposing the mutable scheduledTasks map directly can lead to unintended modifications; consider returning an unmodifiable view (e.g., Collections.unmodifiableMap(scheduledTasks)) to preserve encapsulation.
public Map<String, ScheduledFuture<?>> getScheduledTasks() { return scheduledTasks; }
|
|
||
| // then | ||
| verify(scheduledFuture).cancel(false); | ||
| assert (scheduledTasks.get(clubId) == null); // 태스크가 맵에서 제거되었는지 확인 |
There was a problem hiding this comment.
Consider replacing the built-in assert with a framework assertion (e.g., assertNull) so that the test failure messages are more descriptive and consistent.
| assert (scheduledTasks.get(clubId) == null); // 태스크가 맵에서 제거되었는지 확인 | |
| assertNull(scheduledTasks.get(clubId)); // 태스크가 맵에서 제거되었는지 확인 |
Test Results14 tests 14 ✅ 1s ⏱️ Results for commit 86eef41. ♻️ This comment has been updated with latest results. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (2)
backend/src/main/java/moadong/club/service/RecruitmentScheduler.java (2)
45-46:⚠️ Potential issue동일한 키로 두 개의 작업이 저장되는 문제 수정 필요
현재 코드에서는 시작 작업과 종료 작업이 동일한
clubId키로 저장되어 있습니다. 이로 인해 두 번째put호출이 첫 번째 호출을 덮어쓰게 되어, 종료 작업만 맵에 저장되는 문제가 있습니다.다음과 같이 수정하는 것을 제안합니다:
-scheduledTasks.put(clubId, startFuture); -scheduledTasks.put(clubId, endFuture); +scheduledTasks.put(clubId + "_start", startFuture); +scheduledTasks.put(clubId + "_end", endFuture);또는 복합 키를 사용하지 않고 두 작업을 함께 관리하기 위한 래퍼 클래스를 도입하는 방법도 고려해볼 수 있습니다:
private class RecruitmentTasks { private final ScheduledFuture<?> startFuture; private final ScheduledFuture<?> endFuture; public RecruitmentTasks(ScheduledFuture<?> startFuture, ScheduledFuture<?> endFuture) { this.startFuture = startFuture; this.endFuture = endFuture; } public void cancel() { if (startFuture != null) { startFuture.cancel(false); } if (endFuture != null) { endFuture.cancel(false); } } }
49-54: 🛠️ Refactor suggestioncancelScheduledTask 메소드 로직 개선 필요
현재
cancelScheduledTask메소드는scheduledTasks맵에서 하나의 항목만 제거하고 취소합니다. 위의 문제와 연결하여, 시작 및 종료 작업을 별도의 키로 저장한다면 이 메소드도 두 작업을 모두 취소하도록 수정해야 합니다.public void cancelScheduledTask(String clubId) { - ScheduledFuture<?> future = scheduledTasks.remove(clubId); - if (future != null) { - future.cancel(false); - } + ScheduledFuture<?> startFuture = scheduledTasks.remove(clubId + "_start"); + if (startFuture != null) { + startFuture.cancel(false); + } + + ScheduledFuture<?> endFuture = scheduledTasks.remove(clubId + "_end"); + if (endFuture != null) { + endFuture.cancel(false); + } }
🧹 Nitpick comments (2)
backend/src/test/java/moadong/club/fixture/ClubFixture.java (1)
1-17: 잘 구성된 클럽 테스트 픽스처 클래스
ClubFixture클래스는 테스트에 필요한Club모의 객체를 생성하는 정적 메소드를 제공합니다. 모의 객체의 기본 동작을 설정하여 테스트 코드의 가독성과 유지보수성을 높이는 좋은 방법입니다.향후 확장을 고려하여, 다양한 시나리오에 대응할 수 있도록 추가적인 팩토리 메소드를 제공하는 것도 고려해 볼 수 있습니다. 예를 들어, 모집 정보나 기타 속성이 설정된 클럽 객체를 생성하는 메소드를 추가할 수 있습니다.
backend/src/test/java/moadong/club/service/RecruitmentSchedulerTest.java (1)
123-139: 모집상태 업데이트 성공 테스트가 잘 구현되었습니다.클럽 모집 상태 업데이트 로직이 적절히 테스트되었습니다:
- 클럽 조회 메서드 호출 검증
- 클럽 저장 메서드 호출 검증
단, 실제 클럽 객체의 상태가 변경되었는지에 대한 검증이 누락되어 있습니다.
아래와 같이 클럽 상태 변경 검증을 추가하는 것을 권장합니다:
// when recruitmentScheduler.updateRecruitmentStatus(clubId, status); // then verify(clubRepository).findClubById(any(ObjectId.class)); + // 클럽의 모집 상태가 실제로 변경되었는지 검증 + ArgumentCaptor<Club> clubCaptor = ArgumentCaptor.forClass(Club.class); verify(clubRepository).save(club); + verify(clubRepository).save(clubCaptor.capture()); + assertEquals(status, clubCaptor.getValue().getRecruitmentStatus());
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
backend/src/main/java/moadong/club/service/ClubMetricService.java(2 hunks)backend/src/main/java/moadong/club/service/RecruitmentScheduler.java(2 hunks)backend/src/main/java/moadong/user/service/UserCommandService.java(5 hunks)backend/src/test/java/moadong/club/fixture/ClubFixture.java(1 hunks)backend/src/test/java/moadong/club/fixture/MetricFixture.java(1 hunks)backend/src/test/java/moadong/club/service/ClubMetricServiceTest.java(1 hunks)backend/src/test/java/moadong/club/service/RecruitmentSchedulerTest.java(1 hunks)backend/src/test/java/moadong/user/service/UserCommandServiceTest.java(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
backend/src/test/java/moadong/club/service/ClubMetricServiceTest.java (2)
backend/src/test/java/moadong/club/fixture/ClubFixture.java (1)
ClubFixture(8-17)backend/src/test/java/moadong/club/fixture/MetricFixture.java (1)
MetricFixture(9-31)
🪛 GitHub Actions: PR Test
backend/src/test/java/moadong/club/service/RecruitmentSchedulerTest.java
[error] 92-92: Test failure in method 모집_스케줄링_성공(): org.opentest4j.AssertionFailedError: expected: <> but was: <>
🔇 Additional comments (14)
backend/src/main/java/moadong/club/service/RecruitmentScheduler.java (1)
66-68: 테스트를 위한 getScheduledTasks() 메소드 추가 적절테스트 코드에서 내부 상태를 확인할 수 있도록
scheduledTasks맵을 반환하는 메소드를 추가한 것은 적절합니다. PR 목표에 명시된 대로 테스트 커버리지를 확장하는 데 도움이 됩니다.backend/src/test/java/moadong/club/fixture/MetricFixture.java (1)
1-31: 잘 구성된 테스트 픽스처 클래스
MetricFixture클래스는 Mockito를 사용하여 테스트에 필요한ClubMetric모의 객체를 생성하는 정적 메소드를 제공합니다. 다양한 시나리오에 대응할 수 있도록 오버로딩된 메소드를 제공한 점이 좋습니다.이러한 테스트 픽스처 클래스는 테스트 코드의 가독성을 높이고 중복을 줄이는 좋은 방법입니다. PR 목표에 명시된 대로 테스트 코드 품질 향상에 기여합니다.
backend/src/main/java/moadong/user/service/UserCommandService.java (3)
5-5: import 구문 재정렬
java.util.Dateimport 구문의 위치가 재정렬되었습니다. 일관된 import 순서는 코드 가독성에 도움이 됩니다.
47-47: 코드 들여쓰기 개선메서드 매개변수 및 체인 호출의 들여쓰기가 조정되어 코드의 가독성이 향상되었습니다. 이러한 스타일 개선은 코드 베이스의 일관성을 유지하는 데 도움이 됩니다.
Also applies to: 58-59, 62-62, 64-64, 82-82, 91-91, 96-96, 99-99, 114-115, 129-129
105-105: refreshToken 갱신 시 명시적인 Date 객체 생성
RefreshToken객체를 생성할 때 현재 시간을 나타내는new Date()를 명시적으로 사용하고 있습니다. 이는 토큰 갱신 시점을 정확히 기록하기 위한 좋은 접근 방식입니다.backend/src/main/java/moadong/club/service/ClubMetricService.java (2)
109-113: 적절한 입력 유효성 검사 추가.n이 0 이하인 경우 빈 목록을 반환하는 검증 로직이 잘 추가되었습니다. 이는 불필요한 데이터베이스 조회를 방지하고 예외 상황을 안전하게 처리합니다.
136-140: 적절한 입력 유효성 검사 추가.n이 0 이하인 경우 빈 배열을 반환하는 검증 로직이 잘 추가되었습니다. 이는 불필요한 데이터베이스 조회를 방지하고 예외 상황을 안전하게 처리합니다.
backend/src/test/java/moadong/club/service/ClubMetricServiceTest.java (6)
41-59: 메트릭 업데이트 테스트가 잘 작성되었습니다.기존 메트릭이 있을 때
update()메서드 호출 및 저장 과정이 올바르게 검증되었습니다. Mockito를 활용한 검증 방식이 적절합니다.
61-91: 일일 활성 사용자 수 계산 테스트가 효과적으로 구현되었습니다.다양한 날짜의 메트릭을 사용하여 일일 활성 사용자 수 계산 로직을 잘 검증하고 있습니다. 특히 특정 인덱스 값과 나머지 인덱스에 대한 검증이 포괄적으로 이루어지고 있습니다.
93-123: 주간 활성 사용자 수 계산 테스트가 효과적으로 구현되었습니다.주간 단위로 메트릭을 집계하는 로직이 잘 검증되었습니다. 요일을 고려한 주간 계산 로직 테스트가 포함되어 있어 적절합니다.
125-156: 월간 활성 사용자 수 계산 테스트가 효과적으로 구현되었습니다.월 단위로 메트릭을 집계하는 로직이 잘 검증되었습니다. 다양한 날짜(월 초, 중간, 말일)에 대한 테스트 케이스가 포함되어 적절합니다.
158-195: 일일 랭킹 테스트가 효과적으로 구현되었습니다.두 가지 중요한 시나리오를 검증하고 있습니다:
- 일부 클럽 정보가 누락된 경우에도 null을 포함하여 정상 동작하는지 검증
- n 값이 0일 경우 빈 리스트를 반환하는지 검증
이는 실제 서비스에서 발생할 수 있는 다양한 상황을 테스트하는 좋은 접근입니다.
197-236: 일일 활성 사용자 계산 테스트가 효과적으로 구현되었습니다.두 가지 중요한 케이스를 검증하고 있습니다:
- 입력값(n)이 음수인 경우 빈 배열 반환 검증
- IP 중복 제거하여 활성 사용자 계산 검증
특히 동일 IP가 여러 번 접속한 경우 중복을 제거하는 로직이 잘 테스트되었습니다.
backend/src/test/java/moadong/club/service/RecruitmentSchedulerTest.java (1)
105-117: 스케줄 취소 테스트가 잘 구현되었습니다.스케줄된 작업 취소 기능에 대한 테스트가 적절합니다:
- 스케줄된 작업이 취소되는지 검증
- 작업이 내부 맵에서 제거되는지 검증
간결하면서도 필요한 검증을 모두 수행하고 있습니다.
| @Test | ||
| void 회원가입시_id는_8자이상_15자이하여야_한다() { | ||
| // //given | ||
| // UserRegisterRequest request = new UserRegisterRequest("123", | ||
| // "wap123456@", | ||
| // "한글좋아", | ||
| // "010-1234-5678"); | ||
| // | ||
| // //when & then | ||
| // assertDoesNotThrow(()->userCommandService.registerUser(request)); | ||
| assertThat(3).isEqualTo(3); | ||
| } |
There was a problem hiding this comment.
테스트 코드 구현이 완료되지 않았습니다.
현재 테스트 메서드는 실제 ID 길이 검증을 테스트하지 않고 있습니다. 주석 처리된 코드를 완성하여 다음을 테스트해야 합니다:
- 8자 미만인 ID에 대한 예외 발생 검증
- 15자 초과인 ID에 대한 예외 발생 검증
- 유효한 범위(8-15자) ID의 정상 처리 검증
아래와 같이 테스트 코드를 구현하는 것을 권장합니다:
@Test
void 회원가입시_id는_8자이상_15자이하여야_한다() {
-// //given
-// UserRegisterRequest request = new UserRegisterRequest("123",
-// "wap123456@",
-// "한글좋아",
-// "010-1234-5678");
-//
-// //when & then
-// assertDoesNotThrow(()->userCommandService.registerUser(request));
- assertThat(3).isEqualTo(3);
+ // given - 7자 ID (최소 길이 미만)
+ UserRegisterRequest tooShortRequest = new UserRegisterRequest(
+ "1234567",
+ "wap123456@",
+ "한글좋아",
+ "010-1234-5678");
+
+ // given - 16자 ID (최대 길이 초과)
+ UserRegisterRequest tooLongRequest = new UserRegisterRequest(
+ "1234567890123456",
+ "wap123456@",
+ "한글좋아",
+ "010-1234-5678");
+
+ // given - 유효한 ID (8-15자)
+ UserRegisterRequest validRequest = new UserRegisterRequest(
+ "12345678",
+ "wap123456@",
+ "한글좋아",
+ "010-1234-5678");
+
+ // when & then - 짧은 ID
+ assertThrows(IllegalArgumentException.class,
+ () -> userCommandService.registerUser(tooShortRequest));
+
+ // when & then - 긴 ID
+ assertThrows(IllegalArgumentException.class,
+ () -> userCommandService.registerUser(tooLongRequest));
+
+ // when & then - 유효한 ID
+ assertDoesNotThrow(() -> userCommandService.registerUser(validRequest));
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Test | |
| void 회원가입시_id는_8자이상_15자이하여야_한다() { | |
| // //given | |
| // UserRegisterRequest request = new UserRegisterRequest("123", | |
| // "wap123456@", | |
| // "한글좋아", | |
| // "010-1234-5678"); | |
| // | |
| // //when & then | |
| // assertDoesNotThrow(()->userCommandService.registerUser(request)); | |
| assertThat(3).isEqualTo(3); | |
| } | |
| @Test | |
| void 회원가입시_id는_8자이상_15자이하여야_한다() { | |
| // given - 7자 ID (최소 길이 미만) | |
| UserRegisterRequest tooShortRequest = new UserRegisterRequest( | |
| "1234567", | |
| "wap123456@", | |
| "한글좋아", | |
| "010-1234-5678"); | |
| // given - 16자 ID (최대 길이 초과) | |
| UserRegisterRequest tooLongRequest = new UserRegisterRequest( | |
| "1234567890123456", | |
| "wap123456@", | |
| "한글좋아", | |
| "010-1234-5678"); | |
| // given - 유효한 ID (8-15자) | |
| UserRegisterRequest validRequest = new UserRegisterRequest( | |
| "12345678", | |
| "wap123456@", | |
| "한글좋아", | |
| "010-1234-5678"); | |
| // when & then - 짧은 ID | |
| assertThrows(IllegalArgumentException.class, | |
| () -> userCommandService.registerUser(tooShortRequest)); | |
| // when & then - 긴 ID | |
| assertThrows(IllegalArgumentException.class, | |
| () -> userCommandService.registerUser(tooLongRequest)); | |
| // when & then - 유효한 ID | |
| assertDoesNotThrow(() -> userCommandService.registerUser(validRequest)); | |
| } |
| @Test | ||
| void 모집상태_업데이트_실패_존재하지않는_클럽() { | ||
| // given | ||
| String clubId = new ObjectId().toHexString(); | ||
| ClubRecruitmentStatus status = ClubRecruitmentStatus.OPEN; | ||
|
|
||
| when(clubRepository.findClubById(any(ObjectId.class))).thenReturn(Optional.empty()); | ||
|
|
||
| // when, then | ||
| try { | ||
| recruitmentScheduler.updateRecruitmentStatus(clubId, status); | ||
| } catch (Exception e) { | ||
| assert (e instanceof RestApiException); | ||
| assert (((RestApiException) e).getErrorCode().equals( | ||
| ErrorCode.CLUB_NOT_FOUND)); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
모집상태 업데이트 실패 테스트가 잘 구현되었습니다.
존재하지 않는 클럽에 대한 모집 상태 업데이트 실패 시나리오가 적절히 테스트되었습니다.
그러나 테스트 방식이 JUnit의 표준 예외 테스트 방식을 따르지 않고 있습니다.
다음과 같이 JUnit의 표준 예외 테스트 방식으로 변경하는 것을 권장합니다:
// when, then
- try {
- recruitmentScheduler.updateRecruitmentStatus(clubId, status);
- } catch (Exception e) {
- assert (e instanceof RestApiException);
- assert (((RestApiException) e).getErrorCode().equals(
- ErrorCode.CLUB_NOT_FOUND));
- }
+ RestApiException exception = assertThrows(RestApiException.class, () ->
+ recruitmentScheduler.updateRecruitmentStatus(clubId, status));
+ assertEquals(ErrorCode.CLUB_NOT_FOUND, exception.getErrorCode());이 방식은 더 간결하고 JUnit의 표준 패턴을 따르며, 테스트 실패 시 더 명확한 오류 메시지를 제공합니다.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Test | |
| void 모집상태_업데이트_실패_존재하지않는_클럽() { | |
| // given | |
| String clubId = new ObjectId().toHexString(); | |
| ClubRecruitmentStatus status = ClubRecruitmentStatus.OPEN; | |
| when(clubRepository.findClubById(any(ObjectId.class))).thenReturn(Optional.empty()); | |
| // when, then | |
| try { | |
| recruitmentScheduler.updateRecruitmentStatus(clubId, status); | |
| } catch (Exception e) { | |
| assert (e instanceof RestApiException); | |
| assert (((RestApiException) e).getErrorCode().equals( | |
| ErrorCode.CLUB_NOT_FOUND)); | |
| } | |
| } | |
| @Test | |
| void 모집상태_업데이트_실패_존재하지않는_클럽() { | |
| // given | |
| String clubId = new ObjectId().toHexString(); | |
| ClubRecruitmentStatus status = ClubRecruitmentStatus.OPEN; | |
| when(clubRepository.findClubById(any(ObjectId.class))).thenReturn(Optional.empty()); | |
| // when, then | |
| RestApiException exception = assertThrows(RestApiException.class, () -> | |
| recruitmentScheduler.updateRecruitmentStatus(clubId, status)); | |
| assertEquals(ErrorCode.CLUB_NOT_FOUND, exception.getErrorCode()); | |
| } |
| @Test | ||
| void 모집_스케줄링_성공() { | ||
| // given | ||
| String clubId = "club-1"; | ||
|
|
||
| LocalDateTime startDate = LocalDateTime.now().plusDays(1); | ||
| Instant expectedStartInstant = startDate.atZone(ZoneId.of("Asia/Seoul")).toInstant() | ||
| .truncatedTo(ChronoUnit.MILLIS); | ||
|
|
||
| LocalDateTime endDate = LocalDateTime.now().plusDays(2); | ||
| Instant expectedEndInstant = endDate.atZone(ZoneId.of("Asia/Seoul")).toInstant() | ||
| .truncatedTo(ChronoUnit.MILLIS); | ||
|
|
||
| when(taskScheduler.schedule(any(Runnable.class), any(Date.class))) | ||
| .thenReturn((ScheduledFuture) scheduledFuture); | ||
|
|
||
| // when | ||
| recruitmentScheduler.scheduleRecruitment(clubId, startDate, endDate); | ||
|
|
||
| //then | ||
| verify(taskScheduler, times(2)).schedule(runnableCaptor.capture(), | ||
| dateCaptor.capture()); | ||
|
|
||
| // 모집 시장 스케줄링 검증 | ||
| List<Date> dates = dateCaptor.getAllValues(); | ||
| Date startScheduleDate = dates.get(0); | ||
| assertEquals(expectedStartInstant, | ||
| startScheduleDate.toInstant()); | ||
|
|
||
| // 모집 종료 스케줄링 검증 | ||
| Date endScheduleDate = dates.get(1); | ||
| assertEquals(expectedEndInstant, | ||
| endScheduleDate.toInstant()); | ||
| } |
There was a problem hiding this comment.
테스트 실패 해결 필요: 시간대 또는 날짜 비교 문제.
파이프라인 실패 로그에 따르면 모집_스케줄링_성공() 테스트가 92번 라인의 날짜 비교에서 실패하고 있습니다.
expected: <> but was: <> 오류는 시간대 처리나 시간 정밀도 문제일 수 있습니다.
다음 중 하나의 방법으로 문제를 해결해보세요:
- 시간 비교 방식 변경:
- assertEquals(expectedStartInstant,
- startScheduleDate.toInstant());
+ // 밀리초 단위로 비교하거나 직접 시간 차이를 계산하여 작은 차이는 허용
+ assertTrue(Math.abs(expectedStartInstant.toEpochMilli() -
+ startScheduleDate.toInstant().toEpochMilli()) < 10);- 캡처된 객체에 Mockito spy 추가:
- when(taskScheduler.schedule(any(Runnable.class), any(Date.class)))
- .thenReturn((ScheduledFuture) scheduledFuture);
+ // Date 객체를 전달하기 전에 Spy로 감싸서 실제 값 확인
+ when(taskScheduler.schedule(any(Runnable.class), any(Date.class)))
+ .thenAnswer(invocation -> {
+ Date date = invocation.getArgument(1);
+ // 예상되는 시간과 동일한지 확인할 수 있도록 로깅
+ System.out.println("Scheduled date: " + date.toInstant());
+ return scheduledFuture;
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Test | |
| void 모집_스케줄링_성공() { | |
| // given | |
| String clubId = "club-1"; | |
| LocalDateTime startDate = LocalDateTime.now().plusDays(1); | |
| Instant expectedStartInstant = startDate.atZone(ZoneId.of("Asia/Seoul")).toInstant() | |
| .truncatedTo(ChronoUnit.MILLIS); | |
| LocalDateTime endDate = LocalDateTime.now().plusDays(2); | |
| Instant expectedEndInstant = endDate.atZone(ZoneId.of("Asia/Seoul")).toInstant() | |
| .truncatedTo(ChronoUnit.MILLIS); | |
| when(taskScheduler.schedule(any(Runnable.class), any(Date.class))) | |
| .thenReturn((ScheduledFuture) scheduledFuture); | |
| // when | |
| recruitmentScheduler.scheduleRecruitment(clubId, startDate, endDate); | |
| //then | |
| verify(taskScheduler, times(2)).schedule(runnableCaptor.capture(), | |
| dateCaptor.capture()); | |
| // 모집 시장 스케줄링 검증 | |
| List<Date> dates = dateCaptor.getAllValues(); | |
| Date startScheduleDate = dates.get(0); | |
| assertEquals(expectedStartInstant, | |
| startScheduleDate.toInstant()); | |
| // 모집 종료 스케줄링 검증 | |
| Date endScheduleDate = dates.get(1); | |
| assertEquals(expectedEndInstant, | |
| endScheduleDate.toInstant()); | |
| } | |
| @Test | |
| void 모집_스케줄링_성공() { | |
| // given | |
| String clubId = "club-1"; | |
| LocalDateTime startDate = LocalDateTime.now().plusDays(1); | |
| Instant expectedStartInstant = startDate.atZone(ZoneId.of("Asia/Seoul")).toInstant() | |
| .truncatedTo(ChronoUnit.MILLIS); | |
| LocalDateTime endDate = LocalDateTime.now().plusDays(2); | |
| Instant expectedEndInstant = endDate.atZone(ZoneId.of("Asia/Seoul")).toInstant() | |
| .truncatedTo(ChronoUnit.MILLIS); | |
| // Date 객체를 전달하기 전에 Spy로 감싸서 실제 값 확인 | |
| when(taskScheduler.schedule(any(Runnable.class), any(Date.class))) | |
| .thenAnswer(invocation -> { | |
| Date date = invocation.getArgument(1); | |
| // 예상되는 시간과 동일한지 확인할 수 있도록 로깅 | |
| System.out.println("Scheduled date: " + date.toInstant()); | |
| return scheduledFuture; | |
| }); | |
| // when | |
| recruitmentScheduler.scheduleRecruitment(clubId, startDate, endDate); | |
| //then | |
| verify(taskScheduler, times(2)).schedule(runnableCaptor.capture(), | |
| dateCaptor.capture()); | |
| // 모집 시작 스케줄링 검증 | |
| List<Date> dates = dateCaptor.getAllValues(); | |
| Date startScheduleDate = dates.get(0); | |
| // 밀리초 단위로 비교하거나 직접 시간 차이를 계산하여 작은 차이는 허용 | |
| assertTrue(Math.abs(expectedStartInstant.toEpochMilli() - | |
| startScheduleDate.toInstant().toEpochMilli()) < 10); | |
| // 모집 종료 스케줄링 검증 | |
| Date endScheduleDate = dates.get(1); | |
| assertEquals(expectedEndInstant, | |
| endScheduleDate.toInstant()); | |
| } |
🧰 Tools
🪛 GitHub Actions: PR Test
[error] 92-92: Test failure in method 모집_스케줄링_성공(): org.opentest4j.AssertionFailedError: expected: <> but was: <>
PororoAndFriends
left a comment
PororoAndFriends
left a comment
There was a problem hiding this comment.
고생하셨습니다! 저도 Mockito 사용법 참고해서 짜야겠네요~
- RecruitmentScheduler의 scheduleRecruitment()에서 ZoneId.systemDefault() -> ZoneId.of("Asia/Seoul")로 바꾸어 시간대를 명시함으로써 테스트코드시 시간대가 맞지 않는 플레이키리 현상을 제거하였습니다.
- RecruitmentSchedulerTest에서 ZoneId 상수를 통합하여 사용합니다.
3 participants
#️⃣연관된 이슈
1. 목표
이 PR의 주요 목표는 테스트 커버리지 향상과 코드 안정성 강화입니다.
2. 주요 변경 내용
(1) ClubMetricService 개선
getDailyRanking()
getDailyActiveUser()
(2) RecruitmentScheduler 개선
getScheduledTasks() 추가
(3) 테스트 코드 대규모 추가
ClubMetricServiceTest (238줄)
RecruitmentSchedulerTest (159줄)
(4) Fixture 클래스 도입
ClubFixture, MetricFixture로 테스트 데이터 생성 간소화
3. 구현 상세
(1) ClubMetricService 로직 검증
활성 사용자 계산
일간/주간/월간: IP기준 중복 제거하여 반환
랭킹 생성 (getDailyRanking)
동아리 조회 시 null 허용 → Optional 처리로 NPE 방지
4. 작은 버그 픽스
ClubMetricService
버그: getDailyRanking(0) 호출 시 EmptyList 대신 전체 랭킹 반환
수정: n ≤ 0 검증 추가 → Collections.emptyList() 반환
5. 요약
이 PR은 테스트 커버리지 확보와 예외 처리 강화에 중점을 두었습니다.
핵심 로직의 신뢰성을 검증하는 400줄 이상의 테스트 코드 추가
경계 조건(Edge Case) 처리로 시스템 안정성 향상
모킹과 Fixture를 활용한 유지보수성 개선
🫡 논의하고 싶은 부분
트러블슈팅 사항이 한 가지 있었습니다. 상황은 다음과 같습니다.
RecruitmentSchedulerTest 코드 작성시 테스트 대상이 되는 객체가 주입받는 객체들 중 scheduledTasks는 recruitmentScheduler가 가지는 "상태"이기 때문에 실제 객체를 이용하여 검증하고 싶었습니다.
처음엔
@Spy를 사용하여 주입을 mockito에게 맡기고 실제 객체처럼 사용하고 싶었으나 다음과 같이 인라인 초기화된 필드는 Mockito에 의해 주입할 수 없다고 합니다.따라서, 다음과 같이 프로덕션 코드에 getter를 추가하여 객체를 꺼내어 조회하는 방법을 사용하였습니다.
위 방법의 장단점은 다음과 같습니다.
이 외에도 다양한 방법이 있지만, 그나마 대체 가능한 방법은 인라인 초기화를 제거하고 생성자를 통해 초기화하게 한 후,
@Spy를 사용하는 방법인것 같습니다. 어떤게 더 나을까요?Summary by CodeRabbit
버그 수정
리팩터링/스타일
새 기능
테스트