Conversation
- ServiceIntegrationTest → ServiceTest로 이름 변경 - @SpringBootTest(webEnvironment = NONE) 기반 서비스 테스트 베이스 클래스 - DatabaseCleaner를 사용한 테스트 격리
- CupServiceUnitTest, MemberServiceUnitTest 등 9개 파일 삭제 - 모든 의존성을 mock하여 리팩터링 내성 없음 - 통합 테스트와 완전 중복되는 테스트 제거 삭제 근거: London-style 테스트는 리팩터링 시 거짓 양성 유발
- ServiceIntegrationTest → ServiceTest로 이름 변경 - 예외 검증 패턴을 assertThrows + assertThat으로 통일 - FriendPair record 삭제 및 배열로 변경 - 실제 DB를 사용하는 통합 테스트 유지
- 13개 컨트롤러 테스트 삭제 (서비스 테스트와 중복) - AuthenticationTest 추가: 인증/인가 검증만 수행 - 토큰 없음 → 401, 유효하지 않은 토큰 → 401 - 유효한 토큰 → 200, ACCOUNT만 있음 → 403 - 인증 불필요 엔드포인트 → 통과 삭제 근거: Bean Validation은 도메인 테스트에서, 비즈니스 로직은 서비스 테스트에서 검증
Repository 테스트 삭제 (6개): - CupRepositoryTest, IntakeHistoryRepositoryTest 등 - Spring Data JPA 메서드 테스트는 프레임워크 검증 - 통합 테스트에서 자연스럽게 검증됨 인프라/필터 테스트 삭제 (5개): - JwtAuthenticationFilterTest → AuthenticationTest로 대체 - MemberResolverTest, OauthAccountResolverTest 등 - 구현 세부사항 검증은 리팩터링 내성 없음
- hasFieldOrPropertyWithValue 제거 (리플렉션 기반) - assertThrows + assertThat(ex.getErrorCode()) 패턴으로 변경 - 타입 안전한 예외 검증 변경 대상: - CupRankTest, CupNicknameTest, CupTargetAmountTest - IntakeTypeTest, CupRanksTest, TargetAmountTest - OauthAccountTest, PhysicalAttributesTest 등
- *IntegrationTest.java 파일들 삭제 - *ServiceTest.java로 이름 변경 완료에 따른 정리
|
Note
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. | |
| Out of Scope Changes check | ❓ Inconclusive | 빌드 시스템에 Jacoco 코드 커버리지 도구가 추가되었으나, 이것이 테스트 리팩터링 범위에 명시적으로 포함되었는지 문서상 명확하지 않습니다. | Jacoco 추가가 테스트 아키텍처 리팩터링의 일부인지, 아니면 별도 개선 사항인지 확인이 필요합니다. |
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | PR 제목은 전체 변경사항을 잘 요약하고 있습니다. 테스트 아키텍처 리팩터링이라는 핵심 목표를 명확히 전달합니다. |
| Description check | ✅ Passed | PR 설명에서 관련 이슈(#1115)를 명시하고, 테스트 아키텍처 가이드 문서를 포함하여 변경사항의 배경과 기준을 상세히 제시합니다. |
| Linked Issues check | ✅ Passed | 변경사항이 연결된 이슈 #1115의 목표를 충족합니다. 광범위한 테스트 리팩터링을 통해 리팩터링 내성, 빠른 피드백, 회귀 방지 원칙을 구현했습니다. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing touches
- 📝 Generate docstrings
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 help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (12)
backend/src/test/java/backend/mulkkam/notification/service/SuggestionNotificationServiceTest.java (1)
88-92: 예외 검증 패턴이 PR의 표준화 목표와 불일치합니다.현재
assertThatThrownBy를 사용하고 있지만, PR의 커밋 메시지에서 "unify exception assertions to assertThrows + assertThat"로 통일하기로 했습니다. 이 파일이 표준화 작업에서 누락된 것으로 보입니다.문제점:
- 코드베이스 전체의 일관성이 깨집니다
- 다른 테스트 파일들과 패턴이 달라 유지보수 시 혼란을 줄 수 있습니다
제안:
아래와 같이assertThrows+assertThat패턴으로 변경해주세요.♻️ 표준화된 패턴으로 변경
+import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy; // when & then - assertThatThrownBy( - () -> suggestionNotificationService.applyTargetAmount(Long.MAX_VALUE, - new MemberDetails(savedMember.getId(), MemberRole.MEMBER)) - ).isInstanceOf(CommonException.class) - .hasMessage(NOT_FOUND_SUGGESTION_NOTIFICATION.name()); + CommonException exception = assertThrows(CommonException.class, + () -> suggestionNotificationService.applyTargetAmount(Long.MAX_VALUE, + new MemberDetails(savedMember.getId(), MemberRole.MEMBER))); + + assertThat(exception.getMessage()).isEqualTo(NOT_FOUND_SUGGESTION_NOTIFICATION.name());backend/src/test/java/backend/mulkkam/intake/controller/IntakeAmountControllerTest.java (2)
92-113: 서로 다른 두 엔드포인트가 같은@Nested클래스에 섞여 있어 조직이 명확하지 않습니다.첫 번째 테스트(76번 줄)는
/intake/amount/recommended엔드포인트를 호출하는데, 이는 인증된 사용자의 정보를 기반으로 권장량을 계산합니다. 반면 두 번째 테스트(99번 줄)는/intake/amount/target/recommended엔드포인트를 호출하며, 이는 입력받은 신체 정보 파라미터를 기반으로 목표량을 계산합니다. 두 엔드포인트는 목적과 입력 방식이 다르므로, 별도의@Nested클래스로 분리하는 것이 테스트 의도를 명확히 드러낼 수 있습니다.♻️ 테스트 조직 개선 제안
@DisplayName("사용자 맞춤 권장 음용량을 조회할 때에") @Nested class GetRecommended { // /intake/amount/recommended 엔드포인트 테스트 } @DisplayName("신체 정보 기반 목표 권장량을 계산할 때에") @Nested class GetRecommendedTargetAmount { // /intake/amount/target/recommended 엔드포인트 테스트 }이렇게 분리하면 각 테스트 묶음이 명확한 책임을 갖게 되고, 향후 엔드포인트별 테스트를 추가할 때도 한층 체계적으로 관리할 수 있습니다.
65-114: 컨트롤러 통합 테스트가 부족합니다.서비스 레벨에서
modifyTarget()과getTarget()에 대한 테스트는IntakeTargetAmountServiceTest.java에서 이미 커버되고 있습니다. 하지만 다음 엔드포인트들에 대한 **컨트롤러 통합 테스트(MockMVC 사용)**가 누락되어 있습니다:
PATCH /intake/amount/target- 목표 음용량 수정GET /intake/amount/target- 목표 음용량 조회왜 필요한가:
서비스 테스트와 컨트롤러 테스트는 관심사가 다릅니다. 컨트롤러 통합 테스트는 HTTP 요청-응답 레이어, 인증 헤더 검증, 요청 직렬화/역직렬화 등을 검증합니다. 현재 코드는 이 부분을 확인할 수 없습니다.제안:
다음과 같은 컨트롤러 테스트를 추가하세요. 기존의GetRecommended테스트와 동일한 패턴으로 작성하면 됩니다:
PATCH /intake/amount/target- 정상 수정 케이스 (새로운 amount 값으로 변경 후 조회 확인)PATCH /intake/amount/target- 유효하지 않은 amount 값 (에러 응답 검증)GET /intake/amount/target- 조회 성공 케이스 (설정된 amount 값 확인)backend/src/test/java/backend/mulkkam/intake/service/IntakeTargetAmountServiceTest.java (2)
194-194: @DisplayName의 용어를 수정해주세요."응용량"은 "음용량"의 오타로 보입니다. 전체 파일에서 일관되게 "음용량(drinking amount)"을 사용하고 있으므로 이를 맞춰주세요.
💬 수정 제안
- @DisplayName("하루 섭취 목표 응용량을 추천받을 때에") + @DisplayName("하루 섭취 목표 음용량을 추천받을 때에")
268-272: 변수명 리팩토링을 완료해주세요.이 파일의 다른 테스트들(
GetRecommended,GetTarget)에서는 응답 변수를result로 통일했는데, 이 테스트들만recommendedTargetAmount를 그대로 사용하고 있습니다.일관성을 위해 다음과 같이 변경하는 것을 권장합니다:
이유:
- 일관성: 같은 파일 내에서 동일한 패턴을 사용하면 코드 이해도가 높아집니다
- 간결성:
result가 더 짧고 BDD 패턴의 "when" 결과를 명확히 나타냅니다- 리팩토링 완성도: 현재 PR의 변수명 통일 작업을 완료할 수 있습니다
♻️ 수정 제안
263번 라인 테스트:
// when - SuggestionIntakeAmountResponse recommendedTargetAmount = intakeAmountService.getRecommendedTargetAmount( + SuggestionIntakeAmountResponse result = intakeAmountService.getRecommendedTargetAmount( physicalAttributesRequest); // then - assertThat(recommendedTargetAmount.amount()).isEqualTo(1_800); + assertThat(result.amount()).isEqualTo(1_800);277번 라인 테스트:
// when - SuggestionIntakeAmountResponse recommendedTargetAmount = intakeAmountService.getRecommendedTargetAmount( + SuggestionIntakeAmountResponse result = intakeAmountService.getRecommendedTargetAmount( physicalAttributesRequest); // then - assertThat(recommendedTargetAmount.amount()).isEqualTo(1_800); + assertThat(result.amount()).isEqualTo(1_800);Also applies to: 282-286
backend/src/test/java/backend/mulkkam/notification/service/NotificationServiceTest.java (2)
405-409: assertSoftly 안에서 assertThat을 쓰고 있어 SoftAssertions가 사실상 동작하지 않아요.현재는
assertSoftly(softly -> { assertThat(...); assertThat(...); })라서, 첫 assert에서 실패하면 그대로 종료될 수 있습니다(softly 누적 안 됨). 아래처럼softly.assertThat로 바꿔주세요.💡 제안 diff
// then assertSoftly(softly -> { - assertThat(notificationRepository.findById(notification.getId())).isEmpty(); - assertThat(suggestionNotificationRepository.findById(notification.getId())).isEmpty(); + softly.assertThat(notificationRepository.findById(notification.getId())).isEmpty(); + softly.assertThat(suggestionNotificationRepository.findById(notification.getId())).isEmpty(); });
424-450: 권한 없음 케이스가delete(..., 1L)하드코딩이라 NotFound로 빠질 가능성이 있어요.이 테스트의 목적은 Forbidden인데, 삭제 대상 id를
1L로 고정해두면 시퀀스가 리셋되지 않는 환경에서 “존재하지 않는 알림”이 되어NotFound로 떨어질 수 있어요. 저장한notification.getId()를 쓰는 게 안전합니다.💡 제안 diff
// when & then - assertThatThrownBy(() -> notificationService.delete(memberDetails, 1L)) + assertThatThrownBy(() -> notificationService.delete(memberDetails, notification.getId())) .isInstanceOf(CommonException.class) .hasMessageContaining(ForbiddenErrorCode.NOT_PERMITTED_FOR_NOTIFICATION.name());backend/src/test/java/backend/mulkkam/intake/service/WeatherServiceTest.java (1)
23-35: @disabled 처리된 테스트와 부적절한 검증 방식이 있습니다.현재 이 테스트는 비활성화되어 있고, 검증 로직도
System.out.println으로만 되어 있습니다.문제점:
- @disabled 테스트: 비활성화된 테스트가 코드베이스에 남아있으면 점진적으로 커버리지가 떨어지고, 해당 기능의 회귀 가능성이 높아집니다
- System.out.println: 실제 검증(assertion)이 없어서 테스트가 실패를 감지하지 못합니다
대안 제안:
- 방안 1 (권장): 실제 API 호출이 필요한 테스트라면, 별도의 통합 테스트 프로파일이나 태그(예:
@Tag("integration"))로 분리하여 필요할 때만 실행하도록 구성- 방안 2: Mock을 사용하는
WeatherServiceUnitTest처럼 외부 의존성을 제거하고 단위 테스트로 전환- 방안 3: 테스트가 더 이상 필요하지 않다면 삭제
어떤 방향으로 개선할지 결정 후, 실제 assertion(
assertThat(averageTemperatureForDate).isGreaterThan(0)등)을 추가하는 것을 권장합니다.이 테스트를 적절한 형태로 복원하는 작업을 도와드릴까요?
backend/src/test/java/backend/mulkkam/member/service/MemberServiceTest.java (1)
504-511: 사용되지 않는 메서드 제거 필요
saveDefaultCupEmojis()메서드가 MemberServiceTest 내에서 호출되지 않고 있습니다. 이는 테스트 코드의 가독성과 유지보수성을 해치는 데드코드로, 불필요한 코드 덩어리가 쌓이면 향후 리팩토링 시 혼란을 초래할 수 있습니다. 다른 테스트 클래스(IntakeHistoryServiceTest)에서는 동일한 메서드가 실제로 사용되고 있으므로, 이 클래스에만 존재하는 중복 정의는 제거하는 것이 좋습니다.제안:
- MemberServiceTest의 504-511줄
saveDefaultCupEmojis()메서드를 삭제하기backend/src/test/java/backend/mulkkam/cup/service/CupServiceTest.java (2)
369-374: 컴파일 에러로 보이는 오타가 있어요 (.cupAmount(...)).뒤에 점이 하나 더 붙음)
Line 372 부근의cupAmount(...)호출 뒤에.가 남아 있으면 Java 문법상 컴파일이 안 됩니다. 단순 오타로 보여서 릴리즈 전에 꼭 잡아야 해요.제안 수정(diff)
Cup cup1 = CupFixtureBuilder .withMemberAndCupEmoji(savedMember, savedCupEmoji) .cupNickname(new CupNickname(beforeCupNickName1)) - .cupAmount(new CupAmount(beforeCupAmount1)). - intakeType(beforeIntakeType1) + .cupAmount(new CupAmount(beforeCupAmount1)) + .intakeType(beforeIntakeType1) .build();
531-657:updateRanks계열 테스트가 “ID=1,2,3” 가정에 강하게 묶여 있어요 (환경 따라 간헐 실패 가능)
여러 테스트에서new CupRankDto(1L, ...)처럼 고정 ID를 쓰는데, 이는 “해당 테스트에서 저장된 Cup의 ID가 항상 1부터 시작한다”는 가정이 필요합니다. DB 클리너가 시퀀스까지 리셋하지 않거나 다른 테스트가 먼저 데이터를 만들면 깨질 수 있어요.
- 선택지 A(추천):
saveAll이후 엔티티의 실제getId()를 꺼내서 요청 구성 (가장 견고)- 선택지 B: 테스트 격리 인프라에서 시퀀스 리셋을 보장 (인프라 의존)
제안 수정 예시(diff, success_cup_ranks_are_updated 기준)
cupRepository.saveAll(List.of(firstCup, secondCup, thirdCup)); -List<CupRankDto> cupRanks = List.of(new CupRankDto(1L, 3), new CupRankDto(2L, 2), new CupRankDto(3L, 1)); +List<CupRankDto> cupRanks = List.of( + new CupRankDto(firstCup.getId(), 3), + new CupRankDto(secondCup.getId(), 2), + new CupRankDto(thirdCup.getId(), 1) +); UpdateCupRanksRequest request = new UpdateCupRanksRequest(cupRanks);backend/src/test/java/backend/mulkkam/friend/controller/FriendControllerTest.java (1)
16-49:FriendControllerTest에서 두 번째 파라미터로nickname대신deviceUuid를 전달해야 합니다
OauthJwtTokenHandler.createAccessToken(OauthAccount account, String deviceUuid)메서드 시그니처를 보면 두 번째 파라미터가 명확하게 **deviceUuid**입니다. 이 값은generateClaims()내에서 JWT claim의"deviceUuid"키에 저장되고, 나중에getDeviceUuid(token)으로 추출할 때 null 또는 빈 문자열이면InvalidTokenException을 발생시킵니다.
FriendControllerTest에서는nickname변수를 전달하고 있어서 JWT의 deviceUuid 필드에 멤버 닉네임이 저장됩니다. 이는:
- 다른 모든 테스트(AuthenticationTest, CupControllerTest, IntakeAmountControllerTest 등)에서는
"deviceUuid"또는 deviceUuid 변수를 사용하는 것과 불일치- 만약 테스트에서
getDeviceUuid(token)을 호출한다면 예상과 다른 값을 받거나, 나중에 실제 디바이스 UUID 검증 로직이 추가될 때 이 테스트만 실패할 가능성두 번째 인자를
"deviceUuid"문자열 또는 테스트용 UUID 상수로 변경하세요. 예:String token = oauthJwtTokenHandler.createAccessToken(oauthAccount, "deviceUuid");
🤖 Fix all issues with AI agents
In @backend/src/test/java/backend/mulkkam/common/auth/AuthenticationTest.java:
- Around line 93-106: The test success_request_reaches_controller_without_auth
in the AuthenticationTest.NoAuthenticationRequired nested class relies on
/auth/kakao returning 5xx to infer the auth filter passed, which is fragile;
change the assertion to explicitly verify the response is not an authentication
failure (e.g., assert status is not 401 or 403) or, preferably, stub the
external Kakao call in the test profile (using WireMock/MockRestServiceServer)
so /auth/kakao returns a stable 2xx/4xx and then assert the controller was
reached; update the test method name and expectations accordingly to reflect the
chosen approach.
In @backend/src/test/java/backend/mulkkam/cup/controller/CupControllerTest.java:
- Around line 60-81: The test is unstable because
saveDefaultCupEmojisAndGetFirst() returns
cupEmojiRepository.findAll().getFirst() without a deterministic order; change
saveDefaultCupEmojisAndGetFirst() to capture and return the first CupEmoji
instance you create in the loop (e.g., assign the first created CupEmoji to a
local variable inside the for loop and return it after saving) so the returned
value is the exact one you saved; alternatively, if you prefer queries, replace
the final call with an explicit ordered query (e.g., findAll(Sort.by("id")) or a
repository method that returns the first by id) but prefer the simple
captured-first approach.
In @backend/src/test/java/backend/mulkkam/cup/domain/vo/CupNicknameTest.java:
- Around line 16-23: The parameterized test name shows the raw string value
instead of the intended length; update the test in CupNicknameTest so it accepts
lengths (use @ValueSource(ints = {...}) and change the method signature to
nickname_with_valid_length_is_created(int length)), build the nickname string
inside the test via "a".repeat(length) (or similar) and then assert new
CupNickname(nickname) does not throw; also update the @ParameterizedTest name
pattern to use {0} as the length if desired (e.g., "닉네임 길이 = {0}글자").
In
@backend/src/test/java/backend/mulkkam/friend/service/FriendRequestServiceTest.java:
- Around line 250-272: The test method name
error_cannotProcessAlreadyProcessedRequest in FriendRequestServiceTest deviates
from the file's naming convention (other tests use success_... / fail_...
snake_case); rename the method to follow the pattern (e.g.,
fail_cannot_accept_already_processed_request) and update any references to that
method (test runner will pick up the new name), leaving the test body,
DisplayName, and assertions unchanged.
- Around line 57-160: Tests and domain currently delete rejected friend
requests; change to a REJECTED status transition for extensibility. Add REJECTED
to FriendRelationStatus, stop calling friendCommandService.deleteFriendRequest()
from friendRequestService.modifyFriendStatus when
PatchFriendStatusRequest.Status.REJECTED; instead add or rename a command (e.g.,
friendCommandService.rejectFriendRequest or updateFriendRequestStatus) that
updates the FriendRelation.entity.status to REJECTED and persists it. Update any
repository queries/filters that assumed deletion (e.g., methods used by
friendQueryRepository or findBy* usages) to include or exclude REJECTED as
appropriate, and change the test success_friend_request_is_rejected() to assert
the relation remains with status REJECTED (and adjust assertions in related
tests that expected deletion or NOT_FOUND accordingly).
In
@backend/src/test/java/backend/mulkkam/intake/domain/vo/AchievementRateTest.java:
- Around line 33-49: The test achievement_rate_is_capped_at_100_percent
currently takes an unused actualPercentage parameter (from CsvSource and the
parameter list) which risks misleading test names; remove actualPercentage from
the CsvSource rows, the method signature, and the @ParameterizedTest name
template so the test only accepts (int intake, int target) and continues to
construct TargetAmount and AchievementRate and assert result.value() == 100.0;
alternatively, if you want to keep it for display, add an assertion comparing
actualPercentage to the computed percentage before capping, but the preferred
fix is to delete the unused actualPercentage to keep the test declaration and
display consistent.
In
@backend/src/test/java/backend/mulkkam/intake/domain/vo/TargetAmountTest.java:
- Around line 1-12: Move the test file from backend.mulkkam.intake.domain.vo to
backend.mulkkam.member.domain.vo by updating the package declaration to
backend.mulkkam.member.domain.vo and relocating the file so it sits alongside
other member-domain tests; inside TargetAmountTest add @Nested inner classes to
group related scenarios (e.g., ValidCases and InvalidCases) and add a "given"
comment before existing "when" & "then" comments to follow BDD naming
consistency; ensure imports remain correct for
backend.mulkkam.member.domain.vo.TargetAmount and update any package references
in the test suite or build config if necessary.
In
@backend/src/test/java/backend/mulkkam/intake/service/IntakeTargetAmountServiceTest.java:
- Line 79: The @DisplayName text in IntakeTargetAmountServiceTest is unclear and
contains a typo; update the annotation value to reflect the actual test intent
by changing the string from "음용량이 음용인 경우 예외가 발생한다" to "음용량이 음수인 경우 예외가 발생한다" on
the test method annotated with @DisplayName in the IntakeTargetAmountServiceTest
class.
🧹 Nitpick comments (37)
backend/src/test/java/backend/mulkkam/cup/domain/vo/CupTargetAmountTest.java (1)
16-23: BDD 주석 일관성 개선 필요현재 이 테스트는
// when & then을 결합해서 사용하고 있는데, 아래 다른 테스트들(lines 28-34, 40-46)은// when과// then을 분리하고 있어요. 팀 내 일관성을 위해 동일한 스타일로 통일하는 것이 좋겠습니다.또한, 예외가 발생하지 않는 경우를 검증하는 현재 방식(
assertThatCode().doesNotThrowAnyException())도 유효하지만, 더 간결한 대안도 고려해볼 수 있어요:대안 1 (권장): BDD 주석을 분리하고 assertThatCode 유지
@DisplayName("용량이 1~2000ml 사이이면 생성에 성공한다") @ParameterizedTest(name = "용량 = {0}ml") @ValueSource(ints = {1, 500, 1_000, 1_500, 2_000}) void amount_within_valid_range_is_created(int value) { // when & then assertThatCode(() -> new CupAmount(value)) .doesNotThrowAnyException(); }
- 장점: 명시적으로 "예외가 발생하지 않음"을 검증
- 단점: 약간의 보일러플레이트 코드
대안 2: 단순 생성으로 간소화
@DisplayName("용량이 1~2000ml 사이이면 생성에 성공한다") @ParameterizedTest(name = "용량 = {0}ml") @ValueSource(ints = {1, 500, 1_000, 1_500, 2_000}) void amount_within_valid_range_is_created(int value) { // when CupAmount cupAmount = new CupAmount(value); // then assertThat(cupAmount).isNotNull(); }
- 장점: 더 간결하고 직관적, 다른 테스트와 BDD 주석 패턴 일치
- 단점: "예외 미발생" 의도가 덜 명확
개인적으로는 대안 2가 Value Object의 단순한 생성 검증에는 더 적합해 보이며, 다른 테스트와의 일관성도 확보할 수 있습니다.
backend/src/test/java/backend/mulkkam/notification/service/SuggestionNotificationServiceTest.java (2)
70-71: Optional.get() 직접 호출이 테스트 가독성을 저하시킬 수 있습니다.
.get()을 직접 호출하면 데이터가 없을 때NoSuchElementException이 발생하는데, 이는 테스트 실패 원인을 파악하기 어렵게 만듭니다.대안:
orElseThrow()사용: 명시적인 예외 메시지로 실패 원인을 명확히 합니다assertThat().isPresent()체크: Optional 존재 여부를 먼저 검증합니다♻️ 명시적 예외 처리로 개선
- SuggestionNotification actual = suggestionNotificationRepository.findById( - savedSuggestionNotification.getId()).get(); + SuggestionNotification actual = suggestionNotificationRepository.findById( + savedSuggestionNotification.getId()) + .orElseThrow(() -> new AssertionError("저장된 제안 알림을 찾을 수 없습니다"));
49-94: 테스트 커버리지를 보강할 필요가 있습니다.현재 성공 케이스와 "존재하지 않는 ID" 에러 케이스만 다루고 있어, 중요한 엣지 케이스들이 누락되었습니다.
누락된 테스트 케이스:
- 중복 적용 시나리오: 이미 적용된 제안 알림에 다시 적용 요청 시 동작 (idempotency 보장 여부)
- 권한 검증: 다른 회원의 제안 알림에 적용 요청 시 (authorization)
- 경계값 테스트: 추천 목표량이 0, 음수, 또는 비정상적으로 큰 값일 때
- Member 엔티티 검증 누락: 현재는
IntakeHistory만 확인하는데,Member엔티티의targetAmount도 올바르게 업데이트되는지 검증 필요- 알림 읽음 상태 변경: 적용 시 알림이 읽음 처리되는지 확인
제안하는 테스트 구조:
@Nested class ApplyTargetAmount { // 기존 테스트들... @Test @DisplayName("이미 적용된 제안 알림에 재적용 요청 시 예외가 발생한다") void error_byAlreadyApplied() { } @Test @DisplayName("다른 회원의 제안 알림에 적용 요청 시 예외가 발생한다") void error_byUnauthorizedMember() { } @Test @DisplayName("적용 시 Member 엔티티의 targetAmount도 업데이트된다") void success_updatesMemberTargetAmount() { } }As per coding guidelines, 서비스 레이어 테스트에서는 비즈니스 로직의 다양한 시나리오를 검증하는 단위 테스트가 필요합니다.
backend/src/test/java/backend/mulkkam/intake/controller/IntakeHistoryControllerTest.java (3)
61-61: 병렬 테스트 실행 시 카운터 격리 검토가 필요합니다.
AtomicLong을 인스턴스 필드로 선언하면 각 테스트 클래스 인스턴스마다 새로운 카운터가 생성됩니다. JUnit 5의 기본 동작은 테스트 메서드마다 새 인스턴스를 생성하므로 일반적으로 문제가 없지만,@TestInstance(Lifecycle.PER_CLASS)를 사용하거나 향후 병렬 실행을 도입할 경우 테스트 간 순서에 의존성이 생길 수 있습니다.대안:
- 현재 방식 유지 + 명시적 문서화: JUnit의 기본 인스턴스 생성 정책에 의존하며, 주석으로 이를 명시
- UUID 또는 랜덤 생성:
UUID.randomUUID().toString()을 사용해 순서 의존성 완전 제거- 정적 필드로 변경:
static AtomicLong으로 선언해 클래스 레벨에서 관리 (단, 병렬 실행 시 주의)현재 구조에서는 각 테스트가 고유한 ID를 받으므로 기능상 문제는 없으나, 테스트 실행 순서에 대한 가정이 숨어있다는 점을 고려하세요.
</invoke_comment_end>
65-86: 테스트 데이터 생성 로직을 필요에 따라 최적화할 수 있습니다.현재
createTestContext()메서드는 모든 테스트에 대해 Member, OauthAccount, CupEmoji, Cup을 항상 생성합니다. 그런데 실제 테스트 코드를 보면 일부 테스트는ctx.cup()을 사용하지 않는 것으로 보입니다. 모든 테스트에서 Cup 관련 엔티티가 필요한 게 아니라면 불필요한 DB 호출이 발생해 테스트 실행 속도가 느려질 수 있습니다.추가 고려사항:
- Line 75의
deviceUuid가"deviceUuid"로 하드코딩되어 있는데, 테스트 간 토큰 충돌이나 현실성 있는 시나리오 재현을 위해 동적 생성을 고려해볼 수 있습니다.개선 방향:
- 오버로딩:
createTestContext()와createTestContextWithCup()으로 분리- 빌더 패턴: TestContext 자체를 빌더로 구성해 선택적 엔티티 생성
- 현재 방식 유지: 간결성을 위해 모든 엔티티를 생성하되, 성능 이슈가 실제로 발생할 때 최적화
현재 테스트가 2개뿐이라면 단순함을 유지하는 게 합리적일 수 있으나, 향후 테스트가 늘어날 경우 선택적 생성 구조를 검토하세요.
</invoke_comment_end>
94-94: 테스트 메서드 명명 규칙 확인이 필요합니다.코딩 가이드라인에 따르면
@DisplayName은methodName_successOrError_condition형식을 따라야 합니다. 현재 메서드명은success_returns_empty_details_when_no_records형태로 작성되어 있는데, 이는returns...처럼 동사로 시작하는 설명형 구조입니다.예상되는 형식 예시:
readSummaryOfIntakeHistories_success_whenNoRecordsreadSummaryOfIntakeHistories_success_whenTodayWithNoRecords하지만 현재 네이밍도 충분히 의미가 명확하고 팀 내에서 일관되게 사용 중이라면 큰 문제는 아닙니다. 다만 팀 전체 코드베이스에서 명명 규칙을 통일하는 것이 코드 탐색과 유지보수에 유리합니다.
확인 필요:
다른 테스트 파일들과 명명 규칙이 일치하는지 점검하고, 팀 내 컨벤션을 명확히 정의하는 것을 권장합니다.As per coding guidelines, the DisplayName format should be
methodName_successOrError_condition.</invoke_comment_end>
Also applies to: 122-122
backend/src/test/java/backend/mulkkam/intake/domain/vo/AchievementRateTest.java (3)
11-16:@DisplayName컨벤션(형식) 불일치 가능성가이드에 “
methodName_successOrError_condition” 형식을 요구하는데, 현재는 자연어 문장 위주라 팀 내 검색/일관성이 깨질 수 있어요.선택지:
- (권장) 가이드 형식으로 통일: 테스트 목록이 “성공/실패/조건”으로 정렬돼 리팩토링 때 탐색이 쉬움
- (대안) 현재 문장 유지 + 형식 접두어만 추가: 가독성은 유지하되 컨벤션도 어느 정도 충족
As per coding guidelines, ...
Also applies to: 33-35
22-31: 중복 Given(TargetAmount생성) 정리로 테스트 의도 더 또렷하게두 테스트 모두
TargetAmount targetAmount = new TargetAmount(target);가 반복돼서, “무엇을 검증하는지” 대비 “준비 코드” 비중이 커져 보여요.선택지:
- (권장) private 헬퍼(또는 fixture)로 추출: 파라미터라이즈드 테스트에서도 깔끔하게 중복 제거
- (대안)
@BeforeEach는 이번 케이스에선target이 매번 달라 큰 이점이 적고, 오히려 헬퍼가 더 직관적일 수 있어요제안 diff
class AchievementRateTest { + private TargetAmount targetAmount(int target) { + return new TargetAmount(target); + } void achievement_rate_is_calculated_correctly(int intake, int target, double expectedRate) { // given - TargetAmount targetAmount = new TargetAmount(target); + TargetAmount targetAmount = targetAmount(target); ... void achievement_rate_is_capped_at_100_percent(int intake, int target, int actualPercentage) { // given - TargetAmount targetAmount = new TargetAmount(target); + TargetAmount targetAmount = targetAmount(target);Based on learnings, ... / As per coding guidelines, ...
Also applies to: 40-48
14-31: 허용 오차within(0.1)가 의도 대비 넓을 수 있음 (버그 은폐 가능)케이스가 전부 “딱 떨어지는 비율”이라면
within(0.1)은 살짝 과한 편이라, 계산/반올림 버그를 조용히 통과시킬 여지가 있어요.선택지:
- (권장) 도메인이 “정확히 몇 자리로 반올림”하는지에 맞춰 오차를 더 타이트하게(예:
within(0.001)) 또는isEqualTo(expectedRate)로 고정- (대안) 오차는 유지하되, 소수 케이스(예: 333/1000=33.3) 같은 “반올림 규칙”을 드러내는 CSV를 추가해 도메인 의도를 테스트로 문서화
backend/src/test/java/backend/mulkkam/intake/controller/IntakeAmountControllerTest.java (4)
41-63: TestContext 패턴이 여러 테스트 파일에서 중복되고 있습니다.
IntakeHistoryControllerTest,AuthenticationTest등 다른 테스트 파일에서도 유사한TestContext생성 패턴이 사용되고 있습니다. 이를 공통 유틸리티나ControllerTest부모 클래스로 추출하면 중복을 줄이고 유지보수성을 높일 수 있습니다.대안 1: ControllerTest에 protected 메서드로 추출
- 장점: 모든 컨트롤러 테스트에서 즉시 사용 가능, 일관성 보장
- 단점: ControllerTest의 책임 증가
대안 2: 별도의 TestContextFactory 유틸리티 클래스 생성
- 장점: SRP 원칙 준수, 테스트 외 다른 곳에서도 재사용 가능
- 단점: 추가 클래스 생성 필요
현재 리팩토링 PR의 범위를 고려하면, 추후 별도 이슈로 추적하는 것도 좋은 선택입니다.
69-71: @DisplayName 형식이 코딩 가이드라인과 일치하지 않습니다.가이드라인에 따르면
@DisplayName은methodName_successOrError_condition형식을 사용해야 합니다. 현재 한글 설명문으로 되어 있어 테스트 메서드와의 연관성이 떨어집니다.-@DisplayName("입력받은 몸무게가 있으면 해당 값 * 30을 추천한다") +@DisplayName("getRecommended_success_whenWeightIsProvided")다만, 팀 내에서 한글 설명문이 가독성 면에서 선호된다면 이 규칙을 재검토하는 것도 방법입니다. 중요한 건 팀 전체에서 일관된 스타일을 유지하는 것입니다.
87-89: 단일 assertion에assertSoftly사용은 불필요합니다.
assertSoftly는 여러 assertion이 있을 때 첫 번째 실패 후에도 나머지를 계속 실행하기 위한 용도입니다. 현재 assertion이 하나뿐이라 soft assertion의 이점이 없고, 오히려 불필요한 오버헤드가 됩니다.♻️ 간결한 assertion으로 변경
-assertSoftly(softly -> { - softly.assertThat(actual.amount()).isEqualTo(2100); -}); +assertThat(actual.amount()).isEqualTo(2100);
110-112: 단일 assertion에assertSoftly사용 - 위와 동일한 문제첫 번째 테스트와 마찬가지로 단일 assertion에
assertSoftly를 사용하고 있습니다. 간결하게assertThat으로 변경하는 것을 권장합니다.♻️ 간결한 assertion으로 변경
-assertSoftly(softly -> { - softly.assertThat(actual.amount()).isEqualTo(1800); -}); +assertThat(actual.amount()).isEqualTo(1800);
backend/src/test/java/backend/mulkkam/cup/domain/vo/CupNicknameTest.java (2)
13-14:@DisplayName컨벤션이 가이드(“methodName_successOrError_condition”)와 불일치할 수 있어요현재 클래스/메서드
@DisplayName이 서술형(한글)인데, 가이드에는methodName_successOrError_condition포맷을 요구하고 있어 컨벤션 충돌이 날 가능성이 있습니다. 팀에서 “서술형 DisplayName”으로 바꾼 게 아니라면, 이 PR에서 맞춰두는 편이 이후 리팩토링/검색성 측면에서 이득입니다. (가이드 기준)
- 선택지 A: 가이드대로
nickname_with_valid_length_is_created_success_validLength같은 규칙형으로 통일(일관성/검색성 ↑, 가독성은 취향)- 선택지 B: 현재처럼 서술형 유지하되,
backend/code-style.md가이드 자체를 업데이트(팀 합의가 있으면 가장 깔끔)
25-47: 실패 케이스가 거의 동일한데(중복) +@Nested로 시나리오를 묶으면 더 읽기 좋아져요Line 28-35, 40-47은 “값만 바뀌는 동일 패턴”이라, 길이 기반으로 파라미터화(+헬퍼)하면 중복이 크게 줄고 의도가 더 선명해집니다. (또한 가이드에
@Nested로 시나리오를 조직하라고 되어 있어, “생성/성공/실패”처럼 묶어두면 테스트 탐색성이 좋아요.)
- 선택지 A(추천):
@ValueSource(ints=...)+nicknameOfLength(int)헬퍼로 통일 (중복↓, 케이스 확장↑)- 선택지 B: 지금 형태 유지(명시적이긴 함)하되, 공통 검증 로직만
assertInvalidNickname(value)로 추출 (변경 최소)추가로, **CupNickname이 길이 외에도 문자 제약(공백/특수문자 등)**을 가진다면 그 규칙도 함께 커버하는 케이스가 필요합니다(현재는 길이만 검증). CupNickname의 실제 검증 규칙을 한 번 확인해 주세요.
제안 diff (선택지 A: 길이 기반 파라미터화 + 중복 제거)
@DisplayName("닉네임이 2글자 미만이면 생성에 실패한다") -@ParameterizedTest(name = "닉네임 = \"{0}\"") -@ValueSource(strings = {"", "1"}) -void nickname_shorter_than_min_length_is_invalid(String value) { +@ParameterizedTest(name = "닉네임 길이 = {0}글자") +@ValueSource(ints = {0, 1}) +void nickname_shorter_than_min_length_is_invalid(int length) { + String value = nicknameOfLength(length); // when CommonException ex = assertThrows(CommonException.class, () -> new CupNickname(value)); // then assertThat(ex.getErrorCode()).isEqualTo(INVALID_CUP_NICKNAME); } @DisplayName("닉네임이 10글자 초과이면 생성에 실패한다") -@ParameterizedTest(name = "닉네임 = \"{0}\"") -@ValueSource(strings = {"12345678901", "123456789012"}) -void nickname_longer_than_max_length_is_invalid(String value) { +@ParameterizedTest(name = "닉네임 길이 = {0}글자") +@ValueSource(ints = {11, 12}) +void nickname_longer_than_max_length_is_invalid(int length) { + String value = nicknameOfLength(length); // when CommonException ex = assertThrows(CommonException.class, () -> new CupNickname(value)); // then assertThat(ex.getErrorCode()).isEqualTo(INVALID_CUP_NICKNAME); } + +private String nicknameOfLength(int length) { + return "a".repeat(length); +}(장기적으로는
MIN_LENGTH,MAX_LENGTH같은 상수를 테스트/도메인에서 공유 가능하면 “매직 넘버(2,10)”도 같이 줄일 수 있습니다.)
backend/src/test/java/backend/mulkkam/intake/service/IntakeHistoryCrudServiceTest.java (1)
51-76: 테스트 로직은 정확하나, 엣지 케이스 커버리지 추가를 고려해보세요.테스트 메서드명이 더 구체적으로 변경되었고(
success_calculates_achievement_rate_from_history), BDD 패턴을 명확히 따르고 있으며, 부동소수점 비교에isCloseTo를 사용한 점이 좋습니다. 계산 로직도 정확합니다 (400ml / 1000ml = 40%).현재 테스트가 검증하는 것:
- 정상적인 섭취 기록이 있을 때 달성률이 올바르게 계산되는지
추가로 고려할 테스트 케이스:
서비스의 견고성을 보장하기 위해 다음과 같은 경계 조건 및 예외 상황 테스트를 추가하면 좋습니다:
섭취 기록 상세가 없는 경우
@Test void success_returns_zero_when_no_intake_details() { // IntakeHistoryDetail이 없을 때 0% 반환 확인 }목표량이 0인 경우
@Test void error_handles_zero_target_amount() { // targetAmount가 0일 때 ZeroDivisionException 방지 확인 }목표량 초과 달성 케이스
@Test void success_calculates_over_100_percent_achievement() { // 섭취량이 목표량을 초과할 때 (예: 150%) 정상 처리 확인 }섭취량이 0인 경우
@Test void success_returns_zero_when_intake_is_zero() { // 섭취 기록은 있지만 모든 섭취량이 0일 때 }이러한 테스트들이 실제로 필요한지는
getAchievementRate메서드의 구현 로직과 도메인 요구사항에 따라 달라집니다. 메서드가 이미 안전하게 처리하고 있다면 필수는 아니지만, 명시적인 테스트가 있으면 향후 리팩토링 시 안전성을 보장할 수 있습니다.Based on coding guidelines - instruction #5 about suggesting missing test cases.
backend/src/test/java/backend/mulkkam/notification/service/NotificationServiceTest.java (5)
56-79: 멤버를 테스트마다 로컬 생성하도록 바꾼 건 👍, 다만 헬퍼 네이밍/중복 제거는 더 갈 수 있어요.
createAndSaveMember()로 “공유 savedMember” 의존성을 제거한 건 격리 관점에서 좋아요.createUnReadNotifications는 네이밍이 어색합니다(일반적으로Unread). 가능하면createUnreadNotifications로 맞추는 편이 읽기 좋아요.- 같은
@Nested안에서Member savedMember = createAndSaveMember();가 반복되므로, 각 Nested 클래스에@BeforeEach로 “테스트당 1회” 생성하면 가독성이 더 좋아질 듯해요(격리도 유지됨). (Based on learnings, ...)
90-169: 커서(10L)·nextCursor(5) 하드코딩은 DB/시퀀스 정책에 따라 깨지기 쉬워요.이 테스트는 “조회 후 DB에서 읽음 처리된다”까지 같이 검증해서 의미가 큰데,
ReadNotificationsRequest(10L, ...)및result.nextCursor() == 5같은 값은 ID가 예측 가능하게 발급된다는 전제라 리팩토링/DB 청소 방식 변경 시 흔들릴 수 있어요.
대안:
- (추천) 저장 직후 실제 ID를 얻어서
lastId/expected cursor를 구성(장점: 안정적 / 단점: 준비 코드 약간 증가)- 또는
DatabaseCleaner가 시퀀스를 항상 리셋함을 보장(장점: 간단 / 단점: 인프라 정책 결합)
171-216: 다른 케이스들도 lastId=6L 같은 하드코딩이 동일한 리스크를 가져요.
success_returns_exact_size_when_data_equals_request_size,success_returns_less_when_available_is_less_than_size,success_next_cursor_is_null_when_no_more_data모두 cursor를 상수로 박고 있는데, 시퀀스 리셋이 보장되지 않으면 한 번에 깨질 수 있습니다. 위 테스트들과 같은 방식으로 “저장된 엔티티에서 ID를 얻어 cursor를 구성”하는 형태로 통일하는 걸 권장해요.Also applies to: 258-279
281-293: 예외 검증 스타일이 PR 목표(assertThrows 통일)와 어긋나요.여기서는
assertThatThrownBy를 계속 쓰고 있는데, PR 요약/목표가 “assertThrows+assertThat로 통일”이라면 이 파일도 맞추는 게 좋아요(스타일 혼재는 유지보수 비용이 큼).
가능하면 메시지 문자열 비교 대신,CommonException의 에러코드(필드/메서드)가 있다면 그쪽을 직접 검증하는 게 더 견고합니다.Also applies to: 411-422
300-357: setupNotificationsForCount 도입은 👍, 다만 “공통 시드”는 @beforeeach로 올리면 더 자연스러워요.지금처럼 helper로 숨기는 것도 방법이지만, 이 Nested는 사실상 “항상 같은 공통 시드 + 추가 데이터” 흐름이라
@BeforeEach로 공통 시드를 올려두면 given/when/then이 더 또렷해질 것 같아요. (Based on learnings, ...)
backend/src/test/java/backend/mulkkam/intake/domain/vo/TargetAmountTest.java (3)
14-24: 유효 범위 생성 테스트는 깔끔합니다. 다만 “경계 바로 옆 값”을 한두 개 더 넣으면 더 단단해져요.지금도 최소/최대(200/5000)를 포함해서 핵심은 커버되고 있어요. 여기에 201, 4_999 같이 “경계 바로 옆”을 추가하면, 실수로
>=/>조건이 바뀌는 류의 회귀를 더 잘 잡습니다.예시 (경계 인접 값 추가)
@ValueSource(ints = {200, 1_000, 2_000, 5_000}) +// @ValueSource(ints = {200, 201, 1_000, 2_000, 4_999, 5_000})
26-48: 실패 케이스 2개가 거의 동일해서 “헬퍼 메서드 1개”로 중복 제거 여지가 있어요.두 테스트 모두
assertThrows(CommonException.class, () -> new TargetAmount(value))후errorCode를 비교합니다. 현재도 읽히긴 하지만, 실패 패턴이 늘어날수록 복붙이 누적되기 쉬워요.
- 선택지 A) private 헬퍼로 공통화 (추천)
- 장점: 중복 제거, 실패 케이스 추가 시 더 빠름
- 단점: 테스트 파일이 아주 작을 땐 오히려 추상화로 느껴질 수 있음
- 선택지 B) 현상 유지
- 장점: 즉시 읽기 쉬움
- 단점: 케이스 증가 시 중복 관리 비용 증가
예시 (헬퍼 메서드로 공통화)
class TargetAmountTest { + private void assertInvalidTargetAmount(int value) { + CommonException ex = assertThrows(CommonException.class, () -> new TargetAmount(value)); + assertThat(ex.getErrorCode()).isEqualTo(INVALID_TARGET_AMOUNT); + } void target_amount_less_than_min_is_invalid(int value) { - CommonException ex = assertThrows(CommonException.class, - () -> new TargetAmount(value)); - assertThat(ex.getErrorCode()).isEqualTo(INVALID_TARGET_AMOUNT); + assertInvalidTargetAmount(value); } void target_amount_greater_than_max_is_invalid(int value) { - CommonException ex = assertThrows(CommonException.class, - () -> new TargetAmount(value)); - assertThat(ex.getErrorCode()).isEqualTo(INVALID_TARGET_AMOUNT); + assertInvalidTargetAmount(value); } }Based on learnings, 중복 셋업/검증이 보이면 이런 식으로 공통화를 제안드리는 편입니다.
14-48:@DisplayName포맷 가이드와 현재 스타일이 충돌합니다 — 팀 기준을 “둘 중 하나”로 확정하는 게 좋아요.가이드에는
methodName_successOrError_condition포맷을 요구하고 있는데, 이 파일(그리고 제공된CupTargetAmountTest)은 한국어 문장형@DisplayName을 사용하고 있어요. 둘 다 장단점이 있어서, 이번 PR에서의 잘잘못이라기보다 “팀 기준 정리” 이슈로 보입니다.
- 한국어 문장형 유지: 읽기 친절(테스트 리포트 가독성↑), 다만 포맷 강제는 어려움
- 포맷 강제형 전환: 일관성/검색성↑, 다만 리포트가 기계적으로 보일 수 있음
As per coding guidelines, 현 가이드 문구를 최신 테스트 스타일과 맞춰 업데이트할지(또는 반대로 테스트들을 가이드에 맞출지) 한 번 합의가 필요해 보여요.
backend/src/test/java/backend/mulkkam/support/service/ServiceTest.java (1)
16-24: 데이터베이스 정리가 @beforeeach와 @AfterEach 양쪽에서 중복 실행됩니다.현재
databaseCleaner.clean()이 테스트 전후로 모두 호출되고 있습니다.현재 구조의 장점:
- 최대한의 테스트 격리를 보장 (이전 테스트의 잔여 데이터 제거 + 현재 테스트의 데이터 정리)
- 테스트 실행 순서에 무관하게 안정적
잠재적 문제:
- 매 테스트마다 정리 작업이 2번 수행되어 테스트 실행 시간이 증가할 수 있습니다
- 일반적으로는
@AfterEach만으로도 충분한 경우가 많습니다제안:
테스트 스위트의 안정성과 실행 속도 간 트레이드오프를 고려하여, 실제로 양쪽 정리가 모두 필요한지 검증해보시는 것을 권장합니다. 만약@AfterEach만으로 충분하다면@BeforeEach의 cleanup은 제거를 고려해볼 수 있습니다.
backend/src/test/java/backend/mulkkam/member/service/MemberServiceTest.java (1)
64-66: 상수 명명 규칙 확인 필요
defaultEmojiUrl은private static final상수이므로 코딩 가이드라인에 따라 대문자와 언더스코어를 사용한DEFAULT_EMOJI_URL형식으로 변경하는 것이 좋습니다. 일관된 컨벤션을 유지하면 코드 가독성이 향상됩니다.♻️ 제안된 수정
class MemberServiceTest extends ServiceTest { - private static final String defaultEmojiUrl = "url"; + private static final String DEFAULT_EMOJI_URL = "url";
backend/src/test/java/backend/mulkkam/intake/service/IntakeHistoryServiceTest.java (2)
58-60: 상수 명명 규칙 수정 권장
defaultEmojiUrl은private static final상수이므로DEFAULT_EMOJI_URL로 변경하는 것이 팀 컨벤션에 부합합니다.
101-109: 비효율적인 중복 체크 로직 개선 필요
saveDefaultCupEmojisIfNeeded()메서드에서 모든CupEmoji를 조회한 후 스트림으로 필터링하는 방식은 데이터가 많아질 경우 비효율적입니다.문제점:
findAll()로 전체 엔티티를 메모리에 로드- URL 비교를 위해
value()메서드 호출 시 NPE 가능성 (Line 104에서e.getUrl().value())대안:
- Repository에
existsByUrl(String url)같은 쿼리 메서드 추가- 또는 테스트 격리가 보장된다면 조건부 저장 자체를 제거하고 항상 저장
현재 테스트는
DatabaseCleaner로 격리되므로, 조건 체크 없이 항상 저장하는 방식이 더 단순할 수 있습니다.♻️ 단순화 제안
- private void saveDefaultCupEmojisIfNeeded() { - // 이미 기본 이모지가 저장되어 있으면 스킵 - long defaultEmojiCount = cupEmojiRepository.findAll().stream() - .filter(e -> defaultEmojiUrl.equals(e.getUrl().value())) - .count(); - if (defaultEmojiCount == 0) { - saveDefaultCupEmojis(); - } - } + // DatabaseCleaner로 테스트 격리가 보장되므로 조건 체크 불필요 + // saveDefaultCupEmojis()를 직접 호출
backend/src/test/java/backend/mulkkam/cup/domain/IntakeTypeTest.java (1)
57-79: 수분량 계산 테스트 경계 케이스 추가 고려WATER와 COFFEE에 대한 수분 환산 비율 테스트가 잘 구성되어 있습니다.
추가로 고려할 수 있는 케이스:
- 0ml 입력 시 동작
- 향후 새로운 IntakeType 추가 시 테스트 누락 방지를 위한
values()순회 테스트이는 선택적 개선 사항입니다.
backend/src/test/java/backend/mulkkam/device/service/DeviceServiceTest.java (1)
45-64: 테스트 커버리지 확장 고려현재 FCM 토큰 삭제에 대해 성공 케이스만 테스트하고 있습니다. 코딩 가이드라인에서 "미작성한 테스트 코드 케이스가 있다면 제안"하라고 명시되어 있으므로, 다음 케이스들을 고려해 볼 수 있습니다:
- 존재하지 않는 Device에 대한 삭제 요청 - 예외가 발생하는지, 아니면 무시되는지
- 다른 멤버의 Device 삭제 시도 - 권한 검증 테스트
- 동일 멤버의 여러 디바이스 중 특정 디바이스만 삭제 - 다른 디바이스 영향 없음 확인
이는 서비스 로직의 안정성을 보장하기 위한 선택적 개선입니다.
backend/src/test/java/backend/mulkkam/auth/service/KakaoAuthServiceTest.java (1)
25-25: 클래스 가시성 일관성 검토
KakaoAuthServiceTest가public으로 선언되어 있는데, 다른 테스트 클래스들(MemberServiceTest,IntakeHistoryServiceTest등)은 package-private입니다.JUnit 5에서는 테스트 클래스가 package-private이어도 실행되므로, 일관성을 위해
public키워드를 제거하는 것을 권장합니다.♻️ 제안된 수정
-public class KakaoAuthServiceTest extends ServiceTest { +class KakaoAuthServiceTest extends ServiceTest {
backend/src/test/java/backend/mulkkam/friend/controller/FriendControllerTest.java (1)
34-49: 컨트롤러 테스트의 인증 토큰 생성 로직이 여러 파일에 중복돼 유지보수 포인트가 늘어났어요
AuthenticatedMember+createAuthenticatedMember(...)패턴이CupControllerTest등에도 등장하는데, 토큰 규격/클레임이 바뀌면 컨트롤러 테스트들이 연쇄적으로 깨질 가능성이 큽니다.
- 선택지 A:
backend.mulkkam.support하위에 공용 테스트 유틸(예:TestAuthTokenFactory)로 추출 → 변경 지점 1곳 (장점: 일관성/유지보수, 단점: 유틸이 비대해질 수 있음)- 선택지 B:
ControllerTest베이스에 protected 헬퍼로 제공 → 컨트롤러 테스트에만 스코프 제한 (장점: 응집도, 단점: 베이스 클래스 책임 증가)Also applies to: 64-65
backend/src/test/java/backend/mulkkam/common/auth/AuthenticationTest.java (1)
33-46: 테스트 데이터(oauthId)가 고정값이라 유니크 제약/병렬 실행에서 깨질 소지가 있어요
createMemberToken()/createAccountOnlyToken()가"oauthId","oauthIdWithoutMember"고정값을 쓰는데, DB 클리너가 시퀀스/데이터를 매 테스트마다 확실히 지우지 않거나 병렬 실행되면 유니크 제약에 걸릴 수 있습니다.
- 선택지 A:
AtomicLong/UUID로 매번 고유값 생성 (가장 안전)- 선택지 B: 테스트 시작마다 관련 테이블 truncate 보장 (인프라 의존)
backend/src/test/java/backend/mulkkam/cup/service/CupServiceTest.java (1)
122-151: “마지막 랭크로 저장” 테스트가 현재는 효과가 약해요 (사전 max가 없음)
success_cup_is_created_at_last_rank()는 기존 컵이 없어서, “마지막 랭크 계산”이 맞는지 검증이 거의 안 됩니다(어차피 최대 랭크 = 방금 만든 컵).
- 선택지 A: 기존 컵 1~2개를 먼저 저장하고, 생성된 랭크가
기존 max + 1인지 확인 (의도 명확, 셋업 조금 증가)- 선택지 B: “빈 상태면 1랭크” 같은 명세라면 기대값을
1로 단정해서 더 단순하게 (명세 의존)
backend/src/test/java/backend/mulkkam/friend/service/FriendServiceTest.java (2)
41-51:Member[]로 페어를 반환하면 인덱스 실수/가독성 비용이 커요
pair[0],pair[1]는 테스트가 커질수록 실수하기 쉽고, “누가 요청자/수락자인지” 매번 역추적해야 합니다.
- 선택지 A(추천):
record FriendPair(Member requester, Member addressee)로 교체 → 의미가 코드에 남음(장점: 가독성/안전, 단점: 테스트 코드 타입 1개 추가)- 선택지 B: 메서드에서 두 멤버를 필드로 세팅(@beforeeach) → 호출부는 단순해지지만 공유 상태가 생김(장점: 테스트 본문 간결, 단점: 상태 추적 필요)
(As per coding guidelines, 테스트 시나리오가 늘어날수록 명명된 구조가 유리합니다.)Also applies to: 65-99
47-49: OAuth ID가 고정 문자열이라 유니크 제약이 있으면 테스트가 깨질 수 있어요
testIdOfRequester,testIdOfAddressee가 고정이라, DB 정리가 완벽하지 않거나 같은 헬퍼를 여러 번 호출하면 충돌 가능성이 있습니다.AtomicLong/UUID로 고유값을 만들면 더 안전합니다.
backend/src/test/java/backend/mulkkam/cup/controller/CupControllerTest.java (1)
73-79:DefaultCup.of(intakeType)의 호출 의도가 테스트만 봐선 안 드러나요
반환값을 사용하지 않아 “사이드이펙트 의존”처럼 보일 수 있습니다.
- 선택지 A: 정말 사이드이펙트(예: enum/정적 초기화) 목적이면 주석으로 목적을 명시
- 선택지 B: 불필요 호출이면 제거해서 테스트를 데이터 준비 로직에만 집중
backend/src/test/java/backend/mulkkam/common/auth/AuthenticationTest.java
Outdated
Show resolved
Hide resolved
backend/src/test/java/backend/mulkkam/cup/controller/CupControllerTest.java
Show resolved
Hide resolved
backend/src/test/java/backend/mulkkam/cup/domain/vo/CupNicknameTest.java
Show resolved
Hide resolved
backend/src/test/java/backend/mulkkam/friend/service/FriendRequestServiceTest.java
Show resolved
Hide resolved
backend/src/test/java/backend/mulkkam/friend/service/FriendRequestServiceTest.java
Show resolved
Hide resolved
backend/src/test/java/backend/mulkkam/intake/domain/vo/AchievementRateTest.java
Show resolved
Hide resolved
backend/src/test/java/backend/mulkkam/intake/domain/vo/TargetAmountTest.java
Show resolved
Hide resolved
backend/src/test/java/backend/mulkkam/intake/service/IntakeTargetAmountServiceTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
backend/src/test/java/backend/mulkkam/member/service/MemberServiceTest.java (2)
65-104:IntakeHistoryDetailRepository주입 필드가 2개라서 테스트 유지보수 시 실수 유발 가능성이 있어요
private IntakeHistoryDetailRepository intakeDetailRepository;와private IntakeHistoryDetailRepository intakeHistoryDetailRepository;가 공존해서, 저장/조회 시 어떤 빈을 쓰는지 헷갈리기 쉽습니다(실제로 둘 다 사용 중). 하나로 통일하고 이름도 역할에 맞게 맞추는 게 안전합니다.제안 diff(예시: 하나로 통일)
@Autowired private IntakeHistoryDetailRepository intakeDetailRepository; - @Autowired - private IntakeHistoryDetailRepository intakeHistoryDetailRepository;그리고 아래 사용처(예: Line 470, 495 등)는
intakeDetailRepository로 통일해 주세요.
108-317: 공유 엔티티 필드(cupEmoji)는 테스트 간 상태(영속성/ID) 누수로 플래키해질 여지가 있어요
private final CupEmoji cupEmoji = ...를 여러 테스트에서 재사용하면, JPA 관점에서 “같은 인스턴스”가 다른 테스트에서 다시save될 때(특히 DB 클리너가 TRUNCATE/DELETE를 해도) 영속성 컨텍스트/ID 세팅 여부에 따라 예상치 못한 merge/update 흐름이 섞일 수 있습니다.
테스트 격리/신뢰성을 위해 “테스트마다 새 인스턴스 생성” 쪽이 더 안전합니다.
- 선택지 A(권장): 헬퍼로 매번 새로 만들고 저장까지 책임지기 → 공유 상태 없이도 given 코드가 짧아짐
- 선택지 B:
@BeforeEach에서 “새 인스턴스 생성만” (저장은 각 테스트 given에서) → 가독성은 좋아지지만, 공용 필드가 생기므로 팀 가이드(공유 상태 최소화)와의 균형이 필요
(이건 예전에 요청주신 “BeforeEach로 가독성 개선” 니즈와도 맞는데, 엔티티 공유/재사용은 피하는 방향이 좋아요. Based on learnings, ...)선택지 A 예시 diff
- private final CupEmoji cupEmoji = new CupEmoji("http://example.com"); + private CupEmoji saveCupEmoji() { + CupEmoji cupEmoji = new CupEmoji("http://example.com"); + return cupEmojiRepository.save(cupEmoji); + } ... - cupEmojiRepository.save(cupEmoji); + CupEmoji cupEmoji = saveCupEmoji(); Cup cup = CupFixtureBuilder .withMemberAndCupEmoji(member, cupEmoji) .build();backend/src/test/java/backend/mulkkam/intake/controller/IntakeAmountControllerTest.java (2)
65-86: 엔드포인트와 파라미터 오류: 테스트가 파라미터를 받지 않는 엔드포인트를 호출하고 있습니다컨트롤러를 확인한 결과 두 가지 문제가 있습니다:
엔드포인트 오류 (라인 72):
/intake/amount/recommended는MemberDetails만 받으며 쿼리 파라미터(weight, GENDER)를 처리하지 않습니다. 대신 인증된 멤버의 DB에 저장된 신체 정보를 사용합니다. 입력 파라미터 기반 계산이 필요하면/intake/amount/target/recommended엔드포인트를 호출해야 합니다.파라미터 명명 오류 (라인 73-74):
PhysicalAttributesRequest의 필드명은gender(lowerCamelCase)이므로,.param("GENDER", "FEMALE")대신.param("gender", "FEMALE")로 수정해야 합니다.라인 95의 두 번째 테스트는 올바른 엔드포인트를 사용하지만, 역시 파라미터명을
gender로 수정해야 합니다.
88-109:weight파라미터 null 처리 방식을 더 명확히 정의하세요.테스트 의도와 구현을 보면:
엔드포인트가 member.weight를 사용하지 않음:
/intake/amount/target/recommended는 요청 파라미터의PhysicalAttributesRequest만 사용하므로,createAuthenticatedMember()에서weight=70.0으로 설정되는 것은 실제 로직에 영향을 주지 않습니다. 하지만 이는 테스트를 읽는 사람에게 혼동을 줄 수 있어요.
.param("weight", (String) null)의 문제: Spring MockMvc에 null을 전달하면 버전에 따라 파라미터를 제외하거나 예외를 일으킬 수 있습니다. 더 명시적인 방식을 권장합니다.권장 개선안:
createAuthenticatedMember(null, 1500)으로 변경해 member.weight도 명시적으로 null로 설정.param("weight", (String) null)라인 자체를 제거해 파라미터 미전달을 명확히 표현이렇게 하면 테스트 의도가 분명해집니다: "weight 파라미터를 제공하지 않으면(요청에 없으면) 기본값 60을 사용한다"
제안된 수정
void success_recommends_default_when_weight_is_null() throws Exception { // given - AuthenticatedMember auth = createAuthenticatedMember(); + AuthenticatedMember auth = createAuthenticatedMember(null, 1500); // when String json = mockMvc.perform(get("/intake/amount/target/recommended") - .param("weight", (String) null) .param("GENDER", "FEMALE") .header(HttpHeaders.AUTHORIZATION, "Bearer " + auth.token()))
🤖 Fix all issues with AI agents
In @backend/src/test/java/backend/mulkkam/cup/controller/CupControllerTest.java:
- Around line 73-84: The helper saveDefaultCupEmojisAndGetFirst currently
branches on cupEmojiRepository.findAll() which makes tests depend on DB state;
remove the conditional and always create the default emojis (loop over
IntakeType.values(), call DefaultCup.of(intakeType), new
CupEmoji(defaultEmojiUrl), setEmojiType(...), cupEmojiRepository.save(...)) and
then return cupEmojiRepository.findAll().getFirst(); alternatively, if you
prefer clearing state explicitly call cupEmojiRepository.deleteAll() at the
method start; ensure this method is used with the test transaction/rollback
setup so data is isolated per test.
In
@backend/src/test/java/backend/mulkkam/intake/controller/IntakeHistoryControllerTest.java:
- Line 81: Rename the test method
success_returns_empty_details_when_no_records() to follow the project convention
(methodName_successOrError_condition), e.g.,
readSummaryOfIntakeHistories_success_emptyDetailsWhenNoRecords(), and update any
other test methods with the same pattern (such as the one around line 109) to
the same naming scheme; ensure to update any references or annotations that
refer to the old method names (method declarations in
IntakeHistoryControllerTest and any IDE/test runner metadata) so tests continue
to run.
In
@backend/src/test/java/backend/mulkkam/intake/service/IntakeHistoryServiceTest.java:
- Around line 102-110: saveDefaultCupEmojisIfNeeded() currently checks count
then writes which can race under parallel tests; instead either (A) make the
save idempotent by iterating IntakeType.values(), constructing CupEmoji, calling
cupEmojiRepository.save(...) and catching DataIntegrityViolationException to
ignore already-existing entries (refer to CupEmoji, IntakeType,
cupEmojiRepository and DataIntegrityViolationException), or (B) run the
initialization once for the test class by moving the logic into a static
synchronized @BeforeAll method (e.g., setupDefaultEmojis) so only one thread
performs creation; pick one approach and remove the non-atomic count check.
- Line 183: Rename the test methods in IntakeHistoryServiceTest to follow the
project's test naming convention methodName_successOrError_condition (camelCase
for the condition part) instead of the current underscored-only style; e.g.,
rename success_streak_starts_at_one_without_yesterday_history to a name that
includes the target method under test such as
createByUserInput_success_streakStartsAtOneWithoutYesterdayHistory, and
similarly change success_returns_only_histories_in_date_range and
success_returns_only_own_member_histories to names like
findHistories_success_returnsOnlyHistoriesInDateRange and
findHistories_success_returnsOnlyOwnMemberHistories (or use the actual method
names under test), then run tests to ensure no references break and update any
test runners or documentation if this renaming is being applied project-wide.
- Line 59: The base test class ServiceTest is annotated with
@SpringBootTest(webEnvironment = WebEnvironment.NONE) but your coding guidelines
require WebEnvironment.MOCK; update the annotation in ServiceTest to
@SpringBootTest(webEnvironment = WebEnvironment.MOCK) so subclasses like
IntakeHistoryServiceTest inherit the mock servlet environment for MockMvc-style
service tests, or if NONE was intentional, add a clear comment in ServiceTest
explaining the reason and keep IntakeHistoryServiceTest unchanged.
- Around line 84-100: Extract the helper methods createAndSaveMember(),
createAndSaveMember(String), createAndSaveCupEmoji(), and
createAndSaveCup(Member, CupEmoji) from IntakeHistoryServiceTest into the
existing ServiceTest base class as protected methods that use the same
repositories (memberRepository, cupEmojiRepository, cupRepository) and fixture
builders (MemberFixtureBuilder, CupFixtureBuilder, MemberNickname) so other
service tests can inherit them; also add a protected @BeforeEach setup in
ServiceTest to persist a default CupEmoji (using createAndSaveCupEmoji) so tests
no longer need to duplicate that initialization and can override when needed.
In @backend/src/test/java/backend/mulkkam/member/service/MemberServiceTest.java:
- Around line 65-66: ServiceTest is annotated with
@SpringBootTest(webEnvironment = WebEnvironment.NONE) which conflicts with the
team convention; change the annotation on the ServiceTest class to
@SpringBootTest(webEnvironment = WebEnvironment.MOCK) so tests like
MemberServiceTest (which extends ServiceTest) load the application context with
MockMvc-style web environment instead of NONE; update any imports or static
references to WebEnvironment if necessary and run the tests to verify behavior.
In
@backend/src/test/java/backend/mulkkam/notification/service/NotificationServiceTest.java:
- Line 91: Rename all test methods so they start with the name of the production
method under test (e.g., change
success_returns_notifications_within_7_days_sorted_by_latest to
readNotificationsAfter_success_returns_notifications_within_7_days_sorted_by_latest,
change fail_size_cannot_be_negative to
readNotificationsAfter_fail_size_cannot_be_negative, change
success_returns_unread_count to
getUnReadNotificationsCount_success_returns_unread_count, change
success_notification_is_deleted to delete_success_notification_is_deleted, and
similarly update the other listed tests at lines 174, 197, 221, 261, 284, 327,
342, 366, 387, 413, 426) so each test method begins with the exact target method
name (readNotificationsAfter, getUnReadNotificationsCount, delete, etc.),
preserving the rest of the descriptive suffix and updating references/usages
accordingly.
🧹 Nitpick comments (29)
backend/src/test/java/backend/mulkkam/support/controller/ControllerTest.java (1)
11-13: 베이스 테스트를abstract로 만든 건 좋지만,webEnvironment명시(및 네이밍 옵션)까지 같이 가면 더 안전합니다.
abstract전환(Line 13)은 “베이스 클래스가 테스트로 발견되는 것”을 원천 차단하는 의도가 명확해서 좋습니다.- 다만 컨트롤러 테스트 가이드(“MockMvc로, Tomcat 없이”) 관점에선
@SpringBootTest(webEnvironment = MOCK)를 명시해두는 편이 오해/리그레션에 더 강합니다. (기본값에 기대지 않게 됨)- 추가 옵션(선택): 클래스명이
ControllerTest처럼*Test를 유지하면, 추후 도구/필터 설정에 따라 “스캔 대상”이 될 여지가 남습니다.
- 옵션 A) 현 상태 유지 + abstract: 변경 범위 최소 (장점), 네이밍만으로 의도가 덜 드러남 (단점)
- 옵션 B)
BaseControllerTest/ControllerTestSupport등으로 리네이밍: 테스트 스캔/의도가 더 명확 (장점), 하위 클래스 수정 필요 (단점)✅ 제안 diff: webEnvironment 명시
@SpringBootTest @AutoConfigureMockMvc public abstract class ControllerTest { + + // 컨트롤러 테스트는 Tomcat 없이 MockMvc로만 구동한다는 의도를 명시 + // (가독성/설정 안정성 측면에서 기본값 의존 제거) }-@SpringBootTest +@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.MOCK) @AutoConfigureMockMvc public abstract class ControllerTest {backend/src/test/java/backend/mulkkam/friend/service/FriendRequestServiceTest.java (3)
116-118: 단일 assertion에는 assertSoftly가 불필요합니다.
assertSoftly는 여러 검증을 묶어서 모든 실패를 한 번에 보고하기 위한 용도입니다. 여기서는 검증이 하나뿐이므로 직접assertThat을 사용하는 것이 더 간결합니다.♻️ 제안하는 수정안
// then List<FriendRelation> friendRelations = friendRelationRepository.findAll(); - assertSoftly(softly -> { - softly.assertThat(friendRelations).hasSize(0); - }); + assertThat(friendRelations).isEmpty();
281-303: 반복문 기반 픽스처 생성 로직을 헬퍼로 추출하면 가독성이 향상될 수 있습니다.현재 반복문 내에서 조건부로 여러 타입의 관계를 생성하고 있는데, 이 로직이 다소 복잡합니다. 테스트 의도(내가 보낸 요청만 반환)를 명확히 드러내기 위해 헬퍼 메서드로 추출하는 것을 고려해보세요.
장점:
- 테스트 본문에서 "무엇을" 테스트하는지가 더 명확해집니다
- 유사한 패턴이 반복될 때 재사용 가능
단점:
- 헬퍼로 감추면 테스트 데이터 구성이 덜 직관적일 수 있음
- 현재 인라인 방식도 주석과 함께 충분히 이해 가능
현재 상태도 나쁘지 않지만, 복잡도가 늘어날 경우 리팩터링을 고려하세요.
💡 예시: 헬퍼 메서드 추출
private void createReceivedRequests(Member addressee, int count) { for (int i = 0; i < count; i++) { Member requester = createAndSaveMember("요청자" + i); friendRelationRepository.save( new FriendRelation(requester.getId(), addressee.getId(), FriendRelationStatus.REQUESTED) ); } } private List<Long> createSentRequests(Member requester, int count) { List<Long> ids = new ArrayList<>(); for (int i = 0; i < count; i++) { Member addressee = createAndSaveMember("수신자" + i); FriendRelation relation = friendRelationRepository.save( new FriendRelation(requester.getId(), addressee.getId(), FriendRelationStatus.REQUESTED) ); ids.add(relation.getId()); } return ids; }테스트에서는:
// given Member requester = createAndSaveMember("요청자"); createReceivedRequests(requester, 3); List<Long> expectedIds = createSentRequests(requester, 4);
317-319: 단일 assertion에 assertSoftly 불필요 (2곳 추가)Lines 317-319와 357-359에서도 동일하게 하나의 검증만 수행하고 있습니다.
assertSoftly래핑을 제거하고 직접assertThat을 사용하세요.♻️ 제안하는 수정안
Line 317-319:
- assertSoftly(softly -> { - softly.assertThat(actual).containsExactlyInAnyOrderElementsOf(idsOfFriendRelation); - }); + assertThat(actual).containsExactlyInAnyOrderElementsOf(idsOfFriendRelation);Line 357-359:
- assertSoftly(softly -> { - softly.assertThat(actual).containsExactlyInAnyOrderElementsOf(expected); - }); + assertThat(actual).containsExactlyInAnyOrderElementsOf(expected);Also applies to: 357-359
backend/build.gradle (3)
58-66:test.finalizedBy jacocoTestReport는 로컬 피드백 속도를 떨어뜨릴 수 있어, “옵트인/CI 전용”을 고려해 주세요.
- 장점(현재 방식):
./gradlew test만으로도 항상 리포트가 생성되어 누락이 거의 없음.- 단점: 로컬에서 빠른 반복 테스트에 비용 증가(불필요한 I/O + 리포트 렌더링).
선택지:
- CI에서만 리포트 생성(로컬 빠름 / CI 일관성 유지)
-Pcoverage같은 플래그로 옵트인(필요할 때만 생성 / 기본은 빠름)제안 diff (옵트인: -Pcoverage 일 때만 리포트 생성)
tasks.named('test') { useJUnitPlatform() systemProperty 'user.timezone', 'Asia/Seoul' testLogging { showStandardStreams = true } - finalizedBy jacocoTestReport + if (project.hasProperty('coverage')) { + finalizedBy jacocoTestReport + } }
72-78: 리포트 생성만으로 끝나지 않게, (선택적으로) 커버리지 검증 태스크를check에 연결하는 것도 고려해볼 만합니다.
- 장점: 테스트 대규모 리팩터링 PR에서 “커버리지 감소”가 자동으로 감지되어 회귀 방지에 직접 연결됨.
- 단점: 초기 기준 설정을 잘못 잡으면 PR 진행이 막힐 수 있음(점진적 도입 권장).
예:
jacocoTestCoverageVerification를 추가하고check.dependsOn로 연결(또는 CI에서만 활성화).
68-70: JaCoCo 0.8.11은 Java 21을 공식 지원하므로 호환성 문제는 없지만, 최신 버전 업그레이드를 검토해보세요.확인 결과 JaCoCo 0.8.11은 2023년 10월에 Java 21 공식 지원을 추가했으므로 현재 설정에서 Java 21 바이트코드 처리 문제는 발생하지 않습니다. 다만 현재(2025년 1월 기준) JaCoCo의 최신 안정 버전은 0.8.14(2025년 10월 릴리스)이므로, 다음을 고려해 업그레이드하는 것을 권장합니다:
업그레이드 시 장점:
- 최신 Java 버전(21 이후)에 대한 개선된 지원
- 누적된 버그 픽스 및 성능 개선
- CI/로컬 환경 버전 일관성 강화
검토 사항:
- 팀의 다른 의존성 버전과의 호환성 확인
- 현재 빌드 성공 시 우선순위 낮음 (호환성 문제는 없음)
번거로움이 크지 않다면 0.8.14로 업데이트하고, 그렇지 않다면 현재 설정도 안정적입니다.
backend/src/test/java/backend/mulkkam/cup/controller/CupControllerTest.java (2)
60-71: 인증 컨텍스트 생성 패턴이 테스트 격리에 효과적입니다
AtomicLong과AuthenticatedMember레코드를 활용한 per-test 인증 패턴은 테스트 간 격리를 보장하며, 이전의 공유 상태 방식보다 명확합니다.선택적 개선사항:
- Line 67: 문자열 결합 방식(
"oauthId" + counter)보다String.format("oauthId_%d", counter)형태가 가독성 측면에서 약간 더 명확할 수 있습니다.♻️ 선택적 개선안
- new OauthAccount(member, "oauthId" + oauthIdCounter.getAndIncrement(), OauthProvider.KAKAO) + new OauthAccount(member, String.format("oauthId_%d", oauthIdCounter.getAndIncrement()), OauthProvider.KAKAO)
192-194: 단일 검증에는assertSoftly가 불필요합니다Line 192-194에서
assertSoftly를 사용했지만 실제로는 단 하나의 assertion만 수행하고 있습니다.문제점:
assertSoftly는 여러 검증을 수행할 때 첫 실패 이후에도 나머지를 계속 실행하기 위한 도구입니다- 단일 검증에 사용하면 불필요한 람다 래핑과 오버헤드가 발생합니다
권장사항:
단순assertThat을 사용하거나, 추가 검증이 필요하다면 (예:actual.getMessage()등) 여러 assertion을 함께 추가하세요.♻️ 권장 수정안
- assertSoftly(softly -> { - softly.assertThat(actual.getCode()).isEqualTo(INVALID_METHOD_ARGUMENT.name()); - }); + assertThat(actual.getCode()).isEqualTo(INVALID_METHOD_ARGUMENT.name());backend/src/test/java/backend/mulkkam/auth/infrastructure/OauthJwtTokenHandlerTest.java (2)
73-74: 예외 검증 방식 변경에 대한 트레이드오프 확인
assertThatThrownBy→assertThrows로의 전환은 JUnit 5 표준 방식으로의 통일이라는 PR 목표에 부합합니다.다만, AssertJ의
assertThatThrownBy는 예외 메시지나 원인(cause) 등을 fluent하게 체이닝해서 검증할 수 있는 장점이 있습니다. 현재 테스트는 예외 타입만 확인하고 있는데, 향후 예외 메시지 검증이 필요해지면 다음과 같이 개선할 수 있습니다:// Option 1: assertThrows + 반환값 활용 InvalidTokenException exception = assertThrows(InvalidTokenException.class, () -> oauthJwtTokenHandler.getAccountId(invalidToken)); assertThat(exception).hasMessageContaining("유효하지 않은 토큰"); // Option 2: AssertJ 스타일 (더 fluent) assertThatThrownBy(() -> oauthJwtTokenHandler.getAccountId(invalidToken)) .isInstanceOf(InvalidTokenException.class) .hasMessageContaining("유효하지 않은 토큰");현재는 표준화가 우선순위이므로 이대로 두되, 나중에 예외 메시지 검증이 필요하면 위 패턴을 참고하시면 좋겠습니다.
24-76: 추가 엣지 케이스 테스트 제안 (선택 사항)현재 테스트는 정상 시나리오와 기본적인 실패 시나리오를 잘 커버하고 있습니다. 다만 JWT 토큰 핸들러의 특성상 다음과 같은 추가 테스트 케이스들도 고려해볼 수 있습니다:
장점: 더욱 견고한 예외 처리 검증
- 만료된 토큰 (expired token)
- 서명이 변조된 토큰 (tampered signature)
- null 또는 빈 문자열
- JWT 형식이 아닌 문자열 (예: "not-a-jwt-format")
단점: 테스트 유지보수 비용 증가, 일부는 라이브러리 레벨에서 이미 검증됨
현재 PR이 테스트 리팩터링에 집중하고 있으므로, 이런 케이스들은 별도 이슈로 분리해서 추후 추가하는 것도 좋은 방법입니다.
참고: 현재
CreateToken과GetSubject네스티드 클래스의 성공 테스트들이 유사해 보이지만, 각각 "토큰 생성" vs "subject 추출"이라는 다른 관점에서 검증하므로 의미적으로 분리되어 있어 적절합니다.backend/src/test/java/backend/mulkkam/member/service/MemberServiceTest.java (2)
3-10: 예외 검증은assertThrows로 좋고, 나머지 assert는 AssertJ로 통일하는 편이 일관성이 좋아요지금은
assertEquals(JUnit)와assertThat/assertSoftly(AssertJ)가 섞여 있어요. 에러코드 비교도 AssertJ로 맞추면 테스트 스타일이 한 가지로 정리되고, 실패 메시지도 더 읽기 좋아집니다.제안 diff
import static org.assertj.core.api.AssertionsForClassTypes.assertThat; import static org.assertj.core.api.AssertionsForClassTypes.assertThatCode; import static org.assertj.core.api.SoftAssertions.assertSoftly; -import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; ... CommonException ex = assertThrows(CommonException.class, () -> memberService.validateDuplicateNickname( newNickname, new MemberDetails(member1) )); - assertEquals(DUPLICATE_MEMBER_NICKNAME, ex.getErrorCode()); + assertThat(ex.getErrorCode()).isEqualTo(DUPLICATE_MEMBER_NICKNAME); ... CommonException ex = assertThrows(CommonException.class, () -> memberService.validateDuplicateNickname( nickname, new MemberDetails(member) )); - assertEquals(SAME_AS_BEFORE_NICKNAME, ex.getErrorCode()); + assertThat(ex.getErrorCode()).isEqualTo(SAME_AS_BEFORE_NICKNAME);Also applies to: 245-271
505-512:saveDefaultCupEmojis()가 현재 파일 내에서 호출되지 않아 보이는데, 유지할 이유를 명확히 해두면 좋아요이 메서드가 “디폴트 컵 이모지 시드” 목적이라면:
- 선택지 A: 실제로 필요한 테스트 given에서 호출(의도 명확, 대신 테스트가 느려질 수 있음)
- 선택지 B: 해당 책임을 fixture/빌더로 이동(재사용성↑, 테스트 코드에서의 의도 표현↑)
- 선택지 C: 더 이상 필요 없다면 제거(코드 노이즈↓)
추가로
DefaultCup.of(intakeType);가 반환값 미사용인데, 순수 호출이 의미(예: static init 트리거)라면 코멘트로 의도를 남겨두는 게 안전합니다.backend/src/test/java/backend/mulkkam/intake/service/WeatherServiceUnitTest.java (2)
51-56: Exception assertion 패턴 변경으로 인한 가독성 저하를 고려해주세요.기존
assertThatThrownBy방식에서assertThrows+assertEquals로 변경하면서 2단계 검증으로 분리되었습니다.문제점:
- AssertJ의 fluent API 장점(
.isInstanceOf().hasMessage()체이닝)을 상실했습니다.- 예외 객체를 변수로 받아야 하므로 코드가 더 길어집니다.
- 다른 파일들(IntakeTargetAmountServiceTest 등)도 동일한 패턴으로 변경되어 일관성은 있으나, 전체적으로 가독성이 저하됩니다.
대안:
AssertJ 유지 (권장):
assertThatThrownBy의 표현력과 간결함을 활용assertThatThrownBy(() -> weatherService.getAverageTemperatureForCityDate(CityDateTime.now(City.SEOUL))) .isInstanceOf(CommonException.class) .extracting(ex -> ((CommonException) ex).getErrorCode()) .isEqualTo(INVALID_FORECAST_TARGET_DATE);assertThrows 간소화: error code 검증만 필요하다면 inline으로 처리
assertEquals(INVALID_FORECAST_TARGET_DATE, assertThrows(CommonException.class, () -> weatherService.getAverageTemperatureForCityDate(CityDateTime.now(City.SEOUL))) .getErrorCode());커스텀 assertion: 팀에서 자주 사용하는 패턴이라면
assertThrowsWithErrorCode같은 헬퍼 메서드 도입을 고려해보세요.이 패턴 변경이 팀 전체의 합의된 스타일이라면 문제없지만, 명확한 이점 없이 가독성만 저하시킨다면 재고가 필요합니다.
60-68: 동일한 exception assertion 패턴이 반복됩니다.Lines 51-56에서 언급한 것과 동일한 패턴이 여러 테스트에서 반복되고 있습니다.
제안:
중복을 줄이기 위해 테스트 유틸리티 메서드를 도입하는 것을 고려해보세요:private void assertThrowsCommonException(ErrorCode expectedErrorCode, Executable executable) { CommonException ex = assertThrows(CommonException.class, executable); assertEquals(expectedErrorCode, ex.getErrorCode()); }사용 예시:
assertThrowsCommonException(INVALID_FORECAST_TARGET_DATE, () -> weatherService.getAverageTemperatureForCityDate(CityDateTime.now(City.SEOUL)));이렇게 하면 코드 중복이 줄고, 향후 assertion 방식을 변경할 때도 한 곳만 수정하면 됩니다.
Also applies to: 142-144
backend/src/test/java/backend/mulkkam/intake/service/IntakeTargetAmountServiceTest.java (3)
208-213: 변수명result로의 통일이 일관성을 높이지만, 더 구체적인 이름도 고려해볼 수 있습니다.여러 테스트에서
intakeSuggestionAmountResponse나actual같은 변수명을result로 통일했습니다.장점:
- 간결함과 일관성 확보
- when-then 구조에서 "결과"를 명확히 표현
고려사항:
일부 테스트에서는 타입 정보가 변수명에 포함되는 것이 가독성에 도움이 될 수 있습니다. 예를 들어:
suggestionResponse(IntakeSuggestionAmountResponse일 때)targetResponse(IntakeTargetAmountResponse일 때)특히 한 테스트 메서드 내에서 여러 결과값을 검증할 때 구분이 필요할 수 있습니다.
제안:
현재 각 테스트가 단일 결과만 검증하므로result로 충분하지만, 팀 내에서 "간결함 vs 명시성" 관점의 합의가 필요합니다. 팀 컨벤션으로 명확히 정의되어 있다면 현재 방식이 적절합니다.Also applies to: 226-231, 250-253
95-98: Exception assertion 패턴에 대한 의견은 WeatherServiceUnitTest 리뷰를 참고해주세요.이 파일도 동일하게
assertThatThrownBy에서assertThrows+assertEquals로 변경되었습니다. 가독성 및 대안에 대한 의견은 WeatherServiceUnitTest.java의 lines 51-56 리뷰 코멘트를 참고해주세요.추가로, 팀 전체에서 이 패턴을 표준화했다면 일관성을 위해 유지하는 것이 합리적이지만, 만약 명확한 이점 없이 변경되었다면 재검토를 권장합니다.
57-57: 테스트 메서드명이 더 명확해졌지만, 일부는 과도하게 길어질 수 있습니다.
success_/fail_prefix를 사용한 새로운 네이밍 컨벤션이 적용되었고, 메서드명이 더 설명적으로 변경되었습니다.장점:
- 테스트 의도가 명확해짐 (예:
success_target_is_saved_when_amount_is_positive)- 성공/실패 케이스 구분이 일관됨
고려사항:
일부 메서드명이 매우 길어져서 오히려 가독성이 떨어질 수 있습니다:
success_target_is_saved_when_amount_is_positive(47자)success_calculates_from_provided_physical_attributes(54자)제안:
@DisplayName 어노테이션에 이미 상세한 설명이 있으므로, 메서드명은 좀 더 간결하게 유지하는 것도 고려해보세요:
success_target_is_saved_when_amount_is_positive→success_saveTargetWhenPositivefail_target_cannot_be_negative→fail_negativeAmount이는 코드에서 메서드를 참조할 때(리팩토링, 디버깅 등) 읽기 편하게 만듭니다. 하지만 이것도 팀 컨벤션의 문제이므로, 명시적 규칙이 있다면 그것을 따르는 것이 맞습니다.
Based on coding guidelines, 코딩 가이드라인에서
methodName_successOrError_condition형식을 요구하고 있으므로, 현재 네이밍이 가이드라인과 일치합니다. 따라서 이는 선택적인 제안일 뿐입니다.Also applies to: 82-82, 103-103, 127-127, 164-164, 200-200, 218-218, 240-240, 263-263, 277-277
backend/src/test/java/backend/mulkkam/intake/controller/IntakeAmountControllerTest.java (2)
17-17: AtomicLong 도입 목적(고유성/동시성)이 불명확합니다.
현재 테스트는 단일 스레드/단건 생성이라면 AtomicLong은 과하고, “고유 suffix” 목적이라면 테스트 간 DB 격리가 전제되지 않으면(또는 병렬 테스트 시)testId1같은 값이 충돌할 수 있어요. 고유성만 원하면 UUID가 더 안전하고, 결정적 suffix가 필요하면 “한 번만 증가시킨 값”을 nickname/oauthId에 공통으로 쓰는 게 의도가 명확합니다.대안 1) suffix를 한 번만 생성해 재사용 (결정적/가독성)
- private final AtomicLong oauthIdCounter = new AtomicLong(1); + private final AtomicLong oauthIdCounter = new AtomicLong(1); private AuthenticatedMember createAuthenticatedMember(Double weight, Integer targetAmount) { + long suffix = oauthIdCounter.getAndIncrement(); Member member = memberRepository.save(MemberFixtureBuilder .builder() - .memberNickname(new MemberNickname("테스터" + oauthIdCounter.get())) + .memberNickname(new MemberNickname("테스터" + suffix)) .weight(weight) .targetAmount(targetAmount) .build()); OauthAccount oauthAccount = oauthAccountRepository.save( - new OauthAccount(member, "testId" + oauthIdCounter.getAndIncrement(), OauthProvider.KAKAO) + new OauthAccount(member, "testId" + suffix, OauthProvider.KAKAO) );
35-59: 인증 컨텍스트(AuthenticatedMember) 생성 로직은 공통 유틸로 올릴지 검토해도 좋겠습니다.
PR 전반에서 유사한 “테스트 내 인증 컨텍스트” 패턴이 반복된다면, 각 테스트 클래스에 중복 구현이 누적될 가능성이 큽니다(추후 토큰 정책/claims 변경 시 수정 지점 증가).
- 선택지 A:
ControllerTest(또는 support 패키지)에AuthenticatedMember/factory를 공통 제공 (중복 제거, 변경 용이)- 선택지 B: 현 위치 유지 (테스트 독립성/로컬 이해도는 좋지만 중복이 커질 수 있음)
backend/src/test/java/backend/mulkkam/notification/service/SuggestionNotificationServiceTest.java (2)
43-48: @beforeeach 제거와 헬퍼 메서드 패턴 전환에 대한 검토가 필요합니다.이전에 테스트 코드에서
@BeforeEach를 활용해 가독성을 향상시키는 방향으로 리뷰를 요청하셨던 것으로 기록되어 있는데, 이번 PR에서는 정반대로@BeforeEach를 제거하고 각 테스트마다createAndSaveMember()를 호출하는 패턴으로 전환되었습니다.현재 패턴의 장단점:
- 장점: 테스트 간 격리성이 높아지고, 각 테스트가 독립적으로 데이터를 생성하므로 테스트 순서에 영향받지 않음
- 단점: 모든 테스트에서 동일한 멤버 생성 코드를 반복 호출해야 하므로 중복이 발생하고, 이전에 선호하셨던
@BeforeEach기반 공유 설정의 가독성 이점을 포기하게 됨제안:
- 만약 테스트 격리를 최우선으로 한다면 현재 패턴 유지
- 가독성과 편의성을 우선한다면
@BeforeEach로 복원하고DatabaseCleaner로 격리 보장- 또는 중간 방안으로, 여러 테스트에서 공통으로 사용하는 픽스처는
@BeforeEach에서 생성하고, 테스트별로 특수한 데이터만 헬퍼 메서드로 생성팀 내에서 테스트 격리 vs 가독성/편의성 중 어느 쪽에 더 무게를 둘지 논의 후 일관된 패턴을 적용하는 것이 좋겠습니다.
Based on learnings, 이전에는 BeforeEach 활용을 선호하셨던 것으로 확인됩니다.
89-92: 예외 검증 패턴을 assertThatThrownBy로 복원하는 것을 권장합니다.현재
assertThrows+assertEquals조합으로 변경되었는데, 이전 패턴인assertThatThrownBy가 더 읽기 쉽고 유창한(fluent) API를 제공합니다.현재 패턴의 단점:
- 예외를 변수로 캡처 후 별도로 검증해야 해서 두 단계로 분리됨
- 가독성이 떨어지고 코드가 길어짐
- AssertJ의 체이닝 방식의 표현력을 활용하지 못함
권장 패턴:
// when & then assertThatThrownBy(() -> suggestionNotificationService.applyTargetAmount(Long.MAX_VALUE, new MemberDetails(savedMember.getId(), MemberRole.MEMBER))) .isInstanceOf(CommonException.class) .extracting(ex -> ((CommonException) ex).getErrorCode()) .isEqualTo(NOT_FOUND_SUGGESTION_NOTIFICATION);또는 더 간결하게:
assertThatThrownBy(() -> suggestionNotificationService.applyTargetAmount(Long.MAX_VALUE, new MemberDetails(savedMember.getId(), MemberRole.MEMBER))) .isInstanceOfSatisfying(CommonException.class, ex -> assertThat(ex.getErrorCode()).isEqualTo(NOT_FOUND_SUGGESTION_NOTIFICATION));이 방식이 AssertJ의 철학에 더 부합하고, 테스트 의도를 한 눈에 파악하기 쉽습니다.
backend/src/test/java/backend/mulkkam/notification/service/NotificationServiceTest.java (3)
57-60: 반복적인 Member 생성 패턴을 @beforeeach로 추출하는 것을 고려해보세요.현재
createAndSaveMember()가 파일 내 거의 모든 테스트(12회 이상)에서 동일하게 호출되고 있습니다. 이는 명확한 중복 코드 패턴입니다.현재 패턴의 문제점:
- 모든 테스트에
Member savedMember = createAndSaveMember();라인이 반복됨- 테스트 의도와 무관한 보일러플레이트 코드가 테스트마다 추가됨
- Member 생성 로직 변경 시 모든 테스트를 수정해야 함 (현재는 문제없지만 향후 리스크)
개선 방안:
옵션 1: @beforeeach 활용 (권장)
private Member savedMember; @BeforeEach void setUp() { savedMember = memberRepository.save(MemberFixtureBuilder.builder().build()); }
- 장점: 중복 제거, 테스트 코드 간결화, 공통 설정의 명확한 표현
- 단점: ServiceTest의 DatabaseCleaner가 각 테스트 후 정리하므로 격리성은 유지됨
옵션 2: 현재 패턴 유지
- 테스트 격리를 극단적으로 강조하는 경우에만 정당화 가능
- 하지만 현재 모든 테스트가 동일한 방식으로 Member를 생성하므로 격리 이점이 크지 않음
특히 이전에 BeforeEach 활용을 통한 가독성 향상을 선호하셨던 점을 고려하면, 이 부분은 @beforeeach로 리팩토링하는 것이 팀의 기존 방향성과도 일치합니다.
Based on learnings, 이전에 BeforeEach를 활용한 가독성 개선을 요청하셨던 것으로 확인됩니다.
Also applies to: 93-93, 176-176, 199-199, 223-223, 263-263, 286-286, 329-329, 344-344, 368-368, 389-389, 415-415, 428-428
62-70: 헬퍼 메서드의 Member 파라미터 요구가 현재 패턴에서는 불필요한 복잡도를 추가합니다.
createUnReadNotifications와createReadNotifications메서드가 이제Member파라미터를 명시적으로 받도록 변경되었습니다.현재 설계의 영향:
// 호출 시마다 member를 전달해야 함 Member savedMember = createAndSaveMember(); List<Notification> notifications = createUnReadNotifications(savedMember, ...);설계 트레이드오프:
- 장점: 데이터 의존성이 명시적으로 드러나 테스트 독립성 강조
- 단점: 매번 같은 member를 전달하는 보일러플레이트 증가, 호출 코드가 길어짐
대안:
만약@BeforeEach로savedMember필드를 공유한다면, 헬퍼 메서드들이 해당 필드를 직접 참조할 수 있어 파라미터 전달이 불필요해집니다:private Member savedMember; @BeforeEach void setUp() { savedMember = memberRepository.save(MemberFixtureBuilder.builder().build()); } private List<Notification> createUnReadNotifications(LocalDate... dates) { return Arrays.stream(dates) .map(date -> NotificationFixtureBuilder .withMember(savedMember) // 필드 직접 참조 .createdAt(date) .isRead(false) .build()) .toList(); }현재 모든 테스트가 동일한 방식으로 member를 생성하고 전달하므로, 명시적 파라미터 전달의 이점보다는 간결함의 손실이 더 큽니다.
Also applies to: 72-80
290-292: 예외 검증 패턴을 assertThatThrownBy로 전환하는 것을 권장합니다.파일 전체에 걸쳐 예외 검증이
assertThrows+assertEquals패턴으로 작성되어 있는데, AssertJ의assertThatThrownBy를 사용하면 더 읽기 쉽고 표현력 있는 테스트 코드를 작성할 수 있습니다.현재 패턴의 한계:
CommonException ex = assertThrows(CommonException.class, () -> notificationService.readNotificationsAfter(request, new MemberDetails(savedMember))); assertEquals(INVALID_PAGE_SIZE_RANGE, ex.getErrorCode());
- 예외를 중간 변수로 캡처해야 함
- 검증이 두 단계로 분리되어 흐름이 끊김
- AssertJ의 체이닝 이점을 활용하지 못함
권장 패턴:
assertThatThrownBy(() -> notificationService.readNotificationsAfter( request, new MemberDetails(savedMember))) .isInstanceOf(CommonException.class) .extracting(ex -> ((CommonException) ex).getErrorCode()) .isEqualTo(INVALID_PAGE_SIZE_RANGE);또는 더 간결하게:
assertThatThrownBy(() -> notificationService.readNotificationsAfter( request, new MemberDetails(savedMember))) .isInstanceOfSatisfying(CommonException.class, ex -> assertThat(ex.getErrorCode()).isEqualTo(INVALID_PAGE_SIZE_RANGE));이점:
- 한 눈에 "이런 예외가 발생하고, 이런 속성을 가져야 한다"는 의도 파악 가능
- AssertJ 생태계와 일관된 스타일 유지
- 추가 검증(메시지, cause 등)을 체이닝으로 쉽게 확장 가능
동일한 파일 내 Lines 290-292, 419-421, 447-449에 모두 적용 가능합니다.
Also applies to: 419-421, 447-449
backend/src/test/java/backend/mulkkam/intake/controller/IntakeHistoryControllerTest.java (1)
52-73: 테스트 헬퍼 중복 가능성 검토
createAuthenticatedMember()메서드는 인증 컨텍스트를 생성하는 훌륭한 패턴이지만, 다른 컨트롤러 테스트에서도 유사한 로직이 필요할 가능성이 높습니다.현재 접근의 장점:
- 테스트별 격리된 인증 컨텍스트 생성
- 필요한 모든 관련 엔티티를 함께 생성
개선 고려사항:
- 코드 중복: 다른 컨트롤러 테스트에서도 동일한 패턴 반복 가능성
- 유연성 부족: 항상 Cup까지 생성하지만, 일부 테스트는 Member와 token만 필요할 수 있음
- 커스터마이징 어려움: Member의 특정 속성(weight, targetAmount)을 변경하려면 메서드 수정 필요
제안:
ControllerTest기본 클래스 또는 별도의AuthTestHelper로 추출- 빌더 패턴으로 선택적 엔티티 생성 지원
createAuthenticatedMember() .withWeight(80.0) .withCup(false) // Cup 생성 스킵 .build()Based on learnings, 다른 컨트롤러 테스트에서도 인증 컨텍스트 생성 패턴을 확인하여 공통화 여부를 결정하는 것이 좋습니다.
backend/src/test/java/backend/mulkkam/intake/service/IntakeHistoryServiceTest.java (3)
625-630: 예외 검증 패턴 변경에 대한 검토예외 검증 방식이 변경되었습니다:
이전 (추정):
assertThatThrownBy(() -> intakeHistoryService.deleteDetailHistory(...)) .isInstanceOf(CommonException.class) .hasFieldOrPropertyWithValue("errorCode", NOT_PERMITTED_FOR_INTAKE_HISTORY);현재:
CommonException ex = assertThrows(CommonException.class, () -> intakeHistoryService.deleteDetailHistory(...)); assertEquals(NOT_PERMITTED_FOR_INTAKE_HISTORY, ex.getErrorCode());현재 방식의 문제점:
- 가독성 저하: 두 줄로 나뉘어 예외와 에러 코드 검증이 분리됨
- 표현력 부족: AssertJ의 fluent API가 더 직관적이고 실패 메시지도 명확
- 일관성: AssertJ를 다른 곳에서 사용 중인데 (assertSoftly 등) 혼용하면 코드 스타일이 불일치
대안:
assertThatThrownBy(() -> intakeHistoryService.deleteDetailHistory(...)) .isInstanceOf(CommonException.class) .extracting("errorCode") .isEqualTo(NOT_PERMITTED_FOR_INTAKE_HISTORY);팀 내에서 JUnit 스타일로 통일하기로 결정했다면 괜찮지만, AssertJ가 이미 전반적으로 사용되고 있다면 일관성을 위해 AssertJ를 유지하는 것을 권장합니다.
동일한 패턴이 line 681-686에도 적용되어 있습니다.
286-289: 테스트 변수 네이밍: 'result' vs 'actual'테스트에서 서비스 호출 결과를 담는 변수명이
result로 변경되었습니다.일반적인 테스트 변수 네이밍 관례:
// 전통적인 방식 List<IntakeHistorySummaryResponse> actual = service.method(); List<IntakeHistorySummaryResponse> expected = ...; assertThat(actual).isEqualTo(expected);현재 방식:
List<IntakeHistorySummaryResponse> result = service.method(); assertThat(result).condition(...);비교:
actual: expected/actual 쌍으로 사용할 때 의미가 명확result: 더 일반적이고 중립적이지만, assertion 문맥에서는 덜 관용적권장:
- expected 값과 비교하는 경우:
actual사용- 단순히 조건 검증만 하는 경우:
result사용 가능- 팀 내에서 하나의 스타일로 통일
현재 이 테스트에서는 주로 조건 검증을 하므로
result도 합리적이지만, 일관성을 위해 팀 전체적으로 통일하는 것을 권장합니다.
59-698: 추가 테스트 케이스 제안현재 테스트 커버리지는 주요 시나리오를 잘 다루고 있지만, 다음 케이스들을 고려해보세요:
1. 경계값 테스트:
- 날짜 범위가 매우 긴 경우 (예: 1년)
- 음용량이 0인 경우
- 목표 음용량이 극단적인 값인 경우 (0, Integer.MAX_VALUE)
2. 입력 검증 테스트:
@Test void fail_invalid_amount_when_negative() { // given Member member = createAndSaveMember(); CreateIntakeHistoryDetailByUserInputRequest request = new CreateIntakeHistoryDetailByUserInputRequest(LocalDateTime.now(), WATER, -100); // when & then // 음수 음용량에 대한 예외 검증 }3. 날짜 경계 테스트:
@Test void success_handles_date_range_boundary_correctly() { // from과 to가 같은 날인 경우 // from이 to보다 나중인 경우 (역순) // 미래 날짜에 대한 처리 }4. 스트릭 계산 엣지 케이스:
- 여러 날짜를 건너뛴 후 기록하는 경우 (스트릭 리셋 검증)
- 같은 날 여러 번 기록하는 경우
5. 성능 관련:
- 대량의 기록이 있을 때 조회 성능
- N+1 쿼리 문제 확인
As per coding guidelines, 컨트롤러는 인수 테스트로, 나머지는 단위 테스트로 커버하는 것이 권장됩니다. 현재 서비스 레이어 테스트이므로 위 케이스들을 추가하면 더 견고한 테스트 스위트가 될 것입니다.
위 테스트 케이스들의 구현이 필요하다면, 구체적인 테스트 코드를 생성해드릴 수 있습니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
backend/build.gradlebackend/src/test/java/backend/mulkkam/auth/infrastructure/OauthJwtTokenHandlerTest.javabackend/src/test/java/backend/mulkkam/cup/controller/CupControllerTest.javabackend/src/test/java/backend/mulkkam/friend/service/FriendRequestServiceTest.javabackend/src/test/java/backend/mulkkam/intake/controller/IntakeAmountControllerTest.javabackend/src/test/java/backend/mulkkam/intake/controller/IntakeHistoryControllerTest.javabackend/src/test/java/backend/mulkkam/intake/service/IntakeHistoryServiceTest.javabackend/src/test/java/backend/mulkkam/intake/service/IntakeTargetAmountServiceTest.javabackend/src/test/java/backend/mulkkam/intake/service/WeatherServiceUnitTest.javabackend/src/test/java/backend/mulkkam/member/service/MemberServiceTest.javabackend/src/test/java/backend/mulkkam/notification/service/NotificationServiceTest.javabackend/src/test/java/backend/mulkkam/notification/service/SuggestionNotificationServiceTest.javabackend/src/test/java/backend/mulkkam/support/controller/ControllerTest.java
🧰 Additional context used
📓 Path-based instructions (4)
backend/**/*Test.java
📄 CodeRabbit inference engine (backend/code-style.md)
backend/**/*Test.java: Use @DisplayName with format: methodName_successOrError_condition in test classes
Use @nested annotation for organizing test scenarios in test classes
Follow BDD pattern in tests: given (optional), when, then
Use Calorie/정팩메 style for test fixtures
For controller tests with authentication/authorization, use MockMVC without starting Tomcat; for service tests, use @SpringBootTest(WebEnvironment.MOCK)
Test JPQL and Query Method implementations and wrap assertions with assertSoftly
Files:
backend/src/test/java/backend/mulkkam/notification/service/SuggestionNotificationServiceTest.javabackend/src/test/java/backend/mulkkam/friend/service/FriendRequestServiceTest.javabackend/src/test/java/backend/mulkkam/intake/controller/IntakeAmountControllerTest.javabackend/src/test/java/backend/mulkkam/cup/controller/CupControllerTest.javabackend/src/test/java/backend/mulkkam/intake/controller/IntakeHistoryControllerTest.javabackend/src/test/java/backend/mulkkam/auth/infrastructure/OauthJwtTokenHandlerTest.javabackend/src/test/java/backend/mulkkam/member/service/MemberServiceTest.javabackend/src/test/java/backend/mulkkam/intake/service/WeatherServiceUnitTest.javabackend/src/test/java/backend/mulkkam/intake/service/IntakeTargetAmountServiceTest.javabackend/src/test/java/backend/mulkkam/notification/service/NotificationServiceTest.javabackend/src/test/java/backend/mulkkam/intake/service/IntakeHistoryServiceTest.javabackend/src/test/java/backend/mulkkam/support/controller/ControllerTest.java
backend/**/*.java
📄 CodeRabbit inference engine (backend/code-style.md)
backend/**/*.java: Use verb-starting method names without entity name, e.g., memberService.findById(Long id)
Avoid else statements; use alternative control flow patterns
Use one dot per line (method chaining rule)
Avoid abbreviations in code
Avoid getters and setters; use behavior-focused methods instead
Annotation order: logs, Lombok, Spring meta, Spring components (most important, placed last)
Add one blank line before class declaration
When method has 2 or more parameters, use line breaks for parameters (does not apply to method calls)
Do not use wildcard imports; apply IntelliJ code conventions for imports
Use method names: create, save, register, add for creation operations
Use method names: read for multiple results, get for always-existing values, find for Optional results
Use method names: update for full replacement (PUT), modify for partial updates (PATCH)
Use method names: delete, remove for deletion operations
Do not use final keyword on class or field declarations
Use uppercase with underscores for constant names, e.g., CONSTANT_NAME
Order class members and methods by access level: public, protected, private; arrange by call order
Files:
backend/src/test/java/backend/mulkkam/notification/service/SuggestionNotificationServiceTest.javabackend/src/test/java/backend/mulkkam/friend/service/FriendRequestServiceTest.javabackend/src/test/java/backend/mulkkam/intake/controller/IntakeAmountControllerTest.javabackend/src/test/java/backend/mulkkam/cup/controller/CupControllerTest.javabackend/src/test/java/backend/mulkkam/intake/controller/IntakeHistoryControllerTest.javabackend/src/test/java/backend/mulkkam/auth/infrastructure/OauthJwtTokenHandlerTest.javabackend/src/test/java/backend/mulkkam/member/service/MemberServiceTest.javabackend/src/test/java/backend/mulkkam/intake/service/WeatherServiceUnitTest.javabackend/src/test/java/backend/mulkkam/intake/service/IntakeTargetAmountServiceTest.javabackend/src/test/java/backend/mulkkam/notification/service/NotificationServiceTest.javabackend/src/test/java/backend/mulkkam/intake/service/IntakeHistoryServiceTest.javabackend/src/test/java/backend/mulkkam/support/controller/ControllerTest.java
backend/**
⚙️ CodeRabbit configuration file
backend/**: - 1. 팀 및 공식 컨벤션, 가독성, 예외처리, 테스트/확장/유지보수성, 모듈화, API/DB/보안 설계 기준을 기반으로 리뷰해주세요.
- 최신 트렌드, 불필요한 로직, 클린코드, 리팩토링, 서비스/도메인 설계, 공통 예외 처리, 확장성도 함께 확인해주세요.
- 각 피드백은 문제점·대안·장단점을 짧고 논리적으로, 예시 코드가 있다면 간결히 포함해 주세요.
- 팀 내 스타일 통일성도 확인해주세요.
- 미작성한 테스트 코드 케이스가 있다면, 어떤 테스트가 필요한지 제안해주세요. (예: 컨트롤러는 인수 테스트, 나머지는 단위 테스트)
- 리뷰 남겨주는 부분은 해당 라인 범위의 코멘트에 작성해주세요.
Files:
backend/src/test/java/backend/mulkkam/notification/service/SuggestionNotificationServiceTest.javabackend/src/test/java/backend/mulkkam/friend/service/FriendRequestServiceTest.javabackend/build.gradlebackend/src/test/java/backend/mulkkam/intake/controller/IntakeAmountControllerTest.javabackend/src/test/java/backend/mulkkam/cup/controller/CupControllerTest.javabackend/src/test/java/backend/mulkkam/intake/controller/IntakeHistoryControllerTest.javabackend/src/test/java/backend/mulkkam/auth/infrastructure/OauthJwtTokenHandlerTest.javabackend/src/test/java/backend/mulkkam/member/service/MemberServiceTest.javabackend/src/test/java/backend/mulkkam/intake/service/WeatherServiceUnitTest.javabackend/src/test/java/backend/mulkkam/intake/service/IntakeTargetAmountServiceTest.javabackend/src/test/java/backend/mulkkam/notification/service/NotificationServiceTest.javabackend/src/test/java/backend/mulkkam/intake/service/IntakeHistoryServiceTest.javabackend/src/test/java/backend/mulkkam/support/controller/ControllerTest.java
backend/**/controller/**/*.java
📄 CodeRabbit inference engine (backend/code-style.md)
Use clear Path Variable names in API endpoints, e.g., /api/{documentId}
Files:
backend/src/test/java/backend/mulkkam/intake/controller/IntakeAmountControllerTest.javabackend/src/test/java/backend/mulkkam/cup/controller/CupControllerTest.javabackend/src/test/java/backend/mulkkam/intake/controller/IntakeHistoryControllerTest.javabackend/src/test/java/backend/mulkkam/support/controller/ControllerTest.java
🧠 Learnings (12)
📓 Common learnings
Learnt from: CheChe903
Repo: woowacourse-teams/2025-mul-kkam PR: 739
File: backend/src/main/java/backend/mulkkam/auth/infrastructure/OauthJwtTokenHandler.java:56-64
Timestamp: 2025-09-10T04:51:38.314Z
Learning: CheChe903 사용자는 테스트 코드에서 BeforeEach를 활용해서 가독성을 향상시킬 수 있는 부분이 있으면 리뷰해달라고 요청했습니다. 앞으로 테스트 코드 리뷰 시 중복되는 객체 생성, 테스트 데이터 셋업, Mock 설정 등을 BeforeEach로 추출할 수 있는 리팩터링 기회를 적극적으로 제안해야 합니다.
Learnt from: CR
Repo: woowacourse-teams/2025-mul-kkam PR: 0
File: backend/code-style.md:0-0
Timestamp: 2025-12-15T16:41:54.690Z
Learning: Applies to backend/**/*Test.java : Use Calorie/정팩메 style for test fixtures
Learnt from: CR
Repo: woowacourse-teams/2025-mul-kkam PR: 0
File: backend/code-style.md:0-0
Timestamp: 2025-12-15T16:41:54.690Z
Learning: Applies to backend/**/*Test.java : For controller tests with authentication/authorization, use MockMVC without starting Tomcat; for service tests, use SpringBootTest(WebEnvironment.MOCK)
📚 Learning: 2025-12-15T16:41:54.690Z
Learnt from: CR
Repo: woowacourse-teams/2025-mul-kkam PR: 0
File: backend/code-style.md:0-0
Timestamp: 2025-12-15T16:41:54.690Z
Learning: Applies to backend/**/*Test.java : Use Nested annotation for organizing test scenarios in test classes
Applied to files:
backend/src/test/java/backend/mulkkam/notification/service/SuggestionNotificationServiceTest.javabackend/src/test/java/backend/mulkkam/cup/controller/CupControllerTest.javabackend/src/test/java/backend/mulkkam/notification/service/NotificationServiceTest.java
📚 Learning: 2025-12-15T16:41:54.690Z
Learnt from: CR
Repo: woowacourse-teams/2025-mul-kkam PR: 0
File: backend/code-style.md:0-0
Timestamp: 2025-12-15T16:41:54.690Z
Learning: Applies to backend/**/*Test.java : For controller tests with authentication/authorization, use MockMVC without starting Tomcat; for service tests, use SpringBootTest(WebEnvironment.MOCK)
Applied to files:
backend/src/test/java/backend/mulkkam/notification/service/SuggestionNotificationServiceTest.javabackend/src/test/java/backend/mulkkam/intake/controller/IntakeAmountControllerTest.javabackend/src/test/java/backend/mulkkam/cup/controller/CupControllerTest.javabackend/src/test/java/backend/mulkkam/intake/controller/IntakeHistoryControllerTest.javabackend/src/test/java/backend/mulkkam/auth/infrastructure/OauthJwtTokenHandlerTest.javabackend/src/test/java/backend/mulkkam/member/service/MemberServiceTest.javabackend/src/test/java/backend/mulkkam/notification/service/NotificationServiceTest.javabackend/src/test/java/backend/mulkkam/intake/service/IntakeHistoryServiceTest.javabackend/src/test/java/backend/mulkkam/support/controller/ControllerTest.java
📚 Learning: 2025-12-15T16:41:54.690Z
Learnt from: CR
Repo: woowacourse-teams/2025-mul-kkam PR: 0
File: backend/code-style.md:0-0
Timestamp: 2025-12-15T16:41:54.690Z
Learning: Applies to backend/**/*.java : Annotation order: logs, Lombok, Spring meta, Spring components (most important, placed last)
Applied to files:
backend/build.gradle
📚 Learning: 2025-12-15T16:41:54.690Z
Learnt from: CR
Repo: woowacourse-teams/2025-mul-kkam PR: 0
File: backend/code-style.md:0-0
Timestamp: 2025-12-15T16:41:54.690Z
Learning: Applies to backend/**/*Test.java : Use Calorie/정팩메 style for test fixtures
Applied to files:
backend/src/test/java/backend/mulkkam/intake/controller/IntakeAmountControllerTest.javabackend/src/test/java/backend/mulkkam/cup/controller/CupControllerTest.javabackend/src/test/java/backend/mulkkam/intake/controller/IntakeHistoryControllerTest.javabackend/src/test/java/backend/mulkkam/member/service/MemberServiceTest.javabackend/src/test/java/backend/mulkkam/intake/service/WeatherServiceUnitTest.javabackend/src/test/java/backend/mulkkam/intake/service/IntakeTargetAmountServiceTest.javabackend/src/test/java/backend/mulkkam/intake/service/IntakeHistoryServiceTest.java
📚 Learning: 2025-09-05T10:27:57.192Z
Learnt from: CheChe903
Repo: woowacourse-teams/2025-mul-kkam PR: 716
File: backend/src/main/java/backend/mulkkam/cup/controller/CupController.java:63-65
Timestamp: 2025-09-05T10:27:57.192Z
Learning: Controller에서 RequestBody 파라미터의 변수명은 해당 DTO 클래스명과 일치시켜야 한다. 예: CreateCupWithoutRankRequest 클래스는 createCupWithoutRankRequest 변수명으로 사용
Applied to files:
backend/src/test/java/backend/mulkkam/cup/controller/CupControllerTest.java
📚 Learning: 2025-09-05T10:27:53.796Z
Learnt from: CheChe903
Repo: woowacourse-teams/2025-mul-kkam PR: 716
File: backend/src/main/java/backend/mulkkam/cup/controller/CupController.java:63-65
Timestamp: 2025-09-05T10:27:53.796Z
Learning: Controller의 RequestBody 파라미터 변수명은 해당 DTO 클래스명과 동일하게 명명한다 (예: CreateCupWithoutRankRequest createCupWithoutRankRequest)
Applied to files:
backend/src/test/java/backend/mulkkam/cup/controller/CupControllerTest.java
📚 Learning: 2025-09-10T04:51:38.314Z
Learnt from: CheChe903
Repo: woowacourse-teams/2025-mul-kkam PR: 739
File: backend/src/main/java/backend/mulkkam/auth/infrastructure/OauthJwtTokenHandler.java:56-64
Timestamp: 2025-09-10T04:51:38.314Z
Learning: CheChe903 사용자는 테스트 코드에서 BeforeEach를 활용해서 가독성을 향상시킬 수 있는 부분이 있으면 리뷰해달라고 요청했습니다. 앞으로 테스트 코드 리뷰 시 중복되는 객체 생성, 테스트 데이터 셋업, Mock 설정 등을 BeforeEach로 추출할 수 있는 리팩터링 기회를 적극적으로 제안해야 합니다.
Applied to files:
backend/src/test/java/backend/mulkkam/cup/controller/CupControllerTest.java
📚 Learning: 2025-12-15T16:41:54.690Z
Learnt from: CR
Repo: woowacourse-teams/2025-mul-kkam PR: 0
File: backend/code-style.md:0-0
Timestamp: 2025-12-15T16:41:54.690Z
Learning: Applies to backend/**/*Test.java : Test JPQL and Query Method implementations and wrap assertions with assertSoftly
Applied to files:
backend/src/test/java/backend/mulkkam/intake/controller/IntakeHistoryControllerTest.javabackend/src/test/java/backend/mulkkam/auth/infrastructure/OauthJwtTokenHandlerTest.javabackend/src/test/java/backend/mulkkam/intake/service/WeatherServiceUnitTest.javabackend/src/test/java/backend/mulkkam/intake/service/IntakeTargetAmountServiceTest.javabackend/src/test/java/backend/mulkkam/notification/service/NotificationServiceTest.javabackend/src/test/java/backend/mulkkam/intake/service/IntakeHistoryServiceTest.java
📚 Learning: 2025-12-15T16:41:54.690Z
Learnt from: CR
Repo: woowacourse-teams/2025-mul-kkam PR: 0
File: backend/code-style.md:0-0
Timestamp: 2025-12-15T16:41:54.690Z
Learning: Applies to backend/**/*.java : Use verb-starting method names without entity name, e.g., memberService.findById(Long id)
Applied to files:
backend/src/test/java/backend/mulkkam/member/service/MemberServiceTest.java
📚 Learning: 2025-12-15T16:41:54.690Z
Learnt from: CR
Repo: woowacourse-teams/2025-mul-kkam PR: 0
File: backend/code-style.md:0-0
Timestamp: 2025-12-15T16:41:54.690Z
Learning: Applies to backend/**/*Test.java : Use DisplayName with format: methodName_successOrError_condition in test classes
Applied to files:
backend/src/test/java/backend/mulkkam/intake/service/WeatherServiceUnitTest.javabackend/src/test/java/backend/mulkkam/intake/service/IntakeTargetAmountServiceTest.java
📚 Learning: 2025-12-15T16:41:54.690Z
Learnt from: CR
Repo: woowacourse-teams/2025-mul-kkam PR: 0
File: backend/code-style.md:0-0
Timestamp: 2025-12-15T16:41:54.690Z
Learning: Applies to backend/**/*Test.java : Follow BDD pattern in tests: given (optional), when, then
Applied to files:
backend/src/test/java/backend/mulkkam/intake/service/WeatherServiceUnitTest.java
🧬 Code graph analysis (5)
backend/src/test/java/backend/mulkkam/cup/controller/CupControllerTest.java (2)
backend/src/test/java/backend/mulkkam/support/fixture/member/MemberFixtureBuilder.java (1)
MemberFixtureBuilder(10-110)backend/src/test/java/backend/mulkkam/support/fixture/cup/dto/CreateCupWithoutRankRequestFixtureBuilder.java (1)
CreateCupWithoutRankRequestFixtureBuilder(6-47)
backend/src/test/java/backend/mulkkam/intake/controller/IntakeHistoryControllerTest.java (2)
backend/src/test/java/backend/mulkkam/support/fixture/member/MemberFixtureBuilder.java (1)
MemberFixtureBuilder(10-110)backend/src/test/java/backend/mulkkam/support/fixture/cup/CupFixtureBuilder.java (1)
CupFixtureBuilder(11-71)
backend/src/test/java/backend/mulkkam/auth/infrastructure/OauthJwtTokenHandlerTest.java (1)
backend/src/main/java/backend/mulkkam/common/exception/InvalidTokenException.java (1)
InvalidTokenException(3-4)
backend/src/test/java/backend/mulkkam/notification/service/NotificationServiceTest.java (2)
backend/src/test/java/backend/mulkkam/support/fixture/member/MemberFixtureBuilder.java (1)
MemberFixtureBuilder(10-110)backend/src/test/java/backend/mulkkam/support/fixture/notification/NotificationFixtureBuilder.java (1)
NotificationFixtureBuilder(10-61)
backend/src/test/java/backend/mulkkam/intake/service/IntakeHistoryServiceTest.java (2)
backend/src/test/java/backend/mulkkam/support/fixture/member/MemberFixtureBuilder.java (1)
MemberFixtureBuilder(10-110)backend/src/test/java/backend/mulkkam/support/fixture/cup/CupFixtureBuilder.java (1)
CupFixtureBuilder(11-71)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (13)
backend/src/test/java/backend/mulkkam/friend/service/FriendRequestServiceTest.java (3)
29-55: 헬퍼 메서드 구성이 깔끔합니다.
createAndSaveMember와createFriendRequestPair헬퍼를 통해 테스트별 on-demand 픽스처 생성 패턴을 잘 따르고 있습니다.- MemberFixtureBuilder를 활용한 Calorie 스타일도 코딩 가이드라인과 일치합니다.
- Record 타입
FriendRequestPair로 테스트 데이터를 구조화한 점도 가독성에 도움이 됩니다.
220-227: 상태 검증이 포괄적입니다.수락 후
FriendRelation의 크기, requester/addressee ID, 상태를 모두 검증하고 있어 상태 전환 로직을 충분히 확인하고 있습니다.assertSoftly를 활용해 모든 검증을 그룹화한 점도 적절합니다.
57-361: 테스트 커버리지와 구조가 우수합니다.
- 권한 검증: 수신자만 수락/거절 가능, 요청자가 본인 요청을 처리하려 할 때 예외 등 권한 경계 케이스를 잘 다루고 있습니다.
- 상태 전환 검증: REQUESTED → ACCEPTED/REJECTED 전환과 중복 처리 방지를 명확히 검증합니다.
- 에러 시나리오:
NOT_FOUND_FRIEND_REQUEST,ALREADY_ACCEPTED등 예외 코드를 정확히 확인합니다.- 조회 필터링: 보낸 요청 목록 조회에서 요청자 필터와 상태 필터를 분리해 테스트한 점이 좋습니다.
BDD 패턴(given/when/then)도 일관되게 적용되어 있고,
@Nested를 활용한 구조화도 명확합니다. PR 목표인 DB 기반 통합 테스트 방향과도 잘 맞아떨어집니다.backend/build.gradle (1)
1-6: JaCoCo 도입 방향은 좋으나, 개발 피드백 속도와 품질 강제 개선이 필요합니다.현재 설정의 두 가지 개선 포인트가 있습니다:
finalizedBy사용의 비효율성
매번./gradlew test실행할 때마다 HTML/XML 리포트를 생성합니다. 로컬 개발 중 반복 테스트 시 불필요한 오버헤드가 발생합니다. 대신 CI 파이프라인에서만 리포트를 생성하는 것을 권장합니다.커버리지 검증(verification) 미설정
현재는 리포트만 생성하고 커버리지 임계값을 강제하지 않아, 측정만 되고 품질이 실제로 보장되지 않습니다.다음과 같이 개선할 수 있습니다:
개선 방안
옵션 A: 로컬 개발 속도 우선 (권장)
tasks.named('test') { useJUnitPlatform() systemProperty 'user.timezone', 'Asia/Seoul' testLogging { showStandardStreams = true } // finalizedBy 제거 → 로컬에서는 test만 실행 } // CI에서만 호출: ./gradlew jacocoTestReport옵션 B: 품질 게이트 강제 (선택적)
jacocoTestReport { dependsOn test reports { xml.required = true html.required = true } } // 커버리지 임계값 추가 jacocoTestCoverageVerification { violationRules { rule { element = 'PACKAGE' excludes = ['**.config.*', '**.dto.*'] limit { minimum = 0.70 // 70% 이상 강제 } } } } // CI: ./gradlew test jacocoTestReport jacocoTestCoverageVerification장단점 비교:
- 옵션 A: 개발 피드백 빠름, 간단함 / CI 파이프라인 설정 필요
- 옵션 B: 커버리지 품질 강제 가능 / 로컬 개발 시 추가 조건 고려 필요
팀 기준과 CI 환경에 맞춰 선택하시면 됩니다.
backend/src/test/java/backend/mulkkam/cup/controller/CupControllerTest.java (2)
145-161: 테스트 구현이 팀 컨벤션을 잘 따르고 있습니다
- Per-test 인증 컨텍스트 생성 (Line 147)
- Fixture 빌더 활용 (Lines 150-152)
- BDD 패턴 준수 (given, when, then 주석)
- 네이밍 컨벤션 준수
이번 PR의 리팩토링 방향과 일치하며, 테스트 격리와 가독성이 모두 확보되었습니다.
92-116: 검증 결과: 현재 테스트 코드가 올바르게 작성되었습니다
/cups/default엔드포인트는@RequireAuth어노테이션이 없어 인증이 불필요한 public API입니다. 따라서 lines 92-116과 120-136의 테스트에서 인증 헤더를 포함하지 않은 것이 정확한 구현입니다.같은 파일의 다른 엔드포인트들(lines 147, 172 등)은
@RequireAuth가 적용되어 있어 인증이 필요하므로AuthenticatedMember를 사용하고 있으며, 이는 각 엔드포인트의 요구사항에 맞게 올바르게 테스트되고 있습니다.Likely an incorrect or invalid review comment.
backend/src/test/java/backend/mulkkam/auth/infrastructure/OauthJwtTokenHandlerTest.java (2)
4-4: 베이스 클래스 및 import 변경 LGTM!
ServiceIntegrationTest→ServiceTest로의 전환과assertThrowsimport 추가가 PR의 전반적인 테스트 표준화 방향과 일치합니다. 테스트는 여전히 Spring 컨텍스트와 실제 Repository를 사용하므로 통합 테스트의 성격을 유지하고 있어 적절합니다.Also applies to: 10-10, 16-16
68-68: 테스트 메서드명 개선 확인
error_withInvalidToken→fail_withInvalidToken로의 변경이 메서드의 실제 동작(실패/예외 발생)을 더 명확하게 표현합니다. 네이밍 컨벤션 가이드라인과도 잘 맞습니다.backend/src/test/java/backend/mulkkam/member/service/MemberServiceTest.java (1)
110-116:@DisplayName컨벤션(가이드) 포맷을 실제로 적용할지 한 번만 정리해두면 좋아요가이드에는
methodName_successOrError_condition형태의@DisplayName을 요구하는데, 현재는 한국어 문장형이라 팀 규칙과 약간 어긋나 보입니다.
(지금 방식이 팀에서 합의된 최신 규칙이면 OK인데, 문서/가이드와 다르면 이후 리팩터링 시 “뭐가 정답인지”가 흔들립니다.)
- 선택지 A: 가이드대로
@DisplayName("get_success_existingId")같은 포맷으로 통일(검색/정렬/일관성↑, 다만 가독성(한국어)↓)- 선택지 B: 한국어 문장형 유지 + 가이드 문구를 업데이트(현 가독성↑, 대신 규칙 기반 자동화/검사엔 불리)
Also applies to: 140-143, 180-183
backend/src/test/java/backend/mulkkam/intake/service/IntakeTargetAmountServiceTest.java (1)
37-37: ServiceTest로의 전환이 PR 목표와 잘 부합합니다.
ServiceIntegrationTest에서ServiceTest로 베이스 클래스가 변경되었고, 실제 DB와 의존성을 사용하는 integration test 방식을 유지하고 있습니다. 이는 PR 목표인 "DB-backed domain/service tests" 선호 방향과 정확히 일치합니다.장점:
- Mock을 최소화하고 실제 컴포넌트 간 상호작용을 테스트합니다.
- 리팩토링에 강한 테스트 구조를 제공합니다.
- ServiceTest 베이스 클래스를 통한 일관된 테스트 인프라 활용이 가능합니다.
backend/src/test/java/backend/mulkkam/intake/service/WeatherServiceUnitTest.java (1)
34-41: 예외 검증 방식 변경에 집중해주세요. Mock 유지는 외부 API 호출 때문에 합리적입니다.이 테스트는 실제 외부 API(openweathermap)를 호출하는 WeatherRestClient를 의존하므로, Mock 기반 unit test로 유지하는 것이 타당합니다. PR의 목표는 "mock-heavy tests 제거"가 아니라 "예외 검증 방식 단일화"(test: 예외 검증 단일화)이므로, 기존 리뷰 코멘트에서 언급한 ServiceTest 전환은 필요하지 않습니다.
다만 한 가지 검토할 점: 현재 exception assertion 패턴(assertThrows + assertEquals)은 JUnit 5 기준 표준이지만, 가독성 측면에서 AssertJ의 fluent API(assertThatThrownBy)가 더 선호되는 경향이 있습니다. 팀의 통일된 스타일이 무엇인지 확인하여 일관성을 유지해주세요. KakaoAuthServiceTest처럼 다른 테스트들이 어떤 예외 검증 방식을 사용하고 있는지 참고하면 좋을 것 같습니다.
backend/src/test/java/backend/mulkkam/notification/service/SuggestionNotificationServiceTest.java (1)
84-84: 테스트 메서드 명명 규칙이 코딩 가이드라인과 불일치합니다.코딩 가이드라인에 따르면 테스트 메서드명은
methodName_successOrError_condition형식을 따라야 하는데, 현재fail_byNonExistingSuggestionNotificationId로 작성되어 테스트 대상 메서드명(applyTargetAmount)이 누락되었습니다.올바른 형식:
void applyTargetAmount_fail_byNonExistingSuggestionNotificationId()이유:
- 메서드명을 접두사로 포함하면 어떤 메서드를 테스트하는지 즉시 파악 가능
- 테스트 리포트에서 실패 시 어느 기능의 테스트인지 명확히 확인 가능
- 리팩토링 시 메서드명 변경을 테스트명에도 반영하도록 유도
동일 파일 내 Line 56의
success_validInput도applyTargetAmount_success_validInput로 수정이 필요합니다.As per coding guidelines, 테스트 메서드명은 반드시 테스트 대상 메서드명으로 시작해야 합니다.
⛔ Skipped due to learnings
Learnt from: CR Repo: woowacourse-teams/2025-mul-kkam PR: 0 File: backend/code-style.md:0-0 Timestamp: 2025-12-15T16:41:54.690Z Learning: Applies to backend/**/*Test.java : Use DisplayName with format: methodName_successOrError_condition in test classesbackend/src/test/java/backend/mulkkam/intake/controller/IntakeHistoryControllerTest.java (1)
48-48: AtomicLong 사용은 현재 구조에서 문제되지 않습니다
oauthIdCounter는 각 테스트 클래스의 인스턴스 필드이므로 클래스 간 공유되지 않으며, 따라서 테스트 컨텍스트 공유로 인한 충돌 위험은 없습니다. 실제로 IntakeAmountControllerTest, FriendControllerTest, CupControllerTest 등 팀 전체에서 동일한 패턴을 사용하고 있으며, AtomicLong은 스레드 안전성을 제공하므로 필요한 경우 테스트 병렬 실행도 안전하게 지원합니다. 현재 구현은 적절합니다.Likely an incorrect or invalid review comment.
| private CupEmoji saveDefaultCupEmojisAndGetFirst() { | ||
| if (!cupEmojiRepository.findAll().isEmpty()) { | ||
| return cupEmojiRepository.findAll().getFirst(); | ||
| } | ||
| for (IntakeType intakeType : IntakeType.values()) { | ||
| DefaultCup.of(intakeType); | ||
| CupEmoji cupEmoji = new CupEmoji(defaultEmojiUrl); | ||
| cupEmoji.setEmojiType(intakeType, EmojiType.DEFAULT); | ||
| cupEmojiRepository.save(cupEmoji); | ||
| } | ||
| return cupEmojiRepository.findAll().getFirst(); | ||
| } |
There was a problem hiding this comment.
헬퍼 메서드의 조건부 로직이 테스트 격리를 저해할 수 있습니다
Line 74의 if (!cupEmojiRepository.findAll().isEmpty()) 조건문은 데이터베이스 상태에 따라 동작이 달라지므로, 테스트 실행 순서나 이전 테스트의 잔여 데이터에 의존하게 됩니다.
문제점:
- 첫 번째 실행 시와 두 번째 실행 시 다른 코드 경로를 따를 수 있음
- 테스트 간 격리가 약화되어 비결정적 동작 가능성
대안:
- 권장: 조건문을 제거하고 매번 새로 생성 (이미 존재하면 트랜잭션 롤백으로 정리)
- 메서드 시작 시 명시적으로
cupEmojiRepository.deleteAll()호출 후 생성 @Transactional+@Rollback조합으로 각 테스트마다 데이터 격리 보장
🔧 권장 수정안 (조건부 로직 제거)
private CupEmoji saveDefaultCupEmojisAndGetFirst() {
- if (!cupEmojiRepository.findAll().isEmpty()) {
- return cupEmojiRepository.findAll().getFirst();
- }
for (IntakeType intakeType : IntakeType.values()) {
DefaultCup.of(intakeType);
CupEmoji cupEmoji = new CupEmoji(defaultEmojiUrl);
cupEmoji.setEmojiType(intakeType, EmojiType.DEFAULT);
cupEmojiRepository.save(cupEmoji);
}
return cupEmojiRepository.findAll().getFirst();
}ControllerTest 베이스 클래스가 이미 트랜잭션 롤백을 지원한다면, 각 테스트마다 데이터가 자동으로 정리되므로 조건부 검사가 불필요합니다.
🤖 Prompt for AI Agents
In @backend/src/test/java/backend/mulkkam/cup/controller/CupControllerTest.java
around lines 73 - 84, The helper saveDefaultCupEmojisAndGetFirst currently
branches on cupEmojiRepository.findAll() which makes tests depend on DB state;
remove the conditional and always create the default emojis (loop over
IntakeType.values(), call DefaultCup.of(intakeType), new
CupEmoji(defaultEmojiUrl), setEmojiType(...), cupEmojiRepository.save(...)) and
then return cupEmojiRepository.findAll().getFirst(); alternatively, if you
prefer clearing state explicitly call cupEmojiRepository.deleteAll() at the
method start; ensure this method is used with the test transaction/rollback
setup so data is isolated per test.
backend/src/test/java/backend/mulkkam/intake/controller/IntakeHistoryControllerTest.java
Show resolved
Hide resolved
backend/src/test/java/backend/mulkkam/intake/service/IntakeHistoryServiceTest.java
Show resolved
Hide resolved
backend/src/test/java/backend/mulkkam/intake/service/IntakeHistoryServiceTest.java
Show resolved
Hide resolved
backend/src/test/java/backend/mulkkam/intake/service/IntakeHistoryServiceTest.java
Show resolved
Hide resolved
backend/src/test/java/backend/mulkkam/intake/service/IntakeHistoryServiceTest.java
Show resolved
Hide resolved
backend/src/test/java/backend/mulkkam/member/service/MemberServiceTest.java
Show resolved
Hide resolved
backend/src/test/java/backend/mulkkam/notification/service/NotificationServiceTest.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @backend/src/test/java/backend/mulkkam/common/auth/AuthenticationTest.java:
- Around line 92-105: The test success_request_reaches_controller_without_auth
in AuthenticationTest currently expects a 5xx which is brittle; instead mock the
Kakao client used by the /auth/kakao controller (add a @MockBean for the Kakao
client/service used in that controller, e.g., KakaoClient or KakaoService) and
stub it to return a successful response, then change the assertion to expect a
2xx (use status().is2xxSuccessful()) to prove the request reached the controller
without relying on external failures.
In @backend/src/test/java/backend/mulkkam/cup/domain/IntakeTypeTest.java:
- Around line 23-36: The nested test class FindByName contains a static provider
validIntakeTypes which causes a compile error because non-static @Nested classes
cannot hold static methods; fix by moving the provider method validIntakeTypes
into the outer test class IntakeTypeTest (make it private static
Stream<Arguments> validIntakeTypes() there) and keep the
@MethodSource("validIntakeTypes") on the parameterized test, or alternatively
convert validIntakeTypes into a non-static instance method and annotate the
nested class with @TestInstance(PER_CLASS) so the @MethodSource can reference
the instance provider.
🧹 Nitpick comments (6)
backend/src/test/java/backend/mulkkam/intake/domain/vo/TargetAmountTest.java (2)
14-14: 클래스 레벨@DisplayName누락
CupTargetAmountTest와 비교했을 때, 해당 클래스에는@DisplayName("CupAmount 도메인")형태의 클래스 레벨 어노테이션이 있지만,TargetAmountTest에는 누락되어 있습니다. 테스트 리포트에서 일관된 가독성을 위해 추가하는 것이 좋습니다.💡 개선 제안
+@DisplayName("TargetAmount 도메인") class TargetAmountTest {
1-9: 테스트 파일 패키지 위치와 테스트 대상 클래스 패키지 불일치현재 테스트 파일은
backend.mulkkam.intake.domain.vo패키지에 위치해 있으나, 테스트 대상인TargetAmount클래스는backend.mulkkam.member.domain.vo패키지에서 import되고 있습니다.일반적으로 테스트 클래스는 테스트 대상 클래스와 동일한 패키지 구조를 유지하는 것이 관례입니다. 이렇게 하면:
- 테스트 대상 클래스를 찾기가 용이해집니다.
- package-private 멤버에 접근할 수 있습니다.
- 코드베이스 탐색 시 혼란을 줄일 수 있습니다.
테스트 파일을
backend.mulkkam.member.domain.vo.TargetAmountTest로 이동하는 것을 고려해 주세요.backend/src/test/java/backend/mulkkam/cup/domain/collection/CupRanksTest.java (1)
14-47: 테스트 케이스가 명확하게 작성되어 있습니다.고유 우선순위와 중복 우선순위 케이스를 분리하여 CupRanks의 핵심 검증 로직을 잘 테스트하고 있습니다.
assertThrows+errorCode검증 패턴도CupRankTest와 일관성이 있어 좋습니다.다만, 코딩 가이드라인에서
@Nested를 사용하여 테스트 시나리오를 그룹화하도록 권장하고 있습니다. 현재는 테스트가 2개뿐이라 당장 필요성이 낮지만, 향후 테스트가 추가될 가능성을 고려하면@Nested구조를 미리 적용해두는 것이 일관성 측면에서 좋을 수 있습니다.♻️ @nested 적용 예시 (선택 사항)
class CupRanksTest { + @DisplayName("CupRanks 생성할 때") + @Nested + class Creation { + @DisplayName("모든 컵의 우선순위가 고유하면 생성에 성공한다") @Test void cup_ranks_with_unique_priorities_is_created() { // ... } @DisplayName("중복된 우선순위가 있으면 생성에 실패한다") @Test void cup_ranks_with_duplicate_priorities_is_invalid() { // ... } + } }장점: 다른 테스트 파일(CupRankTest 등)과 구조적 일관성 유지, 향후 swap/reorder 등 기능 추가 시 확장 용이
단점: 현재 테스트가 2개뿐이라 오버엔지니어링으로 느껴질 수 있음
팀의 판단에 따라 적용 여부를 결정해 주세요.
backend/src/test/java/backend/mulkkam/common/auth/AuthenticationTest.java (2)
21-31: 클래스 레벨@DisplayName추가를 권장합니다.테스트 리포트에서 이 테스트 클래스의 목적이 명확하게 드러나도록 클래스 레벨에
@DisplayName을 추가하면 좋겠습니다. 예:@DisplayName("인증/인가 통합 테스트").필드 주입과 전체 구조는 깔끔하게 잘 작성되어 있습니다.
♻️ 제안 코드
+@DisplayName("인증/인가 통합 테스트") class AuthenticationTest extends ControllerTest {
32-45: 헬퍼 메서드 구성이 좋습니다.
createMemberToken()과createAccountOnlyToken()으로 테스트 시나리오별 토큰 생성을 명확하게 분리한 점이 좋습니다.MemberFixtureBuilder를 활용한 것도 팀 컨벤션(Calorie/정팩메 스타일)에 잘 맞습니다.다만,
"oauthId","deviceUuid"같은 매직 스트링들은 테스트 클래스 상단에 상수로 추출하면 의도가 더 명확해지고, 추후 수정 시에도 유지보수가 쉬워집니다. 필수는 아니지만 고려해보시면 좋겠습니다.backend/src/test/java/backend/mulkkam/cup/domain/IntakeTypeTest.java (1)
19-21:@DisplayName포맷이 팀 가이드( methodName_successOrError_condition )와 불일치합니다.
지금처럼 한글 서술형도 읽기 좋지만, 코딩 가이드에 맞춰 통일하면 “검색/정렬/리뷰 기준”이 안정적으로 맞춰집니다.선택지:
- 옵션 A: 가이드 포맷으로 통일(장점: 일관성/검색성, 단점: 한글 설명력 약해질 수 있음)
- 옵션 B: 지금처럼 한글 유지하되, 포맷 규칙을 문서로 예외 허용(장점: 가독성, 단점: 파일마다 스타일이 갈릴 수 있음)
예시(옵션 A):
@DisplayName("findByName_success_caseInsensitive")@DisplayName("calculateHydration_success_typeRatio")Also applies to: 56-58
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
backend/src/test/java/backend/mulkkam/common/auth/AuthenticationTest.javabackend/src/test/java/backend/mulkkam/cup/domain/IntakeTypeTest.javabackend/src/test/java/backend/mulkkam/cup/domain/collection/CupRanksTest.javabackend/src/test/java/backend/mulkkam/cup/domain/vo/CupNicknameTest.javabackend/src/test/java/backend/mulkkam/cup/domain/vo/CupRankTest.javabackend/src/test/java/backend/mulkkam/intake/domain/vo/AchievementRateTest.javabackend/src/test/java/backend/mulkkam/intake/domain/vo/TargetAmountTest.java
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/src/test/java/backend/mulkkam/cup/domain/vo/CupNicknameTest.java
- backend/src/test/java/backend/mulkkam/intake/domain/vo/AchievementRateTest.java
🧰 Additional context used
📓 Path-based instructions (4)
backend/**/*Test.java
📄 CodeRabbit inference engine (backend/code-style.md)
backend/**/*Test.java: Use @DisplayName with format: methodName_successOrError_condition in test classes
Use @nested annotation for organizing test scenarios in test classes
Follow BDD pattern in tests: given (optional), when, then
Use Calorie/정팩메 style for test fixtures
For controller tests with authentication/authorization, use MockMVC without starting Tomcat; for service tests, use @SpringBootTest(WebEnvironment.MOCK)
Test JPQL and Query Method implementations and wrap assertions with assertSoftly
Files:
backend/src/test/java/backend/mulkkam/intake/domain/vo/TargetAmountTest.javabackend/src/test/java/backend/mulkkam/cup/domain/collection/CupRanksTest.javabackend/src/test/java/backend/mulkkam/common/auth/AuthenticationTest.javabackend/src/test/java/backend/mulkkam/cup/domain/vo/CupRankTest.javabackend/src/test/java/backend/mulkkam/cup/domain/IntakeTypeTest.java
backend/**/*.java
📄 CodeRabbit inference engine (backend/code-style.md)
backend/**/*.java: Use verb-starting method names without entity name, e.g., memberService.findById(Long id)
Avoid else statements; use alternative control flow patterns
Use one dot per line (method chaining rule)
Avoid abbreviations in code
Avoid getters and setters; use behavior-focused methods instead
Annotation order: logs, Lombok, Spring meta, Spring components (most important, placed last)
Add one blank line before class declaration
When method has 2 or more parameters, use line breaks for parameters (does not apply to method calls)
Do not use wildcard imports; apply IntelliJ code conventions for imports
Use method names: create, save, register, add for creation operations
Use method names: read for multiple results, get for always-existing values, find for Optional results
Use method names: update for full replacement (PUT), modify for partial updates (PATCH)
Use method names: delete, remove for deletion operations
Do not use final keyword on class or field declarations
Use uppercase with underscores for constant names, e.g., CONSTANT_NAME
Order class members and methods by access level: public, protected, private; arrange by call order
Files:
backend/src/test/java/backend/mulkkam/intake/domain/vo/TargetAmountTest.javabackend/src/test/java/backend/mulkkam/cup/domain/collection/CupRanksTest.javabackend/src/test/java/backend/mulkkam/common/auth/AuthenticationTest.javabackend/src/test/java/backend/mulkkam/cup/domain/vo/CupRankTest.javabackend/src/test/java/backend/mulkkam/cup/domain/IntakeTypeTest.java
backend/**/domain/**/*.java
📄 CodeRabbit inference engine (backend/code-style.md)
backend/**/domain/**/*.java: Do not use static factory methods; use new operator for domain and DTO creation
Use Value Objects (VO) for domains that require validation
Files:
backend/src/test/java/backend/mulkkam/intake/domain/vo/TargetAmountTest.javabackend/src/test/java/backend/mulkkam/cup/domain/collection/CupRanksTest.javabackend/src/test/java/backend/mulkkam/cup/domain/vo/CupRankTest.javabackend/src/test/java/backend/mulkkam/cup/domain/IntakeTypeTest.java
backend/**
⚙️ CodeRabbit configuration file
backend/**: - 1. 팀 및 공식 컨벤션, 가독성, 예외처리, 테스트/확장/유지보수성, 모듈화, API/DB/보안 설계 기준을 기반으로 리뷰해주세요.
- 최신 트렌드, 불필요한 로직, 클린코드, 리팩토링, 서비스/도메인 설계, 공통 예외 처리, 확장성도 함께 확인해주세요.
- 각 피드백은 문제점·대안·장단점을 짧고 논리적으로, 예시 코드가 있다면 간결히 포함해 주세요.
- 팀 내 스타일 통일성도 확인해주세요.
- 미작성한 테스트 코드 케이스가 있다면, 어떤 테스트가 필요한지 제안해주세요. (예: 컨트롤러는 인수 테스트, 나머지는 단위 테스트)
- 리뷰 남겨주는 부분은 해당 라인 범위의 코멘트에 작성해주세요.
Files:
backend/src/test/java/backend/mulkkam/intake/domain/vo/TargetAmountTest.javabackend/src/test/java/backend/mulkkam/cup/domain/collection/CupRanksTest.javabackend/src/test/java/backend/mulkkam/common/auth/AuthenticationTest.javabackend/src/test/java/backend/mulkkam/cup/domain/vo/CupRankTest.javabackend/src/test/java/backend/mulkkam/cup/domain/IntakeTypeTest.java
🧠 Learnings (6)
📓 Common learnings
Learnt from: CheChe903
Repo: woowacourse-teams/2025-mul-kkam PR: 739
File: backend/src/main/java/backend/mulkkam/auth/infrastructure/OauthJwtTokenHandler.java:56-64
Timestamp: 2025-09-10T04:51:38.314Z
Learning: CheChe903 사용자는 테스트 코드에서 BeforeEach를 활용해서 가독성을 향상시킬 수 있는 부분이 있으면 리뷰해달라고 요청했습니다. 앞으로 테스트 코드 리뷰 시 중복되는 객체 생성, 테스트 데이터 셋업, Mock 설정 등을 BeforeEach로 추출할 수 있는 리팩터링 기회를 적극적으로 제안해야 합니다.
Learnt from: CR
Repo: woowacourse-teams/2025-mul-kkam PR: 0
File: backend/code-style.md:0-0
Timestamp: 2025-12-15T16:41:54.690Z
Learning: Applies to backend/**/*Test.java : Use Calorie/정팩메 style for test fixtures
Learnt from: CR
Repo: woowacourse-teams/2025-mul-kkam PR: 0
File: backend/code-style.md:0-0
Timestamp: 2025-12-15T16:41:54.690Z
Learning: Applies to backend/**/*Test.java : For controller tests with authentication/authorization, use MockMVC without starting Tomcat; for service tests, use SpringBootTest(WebEnvironment.MOCK)
📚 Learning: 2025-12-15T16:41:54.690Z
Learnt from: CR
Repo: woowacourse-teams/2025-mul-kkam PR: 0
File: backend/code-style.md:0-0
Timestamp: 2025-12-15T16:41:54.690Z
Learning: Applies to backend/**/*Test.java : Use Nested annotation for organizing test scenarios in test classes
Applied to files:
backend/src/test/java/backend/mulkkam/intake/domain/vo/TargetAmountTest.java
📚 Learning: 2025-12-15T16:41:54.690Z
Learnt from: CR
Repo: woowacourse-teams/2025-mul-kkam PR: 0
File: backend/code-style.md:0-0
Timestamp: 2025-12-15T16:41:54.690Z
Learning: Applies to backend/**/*Test.java : Use Calorie/정팩메 style for test fixtures
Applied to files:
backend/src/test/java/backend/mulkkam/intake/domain/vo/TargetAmountTest.javabackend/src/test/java/backend/mulkkam/cup/domain/vo/CupRankTest.javabackend/src/test/java/backend/mulkkam/cup/domain/IntakeTypeTest.java
📚 Learning: 2025-12-15T16:41:54.690Z
Learnt from: CR
Repo: woowacourse-teams/2025-mul-kkam PR: 0
File: backend/code-style.md:0-0
Timestamp: 2025-12-15T16:41:54.690Z
Learning: Applies to backend/**/*Test.java : For controller tests with authentication/authorization, use MockMVC without starting Tomcat; for service tests, use SpringBootTest(WebEnvironment.MOCK)
Applied to files:
backend/src/test/java/backend/mulkkam/common/auth/AuthenticationTest.java
📚 Learning: 2025-09-10T04:46:55.219Z
Learnt from: CheChe903
Repo: woowacourse-teams/2025-mul-kkam PR: 739
File: backend/src/test/java/backend/mulkkam/auth/controller/AuthControllerTest.java:113-114
Timestamp: 2025-09-10T04:46:55.219Z
Learning: CheChe903 has instructed not to consider Android code compatibility when reviewing backend code in the woowacourse-teams/2025-mul-kkam repository. Focus only on backend code concerns and do not flag client-server DTO mismatches or Android-related issues.
Applied to files:
backend/src/test/java/backend/mulkkam/common/auth/AuthenticationTest.java
📚 Learning: 2025-12-15T16:41:54.690Z
Learnt from: CR
Repo: woowacourse-teams/2025-mul-kkam PR: 0
File: backend/code-style.md:0-0
Timestamp: 2025-12-15T16:41:54.690Z
Learning: Applies to backend/**/*Test.java : Test JPQL and Query Method implementations and wrap assertions with assertSoftly
Applied to files:
backend/src/test/java/backend/mulkkam/cup/domain/vo/CupRankTest.javabackend/src/test/java/backend/mulkkam/cup/domain/IntakeTypeTest.java
🧬 Code graph analysis (2)
backend/src/test/java/backend/mulkkam/intake/domain/vo/TargetAmountTest.java (1)
backend/src/test/java/backend/mulkkam/cup/domain/vo/CupTargetAmountTest.java (1)
DisplayName(13-48)
backend/src/test/java/backend/mulkkam/common/auth/AuthenticationTest.java (1)
backend/src/test/java/backend/mulkkam/support/fixture/member/MemberFixtureBuilder.java (1)
MemberFixtureBuilder(10-110)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (10)
backend/src/test/java/backend/mulkkam/intake/domain/vo/TargetAmountTest.java (1)
16-47: 테스트 구조 및 패턴 승인테스트 구현이 잘 되어 있습니다:
@ParameterizedTest와@ValueSource를 활용한 경계값 테스트가 적절합니다.- BDD 패턴(when/then)을 일관되게 따르고 있습니다.
assertThrows+assertThat을 사용한 예외 검증 패턴이CupTargetAmountTest와 일관됩니다.- 유효 범위(200~5000ml), 최솟값 미만, 최댓값 초과 케이스를 모두 커버하고 있습니다.
@DisplayName의 한글 설명이 테스트 의도를 명확히 전달합니다.backend/src/test/java/backend/mulkkam/cup/domain/vo/CupRankTest.java (3)
15-41: 테스트 구조가 잘 정리되어 있습니다.
@Nested를 활용해 Creation, Promote, Demote 시나리오를 명확하게 그룹화했고, BDD 패턴(given/when/then)을 일관되게 적용한 점이 좋습니다.sut(System Under Test) 네이밍과assertThrows+errorCode검증 방식도 예외 테스트의 가독성을 높여주네요.다만 코딩 가이드라인에서
@DisplayName형식을methodName_successOrError_condition으로 권장하고 있는데, 현재는 한글 설명 기반으로 작성되어 있습니다. 팀 내에서 한글@DisplayName을 사용하기로 합의된 사항이라면 문제없지만, 그렇지 않다면 통일성 검토가 필요할 수 있습니다.팀 컨벤션에서
@DisplayName에 한글 설명을 사용하는 것이 허용되는지 확인해 주세요.
43-75: 승격(Promote) 시나리오 테스트가 적절합니다.2위/3위 컵의 승격 성공 케이스와 1위 컵의 승격 실패 케이스를
@ParameterizedTest와 단위 테스트로 분리하여 잘 커버하고 있습니다. 경계값(1위, 2위, 3위)을 모두 테스트하는 점도 좋습니다.
77-109: 강등(Demote) 시나리오 테스트도 대칭적으로 잘 구성되어 있습니다.Promote 테스트와 대칭 구조로 작성되어 일관성이 있고, 3위 컵의 강등 불가 케이스까지 커버하고 있어 경계 조건 테스트가 충실합니다.
backend/src/test/java/backend/mulkkam/cup/domain/collection/CupRanksTest.java (2)
16-28: 성공 케이스 테스트가 간결하게 작성되었습니다.
Map.of()를 사용한 인라인 픽스처 생성이 테스트 의도를 명확하게 전달합니다.assertThatCode(...).doesNotThrowAnyException()패턴도 생성 성공 검증에 적합합니다.
31-47: 예외 케이스 테스트가 적절합니다.중복 우선순위(2L과 3L 모두 rank 2) 케이스를 통해 검증 로직이 제대로 동작하는지 확인하고 있습니다.
DUPLICATED_CUP_RANKS에러 코드까지 검증하여 예외의 정확성을 보장하는 점이 좋습니다.backend/src/test/java/backend/mulkkam/common/auth/AuthenticationTest.java (2)
51-74: 테스트 메서드 네이밍 컨벤션 확인이 필요합니다.코딩 가이드라인에서는
methodName_successOrError_condition형식을 권장하고 있는데, 현재는fail_returns_401_when_no_token형식을 사용하고 있습니다.두 가지 선택지가 있습니다:
- 가이드라인 준수:
getMembers_fail_noToken,getMembers_fail_invalidToken,getMembers_success_validToken- 현재 방식 유지: 현재 네이밍도 의미가 명확하고
@DisplayName이 한글로 잘 설명하고 있어, 팀 내 합의가 있다면 유지 가능개인적으로는
@DisplayName이 충분히 설명적이라 현재 방식도 괜찮아 보이지만, 팀 컨벤션 통일 차원에서 확인해보시기 바랍니다.
77-90: 인증(Authentication)과 인가(Authorization) 분리가 잘 되어 있습니다.
RequireAuthentication에서는 토큰 유무/유효성을,RequireMemberLevel에서는 권한 수준을 검증하는 구조가 명확합니다. 온보딩 미완료 계정(Account만 존재)에 대한 403 응답 테스트도 적절합니다.backend/src/test/java/backend/mulkkam/cup/domain/IntakeTypeTest.java (2)
44-53: 예외 케이스에서 errorCode까지 검증한 건 리팩터링 내성이 좋아졌습니다.
assertThrows로 예외를 잡고(Line 48-50),NOT_FOUND_INTAKE_TYPE까지 확인(Line 52)해서 “실패 원인”이 명확해졌네요.자잘한 선택지(취향 영역):
isEqualTo대신 errorCode가 enum이면isSameAs가 의도를 더 또렷하게 줄 수는 있습니다(동작 동일).
60-77: 수분량 파라미터 케이스 확장은 좋고, 반올림 규칙 경계값만 한 번 더 확인해두면 안전합니다.
150 → 143(Line 66)은142.5를 어떤 규칙으로 처리하는지(반올림/올림/버림)까지 암묵적으로 고정하는 테스트라, 도메인 정책이 “반올림”이 맞는지만 팀 합의가 되어 있으면 아주 탄탄합니다.추가로 고려할 만한 케이스(정책이 있다면):
0ml입력 처리(0 반환 vs 예외)- 음수 입력 처리(예외가 맞는지)
| @DisplayName("인증이 필요 없는 엔드포인트면") | ||
| @Nested | ||
| class NoAuthenticationRequired { | ||
|
|
||
| @DisplayName("인증 없이도 요청은 컨트롤러까지 도달한다.") | ||
| @Test | ||
| void success_request_reaches_controller_without_auth() throws Exception { | ||
| // 카카오 API 호출 실패로 500이 나오지만, 인증 필터는 통과한 것 | ||
| mockMvc.perform(post("/auth/kakao") | ||
| .contentType("application/json") | ||
| .content("{\"kakaoAccessToken\": \"test\", \"deviceUuid\": \"uuid\"}")) | ||
| .andExpect(status().is5xxServerError()); | ||
| } | ||
| } |
There was a problem hiding this comment.
5xx 응답을 기대하는 테스트는 취약할 수 있습니다.
현재 테스트의 의도(인증 필터를 통과해서 컨트롤러까지 도달하는지 확인)는 이해되지만, 5xx 응답을 기대하는 방식은 몇 가지 문제가 있습니다:
- 외부 의존성: Kakao API 호출 실패에 의존하므로, 테스트 환경에서 Kakao 클라이언트가 mock 처리되면 테스트 결과가 달라질 수 있습니다.
- 의도 불명확: 5xx는 "컨트롤러 도달"보다 "서버 오류"를 의미하므로, 테스트 실패 시 원인 파악이 어렵습니다.
- 거짓 양성: 다른 이유로 5xx가 발생해도 테스트가 통과합니다.
대안 제안:
- 인증이 필요 없는 별도의 테스트용 엔드포인트(예: health check)를 사용하거나
@MockBean으로 Kakao 클라이언트를 mock하여 정상 응답(2xx)을 검증하거나- 최소한
is5xxServerError()대신status().isInternalServerError()로 구체화
현재 방식이 실용적인 절충안일 수 있지만, 장기적으로는 개선이 필요해 보입니다.
🤖 Prompt for AI Agents
In @backend/src/test/java/backend/mulkkam/common/auth/AuthenticationTest.java
around lines 92 - 105, The test success_request_reaches_controller_without_auth
in AuthenticationTest currently expects a 5xx which is brittle; instead mock the
Kakao client used by the /auth/kakao controller (add a @MockBean for the Kakao
client/service used in that controller, e.g., KakaoClient or KakaoService) and
stub it to return a successful response, then change the assertion to expect a
2xx (use status().is2xxSuccessful()) to prove the request reached the controller
without relying on external failures.
There was a problem hiding this comment.
@CheChe903 controller는 통과햇다는 테스트 코드를 작성한 이유에 대해 들어보고 싶어요 ! 또한 토끼가 말한 것 처럼 500에러로의 검증은 포괄적이여보이기도 해요
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| private static Stream<Arguments> validIntakeTypes() { | ||
| return Stream.of( | ||
| Arguments.of("WATER", IntakeType.WATER), | ||
| Arguments.of("water", IntakeType.WATER), | ||
| Arguments.of("Water", IntakeType.WATER), | ||
| Arguments.of("COFFEE", IntakeType.COFFEE), | ||
| Arguments.of("coffee", IntakeType.COFFEE) | ||
| ); | ||
| } | ||
|
|
||
| @DisplayName("대소문자 구분 없이 적절한 타입을 찾는다") | ||
| @ParameterizedTest | ||
| @MethodSource("testCasesForSuccess_ignoreCasesOfName") | ||
| void success_ignoreCasesOfName( | ||
| String input, | ||
| IntakeType expected | ||
| ) { | ||
| @DisplayName("대소문자 구분 없이 음용 타입을 찾을 수 있다") | ||
| @ParameterizedTest(name = "\"{0}\" → {1}") | ||
| @MethodSource("validIntakeTypes") | ||
| void intake_type_is_found_case_insensitively(String input, IntakeType expected) { |
There was a problem hiding this comment.
@Nested 비정적 내부 클래스에 static 메서드 선언은 컴파일 에러입니다 (테스트가 아예 안 돕니다).
현재 FindByName는 비정적 내부 클래스인데(Line 21), 그 안에 private static Stream<Arguments> validIntakeTypes()(Line 23)가 있어 Java 문법 위반입니다. JUnit 이슈가 아니라 “컴파일 단계”에서 막힐 확률이 큽니다.
대안은 2가지가 깔끔합니다.
- 옵션 A(추천): provider를 바깥 테스트 클래스(최상위)로 이동 → 문법/가독성 모두 무난
- 옵션 B: provider를 인스턴스 메서드로 바꾸고
@TestInstance(PER_CLASS)를 붙여 non-static method source 허용 → 클래스 생명주기 변경이 들어가서 팀 컨벤션에 따라 호불호
✅ 옵션 A 제안 diff (provider를 IntakeTypeTest로 이동)
class IntakeTypeTest {
+ private static Stream<Arguments> validIntakeTypes() {
+ return Stream.of(
+ Arguments.of("WATER", IntakeType.WATER),
+ Arguments.of("water", IntakeType.WATER),
+ Arguments.of("Water", IntakeType.WATER),
+ Arguments.of("COFFEE", IntakeType.COFFEE),
+ Arguments.of("coffee", IntakeType.COFFEE)
+ );
+ }
+
@DisplayName("음용 타입 조회할 때")
@Nested
class FindByName {
-
- private static Stream<Arguments> validIntakeTypes() {
- return Stream.of(
- Arguments.of("WATER", IntakeType.WATER),
- Arguments.of("water", IntakeType.WATER),
- Arguments.of("Water", IntakeType.WATER),
- Arguments.of("COFFEE", IntakeType.COFFEE),
- Arguments.of("coffee", IntakeType.COFFEE)
- );
- }
@DisplayName("대소문자 구분 없이 음용 타입을 찾을 수 있다")
@ParameterizedTest(name = "\"{0}\" → {1}")
@MethodSource("validIntakeTypes")
void intake_type_is_found_case_insensitively(String input, IntakeType expected) {📝 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.
| private static Stream<Arguments> validIntakeTypes() { | |
| return Stream.of( | |
| Arguments.of("WATER", IntakeType.WATER), | |
| Arguments.of("water", IntakeType.WATER), | |
| Arguments.of("Water", IntakeType.WATER), | |
| Arguments.of("COFFEE", IntakeType.COFFEE), | |
| Arguments.of("coffee", IntakeType.COFFEE) | |
| ); | |
| } | |
| @DisplayName("대소문자 구분 없이 적절한 타입을 찾는다") | |
| @ParameterizedTest | |
| @MethodSource("testCasesForSuccess_ignoreCasesOfName") | |
| void success_ignoreCasesOfName( | |
| String input, | |
| IntakeType expected | |
| ) { | |
| @DisplayName("대소문자 구분 없이 음용 타입을 찾을 수 있다") | |
| @ParameterizedTest(name = "\"{0}\" → {1}") | |
| @MethodSource("validIntakeTypes") | |
| void intake_type_is_found_case_insensitively(String input, IntakeType expected) { | |
| @DisplayName("대소문자 구분 없이 음용 타입을 찾을 수 있다") | |
| @ParameterizedTest(name = "\"{0}\" → {1}") | |
| @MethodSource("validIntakeTypes") | |
| void intake_type_is_found_case_insensitively(String input, IntakeType expected) { |
🤖 Prompt for AI Agents
In @backend/src/test/java/backend/mulkkam/cup/domain/IntakeTypeTest.java around
lines 23 - 36, The nested test class FindByName contains a static provider
validIntakeTypes which causes a compile error because non-static @Nested classes
cannot hold static methods; fix by moving the provider method validIntakeTypes
into the outer test class IntakeTypeTest (make it private static
Stream<Arguments> validIntakeTypes() there) and keep the
@MethodSource("validIntakeTypes") on the parameterized test, or alternatively
convert validIntakeTypes into a non-static instance method and annotate the
nested class with @TestInstance(PER_CLASS) so the @MethodSource can reference
the instance provider.
| @Nested | ||
| class UpdateRanks { | ||
|
|
||
| @BeforeEach |
| void highest_priority_cup_cannot_be_promoted() { | ||
| // given | ||
| CupRank originRank = new CupRank(1); | ||
| CupRank sut = new CupRank(1); |
There was a problem hiding this comment.
변수명 네이밍 한번 확인해주시면 좋을 것 같습니다~ sut 을 봤을 때 무슨 의미인지 한번에 이해되지 않는 것 같아서요!
|
고생했어요~ 체체 기존 프로젝트 테스트 방식에서 테스트 원칙을 고려하여 전반적인 부분을 수정해준 것으로 확인했는데요. 이외에도 의도한 부분이 있지만 제가 놓친 부분이 있다면 알려주시면 감사하겟습니다 아 그리고, 변경된 방식이 프로젝트 테스트 컨벤션 문법과는 다르게 작성되어 있는 것 같아 이 부분만 한번 확인해주시면 좋을 것 같습니다. 이후 어푸 진행하겠습니다~! (Tmp은 무시해주시세여.. 좀 더 이야기해보고 싶은 부분이라 제가 좀 더 확인 후 다시 말씀 드리겠습니다) |
2 participants
🔗 관련 이슈
물깜 테스트 아키텍처 가이드
📋 목차
1. 테스트 철학
핵심 원칙
테스트 우선순위 (충돌 시 상위 우선):
금지 사항
2. 테스트 피라미드
현재 상태 평가
새 테스트 작성 체크리스트
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.