[DDING-000] FormApplication 도메인 Repository 테스트 코드 작성 및 리팩토링 #354
Conversation
Seooooo24
requested review from
5uhwann,
KoSeonJe and
wonjunYou
as code owners
Walkthrough테스트 픽스처들이 재구성되고 신규 픽스처들이 추가되었습니다. 여러 저장소 테스트가 삭제되거나 픽스처 기반으로 리팩토링되어 DTO 기반 쿼리/검증으로 교체되었으며, CI 워크플로우의 테스트 명령과 타임존 설정이 변경되었습니다. Changes
Sequence Diagram(s)(생략) 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
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Gitleaks (8.30.0).github/workflows/build-test.ymlpanic: allocator_unix: failed to reserve memory: cannot allocate memory goroutine 1 [running]: ... [truncated 302 characters] ... 2a0, {0xf8a300, 0xc0001a4a80}, 0x0?, {0xe18d1d?, 0x0?}, 0x0?, {0x1932260, 0x0, 0x0}) src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.javapanic: allocator_unix: failed to reserve memory: cannot allocate memory goroutine 1 [running]: ... [truncated 302 characters] ... 1e0, {0xf8a300, 0xc000242a20}, 0x0?, {0xe18d1d?, 0x0?}, 0x0?, {0x1932260, 0x0, 0x0}) 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
src/test/java/ddingdong/ddingdongBE/common/fixture/FileMetaDataFixture.java (1)
10-30: LGTM!메서드명
create로 통일하고filename커스터마이징을 위한 오버로드 추가가 적절합니다.🔎 중복 코드 줄이기 (선택 사항)
두 메서드 간 중복을 줄이려면 기본 메서드가 오버로드를 호출하도록 변경할 수 있습니다:
public static FileMetaData create(UUID id, Long entityId) { - return FileMetaData.builder() - .id(id) - .fileKey("1") - .fileStatus(FileStatus.COUPLED) - .fileName("파일") - .domainType(DomainType.FORM_FILE) - .entityId(entityId) - .build(); + return create(id, entityId, "파일"); }src/test/java/ddingdong/ddingdongBE/common/fixture/FormFixture.java (1)
27-37:LocalDate파라미터를 직접 받는 기존 메서드 사용을 권장합니다.
List<Integer>를 사용하면 리스트 크기가 3 미만일 경우IndexOutOfBoundsException이 발생할 수 있고, 타입 안전성도 떨어집니다. 라인 81-91에 이미createFormWithStartAndEndDate(Club, LocalDate, LocalDate)메서드가 존재하므로 해당 메서드를 활용하는 것이 더 안전합니다.🔎 테스트에서 기존 메서드 활용 예시
// List<Integer> 대신 LocalDate 사용 Form savedForm1 = formRepository.save( FormFixture.createFormWithStartAndEndDate( savedClub, LocalDate.of(2020, 3, 1), LocalDate.of(2020, 4, 1) ) );만약
List<Integer>방식을 유지해야 한다면, 최소한 크기 검증을 추가하세요:public static Form createForm(Club club, List<Integer> startDate, List<Integer> endDate) { + if (startDate.size() < 3 || endDate.size() < 3) { + throw new IllegalArgumentException("Date list must have at least 3 elements: [year, month, day]"); + } return Form.builder()src/test/java/ddingdong/ddingdongBE/common/fixture/FormApplicationFixture.java (1)
9-59: 기본값 불일치:department필드.메서드별로
department기본값이 다릅니다:
create(Form),create(Form, FormApplicationStatus):"학과"createWithName(Form, String):"컴퓨터공학과"테스트 데이터 일관성을 위해 기본값을 통일하는 것이 좋습니다.
🔎 통일된 기본값 적용
public static FormApplication createWithName(Form form, String name) { return FormApplication.builder() .name(name) .studentNumber("20231234") - .department("컴퓨터공학과") + .department("학과") .phoneNumber("010-1234-5678")src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormAnswerRepositoryTest.java (2)
31-44:@BeforeEach정리 메서드 누락.
FormApplicationRepositoryTest와 달리 이 테스트 클래스에는@BeforeEach정리 메서드가 없습니다. 테스트 간 데이터 격리를 위해 정리 메서드 추가를 권장합니다.🔎 추가 제안
@Autowired FileMetaDataRepository fileMetaDataRepository; + @BeforeEach + void setUp() { + formAnswerRepository.deleteAllInBatch(); + fileMetaDataRepository.deleteAllInBatch(); + formApplicationRepository.deleteAllInBatch(); + formFieldRepository.deleteAllInBatch(); + formRepository.deleteAllInBatch(); + clubRepository.deleteAllInBatch(); + } + @Test @DisplayName("특정 질문에 대한 답변 개수를 조회한다")
186-203: Fixture 사용 일관성.이 테스트에서는
FormFixture.formWithClubNull(),FormFixture.formFieldWithFormNull(),FormFixture.formAnswerByFormField()를 사용하고 있으나, 같은 파일의 다른 테스트들은 새로 추가된FormFieldFixture,FormAnswerFixture를 사용합니다. 일관성을 위해 새 fixture로 통일하는 것을 고려해보세요.🔎 새 fixture 활용 예시
void getAllFileByForm() { // given - Form form = FormFixture.formWithClubNull(); - Form savedForm = formRepository.save(form); - FormField formField = FormFixture.formFieldWithFormNull(); - - formField.setFormForConvenience(savedForm); - FormField savedField = formFieldRepository.save(formField); - - FormAnswer formAnswer1 = FormFixture.formAnswerByFormField(savedField); + Form savedForm = formRepository.save(FormFixture.formWithClubNull()); + FormField savedField = formFieldRepository.save(FormFieldFixture.create(savedForm)); + + FormAnswer formAnswer1 = FormAnswerFixture.create(null, savedField);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/test/java/ddingdong/ddingdongBE/common/fixture/FileMetaDataFixture.javasrc/test/java/ddingdong/ddingdongBE/common/fixture/FormAnswerFixture.javasrc/test/java/ddingdong/ddingdongBE/common/fixture/FormApplicationFixture.javasrc/test/java/ddingdong/ddingdongBE/common/fixture/FormFieldFixture.javasrc/test/java/ddingdong/ddingdongBE/common/fixture/FormFixture.javasrc/test/java/ddingdong/ddingdongBE/domain/form/repository/FormApplicationRepositoryTest.javasrc/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormAnswerRepositoryTest.javasrc/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java
💤 Files with no reviewable changes (1)
- src/test/java/ddingdong/ddingdongBE/domain/form/repository/FormApplicationRepositoryTest.java
🧰 Additional context used
🧬 Code graph analysis (2)
src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java (5)
src/test/java/ddingdong/ddingdongBE/common/fixture/ClubFixture.java (1)
ClubFixture(11-47)src/test/java/ddingdong/ddingdongBE/common/fixture/FileMetaDataFixture.java (1)
FileMetaDataFixture(8-31)src/test/java/ddingdong/ddingdongBE/common/fixture/FormAnswerFixture.java (1)
FormAnswerFixture(8-25)src/test/java/ddingdong/ddingdongBE/common/fixture/FormApplicationFixture.java (1)
FormApplicationFixture(7-84)src/test/java/ddingdong/ddingdongBE/common/fixture/FormFixture.java (1)
FormFixture(13-92)
src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormAnswerRepositoryTest.java (5)
src/test/java/ddingdong/ddingdongBE/common/fixture/ClubFixture.java (1)
ClubFixture(11-47)src/test/java/ddingdong/ddingdongBE/common/fixture/FileMetaDataFixture.java (1)
FileMetaDataFixture(8-31)src/test/java/ddingdong/ddingdongBE/common/fixture/FormAnswerFixture.java (1)
FormAnswerFixture(8-25)src/test/java/ddingdong/ddingdongBE/common/fixture/FormApplicationFixture.java (1)
FormApplicationFixture(7-84)src/test/java/ddingdong/ddingdongBE/common/fixture/FormFieldFixture.java (1)
FormFieldFixture(8-29)
⏰ 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 (2)
src/test/java/ddingdong/ddingdongBE/common/fixture/FormAnswerFixture.java (1)
1-25: LGTM!Fixture 클래스가 잘 구성되어 있습니다. 기본값을 제공하는 오버로드와 커스텀 값을 받는 오버로드를 분리하여 테스트 코드의 가독성과 유연성을 높였습니다.
src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java (1)
55-63:deleteAllInBatch호출 순서가 FK 제약조건을 위반할 수 있습니다.현재 순서에서
formRepository.deleteAllInBatch()가formApplicationRepository.deleteAllInBatch()이후에 호출되지만,formAnswerRepository가 마지막에 호출됩니다. FK 관계에 따라 삭제 순서를 조정해야 할 수 있습니다.🔎 권장 삭제 순서
일반적으로 자식 테이블부터 부모 테이블 순서로 삭제해야 합니다:
@BeforeEach void setUp() { + formAnswerRepository.deleteAllInBatch(); + fileMetaDataRepository.deleteAllInBatch(); formApplicationRepository.deleteAllInBatch(); - clubRepository.deleteAllInBatch(); - userRepository.deleteAllInBatch(); formRepository.deleteAllInBatch(); - fileMetaDataRepository.deleteAllInBatch(); - formAnswerRepository.deleteAllInBatch(); + clubRepository.deleteAllInBatch(); + userRepository.deleteAllInBatch(); }
src/test/java/ddingdong/ddingdongBE/common/fixture/FormFieldFixture.java
Show resolved
Hide resolved
...a/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java
Show resolved
Hide resolved
Gopistol
left a comment
Gopistol
left a comment
There was a problem hiding this comment.
고생하셨습니다. 리뷰 확인해주시면 감사하겠습니다.
| FileMetaDataRepository fileMetaDataRepository; | ||
|
|
||
| @Test | ||
| @DisplayName("특정 질문에 대한 답변 개수를 조회한다") |
There was a problem hiding this comment.
countByFormField, findAllByFormApplication에 대한 테스트 검증을 추가하셨는데 어떤 이유로 추가하셨는지 알 수 있을까요?
There was a problem hiding this comment.
필요한 테스트에 대한 고민 없이 작성했던 걸 덜 삭제한 것 같습니다... 확인해주셔서 감사합니다! 반영 완료했습니다.
| List<Long> ids = List.of(savedFormAnswer1.getId(), savedFormAnswer2.getId()); | ||
|
|
||
| // when | ||
| List<FileAnswerInfo> fileAnswerInfos = formAnswerRepository.findAllFileAnswerInfo( |
There was a problem hiding this comment.
formAnswerRepository의 동작을 검증하는 테스트가 FormApplicationRepositoryTest에 있는 것 같습니다.
There was a problem hiding this comment.
리뷰 반영하였습니다. 확인 감사합니다.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormAnswerRepositoryTest.java (1)
139-153: 테스트 픽스처 사용을 표준화하세요.이 테스트는 다른 테스트들과 달리 구버전 픽스처 메서드를 사용하고 있습니다:
FormFixture.formWithClubNull()→FormFixture.createForm(null)사용 권장FormFixture.formFieldWithFormNull()→FormFieldFixture.create(form)사용 권장FormFixture.formAnswerByFormField()→FormAnswerFixture.create()사용 권장또한 Line 143의
setFormForConvenience()메서드는 캡슐화를 깨뜨리는 테스트 전용 편의 메서드로 보입니다. 픽스처에서 관계를 직접 설정하는 방식으로 개선할 수 있습니다.PR 목표에서 "테스트에서 필요한 객체 생성은 빌더 대신 Fixture 클래스의 메서드를 사용하도록 통일"을 명시했으므로, 이 테스트도 다른 테스트들과 일관되게 리팩토링하는 것을 권장합니다.
🔎 제안하는 리팩토링
// given - Form form = FormFixture.formWithClubNull(); - Form savedForm = formRepository.save(form); - FormField formField = FormFixture.formFieldWithFormNull(); - - formField.setFormForConvenience(savedForm); - FormField savedField = formFieldRepository.save(formField); - - FormAnswer formAnswer1 = FormFixture.formAnswerByFormField(savedField); - FormAnswer savedFormAnswer1 = formAnswerRepository.save(formAnswer1); - - FormAnswer formAnswer2 = FormFixture.formAnswerByFormField(savedField); - FormAnswer savedFormAnswer2 = formAnswerRepository.save(formAnswer2); - - FormAnswer formAnswer3 = FormFixture.formAnswerByFormField(savedField); - FormAnswer savedFormAnswer3 = formAnswerRepository.save(formAnswer3); + Form savedForm = formRepository.save(FormFixture.createForm(null)); + FormField savedField = formFieldRepository.save(FormFieldFixture.create(savedForm)); + + FormAnswer savedFormAnswer1 = formAnswerRepository.save( + FormAnswerFixture.create(null, savedField, List.of("value1"))); + FormAnswer savedFormAnswer2 = formAnswerRepository.save( + FormAnswerFixture.create(null, savedField, List.of("value2"))); + FormAnswer savedFormAnswer3 = formAnswerRepository.save( + FormAnswerFixture.create(null, savedField, List.of("value3")));
♻️ Duplicate comments (1)
src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java (1)
139-145: 불필요한 중복 쿼리 호출.
findAllFinalPassedByFormId가 두 번 호출되고 있습니다. 라인 140의 첫 번째 호출 결과가 사용되지 않고, 라인 143-144에서 다시 호출하여 검증합니다. 한 번만 호출하고 결과를 변수에 저장하여 검증하세요.🔎 수정 제안
// when - formApplicationRepository.findAllFinalPassedByFormId(savedForm.getId()); + List<FormApplication> results = formApplicationRepository.findAllFinalPassedByFormId(savedForm.getId()); // then - assertThat(formApplicationRepository.findAllFinalPassedByFormId(savedForm.getId()) - .size()).isEqualTo(3); + assertThat(results).hasSize(3);
🧹 Nitpick comments (2)
src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java (2)
105-106:savedForm3의 테스트 목적을 명확히 해주세요.
savedForm3가 생성되지만 이후 assertion에서 참조되지 않습니다. 날짜 필터링 동작 확인을 위해 의도적으로 추가한 것으로 보이지만, 주석을 추가하거나 결과에서 제외되었음을 명시적으로 검증하면 테스트 의도가 더 명확해집니다.Form savedForm3 = formRepository.save(FormFixture.createForm(savedClub, List.of(2020, 5, 1), List.of(2020, 6, 1))); +// savedForm3는 기준일(savedForm1.endDate) 이후이므로 결과에서 제외되어야 함
114-114: AssertJ의hasSize()사용을 권장합니다.
assertThat(list.size()).isEqualTo(n)대신assertThat(list).hasSize(n)을 사용하면 실패 시 더 유용한 에러 메시지를 제공합니다.-assertThat(recentFormInfos.size()).isEqualTo(2); +assertThat(recentFormInfos).hasSize(2);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormAnswerRepositoryTest.javasrc/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java (3)
src/test/java/ddingdong/ddingdongBE/common/fixture/ClubFixture.java (1)
ClubFixture(11-47)src/test/java/ddingdong/ddingdongBE/common/fixture/FormApplicationFixture.java (1)
FormApplicationFixture(7-84)src/test/java/ddingdong/ddingdongBE/common/fixture/FormFixture.java (1)
FormFixture(13-92)
🔇 Additional comments (2)
src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java (1)
56-88: LGTM!테스트 로직이 명확하고, 학과별 지원자 수 상위 n개 조회 기능을 정확하게 검증하고 있습니다. Fixture를 활용한 데이터 구성도 적절합니다.
src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormAnswerRepositoryTest.java (1)
178-207: LGTM!테스트가 픽스처를 일관되게 사용하고 있으며, enum 상수를 올바르게 활용하고 있습니다. 조인 쿼리에 대한 검증도 적절합니다.
...t/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormAnswerRepositoryTest.java
Outdated
Show resolved
Hide resolved
...a/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java (1)
46-54: 외래 키 제약 조건 위반 가능성이 여전히 존재합니다.이전 리뷰에서 지적된
deleteAllInBatch()호출 순서 문제가 아직 해결되지 않았습니다. 현재 순서에서:
clubRepository가formRepository보다 먼저 삭제되고 있습니다 (라인 49, 51)formAnswerRepository가formRepository보다 나중에 삭제되고 있습니다 (라인 53, 51)
Form이Club을 참조하고,FormAnswer가FormField를 통해Form을 참조하므로 FK 제약 조건 위반이 발생할 수 있습니다.
🧹 Nitpick comments (1)
src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java (1)
114-114: 일관된 assertion 스타일 사용을 권장합니다.동일 파일 내 다른 테스트에서는
assertThat().hasSize()를 사용하고 있습니다(라인 79, 144). 일관성을 위해 여기서도 동일한 스타일을 사용하는 것이 좋습니다.🔎 제안하는 수정
- assertThat(recentFormInfos.size()).isEqualTo(2); + assertThat(recentFormInfos).hasSize(2);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/test/java/ddingdong/ddingdongBE/common/fixture/FormFieldFixture.javasrc/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/test/java/ddingdong/ddingdongBE/common/fixture/FormFieldFixture.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java (3)
src/test/java/ddingdong/ddingdongBE/common/fixture/ClubFixture.java (1)
ClubFixture(11-47)src/test/java/ddingdong/ddingdongBE/common/fixture/FormApplicationFixture.java (1)
FormApplicationFixture(7-84)src/test/java/ddingdong/ddingdongBE/common/fixture/FormFixture.java (1)
FormFixture(13-92)
🔇 Additional comments (3)
src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java (3)
56-88: 테스트 로직이 명확하고 정확합니다.학과별 지원자 수 집계 테스트가 잘 작성되었습니다. 픽스처를 활용한 테스트 데이터 구성과 검증 로직이 적절합니다.
121-145: 상태별 필터링 테스트가 정확합니다.다양한
FormApplicationStatus값을 사용하여FINAL_PASS필터링을 검증하는 테스트가 잘 작성되었습니다. 이전 리뷰에서 지적된 중복 쿼리 호출도 해결되었습니다.
90-119: 메서드 이름과 실제 정렬 순서가 불일치합니다.리포지토리 구현(FormApplicationRepository.java)을 확인한 결과, 쿼리는
ORDER BY start_date(ASC)로 설정되어 있습니다:SELECT recent_forms.start_date AS date, COUNT(fa.id) AS count FROM ( SELECT * FROM form WHERE club_id = :clubId AND start_date <= :date ORDER BY start_date LIMIT :size )이는 가장 오래된 폼부터 반환합니다(2020-01-01 → 2020-03-01). 하지만 메서드명의 "Recent"와 한글 설명의 "최신 폼지"는 가장 최근의 폼을 의미하므로 명칭이 실제 동작과 맞지 않습니다.
테스트 기대값 자체는 구현과 일치하므로 정확하지만, 메서드명이나 설명을 수정하여 "과거 폼지" 또는 "지정 날짜 이전 폼지"로 변경하는 것이 더 명확합니다.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java (1)
114-114: AssertJ의hasSize()메서드 사용을 권장합니다.다른 테스트(라인 79, 144)와의 일관성을 위해
.size()를.hasSize()로 변경하는 것이 좋습니다.hasSize()는 실패 시 더 명확한 에러 메시지를 제공합니다.🔎 수정 제안
- assertThat(recentFormInfos.size()).isEqualTo(2); + assertThat(recentFormInfos).hasSize(2);src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormAnswerRepositoryTest.java (1)
145-186: Fixture 사용 패턴의 불일치.이 테스트는
FormFixture.formWithClubNull(),FormFixture.formFieldWithFormNull(),FormFixture.formAnswerByFormField()를 사용하지만, 다른 테스트들은FormFieldFixture,FormAnswerFixture등 전용 Fixture 클래스를 사용합니다. PR 목표에서 "Fixture 클래스의 메서드를 사용하도록 통일"한다고 했으므로, 일관성을 위해 전용 Fixture로 변경하는 것을 고려해 주세요.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormAnswerRepositoryTest.javasrc/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java
🧰 Additional context used
🧬 Code graph analysis (2)
src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormAnswerRepositoryTest.java (6)
src/test/java/ddingdong/ddingdongBE/common/fixture/ClubFixture.java (1)
ClubFixture(11-47)src/test/java/ddingdong/ddingdongBE/common/fixture/FileMetaDataFixture.java (1)
FileMetaDataFixture(8-31)src/test/java/ddingdong/ddingdongBE/common/fixture/FormAnswerFixture.java (1)
FormAnswerFixture(8-25)src/test/java/ddingdong/ddingdongBE/common/fixture/FormApplicationFixture.java (1)
FormApplicationFixture(7-84)src/test/java/ddingdong/ddingdongBE/common/fixture/FormFieldFixture.java (1)
FormFieldFixture(8-33)src/test/java/ddingdong/ddingdongBE/common/fixture/FormFixture.java (1)
FormFixture(13-92)
src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java (3)
src/test/java/ddingdong/ddingdongBE/common/fixture/ClubFixture.java (1)
ClubFixture(11-47)src/test/java/ddingdong/ddingdongBE/common/fixture/FormApplicationFixture.java (1)
FormApplicationFixture(7-84)src/test/java/ddingdong/ddingdongBE/common/fixture/FormFixture.java (1)
FormFixture(13-92)
🔇 Additional comments (6)
src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java (3)
46-54: 삭제 순서가 올바르게 수정되었습니다.FK 제약 조건을 고려하여 자식 테이블부터 삭제하는 올바른 순서로 수정되었습니다:
formAnswer→formApplication→form→club→user.
56-88: LGTM!테스트 데이터 설정과 검증 로직이 적절합니다. Fixture를 활용한 데이터 생성이 일관성 있게 적용되었습니다.
139-145: 중복 쿼리 호출 문제가 해결되었습니다.이전 리뷰에서 지적된 중복 쿼리 호출이 제거되고, 결과를 변수에 저장하여 한 번만 검증하도록 수정되었습니다.
src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormAnswerRepositoryTest.java (3)
58-75: LGTM!
FormFieldFixture를 활용한 테스트 데이터 생성과 검증 로직이 적절합니다.
77-107: LGTM!전체 엔티티 그래프를 구성하여
TextAnswerInfoDTO 매핑을 적절히 검증하고 있습니다.
109-143: enum 상수 사용 문제가 해결되었습니다.이전 리뷰에서 지적된 하드코딩된 문자열이
DomainType.FORM_FILE.name()과FileStatus.COUPLED.name()으로 적절히 수정되었습니다.
...t/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormAnswerRepositoryTest.java
Outdated
Show resolved
Hide resolved
...t/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormAnswerRepositoryTest.java
Show resolved
Hide resolved
KoSeonJe
left a comment
KoSeonJe
left a comment
There was a problem hiding this comment.
고생하셨습니다~ 잘해주셨네요 코멘트 확인해주세요
| public class FileMetaDataFixture { | ||
|
|
||
| public static FileMetaData formFileMetaData(UUID id, Long entityId) { | ||
| public static FileMetaData create(UUID id, Long entityId) { |
There was a problem hiding this comment.
p2)
지금 보니까, domainType도 같이 받아야할 것 같아요! 도메인 타입 + 엔티티 id가 세트입니다
| .build(); | ||
| } | ||
|
|
||
| public static FormApplication createWithDepartment(Form form, String department) { |
There was a problem hiding this comment.
p2)
위에 네이밍은 create로 오버로딩 다 해놨는데, createWithName, createWithDepartment 등이 추가되면 통일성이 없어지는 것 같아요.
통일되면 좋을 것 같습니다
There was a problem hiding this comment.
name과 department 둘 다 String 타입이라 둘을 따로 추가했는데, 리뷰 반영하여 두 메소드를 삭제하고 name과 department를 받는 create 메소드 하나를 추가하였습니다.
| @BeforeEach | ||
| void setUp() { | ||
| formAnswerRepository.deleteAllInBatch(); | ||
| formFieldRepository.deleteAllInBatch(); | ||
| formApplicationRepository.deleteAllInBatch(); | ||
| formRepository.deleteAllInBatch(); | ||
| clubRepository.deleteAllInBatch(); | ||
| fileMetaDataRepository.deleteAllInBatch(); | ||
| } |
There was a problem hiding this comment.
p2)
초기화 코드인 것 같은데 서포트 클래스 내 테이블 초기화 로직으로는 해결되지 않은 것일까요?
There was a problem hiding this comment.
예전에 fixtureMonkey 에러 해결할 때 넣어놨어서 있어야 하는 줄 알았습니다... 삭제했습니다. 서포트 클래스에 대해서 알아보겠습니다. 감사합니다!
| } | ||
|
|
||
| @Test | ||
| @DisplayName("특정 질문에 대한 지원자 이름과 답변 내용을 조회한다") |
There was a problem hiding this comment.
| @DisplayName("특정 질문에 대한 지원자 이름과 답변 내용을 조회한다") | |
| @DisplayName("특정 질문에 답한 모든 지원자 이름과 답변 내용을 조회한다") |
p3)
이건 취향입니다. 조금 더 직관적인 것 같아요! 위처럼 하는 것이
There was a problem hiding this comment.
저도 변경하는 것이 더 좋다고 생각합니다! 반영했습니다.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java (1)
39-71: 쿼리의 ORDER BY count DESC는 정상적으로 적용되어 있습니다.
findTopDepartmentsByFormId메서드의 쿼리에ORDER BY count DESC가 명시되어 있으므로 테스트는 정상적으로 작동합니다. 다만, 동일한 count를 가진 학과가 존재할 경우 결과 순서가 비결정적일 수 있으므로, 추가 정렬 기준(예:ORDER BY count DESC, f.department ASC)을 고려하면 더욱 견고한 쿼리가 될 것입니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java (3)
src/test/java/ddingdong/ddingdongBE/common/fixture/ClubFixture.java (1)
ClubFixture(11-47)src/test/java/ddingdong/ddingdongBE/common/fixture/FormApplicationFixture.java (1)
FormApplicationFixture(7-71)src/test/java/ddingdong/ddingdongBE/common/fixture/FormFixture.java (1)
FormFixture(13-92)
⏰ 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 (2)
src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java (2)
107-131: 테스트가 잘 작성되었습니다!
findAllFinalPassedByFormId메서드에 대한 테스트가 명확하고 올바르게 구현되었습니다. 다양한 상태의 지원서를 생성하고 FINAL_PASS 상태만 정확히 필터링하는지 검증합니다. 이전 리뷰에서 지적된 중복 쿼리 호출 문제도 해결되었습니다.
73-105: 제안된 form ID 추출 검증은 실행 불가능합니다.
RecentFormInfo는getDate()와getCount()메서드만 노출하는 인터페이스입니다. 리뷰 댓글에서 제안한extracting(RecentFormInfo::getFormId)는 컴파일 오류가 발생합니다.현재 테스트는 다음을 통해 이미 올바르게 검증하고 있습니다:
recentFormInfos.size() == 2: form3이 제외되었음을 확인- 반환된 결과의 개수, 날짜, 지원자 수를 검증
추가로 필요하다면 다른 동아리의 폼을 포함한 테스트 케이스를 추가하여 clubId 필터링을 더 명확히 검증할 수 있습니다.
Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java (1)
92-92: 일관된 assertion 스타일을 사용하세요.다른 테스트(57번, 122번 라인)에서는
hasSize()를 사용하고 있습니다. 일관성을 위해 동일한 스타일을 적용하는 것이 좋습니다.🔎 제안하는 수정
- assertThat(recentFormInfos.size()).isEqualTo(2); + assertThat(recentFormInfos).hasSize(2);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java (3)
src/test/java/ddingdong/ddingdongBE/common/fixture/ClubFixture.java (1)
ClubFixture(11-47)src/test/java/ddingdong/ddingdongBE/common/fixture/FormApplicationFixture.java (1)
FormApplicationFixture(7-71)src/test/java/ddingdong/ddingdongBE/common/fixture/FormFixture.java (1)
FormFixture(13-92)
⏰ 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 (2)
src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java (2)
34-66: 테스트 로직이 올바르게 구현되었습니다.학과별 지원자 수를 집계하고 상위 N개를 조회하는 로직이 명확하게 검증되고 있으며, 픽스처를 활용한 테스트 데이터 생성도 적절합니다.
99-123: LGTM!최종 합격 지원자 필터링 로직이 올바르게 검증되고 있으며, 다양한 상태값에 대한 테스트 커버리지가 충분합니다. 이전 리뷰에서 지적된 중복 쿼리 호출 문제도 해결되었습니다.
There was a problem hiding this comment.
Actionable comments posted: 2
Fix all issues with AI Agents 🤖
In @src/test/java/ddingdong/ddingdongBE/common/fixture/FormFixture.java:
- Around line 27-37: There are two identical factory methods, createForm(Club,
LocalDate, LocalDate) and createFormWithStartAndEndDate(Club, LocalDate,
LocalDate); remove the duplicate createFormWithStartAndEndDate, keep createForm
as the single implementation, and update any callers/tests that reference
createFormWithStartAndEndDate to call createForm instead so compilation and
behavior remain unchanged.
In
@src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java:
- Around line 68-97: The method name findRecentFormByDateWithApplicationCount
implies newest-first but the query uses ORDER BY start_date ASC; update the
query used in FormApplicationRepository (the method
findRecentFormByDateWithApplicationCount) to ORDER BY start_date DESC so the
returned RecentFormInfo list is sorted by newest start date first, or
alternatively rename the method to reflect ascending order if you prefer to keep
ASC.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/test/java/ddingdong/ddingdongBE/common/fixture/FormFixture.javasrc/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java (3)
src/test/java/ddingdong/ddingdongBE/common/fixture/ClubFixture.java (1)
ClubFixture(11-47)src/test/java/ddingdong/ddingdongBE/common/fixture/FormApplicationFixture.java (1)
FormApplicationFixture(7-71)src/test/java/ddingdong/ddingdongBE/common/fixture/FormFixture.java (1)
FormFixture(13-92)
⏰ 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 (2)
src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java (2)
34-66: LGTM!상위 N개 학과 조회 로직이 올바르게 검증되고 있습니다. 픽스처 기반 데이터 생성과 DTO 검증이 적절합니다.
99-123: LGTM!최종 합격자 필터링 로직이 올바르게 검증되고 있습니다. 이전 리뷰에서 지적된 중복 쿼리 호출 문제도 해결되었습니다.
| public static Form createForm(Club club, LocalDate startDate, LocalDate endDate) { | ||
| return Form.builder() | ||
| .title("모집 지원서") | ||
| .description("동아리 모집을 위한 지원서입니다.") | ||
| .startDate(startDate) | ||
| .endDate(endDate) | ||
| .hasInterview(true) | ||
| .sections(List.of("자기소개", "지원 동기", "경력 및 경험")) | ||
| .club(club) | ||
| .build(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
중복된 메서드를 제거하세요.
createForm(Club, LocalDate, LocalDate) (27-37번 줄)과 createFormWithStartAndEndDate(Club, LocalDate, LocalDate) (81-91번 줄)는 메서드 이름만 다를 뿐 완전히 동일한 구현을 가지고 있습니다. 둘 중 하나를 제거하여 코드 중복을 해소하세요.
🔎 수정 제안
createFormWithStartAndEndDate 메서드를 제거하고 createForm(Club, LocalDate, LocalDate)만 사용하도록 통일하세요:
- public static Form createFormWithStartAndEndDate(Club club, LocalDate startDate, LocalDate endDate) {
- return Form.builder()
- .title("모집 지원서")
- .description("동아리 모집을 위한 지원서입니다.")
- .startDate(startDate)
- .endDate(endDate)
- .hasInterview(true)
- .sections(List.of("자기소개", "지원 동기", "경력 및 경험"))
- .club(club)
- .build();
- }Also applies to: 81-91
🤖 Prompt for AI Agents
In @src/test/java/ddingdong/ddingdongBE/common/fixture/FormFixture.java around
lines 27-37, There are two identical factory methods, createForm(Club,
LocalDate, LocalDate) and createFormWithStartAndEndDate(Club, LocalDate,
LocalDate); remove the duplicate createFormWithStartAndEndDate, keep createForm
as the single implementation, and update any callers/tests that reference
createFormWithStartAndEndDate to call createForm instead so compilation and
behavior remain unchanged.
KoSeonJe
left a comment
KoSeonJe
left a comment
There was a problem hiding this comment.
어프로브해놨는데, 테스트 실패하는데 다시 한번 확인 부탁드려요!
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java (1)
92-92: AssertJ 스타일 일관성 개선을 권장합니다.57번, 122번 라인에서는
hasSize()를 사용하는 반면, 이 라인에서는size()).isEqualTo()를 사용하고 있습니다. 일관성을 위해hasSize()를 사용하는 것이 더 간결하고 가독성이 좋습니다.🔎 제안하는 수정
- assertThat(recentFormInfos.size()).isEqualTo(2); + assertThat(recentFormInfos).hasSize(2);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepository.javasrc/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepository.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java (3)
src/test/java/ddingdong/ddingdongBE/common/fixture/ClubFixture.java (1)
ClubFixture(11-47)src/test/java/ddingdong/ddingdongBE/common/fixture/FormApplicationFixture.java (1)
FormApplicationFixture(7-71)src/test/java/ddingdong/ddingdongBE/common/fixture/FormFixture.java (1)
FormFixture(13-92)
⏰ 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 (3)
src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java (3)
3-32: 테스트 클래스 구조와 Fixture 사용이 적절합니다.Fixture 클래스를 활용한 테스트 데이터 생성 방식으로 변경되었으며, 필요한 Repository 의존성이 올바르게 주입되었습니다. PR 목표에 부합하는 구조입니다.
34-66: 테스트 로직이 명확하고 올바릅니다.학과별 지원자 수를 정확히 검증하고 있으며, Fixture를 통한 테스트 데이터 생성이 일관성 있게 적용되었습니다.
99-123: 최종 합격자 조회 테스트가 정확합니다.다양한 지원 상태를 포함한 테스트 데이터로 FINAL_PASS 상태만 올바르게 조회하는지 검증하고 있습니다. 이전 리뷰에서 지적된 중복 쿼리 호출 문제도 해결되었습니다.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In @.github/workflows/build-test.yml:
- Around line 38-43: The workflow step using actions/upload-artifact currently
references actions/upload-artifact@v4; update that step (the "test report save"
step) to use actions/upload-artifact@v6 to pick up fixes and improvements, keep
the existing if: always() behavior, and optionally add a retention-days value
(e.g., 30) under the with: section to control artifact retention.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/build-test.yml
⏰ 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
.github/workflows/build-test.yml
Outdated
| - name: test report save | ||
| if: always() # 성공하든 실패하든 무조건 실행 | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: test-report | ||
| path: build/reports/tests/test/ |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
latest stable version of actions/upload-artifact GitHub Actions
💡 Result:
The latest stable release is actions/upload-artifact@v6 (as of January 6, 2026). [1][2]
Sources:
[1] actions/upload-artifact repository (README).
[2] GitHub Changelog (deprecation notices and v4+ context).
🌐 Web query:
actions/upload-artifact v4 known issues release notes
💡 Result:
Summary of actions/upload-artifact@v4 release notes and known issues
Key changes and breaking behavior
- Major backend rewrite: job-scoped, immutable artifacts, much faster uploads, immediate artifact IDs/API availability. [1][2]
- Breaking changes: cannot upload to the same artifact name multiple times (immutability); v4 artifacts are not cross-compatible with v3; v4+ not supported on GHES (use v3 on GHES). [1][2]
Limits & defaults
- Per-job artifact limit: 500 artifacts. [1]
- With v4.4+ hidden files are excluded by default. [1]
Operational notes (self-hosted / networking)
- Self‑hosted runners may require additional firewall/network rules to reach new storage endpoints. [1][2]
Notable reported issues
- Public-artifact access bug: some users reported newly uploaded public artifacts being inaccessible (HTTP 409 / "Public access is not permitted on this storage account") with v4; issue tracked in the repo. [3]
- Other user-reported bugs and migration questions are tracked in the actions/upload-artifact GitHub issues. [3][4]
Where to read more / primary sources
- actions/upload-artifact README (v4 changelog, migration notes, limits). [1]
- GitHub Marketplace action page (v4 highlights & GHES note). [2]
- Repo issues (example public-access bug #486). [3]
- GitHub Changelog posts about Artifacts v4 and v3 deprecation. [4][5]
References
[1] actions/upload-artifact README (v4) — GitHub repo.
[2] Upload a Build Artifact — GitHub Marketplace (v4 notes).
[3] Issue: "Since actions/upload-artifact@v4 new public artifacts are no longer accessible" — actions/upload-artifact #486.
[4] GitHub Changelog: "GitHub Actions - Artifacts v4 is now Generally Available".
[5] GitHub Changelog: "Deprecation notice: v3 of the artifact actions".
Would you like direct links to any of those sources or help troubleshooting a specific v4 issue?
actions/upload-artifact 버전을 v4에서 v6으로 업그레이드하세요.
if: always() 조건을 사용하여 테스트 실패 시에도 리포트가 저장되도록 한 것은 디버깅과 분석에 유용한 좋은 접근입니다. 다만 현재 사용 중인 actions/upload-artifact@v4는 2026년 1월 기준 최신 안정 버전인 v6으로 업그레이드되어야 합니다.
권장 사항:
-
v4에서 v6으로 업그레이드: v6은 주요 개선사항과 안정성 개선이 포함되어 있으며, v4는 공개 아티팩트 접근 문제 등 알려진 이슈가 있습니다.
-
선택적: 보관 기간 명시 - 명확성을 위해
retention-days를 추가하는 것을 고려하세요:
권장 구성
- name: test report save
if: always()
uses: actions/upload-artifact@v6
with:
name: test-report
path: build/reports/tests/test/
retention-days: 30🤖 Prompt for AI Agents
In @.github/workflows/build-test.yml around lines 38 - 43, The workflow step
using actions/upload-artifact currently references actions/upload-artifact@v4;
update that step (the "test report save" step) to use actions/upload-artifact@v6
to pick up fixes and improvements, keep the existing if: always() behavior, and
optionally add a retention-days value (e.g., 30) under the with: section to
control artifact retention.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In
@src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java:
- Around line 92-104: Remove the CI debugging System.out.println statements in
FormApplicationRepositoryTest: delete the entire debug block that prints
"================ [CI 디버깅 시작] ================", the loop over recentFormInfos
(including prints of info.getDate(), info.getCount(), and
info.getCount().getClass()), and the closing "================ [CI 디버깅 끝]
================" line; rely on assertions within the test that use
recentFormInfos/RecentFormInfo instead of printing, or replace with proper
logger calls if persistent logging is required.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/build-test.ymlsrc/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/build-test.yml
🧰 Additional context used
🧬 Code graph analysis (1)
src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java (3)
src/test/java/ddingdong/ddingdongBE/common/fixture/ClubFixture.java (1)
ClubFixture(11-47)src/test/java/ddingdong/ddingdongBE/common/fixture/FormApplicationFixture.java (1)
FormApplicationFixture(7-71)src/test/java/ddingdong/ddingdongBE/common/fixture/FormFixture.java (1)
FormFixture(13-92)
⏰ 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 (2)
src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java (2)
34-66: 테스트 로직이 명확하고 올바릅니다.학과별 지원자 수 상위 N개 조회 기능을 잘 검증하고 있습니다. 픽스처 사용과 순위 및 카운트에 대한 구체적인 검증이 적절합니다.
113-137: 최종 합격자 필터링 로직이 올바르게 검증되었습니다.다양한 지원 상태를 가진 데이터를 생성하여
FINAL_PASS상태만 정확히 필터링되는지 검증하고 있습니다.
| System.out.println("================ [CI 디버깅 시작] ================"); | ||
| System.out.println("결과 개수: " + recentFormInfos.size()); | ||
|
|
||
| for (RecentFormInfo info : recentFormInfos) { | ||
| System.out.println("---------------------------------------------"); | ||
| System.out.println("날짜(Date): " + info.getDate()); | ||
| System.out.println("개수(Count): " + info.getCount()); | ||
|
|
||
| if (info.getCount() != null) { | ||
| System.out.println("개수 타입: " + info.getCount().getClass().getName()); | ||
| } | ||
| } | ||
| System.out.println("================ [CI 디버깅 끝] ================"); |
There was a problem hiding this comment.
디버깅용 출력문을 제거해야 합니다.
CI 디버깅을 위한 System.out.println 구문이 남아있습니다. 이는 프로덕션 테스트 코드에 적합하지 않으며, 테스트 출력을 어지럽힙니다.
🔎 디버깅 코드 제거 제안
);
- System.out.println("================ [CI 디버깅 시작] ================");
- System.out.println("결과 개수: " + recentFormInfos.size());
-
- for (RecentFormInfo info : recentFormInfos) {
- System.out.println("---------------------------------------------");
- System.out.println("날짜(Date): " + info.getDate());
- System.out.println("개수(Count): " + info.getCount());
-
- if (info.getCount() != null) {
- System.out.println("개수 타입: " + info.getCount().getClass().getName());
- }
- }
- System.out.println("================ [CI 디버깅 끝] ================");
// then
assertThat(recentFormInfos.size()).isEqualTo(2);🤖 Prompt for AI Agents
In
@src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java
around lines 92 - 104, Remove the CI debugging System.out.println statements in
FormApplicationRepositoryTest: delete the entire debug block that prints
"================ [CI 디버깅 시작] ================", the loop over recentFormInfos
(including prints of info.getDate(), info.getCount(), and
info.getCount().getClass()), and the closing "================ [CI 디버깅 끝]
================" line; rely on assertions within the test that use
recentFormInfos/RecentFormInfo instead of printing, or replace with proper
logger calls if persistent logging is required.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/build-test.yml (1)
35-36:build대신test만 실행하는 것이 의도된 변경인지 확인해 주세요.
./gradlew clean build에서./gradlew clean test --stacktrace로 변경되었습니다.--stacktrace옵션 추가는 테스트 실패 시 디버깅에 유용하여 좋은 개선입니다.다만
build태스크는test+ 컴파일 + 패키징을 포함하는 반면,test만 실행하면 다음을 검증하지 못합니다:
- 전체 컴파일 오류 (테스트에서 감지되지 않는 컴파일 이슈)
- 패키징 및 리소스 처리 문제
- 빌드 타임 검증
PR 브랜치의 CI에서는 일반적으로
build를 실행하여 완전한 검증을 수행하는 것이 권장됩니다. 테스트 중심 PR이라도 빌드 전체를 검증하는 것이 안전합니다.🔎 권장 변경사항
- name: CI Test - run: ./gradlew clean test --stacktrace + run: ./gradlew clean build --stacktrace
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/build-test.ymlsrc/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java (3)
src/test/java/ddingdong/ddingdongBE/common/fixture/ClubFixture.java (1)
ClubFixture(11-47)src/test/java/ddingdong/ddingdongBE/common/fixture/FormApplicationFixture.java (1)
FormApplicationFixture(7-71)src/test/java/ddingdong/ddingdongBE/common/fixture/FormFixture.java (1)
FormFixture(13-92)
⏰ 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 (3)
src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java (3)
34-66: 테스트 로직이 명확하고 올바릅니다.상위 n개 학과 조회 테스트가 Fixture를 활용하여 깔끔하게 구현되었으며, 검증 로직도 정확합니다.
68-91: 최신 폼지 조회 테스트가 올바르게 구현되었습니다.디버깅 라인을 제외하면, 테스트 로직은 명확하고 검증 로직도 정확합니다. Fixture를 활용한 테스트 데이터 구성이 깔끔합니다.
Also applies to: 93-99
101-125: 최종 합격자 조회 테스트가 정확합니다.다양한 상태의 지원서를 생성하고 FINAL_PASS 상태만 필터링하는 로직을 올바르게 검증하고 있습니다. 이전 리뷰에서 지적된 중복 쿼리 문제도 해결되었습니다.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In
@src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java:
- Around line 68-98: The test DisplayName and assertions in
FormApplicationRepositoryTest.findRecentFormByDateWithApplicationCount are
inconsistent with the expected "latest" ordering: currently assertions check
ascending (oldest-first) dates; either update the DisplayName to reflect
"oldest" ordering and keep assertions, or change the repository query used by
formApplicationRepository.findRecentFormByDateWithApplicationCount to sort by
start date descending (DESC) so results are newest-first and then adjust the
assertions to expect 2020-05-10/2020-03-10/etc.; ensure the chosen fix updates
the DisplayName, the repository query ordering, and the assertions to remain
consistent.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/build-test.ymlsrc/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/build-test.yml
⏰ 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 (3)
src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java (3)
1-32: LGTM!테스트 클래스 구조와 의존성 설정이 적절하며, Fixture 기반 테스트 데이터 생성 방식이 PR 목표와 일치합니다.
34-66: LGTM!학과별 지원자 수 상위 조회 테스트가 명확하게 작성되었으며, 테스트 데이터와 검증 로직이 정확합니다.
100-124: LGTM!최종 합격자 조회 테스트가 명확하게 작성되었으며, 이전 리뷰에서 지적된 중복 쿼리 호출 문제가 해결되었습니다.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In
@src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java:
- Around line 68-98: The test's DisplayName says "latest forms" but assertions
check ascending order; update the test
(FormApplicationRepositoryTest.findRecentFormByDateWithApplicationCount) so it
asserts results are sorted newest-first: expect recentFormInfos.get(0).getDate()
== 2020-03-01 with count 2 and recentFormInfos.get(1).getDate() == 2020-01-01
with count 1, or alternatively change the repository method
findRecentFormByDateWithApplicationCount to return results in descending order;
pick one approach and make the assertions or query sort consistent with the "최신"
description.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/build-test.ymlsrc/test/java/ddingdong/ddingdongBE/domain/banner/entity/BannerTest.javasrc/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java
💤 Files with no reviewable changes (1)
- src/test/java/ddingdong/ddingdongBE/domain/banner/entity/BannerTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/build-test.yml
⏰ 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 (1)
src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java (1)
34-66: 픽스처 기반 테스트 리팩토링 잘 적용되었습니다.PR 목표에 맞춰 빌더 대신 Fixture 클래스를 사용하도록 테스트가 잘 리팩토링되었습니다. 테스트 데이터 생성이 일관되고 가독성도 향상되었습니다. 각 저장소 쿼리에 대한 검증 로직도 명확합니다.
Also applies to: 100-124
| @DisplayName("주어진 날짜를 기준으로 주어진 개수만큼 최신 폼지의 시작일과 지원 수를 반환한다.") | ||
| @Test | ||
| void findAllFileApplicationInfo() { | ||
| void findRecentFormByDateWithApplicationCount() { | ||
| // given | ||
| FormApplication formApplication = FormApplicationFixture.create("이름1", null); | ||
| FormApplication savedFormApplication = formApplicationRepository.save(formApplication); | ||
| FormAnswer formAnswer = FormAnswer.builder() | ||
| .formApplication(savedFormApplication) | ||
| .value(List.of()) | ||
| .build(); | ||
| FormAnswer savedFormAnswer = formAnswerRepository.save(formAnswer); | ||
| FormApplication formApplication2 = FormApplicationFixture.create("이름2", null); | ||
|
|
||
| FormApplication savedFormApplication2 = formApplicationRepository.save(formApplication2); | ||
| FormAnswer formAnswer2 = FormAnswer.builder() | ||
| .formApplication(savedFormApplication2) | ||
| .value(List.of()) | ||
| .build(); | ||
| FormAnswer savedFormAnswer2 = formAnswerRepository.save(formAnswer2); | ||
| FileMetaData fileMetaData = FileMetaData.builder() | ||
| .id(UUID.randomUUID()) | ||
| .entityId(savedFormAnswer.getId()) | ||
| .fileName("파일 이름1") | ||
| .domainType(DomainType.FORM_FILE) | ||
| .fileStatus(FileStatus.COUPLED) | ||
| .fileKey("1") | ||
| .build(); | ||
| FileMetaData fileMetaData2 = FileMetaData.builder() | ||
| .id(UUID.randomUUID()) | ||
| .entityId(savedFormAnswer2.getId()) | ||
| .fileName("파일 이름2") | ||
| .domainType(DomainType.FORM_FILE) | ||
| .fileStatus(FileStatus.COUPLED) | ||
| .fileKey("1") | ||
| .build(); | ||
| fileMetaDataRepository.saveAll(List.of(fileMetaData, fileMetaData2)); | ||
| List<Long> ids = List.of(savedFormAnswer.getId(), savedFormAnswer2.getId()); | ||
| Club savedClub = clubRepository.save(ClubFixture.createClub()); | ||
|
|
||
| Form savedForm1 = formRepository.save(FormFixture.createForm(savedClub, LocalDate.of(2020, 3, 1), LocalDate.of(2020, 4, 1))); | ||
| FormApplication formApplication1 = FormApplicationFixture.create(savedForm1); | ||
| FormApplication formApplication2 = FormApplicationFixture.create(savedForm1); | ||
| formApplicationRepository.saveAll(List.of(formApplication1, formApplication2)); | ||
|
|
||
| Form savedForm2 = formRepository.save(FormFixture.createForm(savedClub, LocalDate.of(2020, 1, 1), LocalDate.of(2020, 2, 1))); | ||
| FormApplication formApplication3 = FormApplicationFixture.create(savedForm2); | ||
| formApplicationRepository.save(formApplication3); | ||
|
|
||
| Form savedForm3 = formRepository.save(FormFixture.createForm(savedClub, LocalDate.of(2020, 5, 1), LocalDate.of(2020, 6, 1))); | ||
|
|
||
| // when | ||
| List<RecentFormInfo> recentFormInfos = formApplicationRepository.findRecentFormByDateWithApplicationCount( | ||
| savedClub.getId(), | ||
| savedForm1.getEndDate(), | ||
| 3 | ||
| ); | ||
|
|
||
| // then | ||
| assertThat(recentFormInfos.size()).isEqualTo(2); | ||
| assertThat(recentFormInfos.get(0).getCount()).isEqualTo(1); | ||
| assertThat(recentFormInfos.get(0).getDate()).isEqualTo(LocalDate.of(2020, 1, 1)); | ||
| assertThat(recentFormInfos.get(1).getCount()).isEqualTo(2); | ||
| assertThat(recentFormInfos.get(1).getDate()).isEqualTo(LocalDate.of(2020, 3, 1)); | ||
| } |
There was a problem hiding this comment.
테스트 설명과 실제 정렬 순서 불일치.
이전 리뷰에서 이미 지적된 내용입니다. @DisplayName에 "최신 폼지"라고 명시되어 있으나, 실제 검증 로직(라인 94-97)은 오름차순(오래된 날짜부터)으로 결과를 확인하고 있습니다.
- 첫 번째 결과: 2020-01-01 (가장 오래된 폼)
- 두 번째 결과: 2020-03-01 (중간 폼)
"최신"의 의미에 맞게 테스트 설명을 수정하거나, 리포지토리 쿼리의 정렬 방향을 내림차순으로 변경해 주세요.
🤖 Prompt for AI Agents
In
@src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java
around lines 68 - 98, The test's DisplayName says "latest forms" but assertions
check ascending order; update the test
(FormApplicationRepositoryTest.findRecentFormByDateWithApplicationCount) so it
asserts results are sorted newest-first: expect recentFormInfos.get(0).getDate()
== 2020-03-01 with count 2 and recentFormInfos.get(1).getDate() == 2020-01-01
with count 1, or alternatively change the repository method
findRecentFormByDateWithApplicationCount to return results in descending order;
pick one approach and make the assertions or query sort consistent with the "최신"
description.
🚀 작업 내용
FormApplication 도메인에 속한 repository 테스트 코드 작성 및 리팩토링 하였습니다.
🤔 고민했던 내용
given에서 필요한 객체 생성은 테스트 코드 내부에서 빌더를 사용하지 않고 전부 Fixture의 메소드를 사용해서 생성되도록 했습니다.
이 테스트가 필요한지에 대해 고민해보고 작성하라고 하셔서 고민해봤는데, 리포지토리 테스트에서는 직접 작성한 모든 쿼리에 대한 테스트가 필요하다고 생각해서 그에 대한 테스트를 작성했습니다.
💬 리뷰 중점사항
formapplication 패키지와 Fixture 클래스가 생긴 것을 늦게 확인해서 1월 2일자 커밋은 무시해주시길 바랍니다...
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.