-
Notifications
You must be signed in to change notification settings - Fork 2
test: 이벤트 참여 신청 정책 변경에 따른 테스트 수정 #1238
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
test: 이벤트 참여 신청 정책 변경에 따른 테스트 수정 #1238
Conversation
📝 WalkthroughWalkthrough테스트 코드에서 EventParticipation.createOnlineForRegistered / createOnlineForUnregistered 호출을 단일 팩토리 메서드 EventParticipation.createOnline으로 교체하고, Participant.of(...)를 통해 Participant를 생성해 첫 인자로 전달하도록 수정했습니다. 등록되지 않은 경우 Member는 null로 전달되며, 나머지 인자 순서는 새 시그니처에 맞춰 정렬되었습니다. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Possibly related PRs
Poem
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
|
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
kckc0608
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
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/com/gdschongik/gdsc/domain/event/domain/ParticipantRoleTest.java (1)
118-119: 표현 일관성: 기본 케이스는 UNPAID 대신 NONE이 더 자연스럽습니다.뒤풀이 미신청(NOT_APPLIED) 시 결제 상태는 미발생(NONE)이 의미상 더 정확합니다.
적용 예시:
- PaymentStatus.UNPAID, - PaymentStatus.UNPAID, + PaymentStatus.NONE, + PaymentStatus.NONE,src/test/java/com/gdschongik/gdsc/domain/event/application/EventParticipationServiceTest.java (1)
1109-1110: 중복 Participant.of(...)를 헬퍼로 추출해 중복 감소 + 가독성 향상여러 헬퍼에서 동일한 Participant 구성 로직이 반복됩니다. private 메서드로 추출하면 유지보수성이 좋아집니다.
대표 적용(diff — 1108~1111 인접 라인 기준):
- EventParticipation eventParticipation = EventParticipation.createOnline( - Participant.of(member.getName(), member.getStudentId(), member.getPhone()), + EventParticipation eventParticipation = EventParticipation.createOnline( + toParticipant(member),헬퍼 추가(파일 하단 임의 위치):
private Participant toParticipant(Member member) { return Participant.of(member.getName(), member.getStudentId(), member.getPhone()); }Also applies to: 1122-1123, 1134-1135, 1146-1147, 1158-1159
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/test/java/com/gdschongik/gdsc/domain/event/application/EventParticipationServiceTest.java(5 hunks)src/test/java/com/gdschongik/gdsc/domain/event/domain/ParticipantRoleTest.java(1 hunks)
⏰ 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-test
🔇 Additional comments (8)
src/test/java/com/gdschongik/gdsc/domain/event/domain/ParticipantRoleTest.java (1)
113-120: createOnline 단일 팩토리로의 전환은 적절합니다. event=null 허용만 확인 부탁드립니다.도메인 제약에서 Event가 @nullable이 아니라면 간단한 Event 픽스처를 만들어 넘겨주세요. 현재 null 전달은 잠재적 NPE 지점입니다.
src/test/java/com/gdschongik/gdsc/domain/event/application/EventParticipationServiceTest.java (7)
1108-1116: LGTM — 새 시그니처(createOnline)로 안전하게 이행되었습니다.
1121-1129: LGTM — 뒤풀이 신청 케이스(APPLIED)도 새 시그니처에 맞게 정확히 반영되었습니다.
1133-1141: LGTM — 뒤풀이 비활성 케이스(NONE/NONE) 설정이 도메인 의미와 일치합니다.
1145-1153: LGTM — 미확인 상태(UNPAID/UNPAID) 초기화가 적절합니다.
1157-1165: LGTM — 확인 완료 상태(ATTENDED, PAID/PAID) 시나리오가 올바르게 구성되었습니다.
1169-1176: LGTM — 비회원 케이스에서 member=null 전달이 새 API 의도와 일치합니다.
1109-1110: 문제 없음 — 테스트 픽스처가 phone을 채웁니다FixtureHelper.createAssociateMember(...)와 IntegrationTest.createAssociateMember()가 updateBasicMemberInfo(..., PHONE_NUMBER, ...)로 phone을 세팅합니다 (src/test/java/com/gdschongik/gdsc/helper/FixtureHelper.java:51-55, src/test/java/com/gdschongik/gdsc/helper/IntegrationTest.java:191-195). Member.phone 필드와 Participant.of(...)의 null 검증을 확인했습니다 (src/main/java/com/gdschongik/gdsc/domain/member/domain/Member.java:62, src/main/java/com/gdschongik/gdsc/domain/event/domain/Participant.java:39-41,63-66). 현재 해당 호출부(EventParticipationServiceTest#createEventParticipation 등)는 phone이 세팅된 멤버를 사용하므로 수정 불필요합니다.
uwoobeat
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
kimsh1017
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
🌱 관련 이슈
📌 작업 내용 및 특이사항
📝 참고사항
📚 기타
Summary by CodeRabbit
사용자 기능상의 변화는 없습니다.