unit test baseline과 서비스 계층 테스트 도입#87
Merged
Merged
Conversation
- unit test runtime과 workflow verification baseline 정렬 - pure component와 domain service 계층 테스트 추가 - locale 및 wall-clock 의존을 줄이는 최소 seam 보강
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthrough단위 테스트 기반을 복구하기 위해 CI/배포 워크플로우를 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Possibly related PRs
Suggested labels
Poem
🚥 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 |
alexization
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (8)
src/test/java/com/gitranker/api/domain/auth/service/RefreshTokenServiceTest.java (1)
44-45: 삭제→저장 호출 순서도 고정해두는 편이 안전합니다.현재는 두 호출의 “존재”만 검증해서 순서가 바뀌어도 테스트가 통과합니다. 순서 역전 회귀(새 토큰까지 삭제)를 막기 위해 순서 검증을 추가하는 것을 권장합니다.
제안 diff
@@ - verify(refreshTokenRepository).deleteAllByUser(user); - verify(refreshTokenRepository).save(any(RefreshToken.class)); + org.mockito.InOrder order = org.mockito.Mockito.inOrder(refreshTokenRepository); + order.verify(refreshTokenRepository).deleteAllByUser(user); + order.verify(refreshTokenRepository).save(any(RefreshToken.class));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/gitranker/api/domain/auth/service/RefreshTokenServiceTest.java` around lines 44 - 45, The test currently only verifies that refreshTokenRepository.deleteAllByUser(user) and refreshTokenRepository.save(any(RefreshToken.class)) were called but not their order; update RefreshTokenServiceTest to enforce call order by creating an InOrder for refreshTokenRepository (e.g., InOrder inOrder = inOrder(refreshTokenRepository)) and use inOrder.verify(refreshTokenRepository).deleteAllByUser(user) followed by inOrder.verify(refreshTokenRepository).save(any(RefreshToken.class)); keep or remove the existing unordered verify(...) assertions as desired.src/test/java/com/gitranker/api/domain/badge/SvgBadgeRendererTest.java (1)
31-41: diff 렌더링은 음수 케이스도 함께 고정해두는 것이 좋습니다.Line 27에서 음수 diff 데이터를 넣고 있으므로, Line 31-41 assertion에
diff-minus/음수 표기까지 추가하면 렌더링 회귀를 더 잘 잡을 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/gitranker/api/domain/badge/SvgBadgeRendererTest.java` around lines 31 - 41, The assertions in SvgBadgeRendererTest currently only check for positive diff rendering ("diff-plus" and "+5") but the test setup includes negative diff data; update the assertion block that checks the rendered svg (the chained assertThat(svg).contains(...)) to also assert the negative diff rendering by including "diff-minus" and the negative value string (e.g., "-5") so the test covers both plus and minus diff cases.src/test/java/com/gitranker/api/domain/user/service/BaselineStatsCalculatorTest.java (1)
33-40: 중복된 spy/날짜 고정 setup은 헬퍼로 추출하는 편이 좋습니다.Line 33-40과 Line 52-64의 패턴이 동일해서, 고정 날짜 기반
BaselineStatsCalculator생성 로직을 메서드로 빼면 테스트 의도가 더 선명해집니다.예시 리팩터링
+ private BaselineStatsCalculator calculatorWithDate(LocalDate fixedDate) { + BaselineStatsCalculator calculator = Mockito.spy(new BaselineStatsCalculator(gitHubDataMapper)); + Mockito.doReturn(fixedDate).when(calculator).currentDate(); + return calculator; + } + void calculatesBaselineForExistingUsers() { - BaselineStatsCalculator calculator = Mockito.spy(new BaselineStatsCalculator(gitHubDataMapper)); + BaselineStatsCalculator calculator = calculatorWithDate(LocalDate.of(2026, 1, 1)); ... - LocalDate fixedDate = LocalDate.of(2026, 1, 1); + LocalDate fixedDate = LocalDate.of(2026, 1, 1); - Mockito.doReturn(fixedDate).when(calculator).currentDate(); ... }Also applies to: 52-64
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/gitranker/api/domain/user/service/BaselineStatsCalculatorTest.java` around lines 33 - 40, Extract the duplicated spy-and-fixed-date setup into a private helper method (e.g., createCalculatorWithFixedDate or setupCalculatorWithDate) that constructs the BaselineStatsCalculator spy, stubs currentDate() via Mockito.doReturn(fixedDate).when(calculator).currentDate(), and returns the prepared spy (and any commonly used fixtures like the fixed LocalDate or User/ActivityStatistics via overloaded helpers if needed); then replace the repeated blocks that create the BaselineStatsCalculator, call Mockito.doReturn(...).when(calculator).currentDate(), and set up user(...) and stats(...) with calls to this helper to remove duplication and clarify intent.src/test/java/com/gitranker/api/domain/user/service/UserPersistenceServiceTest.java (1)
108-113: 통계 갱신 성공 케이스에 영속화 호출 검증을 추가해 주세요.현재는 값 변경과 오케스트레이션 호출만 확인합니다.
userRepository.save(...)검증을 추가하면 저장 누락 회귀까지 막을 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/gitranker/api/domain/user/service/UserPersistenceServiceTest.java` around lines 108 - 113, Add a verification that the repository persist call happened: in UserPersistenceServiceTest after calling updateUserStatisticsWithLog(1L, totalStats, baselineStats) and after the existing verifies for activityLogOrchestrator and rankingRecalculationService, add a verify on the mocked userRepository that save(...) was called (e.g., verify(userRepository).save(...) or verify(userRepository).save(same(user)) using the existing user test fixture) to ensure the persistence path in updateUserStatisticsWithLog is exercised.src/test/java/com/gitranker/api/domain/user/service/UserRefreshServiceTest.java (1)
92-95: 성공 경로에서는rawResponse를 null 대신 명시 객체로 두는 편이 안전합니다.현재 null 기반 스텁은 통과는 되지만, 실제 호출 계약이 바뀌어도 테스트가 의도를 놓칠 수 있습니다. mock 또는 fixture 응답 객체를 두고 연결하는 구성이 더 견고합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/gitranker/api/domain/user/service/UserRefreshServiceTest.java` around lines 92 - 95, Replace the null-based stubbing with a concrete fixture for the raw GitHub response: create a mock or fixture object (e.g., rawResponse) and use it in the when(...) for gitHubActivityService.fetchRawAllActivities("alice", user.getGithubCreatedAt()) instead of null, then update the dependent stubs to reference that same rawResponse (gitHubDataMapper.toActivityStatistics(rawResponse) and baselineStatsCalculator.calculate(user, rawResponse)) while keeping the final updateUserStatisticsWithLog(1L, totalStats, baselineStats) stub unchanged; this ensures the test exercises a realistic non-null contract for fetchRawAllActivities and remains robust to API changes.src/test/java/com/gitranker/api/domain/badge/BadgeServiceTest.java (1)
56-65: USER_NOT_FOUND 경로에 부수효과 부재 검증을 추가하는 것을 권장합니다.예외 발생 시
businessMetrics/svgBadgeRenderer가 호출되지 않음을 함께 검증하면 실패 경로 회귀를 더 잘 방지할 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/gitranker/api/domain/badge/BadgeServiceTest.java` around lines 56 - 65, Update the test throwsWhenUserDoesNotExist in BadgeServiceTest to also assert that failure-side collaborators are not called: after stubbing userRepository.findByNodeId("missing") to return Optional.empty() and asserting badgeService.generateBadge("missing") throws USER_NOT_FOUND, add verifications that businessMetrics and svgBadgeRenderer were not invoked (e.g., verifyNoInteractions(svgBadgeRenderer) and verifyNoInteractions(businessMetrics) or verify(svgBadgeRenderer, never()).render... / verify(businessMetrics, never()).increment... as appropriate) so the USER_NOT_FOUND path has no side effects.src/test/java/com/gitranker/api/support/TestFixtures.java (1)
47-53: 반복되는 날짜/식별자 리터럴은 상수로 추출해 drift를 줄여주세요.Line 48, Line 52, Line 65-67, Line 70의 하드코딩 값이 중복되어 이후 수정 시 테스트 간 불일치가 생기기 쉽습니다. 파일 상단 상수로 모아두는 편이 안전합니다.
예시 리팩터링 diff
public final class TestFixtures { + private static final LocalDate FIXED_ACTIVITY_DATE = LocalDate.of(2025, 1, 1); + private static final int FIXED_GITHUB_ID = 12345; + private static final String FIXED_GITHUB_NODE_ID = "MDQ6VXNlcjEyMzQ1"; + private static final String FIXED_GITHUB_CREATED_AT = "2020-01-01T00:00:00Z"; private TestFixtures() { } @@ public static ActivityLog emptyActivityLog(User user) { - return ActivityLog.empty(user, LocalDate.of(2025, 1, 1)); + return ActivityLog.empty(user, FIXED_ACTIVITY_DATE); } @@ public static ActivityLog activityLog(User user, ActivityStatistics totals, ActivityStatistics diff) { - return ActivityLog.of(user, totals, diff, LocalDate.of(2025, 1, 1)); + return ActivityLog.of(user, totals, diff, FIXED_ACTIVITY_DATE); } @@ public static OAuthAttributes oauthAttributes(String username, String email, String profileImage) { Map<String, Object> attributes = new HashMap<>(); - attributes.put("id", 12345); - attributes.put("node_id", "MDQ6VXNlcjEyMzQ1"); + attributes.put("id", FIXED_GITHUB_ID); + attributes.put("node_id", FIXED_GITHUB_NODE_ID); @@ - attributes.put("created_at", "2020-01-01T00:00:00Z"); + attributes.put("created_at", FIXED_GITHUB_CREATED_AT); return OAuthAttributes.of("id", attributes); } }Also applies to: 63-71
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/gitranker/api/support/TestFixtures.java` around lines 47 - 53, Extract the hardcoded repeated literals used in TestFixtures (e.g., the LocalDate.of(2025, 1, 1) in emptyActivityLog and activityLog and any repeated identifier literals referenced around lines 63-71) into private static final constants at the top of the class (e.g., TEST_DATE and any repeated IDs/names), then replace the inline calls in ActivityLog.empty, ActivityLog.of and related helper methods with those constants so all tests share the same single source of truth.src/test/java/com/gitranker/api/domain/user/service/UserRegistrationServiceTest.java (1)
95-100: 기존 사용자 시나리오의 응답 단언 깊이를 맞춰주세요.Line 95-100, Line 122-124에서 일부 필드만 단언하고 있어 response 매핑 회귀를 놓칠 수 있습니다.
userId,commitCount,isNewUser까지 함께 검증하면 테스트 일관성이 좋아집니다.Also applies to: 122-124
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/gitranker/api/domain/user/service/UserRegistrationServiceTest.java` around lines 95 - 100, In UserRegistrationServiceTest, extend the assertions on the existing "existing user" scenario to cover all relevant response fields so mapping regressions are caught: after asserting response.username(), response.isNewUser(), response.email(), and response.profileImage(), also assert response.userId() and response.commitCount() (and keep response.isNewUser() assertion) to match the expected values for an existing user; ensure the same additional assertions are added in the other case mentioned (lines 122-124). Leave the existing verify(...) checks for gitHubActivityService.fetchRawAllActivities(...) and businessMetrics.incrementRegistrations() intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/test/java/com/gitranker/api/domain/auth/service/AuthServiceTest.java`:
- Around line 36-37: The hard-coded expiry constants VALID_EXPIRY and EXPIRED_AT
in AuthServiceTest make tests brittle; change them to be relative to the current
time (e.g., use LocalDateTime.now().plus... for a valid future expiry and
LocalDateTime.now().minus... for an expired time) so tests like
refreshesAccessTokenForValidRefreshToken and any other assertions using
VALID_EXPIRY/EXPIRED_AT do not break with real-world calendar drift.
In
`@src/test/java/com/gitranker/api/domain/user/service/UserRegistrationServiceTest.java`:
- Around line 64-66: The test stubs use nulls which can hide regressions; change
the fetchRawAllActivities and related mapper stub to use a non-null empty
collection (e.g., Collections.emptyList() or List.of()) instead of null, update
the gitHubDataMapper.toActivityStatistics stub to accept that empty collection,
and change the baselineStatsCalculator.calculate matcher from
org.mockito.ArgumentMatchers.isNull() to a matcher that expects the empty
collection (e.g., eq(Collections.emptyList()) or anyList()) so the data path
(gitHubActivityService.fetchRawAllActivities ->
gitHubDataMapper.toActivityStatistics -> baselineStatsCalculator.calculate) is
exercised with a real, non-null value.
---
Nitpick comments:
In
`@src/test/java/com/gitranker/api/domain/auth/service/RefreshTokenServiceTest.java`:
- Around line 44-45: The test currently only verifies that
refreshTokenRepository.deleteAllByUser(user) and
refreshTokenRepository.save(any(RefreshToken.class)) were called but not their
order; update RefreshTokenServiceTest to enforce call order by creating an
InOrder for refreshTokenRepository (e.g., InOrder inOrder =
inOrder(refreshTokenRepository)) and use
inOrder.verify(refreshTokenRepository).deleteAllByUser(user) followed by
inOrder.verify(refreshTokenRepository).save(any(RefreshToken.class)); keep or
remove the existing unordered verify(...) assertions as desired.
In `@src/test/java/com/gitranker/api/domain/badge/BadgeServiceTest.java`:
- Around line 56-65: Update the test throwsWhenUserDoesNotExist in
BadgeServiceTest to also assert that failure-side collaborators are not called:
after stubbing userRepository.findByNodeId("missing") to return Optional.empty()
and asserting badgeService.generateBadge("missing") throws USER_NOT_FOUND, add
verifications that businessMetrics and svgBadgeRenderer were not invoked (e.g.,
verifyNoInteractions(svgBadgeRenderer) and verifyNoInteractions(businessMetrics)
or verify(svgBadgeRenderer, never()).render... / verify(businessMetrics,
never()).increment... as appropriate) so the USER_NOT_FOUND path has no side
effects.
In `@src/test/java/com/gitranker/api/domain/badge/SvgBadgeRendererTest.java`:
- Around line 31-41: The assertions in SvgBadgeRendererTest currently only check
for positive diff rendering ("diff-plus" and "+5") but the test setup includes
negative diff data; update the assertion block that checks the rendered svg (the
chained assertThat(svg).contains(...)) to also assert the negative diff
rendering by including "diff-minus" and the negative value string (e.g., "-5")
so the test covers both plus and minus diff cases.
In
`@src/test/java/com/gitranker/api/domain/user/service/BaselineStatsCalculatorTest.java`:
- Around line 33-40: Extract the duplicated spy-and-fixed-date setup into a
private helper method (e.g., createCalculatorWithFixedDate or
setupCalculatorWithDate) that constructs the BaselineStatsCalculator spy, stubs
currentDate() via Mockito.doReturn(fixedDate).when(calculator).currentDate(),
and returns the prepared spy (and any commonly used fixtures like the fixed
LocalDate or User/ActivityStatistics via overloaded helpers if needed); then
replace the repeated blocks that create the BaselineStatsCalculator, call
Mockito.doReturn(...).when(calculator).currentDate(), and set up user(...) and
stats(...) with calls to this helper to remove duplication and clarify intent.
In
`@src/test/java/com/gitranker/api/domain/user/service/UserPersistenceServiceTest.java`:
- Around line 108-113: Add a verification that the repository persist call
happened: in UserPersistenceServiceTest after calling
updateUserStatisticsWithLog(1L, totalStats, baselineStats) and after the
existing verifies for activityLogOrchestrator and rankingRecalculationService,
add a verify on the mocked userRepository that save(...) was called (e.g.,
verify(userRepository).save(...) or verify(userRepository).save(same(user))
using the existing user test fixture) to ensure the persistence path in
updateUserStatisticsWithLog is exercised.
In
`@src/test/java/com/gitranker/api/domain/user/service/UserRefreshServiceTest.java`:
- Around line 92-95: Replace the null-based stubbing with a concrete fixture for
the raw GitHub response: create a mock or fixture object (e.g., rawResponse) and
use it in the when(...) for gitHubActivityService.fetchRawAllActivities("alice",
user.getGithubCreatedAt()) instead of null, then update the dependent stubs to
reference that same rawResponse
(gitHubDataMapper.toActivityStatistics(rawResponse) and
baselineStatsCalculator.calculate(user, rawResponse)) while keeping the final
updateUserStatisticsWithLog(1L, totalStats, baselineStats) stub unchanged; this
ensures the test exercises a realistic non-null contract for
fetchRawAllActivities and remains robust to API changes.
In
`@src/test/java/com/gitranker/api/domain/user/service/UserRegistrationServiceTest.java`:
- Around line 95-100: In UserRegistrationServiceTest, extend the assertions on
the existing "existing user" scenario to cover all relevant response fields so
mapping regressions are caught: after asserting response.username(),
response.isNewUser(), response.email(), and response.profileImage(), also assert
response.userId() and response.commitCount() (and keep response.isNewUser()
assertion) to match the expected values for an existing user; ensure the same
additional assertions are added in the other case mentioned (lines 122-124).
Leave the existing verify(...) checks for
gitHubActivityService.fetchRawAllActivities(...) and
businessMetrics.incrementRegistrations() intact.
In `@src/test/java/com/gitranker/api/support/TestFixtures.java`:
- Around line 47-53: Extract the hardcoded repeated literals used in
TestFixtures (e.g., the LocalDate.of(2025, 1, 1) in emptyActivityLog and
activityLog and any repeated identifier literals referenced around lines 63-71)
into private static final constants at the top of the class (e.g., TEST_DATE and
any repeated IDs/names), then replace the inline calls in ActivityLog.empty,
ActivityLog.of and related helper methods with those constants so all tests
share the same single source of truth.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: e4b7e650-6f24-4e4c-a8a5-c6049d9030ed
📒 Files selected for processing (26)
.github/workflows/ci.yml.github/workflows/deploy.ymlAGENTS.mdbuild.gradlesrc/main/java/com/gitranker/api/domain/badge/BadgeFormatter.javasrc/main/java/com/gitranker/api/domain/badge/BadgeService.javasrc/main/java/com/gitranker/api/domain/badge/SvgBadgeRenderer.javasrc/main/java/com/gitranker/api/domain/user/service/BaselineStatsCalculator.javasrc/test/java/com/gitranker/api/domain/auth/service/AuthServiceTest.javasrc/test/java/com/gitranker/api/domain/auth/service/RefreshTokenServiceTest.javasrc/test/java/com/gitranker/api/domain/badge/BadgeFormatterTest.javasrc/test/java/com/gitranker/api/domain/badge/BadgeServiceTest.javasrc/test/java/com/gitranker/api/domain/badge/SvgBadgeRendererTest.javasrc/test/java/com/gitranker/api/domain/badge/TierGradientProviderTest.javasrc/test/java/com/gitranker/api/domain/failure/BatchFailureLogServiceTest.javasrc/test/java/com/gitranker/api/domain/log/ActivityLogServiceTest.javasrc/test/java/com/gitranker/api/domain/ranking/RankingRecalculationServiceTest.javasrc/test/java/com/gitranker/api/domain/ranking/RankingServiceTest.javasrc/test/java/com/gitranker/api/domain/user/service/BaselineStatsCalculatorTest.javasrc/test/java/com/gitranker/api/domain/user/service/UserDeletionServiceTest.javasrc/test/java/com/gitranker/api/domain/user/service/UserPersistenceServiceTest.javasrc/test/java/com/gitranker/api/domain/user/service/UserQueryServiceTest.javasrc/test/java/com/gitranker/api/domain/user/service/UserRefreshServiceTest.javasrc/test/java/com/gitranker/api/domain/user/service/UserRegistrationServiceTest.javasrc/test/java/com/gitranker/api/global/util/TimeUtilsTest.javasrc/test/java/com/gitranker/api/support/TestFixtures.java
- unresolved PR 리뷰 스레드의 테스트 안정성 피드백 반영 - TestFixtures 의존을 단순화해 IDE 해석 안정성 개선 - badge diff 포맷과 응답 단언 범위를 보강
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
1) 요약
src/test/**에 pure component + domain service 계층 unit test를 추가했습니다../gradlew test를 넣고 packaging build는./gradlew build -x test로 분리했습니다.build-only검증이라 비즈니스 service 회귀를 물리적 gate에서 잡지 못했습니다.2) 연관 이슈
3) 문제와 목표
git-ranker는 test runtime과src/test/tree가 없어 backend 변경이./gradlew build만으로 검증되고 있었습니다.4) 영향 범위
domain.badgedomain.auth.servicedomain.logdomain.rankingdomain.user.serviceglobal.utilsrc/test/**build.gradle,.github/workflows/*,AGENTS.md5) 검증 증거
./gradlew build./gradlew test미실행(이번 spec 범위에서 제외)미실행(coverage gate는 이번 spec 비범위)미실행(unit-test baseline 작업)6) 관측성 확인
7) AI 리뷰 메모 (선택)
./gradlew test,./gradlew build통과8) 리스크 및 롤백
check에 새 검증이 묶이면 workflow의build -x test사용 여부를 다시 점검해야 합니다.build.gradle, workflow,AGENTS.md,src/test/**를 되돌립니다.9) 체크리스트
Summary by CodeRabbit
릴리스 노트
버그 수정
테스트
Chores