-
Notifications
You must be signed in to change notification settings - Fork 0
[Refactor] 테스트 코드 @MockitoBean, 중복 코드 추상화 #37
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
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (3)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note
|
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.
Pull request overview
This PR refactors test code to eliminate duplication by introducing an AcceptanceTestSupport base class that consolidates common test setup, annotations, and mock beans. This allows for better context reuse across acceptance tests.
- Introduced
AcceptanceTestSupportabstract class with shared test configuration - Migrated all acceptance test classes to extend the new base class
- Changed mock bean dependency from concrete
FcmNotificationManagertoFestivalNotificationManagerinterface
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
AcceptanceTestSupport.java |
New abstract base class providing common test annotations, mock beans (Clock, FestivalNotificationManager, ShuffleManager), and RestAssured port setup |
| Multiple controller test files | Removed duplicated annotations, setup methods, and mock bean declarations by extending AcceptanceTestSupport |
FestabookApplicationTests.java |
Updated to extend AcceptanceTestSupport for context loading test |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
changuii
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.
LGTM
| import org.springframework.context.annotation.Import; | ||
| import org.springframework.test.context.bean.override.mockito.MockitoBean; | ||
|
|
||
| @Import(TestSecurityConfig.class) |
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.
TestSecurityConfig는 테스트 용으로 따로 만든 security filter입니다.
테스트용 api 엔드포인트의 보안을 뚫어줍니다.
|
FestivalNotificationManager 바꾼거 좋습니다. |
#️⃣ 이슈 번호
🛠️ 작업 내용
Notion 문서
🙇🏻 중점 리뷰 요청
SecurityConfigTest클래스안에@Import({TestSecurityConfig.class, SecurityConfig.class})코드가 존재했습니다.SecurityConfig.class는 왜 필요해서 선언했는 지 모르겠습니다. 이미 컨텍스트 로딩할 때 적용되고 있었습니다. 지금은 삭제한 상황이구요!기존 @MockitoBean 사용할 때 아래와 같이
FcmNotificationManager인터페이스가 아닌 구현체에 의존하고 있었습니다. 이를FestivalNotificationManager인터페이스로 변경했는데, 문제가 없겠죠? 🤔📸 이미지 첨부 (Optional)