-
Notifications
You must be signed in to change notification settings - Fork 3
[DDING-000] Banner Entity, Service 테스트코드 작성 및 리팩토링 #353
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테스트 픽스처 클래스 추가(BannerFixture) 및 기존 픽스처(FileMetaDataFixture) 개선과 함께, FixtureMonkey 기반 테스트를 dedicated 픽스처로 마이그레이션합니다. 배너 도메인의 JPA 테스트 추가 및 여러 테스트에서 메서드 호출 업데이트가 포함됩니다. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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 (4)
src/test/java/ddingdong/ddingdongBE/domain/banner/repository/BannerRepositoryTest.java (1)
26-38: Soft delete 검증 시 영속성 컨텍스트 캐시 주의
delete()호출 후 바로findById()를 수행하면 1차 캐시에서 조회될 수 있어 실제 DB의 soft delete 동작을 정확히 검증하지 못할 수 있습니다.EntityManager.flush()및clear()를 호출하여 영속성 컨텍스트를 초기화한 후 조회하는 것이 더 정확한 검증입니다.🔎 제안하는 수정 사항
DataJpaTestSupport에EntityManager가 주입되어 있다면 다음과 같이 수정할 수 있습니다:+ @Autowired + private EntityManager entityManager; + @DisplayName("Banner 삭제 시 soft delete 되어 조회에서 제외된다.") @Test void softDeleteBanner() { // given User savedUser = userRepository.save(UserFixture.createGeneralUser()); - Banner banner = bannerRepository.save(BannerFixture.createBanner(savedUser)); // when bannerRepository.delete(banner); + entityManager.flush(); + entityManager.clear(); // then Optional<Banner> found = bannerRepository.findById(banner.getId()); assertThat(found).isEmpty(); }src/test/java/ddingdong/ddingdongBE/domain/banner/service/FacadeUserBannerServiceImplTest.java (1)
47-52: 메서드 이름이 실제 동작과 맞지 않음
@BeforeEach로 각 테스트 전에 실행되므로 "tearDown"보다는 "setUp" 또는 "cleanUp"이 더 적절합니다. 일반적으로 "tearDown"은@AfterEach에서 사용하는 네이밍 컨벤션입니다.🔎 제안하는 수정 사항
@BeforeEach - void tearDown() { + void setUp() { fileMetaDataRepository.deleteAllInBatch(); bannerRepository.deleteAllInBatch(); userRepository.deleteAllInBatch(); }src/test/java/ddingdong/ddingdongBE/domain/banner/service/FacadeAdminBannerServiceImplTest.java (2)
50-55: 메서드 이름이 실제 동작과 맞지 않음
FacadeUserBannerServiceImplTest와 동일하게,@BeforeEach에서 실행되므로 "tearDown"보다는 "setUp" 또는 "cleanUp"이 더 적절합니다.🔎 제안하는 수정 사항
@BeforeEach - void tearDown() { + void setUp() { fileMetaDataRepository.deleteAllInBatch(); bannerRepository.deleteAllInBatch(); userRepository.deleteAllInBatch(); }
199-206: 헬퍼 메서드를 FileMetaDataFixture로 이동 고려
createPendingFileMetaDataWithId는PENDING상태의FileMetaData를 생성하는 재사용 가능한 패턴입니다. 다른 테스트에서도 동일한 패턴이 필요할 수 있으므로FileMetaDataFixture에 추가하면 일관성이 높아집니다.🔎 FileMetaDataFixture에 추가하는 예시
// FileMetaDataFixture.java에 추가 public static FileMetaData pendingFileMetaData(UUID id) { return FileMetaData.builder() .id(id) .fileKey("test") .fileName("test") .fileStatus(FileStatus.PENDING) .build(); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/test/java/ddingdong/ddingdongBE/common/fixture/BannerFixture.javasrc/test/java/ddingdong/ddingdongBE/common/fixture/FileMetaDataFixture.javasrc/test/java/ddingdong/ddingdongBE/domain/banner/repository/BannerRepositoryTest.javasrc/test/java/ddingdong/ddingdongBE/domain/banner/service/FacadeAdminBannerServiceImplTest.javasrc/test/java/ddingdong/ddingdongBE/domain/banner/service/FacadeUserBannerServiceImplTest.java
🧰 Additional context used
🧬 Code graph analysis (2)
src/test/java/ddingdong/ddingdongBE/domain/banner/repository/BannerRepositoryTest.java (2)
src/test/java/ddingdong/ddingdongBE/common/fixture/BannerFixture.java (1)
BannerFixture(6-15)src/test/java/ddingdong/ddingdongBE/common/fixture/UserFixture.java (1)
UserFixture(6-61)
src/test/java/ddingdong/ddingdongBE/domain/banner/service/FacadeUserBannerServiceImplTest.java (3)
src/test/java/ddingdong/ddingdongBE/common/fixture/BannerFixture.java (1)
BannerFixture(6-15)src/test/java/ddingdong/ddingdongBE/common/fixture/FileMetaDataFixture.java (1)
FileMetaDataFixture(8-32)src/test/java/ddingdong/ddingdongBE/common/fixture/UserFixture.java (1)
UserFixture(6-61)
⏰ 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: build_and_push
🔇 Additional comments (6)
src/test/java/ddingdong/ddingdongBE/common/fixture/BannerFixture.java (1)
6-15: LGTM!간결하고 일관된 테스트 픽스처입니다. 다른 테스트 파일들에서 재사용되어 테스트 코드의 가독성과 유지보수성을 높여줍니다.
src/test/java/ddingdong/ddingdongBE/common/fixture/FileMetaDataFixture.java (1)
20-31: LGTM!기존
formFileMetaData보다 유연한 팩토리 메서드를 추가하여 다양한DomainType과 파일 정보를 지정할 수 있게 되었습니다. 배너 관련 테스트에서 web/mobile 이미지 메타데이터를 구분하여 생성하는 데 적절합니다.src/test/java/ddingdong/ddingdongBE/domain/banner/service/FacadeUserBannerServiceImplTest.java (2)
82-146: URL 매핑 검증 테스트 구조 적절함PR 설명에서 언급된 우려처럼, 이 테스트는 구현 세부사항(특정 mock 객체 인스턴스)에 의존합니다. 그러나 배너별로 올바른 이미지 URL이 매핑되는지 검증하는 것은 중요한 비즈니스 로직이므로 현재 접근 방식은 합리적입니다.
향후 유지보수를 위해, mock 객체 대신 실제 URL 값을 비교하는 방식으로 전환하면 구현 세부사항에 대한 의존도를 줄일 수 있습니다.
148-152: LGTM!
Stream.generate를 활용한 헬퍼 메서드가 간결하고 재사용 가능합니다.src/test/java/ddingdong/ddingdongBE/domain/banner/service/FacadeAdminBannerServiceImplTest.java (2)
57-97: LGTM!배너 생성 후
FileMetaData가PENDING에서COUPLED로 상태 전환되는지 정확히 검증합니다.tuple()을 사용한 다중 속성 추출이 가독성을 높여줍니다.
165-197: LGTM!배너 삭제 시 연관된
FileMetaData가DELETED상태로 변경되는 것을 정확히 검증합니다.DomainType과FileStatus를 함께 확인하여 web/mobile 이미지 모두 올바르게 처리되는지 검증합니다.
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 (2)
src/test/java/ddingdong/ddingdongBE/domain/banner/service/FacadeUserBannerServiceImplTest.java (1)
82-146: URL 매핑 검증이 구현 디테일에 다소 의존적입니다.PR 설명에서 언급하신 우려가 타당합니다. 현재 테스트는
isSameAs()를 사용하여 모킹된 객체의 동일성(identity)을 검증하고 있어, 서비스 내부의 매핑 메커니즘에 강하게 결합되어 있습니다.예를 들어, 서비스가 나중에 캐싱이나 지연 로딩을 추가하면 이 테스트가 실패할 수 있습니다.
더 행위 중심적인 접근 방식을 고려해보세요:
- 모킹된
UploadedFileUrlAndNameQuery객체에 실제 URL 값을 반환하도록 설정- 결과의 각 배너에서 올바른 URL 값이 반환되는지 검증
다만, 현재 테스트도 배너별 이미지 매핑이 올바르게 되는지는 검증하고 있으며, 팀에서 이 수준의 검증이 필요하다고 판단했다면 유지하는 것도 합리적입니다.
🔎 대안적 접근 방식 예시
- UploadedFileUrlAndNameQuery b1WebQuery = mock(UploadedFileUrlAndNameQuery.class); - UploadedFileUrlAndNameQuery b1MobileQuery = mock(UploadedFileUrlAndNameQuery.class); - UploadedFileUrlAndNameQuery b2WebQuery = mock(UploadedFileUrlAndNameQuery.class); - UploadedFileUrlAndNameQuery b2MobileQuery = mock(UploadedFileUrlAndNameQuery.class); + UploadedFileUrlAndNameQuery b1WebQuery = new UploadedFileUrlAndNameQuery("https://b1-web-url", "b1-web.png"); + UploadedFileUrlAndNameQuery b1MobileQuery = new UploadedFileUrlAndNameQuery("https://b1-mobile-url", "b1-m.png"); + UploadedFileUrlAndNameQuery b2WebQuery = new UploadedFileUrlAndNameQuery("https://b2-web-url", "b2-web.png"); + UploadedFileUrlAndNameQuery b2MobileQuery = new UploadedFileUrlAndNameQuery("https://b2-mobile-url", "b2-m.png"); // ... stubbing remains same ... // then UserBannerListQuery banner1Result = result.stream() .filter(it -> it.id().equals(savedBanner1.getId())) .findFirst() .orElseThrow(); UserBannerListQuery banner2Result = result.stream() .filter(it -> it.id().equals(savedBanner2.getId())) .findFirst() .orElseThrow(); - assertThat(banner1Result.webImageUrlQuery()).isSameAs(b1WebQuery); - assertThat(banner1Result.mobileImageUrlQuery()).isSameAs(b1MobileQuery); + assertThat(banner1Result.webImageUrlQuery().url()).isEqualTo("https://b1-web-url"); + assertThat(banner1Result.mobileImageUrlQuery().url()).isEqualTo("https://b1-mobile-url"); - assertThat(banner2Result.webImageUrlQuery()).isSameAs(b2WebQuery); - assertThat(banner2Result.mobileImageUrlQuery()).isSameAs(b2MobileQuery); + assertThat(banner2Result.webImageUrlQuery().url()).isEqualTo("https://b2-web-url"); + assertThat(banner2Result.mobileImageUrlQuery().url()).isEqualTo("https://b2-mobile-url");src/test/java/ddingdong/ddingdongBE/domain/banner/service/FacadeAdminBannerServiceImplTest.java (1)
100-164: URL 매핑 검증 패턴이 UserBannerService 테스트와 일관성 있습니다.FacadeUserBannerServiceImplTest의
findAllMapsImagesByBannerId()테스트와 동일한 패턴으로 구현되어 일관성이 있습니다. 앞서 언급한 것처럼isSameAs()를 사용한 객체 동일성 검증이 구현 디테일에 의존적이지만, 두 서비스에서 일관되게 적용되었다면 팀의 의도적인 선택으로 보입니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/test/java/ddingdong/ddingdongBE/common/fixture/FileMetaDataFixture.javasrc/test/java/ddingdong/ddingdongBE/domain/banner/repository/BannerRepositoryTest.javasrc/test/java/ddingdong/ddingdongBE/domain/banner/service/FacadeAdminBannerServiceImplTest.javasrc/test/java/ddingdong/ddingdongBE/domain/banner/service/FacadeUserBannerServiceImplTest.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/test/java/ddingdong/ddingdongBE/domain/banner/repository/BannerRepositoryTest.java (2)
src/test/java/ddingdong/ddingdongBE/common/fixture/BannerFixture.java (1)
BannerFixture(6-15)src/test/java/ddingdong/ddingdongBE/common/fixture/UserFixture.java (1)
UserFixture(6-61)
⏰ 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: build_and_push
🔇 Additional comments (10)
src/test/java/ddingdong/ddingdongBE/domain/banner/repository/BannerRepositoryTest.java (1)
27-43: LGTM! 소프트 삭제 검증 테스트가 잘 작성되었습니다.소프트 삭제 후 조회에서 제외되는지 명확하게 검증하고 있으며,
flush()와clear()를 사용하여 영속성 컨텍스트를 정리한 후 데이터베이스 상태를 확인하는 것이 적절합니다.src/test/java/ddingdong/ddingdongBE/domain/banner/service/FacadeUserBannerServiceImplTest.java (4)
41-52: LGTM! 테스트 셋업이 올바르게 구성되었습니다.
FileMetaDataRepository와S3FileService모킹이 추가되었고,setUp()메서드에서 외래 키 순서를 고려하여 올바른 순서로 데이터를 정리하고 있습니다.
54-62: LGTM! 엣지 케이스를 잘 커버하는 테스트입니다.배너가 없는 경우 빈 리스트 반환을 검증하는 명확한 테스트입니다.
65-80: LGTM! 픽스처 기반 리팩토링이 잘 되었습니다.FixtureMonkey에서 전용 픽스처로 변경되어 테스트가 더 명확해졌으며, 정렬 검증도 올바르게 작동합니다.
148-152: LGTM! 깔끔한 헬퍼 메서드입니다.
Stream.generate()를 사용하여 지정된 개수의 배너를 생성하는 간결하고 재사용 가능한 구현입니다.src/test/java/ddingdong/ddingdongBE/domain/banner/service/FacadeAdminBannerServiceImplTest.java (3)
47-55: LGTM! 테스트 설정이 개선되었습니다.
@MockitoBean을 사용한 S3FileService 모킹이 추가되었고,setUp()에서 배치 삭제로 변경되어 더 간결해졌습니다.
57-98: LGTM! 배너 생성 테스트가 강화되었습니다.픽스처 기반으로 리팩토링되었고, FileStatus 전환(PENDING → COUPLED)을 명확히 검증하여 테스트 커버리지가 향상되었습니다.
166-198: LGTM! 배너 삭제의 연쇄 효과를 잘 검증합니다.배너 삭제 시 연관된 FileMetaData의 상태가 DELETED로 변경되고 DomainType이 보존되는지 명확히 검증하여, 삭제 로직의 부수 효과를 적절히 테스트하고 있습니다.
src/test/java/ddingdong/ddingdongBE/common/fixture/FileMetaDataFixture.java (2)
21-31: LGTM! 유연한 픽스처 헬퍼 메서드입니다.도메인 타입, 파일 키, 파일명을 매개변수화하면서 FileStatus는 COUPLED로 고정하여, 연결된 파일 메타데이터를 생성하는 일반적인 케이스를 잘 지원합니다.
33-40: LGTM! FileStatus 중심 테스트를 위한 간소화된 헬퍼입니다.FileStatus별 시나리오 테스트를 위해 최소한의 매개변수만 받는 간결한 헬퍼 메서드로, create 테스트에서 PENDING 상태 메타데이터를 생성하는 데 적절히 사용되고 있습니다.
Seooooo24
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.
고생하셨습니다!! 코멘트 확인 후 답변 남겨주시면 감사하겠습니다!
| given(s3FileService.getUploadedFileUrlAndName("b1-web-key", "b1-web.png")).willReturn( | ||
| b1WebQuery); | ||
| given(s3FileService.getUploadedFileUrlAndName("b1-m-key", "b1-m.png")).willReturn( | ||
| b1MobileQuery); | ||
| given(s3FileService.getUploadedFileUrlAndName("b2-web-key", "b2-web.png")).willReturn( | ||
| b2WebQuery); | ||
| given(s3FileService.getUploadedFileUrlAndName("b2-m-key", "b2-m.png")).willReturn( | ||
| b2MobileQuery); |
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.
혹시 웹키와 파일명을 b1Web, b1Mobile, b2Web, b2Mobile 객체에서 getter로 꺼내오지 않고 문자열로 직접 입력하신 이유가 있을까요?
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.
어떤 key/name으로 S3가 호출돼야 하는지 바로 드러내려고 문자열을 하드코딩했습니다. 하지만 말씀하신 Getter를 사용하면 Fixture 생성 로직등의 변경에도 유지보수성이 좋아질 거 같아 수정하였습니다.
| .findFirst() | ||
| .orElseThrow(); | ||
|
|
||
| assertThat(banner1Result.webImageUrlQuery()).isSameAs(b1WebQuery); |
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.
여기서 isEqualTo()가 아닌 isSameAs()를 쓰신 이유가 객체 내부에 equals() 메소드가 없어서가 맞을까요? 그런 경우에 테스트 코드에서 usingRecursiveComparison().isEqualTo()를 사용해서 비교하던데, 어떻게 생각하시는지 궁금합니다! 혹시 다른 이유로 isSameAs를 사용하셨다면 말씀해주시면 감사하겠습니다!!
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.
기존에는 S3FileService가 반환한 객체가 결과 DTO에 그대로 들어갔는지를 확인하려고 isSameAs를 사용했습니다. 이 방식은 배너별 key/name 매핑이 틀리면 스텁이 맞지 않아 테스트가 바로 실패하므로 매핑 오류를 강하게 잡을 수 있다고 생각했습니다.
UserBannerListQuery의 UploadedFileUrlAndNameQuery를 찾아보니 record라 값 기반 equals가 제공되므로, 결과 값이 올바른지만 검증하도록 isEqualTo로 변경했습니다.
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.
이해했습니다. 감사합니다!
KoSeonJe
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.
고생하셨습니다~ 코멘트 확인해주세요
| public static FileMetaData fileMetaData(UUID id, Long entityId, DomainType domainType, | ||
| String fileKey, String fileName) { | ||
| return FileMetaData.builder() | ||
| .id(id) | ||
| .fileKey(fileKey) | ||
| .fileStatus(FileStatus.COUPLED) |
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.
p2)
coupled 상태 파일 메타데이터를 생성한다는 것을 메서드명에 표시해주면 좋을 것 같아요
다른 fixture에는 create와 같은 행위를 적어서 파일메타데이터도 같이 적어주면 좋을 것 같아요
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.
수정했습니다.
| @MockitoBean | ||
| private S3FileService s3FileService; |
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.
p3)
스프링 컨텍스트에 등록되는 빈이 변경될 때 새로 스프링 컨텍스트가 띄어지게 되는데,
이로 인해 테스트 속도가 느려질 수 있어요. 그래서 통일된 컨텍스트를 사용해야 하는데, 이는 추후에 고민해보죠!
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.
Fake구현체를 만들어서 TestConfig에 등록하는 방법이 있을 것 같습니다
| void setUp() { | ||
| fileMetaDataRepository.deleteAllInBatch(); | ||
| bannerRepository.deleteAllInBatch(); | ||
| userRepository.deleteAllInBatch(); |
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.
p2)
support 클래스에 정의된 셋업으로 초기화되지 않나요?
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.
삭제하도록 하겠습니다!
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 (2)
src/test/java/ddingdong/ddingdongBE/domain/banner/entity/BannerTest.java (1)
50-51: 중복 단언문 제거 고려Line 50에서
deletedAt이null이 아님을 이미 검증하였으므로, Line 51의deletedAt.toLocalDateTime()이null이 아님을 검증하는 것은 중복입니다.Timestamp.toLocalDateTime()은 non-nullTimestamp에서 항상 non-null 값을 반환합니다.🔎 제안된 수정
assertThat(deletedAt).isNotNull(); - assertThat(deletedAt.toLocalDateTime()).isNotNull();src/test/java/ddingdong/ddingdongBE/domain/banner/service/FacadeAdminBannerServiceImplTest.java (1)
100-164:FacadeUserBannerServiceImplTest와 중복되는 테스트 구조이 테스트의 설정 및 검증 로직이
FacadeUserBannerServiceImplTest.findAllMapsImagesByBannerId()와 거의 동일합니다. Admin/User 서비스의 동작이 다르므로 각각 테스트하는 것은 맞지만, 공통 설정 로직(FileMetaData 생성, Mock 설정)을 헬퍼 메서드나 별도 클래스로 추출하면 유지보수성이 향상될 수 있습니다. 현재 상태로도 문제는 없으니 참고용으로 남깁니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/test/java/ddingdong/ddingdongBE/common/fixture/FileMetaDataFixture.javasrc/test/java/ddingdong/ddingdongBE/domain/banner/entity/BannerTest.javasrc/test/java/ddingdong/ddingdongBE/domain/banner/service/FacadeAdminBannerServiceImplTest.javasrc/test/java/ddingdong/ddingdongBE/domain/banner/service/FacadeUserBannerServiceImplTest.javasrc/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormAnswerRepositoryTest.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-11-23T08:57:46.445Z
Learnt from: KoSeonJe
Repo: COW-dev/ddingdong-be PR: 188
File: src/main/java/ddingdong/ddingdongBE/domain/filemetadata/service/FileMetaDataServiceImpl.java:110-122
Timestamp: 2024-11-23T08:57:46.445Z
Learning: Soft delete를 수행하며 엔티티의 상태를 삭제 전에 업데이트할 때는 `entityManager.flush()` 호출이 필요하다. 이는 상태 변경이 삭제 작업 전에 반영되도록 보장합니다.
Applied to files:
src/test/java/ddingdong/ddingdongBE/domain/banner/entity/BannerTest.java
🧬 Code graph analysis (3)
src/test/java/ddingdong/ddingdongBE/domain/banner/service/FacadeAdminBannerServiceImplTest.java (3)
src/test/java/ddingdong/ddingdongBE/common/fixture/BannerFixture.java (1)
BannerFixture(6-15)src/test/java/ddingdong/ddingdongBE/common/fixture/FileMetaDataFixture.java (1)
FileMetaDataFixture(8-41)src/test/java/ddingdong/ddingdongBE/common/fixture/UserFixture.java (1)
UserFixture(6-61)
src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormAnswerRepositoryTest.java (1)
src/test/java/ddingdong/ddingdongBE/common/fixture/FileMetaDataFixture.java (1)
FileMetaDataFixture(8-41)
src/test/java/ddingdong/ddingdongBE/domain/banner/service/FacadeUserBannerServiceImplTest.java (3)
src/test/java/ddingdong/ddingdongBE/common/fixture/BannerFixture.java (1)
BannerFixture(6-15)src/test/java/ddingdong/ddingdongBE/common/fixture/FileMetaDataFixture.java (1)
FileMetaDataFixture(8-41)src/test/java/ddingdong/ddingdongBE/common/fixture/UserFixture.java (1)
UserFixture(6-61)
⏰ 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: build_and_push
🔇 Additional comments (12)
src/test/java/ddingdong/ddingdongBE/common/fixture/FileMetaDataFixture.java (3)
10-19: LGTM!메서드 이름에
create접두사를 추가하여 다른 fixture들과 일관성을 맞춘 좋은 변경입니다.
21-31: LGTM!
DomainType,fileKey,fileName을 파라미터로 받아 다양한 테스트 시나리오에서 유연하게 사용할 수 있는 팩토리 메서드입니다.
33-40:domainType과entityId미설정 확인 필요이 메서드는
domainType과entityId를 설정하지 않습니다. 테스트 대상 코드에서 이 필드들에 접근하는 경우null관련 문제가 발생할 수 있습니다. 의도된 설계라면 무시하셔도 됩니다.src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormAnswerRepositoryTest.java (1)
88-91: LGTM!
FileMetaDataFixture의 리네임된 메서드createFormFileMetaData를 일관되게 사용하도록 업데이트되었습니다.src/test/java/ddingdong/ddingdongBE/domain/banner/entity/BannerTest.java (1)
29-52: LGTM!Soft delete 동작을 검증하는 테스트가 잘 구성되었습니다. Native query를 사용하여 물리적 데이터 상태(
deleted_at컬럼)를 검증하는 방식이 적절합니다. Retrieved learnings에서 언급된 대로flush()호출을 통해 상태 변경이 삭제 작업 전에 반영되도록 보장하고 있습니다.src/test/java/ddingdong/ddingdongBE/domain/banner/service/FacadeUserBannerServiceImplTest.java (4)
47-55: LGTM!배너가 없을 때 빈 리스트를 반환하는지 검증하는 테스트가 추가되었습니다. 엣지 케이스를 명확하게 검증합니다.
75-141: LGTM!배너별 web/mobile URL 쿼리 매핑을 검증하는 테스트입니다. PR 목표에서 언급한 "구현 디테일에 과도하게 의존하지 않는지"에 대한 고민이 있었는데, 이 테스트는 서비스의 핵심 동작(배너별 이미지 매핑)을 검증하므로 적절한 수준의 테스트라고 판단됩니다. Mock 객체의 동일성(identity)이 아닌 반환값의 동등성(equality)을 검증하도록
isEqualTo를 사용한 점도 좋습니다.
143-147: LGTM!헬퍼 메서드를 통해 테스트 데이터 생성 로직을 추출하여 가독성이 향상되었습니다.
32-45: 테스트 격리는 부모 클래스TestContainerSupport에서 이미 처리되고 있으므로 추가 조치가 필요하지 않습니다.TestContainerSupport는@Transactional클래스 어노테이션과@BeforeEach delete()메서드를 통해 각 테스트 전에 데이터를 정리하고 트랜잭션을 롤백합니다. 별도의@BeforeEach메서드를 이 클래스에 추가할 필요가 없습니다.src/test/java/ddingdong/ddingdongBE/domain/banner/service/FacadeAdminBannerServiceImplTest.java (3)
50-55: LGTM!
@BeforeEach에서 FK 제약 조건을 고려한 올바른 순서(fileMetaData → banner → user)로 데이터를 정리하여 테스트 격리를 보장합니다.
166-198: LGTM!배너 삭제 시 연관된
FileMetaData의 상태가DELETED로 변경되는지 검증하는 테스트입니다.domainType과fileStatus를 함께 검증하여 올바른 파일들이 삭제 처리되었는지 확인합니다.
57-98: LGTM!배너 생성 테스트가 fixture 기반으로 잘 리팩토링되었습니다.
FileStatus.PENDING→FileStatus.COUPLED상태 전환을 검증하여 파일 메타데이터 연결 동작을 확인합니다.
🚀 작업 내용
🤔 고민했던 내용
💬 리뷰 중점사항
Summary by CodeRabbit
릴리스 노트
✏️ Tip: You can customize this high-level summary in your review settings.