-
Notifications
You must be signed in to change notification settings - Fork 0
test: 앨범, 사진, 치즈네컷 테스트 작성 #94
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Walkthrough여러 서비스의 단위 테스트 및 동시성 벤치마크 테스트를 추가했습니다. AlbumServiceTest, Cheese4cutServiceTest, PhotoServiceTest의 새로운 테스트 클래스와 AlbumConcurrencyTest 동시성 테스트를 포함합니다. build.gradle에 H2 데이터베이스 테스트 의존성과 JVM 인자를 추가했습니다. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (5)
build.gradle (1)
61-71:benchmark태그와excludeTags설정으로 새 테스트들이 기본test태스크에서 모두 제외됩니다
tasks.named('test')에서excludeTags 'benchmark'를 설정했는데, 이번 PR에서 추가된AlbumServiceTest,PhotoServiceTest,Cheese4cutServiceTest,AlbumConcurrencyTest가 모두@Tag("benchmark")를 사용하고 있어서 Gradletest실행 시 전부 스킵됩니다.
성능/부하 테스트만 제외하고 일반 단위 테스트는 항상 돌리고 싶다면,
- 일반 단위 테스트 클래스에서는
@Tag("benchmark")를 제거하거나,- 성능/동시성 전용 테스트만 별도의 태그(예:
"load","concurrency"등)를 사용하고 그 태그만excludeTags로 필터링하는 방식으로 태그/필터 구성을 다시 한 번 정리하는 것을 추천드립니다.
현재 H2 test 의존성 추가와 JVM 옵션 자체는 무난해 보입니다.src/test/java/com/cheeeese/album/benchmark/AlbumConcurrencyTest.java (1)
22-91: 동시성 테스트의 의도(100명 입장)와 실제 검증 내용이 조금 어긋나 있습니다
FixtureFactory.createAlbum(maker.getId())가 현재 participant 정원을 4로 두고 있어서, 주석에서 말하는 “정원이 100명 이상인 앨범” 시나리오가 실제로는 구성되지 않습니다. 이 테스트를 진짜 100명 동시 입장 성능/정합성 벤치마크로 쓰고 싶다면,
- FixtureFactory의
createAlbum에 maxParticipant를 늘리거나,- 테스트에서
album에 대해 별도 setter/업데이트 메서드로 정원을 크게 올린 뒤 저장하는 쪽으로 맞추는 편이 좋습니다.또한 현재 검증은
updatedAlbum.getCurrentParticipant() == 1 + successCount만 확인해서, 극단적으로 모든 입장이 실패해도(successCount == 0, 참여자도 1명만) 테스트가 통과할 수 있습니다. 최소한successCount가 0이 아닌지(또는 정원과 일치하는지)도 함께 assert 하면 동시성 이슈를 더 잘 잡을 수 있습니다.부가적으로,
latch.await()이후에executorService.shutdown()(또는shutdownNow()+awaitTermination)을 호출해 스레드 풀을 정리해 두면, 벤치마크를 반복 실행할 때 리소스 누수를 줄일 수 있습니다.전반적인 테스트 구조는 잘 잡혀 있어서, 위 수정들은 테스트 신뢰도를 높이기 위한 개선 사항 정도로 보입니다.
src/test/java/com/cheeeese/photo/PhotoServiceTest.java (1)
35-143: 단위 테스트에@Tag("benchmark")가 붙어 있어 Gradletest에서 실행되지 않습니다이 클래스는 순수 Mockito 기반 단위 테스트라서 일반 테스트처럼 항상 돌려도 부담이 크지 않은데,
build.gradle에서excludeTags 'benchmark'를 설정해둔 상태라 현재 구성대로면PhotoServiceTest는 기본test태스크에서 전부 스킵됩니다. 의도한 동작이 아니라면 태그를 제거하는 편을 권장합니다.예를 들어 다음처럼 수정할 수 있습니다:
-@ExtendWith(MockitoExtension.class) -@Tag("benchmark") -class PhotoServiceTest { +@ExtendWith(MockitoExtension.class) +class PhotoServiceTest {테스트 내용 자체는 Presigned URL 생성 성공/실패, 업로드 실패 보고 흐름까지 잘 커버하고 있어서, 태그만 정리하면 CI 커버리지에 바로 도움이 될 것 같습니다.
src/test/java/com/cheeeese/cheese4cut/Cheese4cutServiceTest.java (1)
38-175: Cheese4cut 단위 테스트에@Tag("benchmark")가 적용되어 기본 테스트 러너에서 제외됩니다이 파일의 테스트들은 전형적인 서비스 단위 테스트이며, 성능/부하 전용 테스트라기보다는 항상 실행되는 것이 더 자연해 보입니다. 하지만
build.gradle에서excludeTags 'benchmark'를 사용하고 있어서, 현재 설정대로라면 CI에서Cheese4cutServiceTest는 실행되지 않습니다.일반 단위 테스트로 돌리려면 태그를 제거하는 쪽을 추천합니다:
-@ExtendWith(MockitoExtension.class) -@Tag("benchmark") -class Cheese4cutServiceTest { +@ExtendWith(MockitoExtension.class) +class Cheese4cutServiceTest {또한
finalizeCheese4cut_Success에서photoRepository.findAllByIdInOrderByLikesDescCreatedDesc(photoIds)를 사용하도록 모킹한 부분은, 이전 PR에서 정의한 “클라이언트가 보낸 순서를 신뢰하지 않고 항상 좋아요/생성일 기준으로 재정렬한다”는 설계 의도와 잘 맞아서 유지하는 것이 좋아 보입니다. (Based on learnings)src/test/java/com/cheeeese/album/AlbumServiceTest.java (1)
41-195: AlbumService 단위 테스트도benchmark태그 때문에 Gradletest에서 스킵됩니다
AlbumServiceTest는 핵심 비즈니스 플로우(신규 입장, 재입장, 정원 초과 예외)를 잘 커버하는 단위 테스트인데, 클래스에@Tag("benchmark")가 붙어 있고build.gradle에서 이 태그를excludeTags로 필터링하고 있어서 기본test태스크에서 실행되지 않습니다. 성능 테스트가 아니라면 태그를 제거하는 편이 자연스럽습니다.예시 수정:
-@ExtendWith(MockitoExtension.class) -@Tag("benchmark") -class AlbumServiceTest { +@ExtendWith(MockitoExtension.class) +class AlbumServiceTest {세 개의 테스트 케이스 구성 자체는
enterAlbum주요 분기(신규 유저, 나갔다가 재입장, 정원 초과)를 잘 표현하고 있으므로, 태그만 정리하면 CI 상에서 바로 유용한 회귀 테스트 역할을 할 수 있을 것 같습니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
build.gradle(1 hunks)src/test/java/com/cheeeese/album/AlbumServiceTest.java(1 hunks)src/test/java/com/cheeeese/album/benchmark/AlbumConcurrencyTest.java(1 hunks)src/test/java/com/cheeeese/cheese4cut/Cheese4cutServiceTest.java(1 hunks)src/test/java/com/cheeeese/photo/PhotoServiceTest.java(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-13T12:56:22.161Z
Learnt from: dahyun24
Repo: Say-Cheeeese/BE PR: 58
File: src/main/java/com/cheeeese/cheese4cut/application/Cheese4cutService.java:149-156
Timestamp: 2025-11-13T12:56:22.161Z
Learning: In src/main/java/com/cheeeese/cheese4cut/application/Cheese4cutService.java, the finalizeCheese4cut method intentionally re-sorts photos using findAllByIdInOrderByLikesDescCreatedDesc(request.photoIds()) instead of preserving the client's requested order. This is a defensive measure to ensure photos are always ordered by likes (DESC) and creation time (DESC), regardless of what order the client sends, preventing incorrect ordering from client errors.
Applied to files:
src/test/java/com/cheeeese/photo/PhotoServiceTest.javasrc/test/java/com/cheeeese/cheese4cut/Cheese4cutServiceTest.java
📚 Learning: 2025-10-31T13:17:52.523Z
Learnt from: dahyun24
Repo: Say-Cheeeese/BE PR: 35
File: src/main/java/com/cheeeese/photo/application/PhotoService.java:46-52
Timestamp: 2025-10-31T13:17:52.523Z
Learning: In src/main/java/com/cheeeese/photo/application/PhotoService.java, the getRecentThumbnailUrls method intentionally returns only the first thumbnail URL when photos.size() < 5, rather than returning all available thumbnails. This is according to product requirements: 0 photos → empty list, 1-4 photos → single thumbnail (most recent), 5 photos → all 5 thumbnails.
Applied to files:
src/test/java/com/cheeeese/photo/PhotoServiceTest.java
zyovn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고하셧습니다
🔗 연관된 이슈
🚀 변경 유형
📝 작업 내용
📸 스크린샷
💬 리뷰 요구사항
📜 리뷰 규칙
Reviewer는 아래 P5 Rule을 참고하여 리뷰를 진행합니다.
P5 Rule을 통해 Reviewer는 Reviewee에게 리뷰의 의도를 보다 정확히 전달할 수 있습니다.
Summary by CodeRabbit
릴리스 노트
이번 업데이트에는 사용자 화면에 직접 영향을 주는 기능 변경 사항이 없습니다.
✏️ Tip: You can customize this high-level summary in your review settings.