[test] 동아리 활동사진 이미지 다중업로드 기능 테스트코드를 작성한다#910
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning
|
| Cohort / File(s) | Summary |
|---|---|
CloudflareImageService 단위 테스트 backend/src/test/java/moadong/media/service/CloudFlareImageServiceTest.java, backend/src/test/java/moadong/media/service/CloudflareImageServiceUpdateFeedsLimitTest.java |
CloudFlareImageServiceTest: 파일명 검증(UNSUPPORTED_FILE_TYPE, FILE_NOT_FOUND), deleteFile 동작(null/올바르지 않은 URL 처리, 유효한 URL로 정상 삭제) 검증. CloudflareImageServiceUpdateFeedsLimitTest: 최대 피드 이미지 개수(MAX_FEED_COUNT=15) 초과 시 TOO_MANY_FILES 예외 발생 검증 |
Estimated code review effort
🎯 2 (Simple) | ⏱️ ~10분
- 테스트 코드만 추가된 변경으로 기존 기능에 대한 영향 최소화
- Mockito, Spring ReflectionTestUtils 등 표준 테스팅 도구 사용
- 각 테스트 케이스가 명확한 검증 목표를 가짐
Possibly related PRs
- [feature] cover image 업로드 및 삭제 기능 구현 #537: CloudflareImageService의 동작(S3 삭제, 파일 검증, 피드 제한) 및 테스트 관련 변경을 함께 다루고 있어 코드 수준의 연관성이 높음
Suggested reviewers
- alsdddk
- lepitaaar
- Zepelown
Pre-merge checks and finishing touches
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | You can run @coderabbitai generate docstrings to improve docstring coverage. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | PR 제목은 '동아리 활동사진 이미지 다중업로드 기능 테스트코드를 작성한다'로, 추가된 두 개의 테스트 클래스(CloudFlareImageServiceTest와 CloudflareImageServiceUpdateFeedsLimitTest)의 목적을 명확하게 설명하고 있습니다. |
| Linked Issues check | ✅ Passed | PR에서 추가된 테스트 코드들이 연관 이슈 MOA-403의 목표인 '동아리 활동사진 이미지 다중업로드 기능 테스트코드 작성'을 완벽하게 충족하고 있습니다. |
| Out of Scope Changes check | ✅ Passed | 모든 변경사항이 테스트 코드 작성에만 한정되어 있으며, 연관 이슈의 범위 내에서 이루어진 변경입니다. |
✨ Finishing touches
- 📝 Generate docstrings
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
test/#909-club-photos-multi-upload-test-MOA-403
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.
Test Results74 tests 71 ✅ 17s ⏱️ Results for commit 11cd9c6. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
backend/src/test/java/moadong/media/service/CloudFlareImageServiceTest.java (3)
27-49: 테스트 클래스/초기화 패턴 전반적으로 적절합니다
@Spy @InjectMocks와ReflectionTestUtils로 private 필드/init을 맞춰주는 패턴 덕분에 실제 환경과 거의 같은 상태에서CloudflareImageService를 exercise 하고 있어서 좋습니다.
다만 테스트 클래스/파일 이름이CloudFlareImageServiceTest로, 서비스명CloudflareImageService와 대소문자/철자가 살짝 다른데, IDE 검색/네비게이션을 생각하면CloudflareImageServiceTest로 통일하는 것도 고려해 볼 만합니다.
52-75: 파일명 검증 테스트가 명확하고 에러 코드 매핑도 잘 드러납니다잘못된 확장자와 빈 문자열에 대해 각각
UNSUPPORTED_FILE_TYPE,FILE_NOT_FOUND를 직접 검증해서 도메인 요구사항이 테스트에 잘 녹아 있습니다.
다만validateFileName이 private 메서드라ReflectionTestUtils.invokeMethod를 사용하고 있는데, 가능하다면 향후에는 이 검증이 노출되는 public API(예: 업로드 메서드) 단에서 같은 시나리오를 검증하도록 리팩터링하면 구현 변경에도 덜 민감한 테스트가 될 것 같습니다.
77-115:deleteFile방어 로직/정상 플로우 테스트 구성이 적절합니다
filePath == null인 경우와 view endpoint 접두어가 다른 경우에 S3deleteObject가 호출되지 않는 것을 검증한 부분이 실제 운영 시 오동작 방지에 도움이 될 것 같습니다.- 정상 URL 케이스에서
ArgumentCaptor<DeleteObjectRequest>로bucket과key를 정확히 검증한 것도 좋습니다. 특히 key 에서 도메인/프리픽스 제거가 원하는 대로 되는지 명확히 보장되네요.추가로 커버하고 싶다면,
filePath가 빈 문자열이거나 공백만 있는 경우 같은 엣지 케이스도 하나 정도 더 넣어두면 방어 범위가 조금 더 넓어질 것 같습니다(필수는 아니고 선택 사항으로 보입니다).backend/src/test/java/moadong/media/service/CloudflareImageServiceUpdateFeedsLimitTest.java (2)
32-57: 서비스 초기화 방식이 다른 테스트와 살짝 달라, 향후 변경 시 차이가 날 수 있습니다다른 클래스(
CloudFlareImageServiceTest)에서는viewEndpoint,bucketName을ReflectionTestUtils.setField로 주입하고init()까지 호출해 실제 빈과 유사하게 초기화하고 있는데, 여기서는 그런 설정 없이 바로updateFeeds를 호출하고 있습니다.
현재 구현이 업로드 개수 제한을 먼저 검사한다면 문제 없겠지만, 나중에updateFeeds내부에서 presigner/bucket 설정을 더 일찍 참조하도록 바뀌면 이 테스트만 NPE 등으로 깨질 여지가 있어, 동일한 초기화 패턴을 맞춰두는 것도 한 번 고려해 볼 만합니다.
59-75: 업로드 제한 시나리오를 잘 검증하지만, 테스트 데이터 표현을 조금 더 명확히 할 여지가 있습니다
Arrays.asList(new String[MAX_FEED_COUNT])로 길이만 맞춘 null 리스트를 만들고, 이를 기존feedImages와 업로드 파라미터 모두에 재사용하는 방식은 개수 제한 관점에서는 충분하지만, 나중에 구현이 각 URL 에 문자열 연산을 수행하면 null 때문에 다른 예외가 먼저 날 수도 있습니다.- 또한 현재는 "이미 MAX 개가 저장된 상태에서 MAX 개를 더 올리려다" 예외가 나는 시나리오라, 테스트 이름의 "MAX_FEED_COUNT 이상의 피드를 업로드" 와는 뉘앙스가 조금 다릅니다.
가독성과 의도를 더 분명히 하려면, 예를 들어:
- 기존
feedImages를MAX_FEED_COUNT개(혹은 0개)로 두고,- 업로드 리스트는
"image-0", "image-1", ...같은 dummy 값으로MAX_FEED_COUNT + 1개를 채워서"총 개수가 제한을 초과할 때" 를 직접적으로 표현하는 것도 고려해 볼 수 있습니다. 로직 자체는 지금도 잘 검증되고 있어서 반드시 바꿔야 하는 수준은 아닙니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
backend/src/test/java/moadong/media/service/CloudFlareImageServiceTest.java(1 hunks)backend/src/test/java/moadong/media/service/CloudflareImageServiceUpdateFeedsLimitTest.java(1 hunks)
alsdddk
left a comment
alsdddk
left a comment
There was a problem hiding this comment.
수고하셨어요!!
다음 스터디에서 테스트코드 네이밍 규칙에 대해서 정하는게 좋을 거 같네여
예를 들어, 파일 관련해서 영어 단어를 쓸 지, 'File_이름'을 'FILE_NAME'이나 '파일이름' 중에 통일 시킬지 같은 거요!
| @Mock | ||
| private S3Presigner s3Presigner; | ||
|
|
||
| private final int MAX_FEED_COUNT = 15; |
There was a problem hiding this comment.
프로덕션 코드 쪽 상수를 직접 참조하는건 어떨까요??
테스트 코드와 도메인 제약이 자동으로 동기화되도록 만드는 것도 유지보수 측면에서 좋을 거 같아요!
|
|
||
| //then | ||
| ArgumentCaptor<DeleteObjectRequest> captor = ArgumentCaptor.forClass(DeleteObjectRequest.class); | ||
| verify(s3Client).deleteObject(captor.capture()); |
There was a problem hiding this comment.
찾아보니 메서드 호출 시 전달된 인자를 가로채서 들고 있는 객체네요! 좋은 거 같아요~
4 participants
#️⃣연관된 이슈
📝작업 내용
이미지 업로드 서비스 중에서 피드 업로드 갯수 초과했을 때, 파일 이름이 잘못되었을 때, 파일 삭제할 때 관련해서 테스트 코드를 집중적으로 만들었습니다.
중점적으로 리뷰받고 싶은 부분(선택)
논의하고 싶은 부분(선택)
🫡 참고사항
Summary by CodeRabbit
릴리스 노트
✏️ Tip: You can customize this high-level summary in your review settings.