Skip to content

Conversation

@hyxklee
Copy link
Collaborator

@hyxklee hyxklee commented Oct 28, 2025

📌 PR 내용

  • 캘린더 도메인 중 repository, service layer에 대한 테스트 코드를 작성했습니다.

🔍 PR 세부사항

  • @DataJpaTest의 경우 TestContainers 라이브러리를 활용해 테스트 전용 Mysql 컨테이너와 직접 통신해 실제 환경과 유사하게 테스트가 가능하게 했습니다
  • 라이브러리, 프레임워크에서 제공하는 기능의 경우는 검증의 영역에서 제외했고, 코드 레벨에서 검증이 가능한 영역 위주로 테스트를 작성했습니다.
  • 해당 도메인에서 공통으로 사용될 테스트 데이터들은 중복을 제거하기 위해 fixture로 미리 정의해서 static import 해와서 사용하도록 했습니다.

📸 관련 스크린샷


📝 주의사항

  • 라이브러리 버전이 spring 버전과 약간 안 맞을 수 있는데, 후에 스프링 버전을 올리도록 하겠습니다.

✅ 체크리스트

  • 리뷰어 설정
  • Assignee 설정
  • Label 설정
  • 제목 양식 맞췄나요? (ex. [WTH-01] PR 템플릿 수정)
  • 변경 사항에 대한 테스트

Summary by CodeRabbit

릴리스 노트

이번 업데이트는 내부 테스트 인프라 개선에 중점을 두었으며, 최종 사용자에게 직접적인 변화는 없습니다.

  • Tests

    • 테스트 환경 안정화를 위한 테스트 인프라 강화
    • 데이터베이스 기반 저장소 및 서비스 테스트 추가
  • Chores

    • 테스트 의존성 및 빌드 구성 업데이트

@coderabbitai
Copy link

coderabbitai bot commented Oct 28, 2025

Walkthrough

테스트 인프라를 구축하는 PR로, Testcontainers 기반 MySQL 컨테이너 설정, 저장소와 서비스 계층의 JUnit 5 테스트, 테스트 픽스처, 그리고 테스트용 애플리케이션 설정을 추가합니다.

Changes

응집(Cohort) / 파일 변경 요약
테스트 인프라 및 설정
build.gradle, src/test/resources/application-test.yml
Gradle에 junit-jupiter, testcontainers 및 spring-boot-testcontainers 테스트 의존성 추가. 테스트 프로파일용 JPA/Hibernate 및 SQL 로깅 설정 추가.
Testcontainers 구성
src/test/java/leets/weeth/config/TestContainersConfig.java, src/test/java/leets/weeth/config/TestContainersTest.java
MySQL 8.0.41 컨테이너를 관리하는 TestContainersConfig 테스트 설정 클래스 추가. 컨테이너 주입 및 작동 여부를 검증하는 TestContainersTest 추가.
저장소 테스트
src/test/java/leets/weeth/domain/schedule/domain/repository/EventRepositoryTest.java, src/test/java/leets/weeth/domain/schedule/domain/repository/MeetingRepositoryTest.java
EventRepository 및 MeetingRepository의 쿼리 메서드를 검증하는 @DataJpaTest 기반 테스트 클래스 추가. 각각 여러 조건의 조회 시나리오 테스트 포함.
서비스 단위 테스트
src/test/java/leets/weeth/domain/schedule/domain/service/EventGetServiceTest.java, src/test/java/leets/weeth/domain/schedule/domain/service/MeetingGetServiceTest.java
Mockito를 사용한 EventGetService 및 MeetingGetService 단위 테스트 추가. find() 성공/실패 시나리오 검증.
테스트 픽스처
src/test/java/leets/weeth/domain/schedule/test/fixture/ScheduleTestFixture.java
Event 및 Meeting 생성 팩토리 메서드를 제공하는 테스트 픽스처 유틸리티 클래스 추가.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

세부 검토 영역:

  • 저장소 테스트 파일들의 쿼리 검증 로직 및 테스트 데이터 설정 일관성 확인 (EventRepositoryTest.java, MeetingRepositoryTest.java)
  • Testcontainers 설정의 MySQL 이미지 버전 및 컨테이너 재사용 정책 검증 (TestContainersConfig.java)
  • 서비스 테스트의 예외 처리 및 Mock 상호작용 검증 (EventGetServiceTest.java, MeetingGetServiceTest.java)
  • 테스트 설정 파일(application-test.yml)의 Hibernate 방언 및 스키마 생성 정책이 MySQL 8.0.41과 호환성 확인

Poem

🐰 테스트의 정원을 심어, 컨테이너 속 씨앗 자라나고,
저장소와 서비스는 픽스처로 견고히,
Testcontainers 마법으로 MySQL 피어난다,
모든 쿼리가 춤을 춘다, 성공의 노래로! 🌱✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning 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 (2 passed)
Check name Status Explanation
Title Check ✅ Passed PR 제목 "WTH-40 캘린터 테스트 코드 작성 (Repository, Service)"는 변경사항의 핵심 내용을 명확하고 구체적으로 요약하고 있습니다. raw_summary에서 확인할 수 있듯이 이 PR은 캘린더 도메인의 repository 계층과 service 계층에 대한 여러 테스트 클래스를 추가하는 것이 주요 목표이며, 제목이 이를 정확하게 반영합니다. 이슈 번호도 포함되어 있어 추적성도 좋으며, 명확한 단일 문장으로 구성되어 있습니다.
Description Check ✅ Passed PR 설명은 제공된 템플릿의 모든 주요 섹션을 포함하고 있습니다. "PR 내용"에서 변경사항의 개요를 제시하고, "PR 세부사항"에서 TestContainers 라이브러리 활용, 테스트 작성 원칙, 테스트 데이터 fixture 활용 등 구현 방식을 상세히 설명합니다. "주의사항"에서 라이브러리 버전 호환성 이슈를 명시하고 있으며, 체크리스트도 모두 완료로 표시되어 있습니다. "관련 스크린샷" 섹션은 비어있으나 테스트 코드 PR의 특성상 선택 사항으로 볼 수 있습니다.
✨ 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 WTH-40-Weeth-캘린터-테스트-코드-작성

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (4)
build.gradle (1)

55-55: 명시적인 JUnit 버전 선언을 제거하는 것을 고려하세요.

JUnit Jupiter 5.8.1은 2021년에 릴리스된 버전으로, Spring Boot 3.3.1이 관리하는 버전(5.10.x)보다 오래되었습니다. 명시적인 버전 선언은 Spring Boot의 의존성 관리와 충돌할 수 있으며, 최신 기능과 버그 수정을 놓칠 수 있습니다.

다음 diff를 적용하여 Spring Boot가 관리하는 JUnit 버전을 사용하세요:

-	testImplementation "org.junit.jupiter:junit-jupiter:5.8.1"

참고: PR 설명에서 언급하신 것처럼 Spring 업그레이드 시 함께 처리하셔도 됩니다.

src/test/java/leets/weeth/config/TestContainersTest.java (1)

23-23: 로깅 방식을 개선하는 것을 고려하세요.

System.out.println 대신 로거를 사용하거나, 테스트 출력이 필요하지 않다면 제거하는 것이 좋습니다. 테스트 실행 시 불필요한 출력이 발생할 수 있습니다.

-        System.out.println("Container JDBC URL: " + mysqlContainer.getJdbcUrl());

또는 로거 사용:

private static final Logger log = LoggerFactory.getLogger(TestContainersTest.class);
// ...
log.info("Container JDBC URL: {}", mysqlContainer.getJdbcUrl());
src/test/java/leets/weeth/domain/schedule/domain/service/MeetingGetServiceTest.java (1)

43-43: 테스트 문서화 방식의 일관성을 고려하세요.

이 테스트는 @DisplayName을 사용하지만, EventGetServiceTest는 사용하지 않습니다. 프로젝트 전체에서 일관된 방식을 선택하는 것이 좋습니다.

Also applies to: 60-60

src/test/java/leets/weeth/domain/schedule/test/fixture/ScheduleTestFixture.java (1)

10-29: 테스트 안정성을 위해 고정된 시간 값 사용을 고려하세요.

LocalDateTime.now()를 사용하면 테스트 실행 시점에 따라 다른 결과가 나올 수 있습니다. 더 결정적인(deterministic) 테스트를 위해 고정된 시간 값을 사용하는 것을 고려해보세요.

예시:

private static final LocalDateTime BASE_TIME = LocalDateTime.of(2024, 1, 1, 10, 0);

public static Event createEvent() {
    return Event.builder()
            .title("Test Meeting")
            .location("Test Location")
            .start(BASE_TIME)
            .end(BASE_TIME.plusDays(2))
            .cardinal(1)
            .build();
}

다만, 현재 테스트 케이스들이 상대적인 시간 범위만 검증한다면 큰 문제는 없을 수 있습니다.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7aacb94 and 18a5164.

📒 Files selected for processing (9)
  • build.gradle (1 hunks)
  • src/test/java/leets/weeth/config/TestContainersConfig.java (1 hunks)
  • src/test/java/leets/weeth/config/TestContainersTest.java (1 hunks)
  • src/test/java/leets/weeth/domain/schedule/domain/repository/EventRepositoryTest.java (1 hunks)
  • src/test/java/leets/weeth/domain/schedule/domain/repository/MeetingRepositoryTest.java (1 hunks)
  • src/test/java/leets/weeth/domain/schedule/domain/service/EventGetServiceTest.java (1 hunks)
  • src/test/java/leets/weeth/domain/schedule/domain/service/MeetingGetServiceTest.java (1 hunks)
  • src/test/java/leets/weeth/domain/schedule/test/fixture/ScheduleTestFixture.java (1 hunks)
  • src/test/resources/application-test.yml (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/test/java/leets/weeth/domain/schedule/domain/service/EventGetServiceTest.java (2)
src/main/java/leets/weeth/domain/schedule/application/exception/EventNotFoundException.java (1)
  • EventNotFoundException (5-9)
src/test/java/leets/weeth/domain/schedule/test/fixture/ScheduleTestFixture.java (1)
  • ScheduleTestFixture (8-30)
src/test/java/leets/weeth/domain/schedule/domain/service/MeetingGetServiceTest.java (2)
src/main/java/leets/weeth/domain/schedule/application/exception/MeetingNotFoundException.java (1)
  • MeetingNotFoundException (5-7)
src/test/java/leets/weeth/domain/schedule/test/fixture/ScheduleTestFixture.java (1)
  • ScheduleTestFixture (8-30)
🔇 Additional comments (4)
src/test/resources/application-test.yml (1)

1-11: 테스트 설정이 적절합니다.

create-drop DDL 전략과 SQL 로깅 활성화는 Testcontainers 기반 통합 테스트에 적합한 설정입니다. MySQL8Dialect도 사용 중인 MySQL 8.0.41 이미지와 일치합니다.

src/test/java/leets/weeth/domain/schedule/domain/service/EventGetServiceTest.java (1)

22-67: 테스트 구조가 잘 작성되었습니다.

Mockito를 사용한 단위 테스트 구조가 명확하고, given/when/then 패턴이 일관되게 적용되었습니다. 정상 케이스와 예외 케이스 모두 적절히 검증하고 있습니다.

src/test/java/leets/weeth/config/TestContainersConfig.java (1)

9-19: Testcontainers 설정이 잘 구성되었습니다.

MySQL 8.0.41 이미지 사용과 withReuse(true) 설정은 적절합니다. @ServiceConnection 어노테이션이 자동으로 데이터소스 속성을 구성해주므로 깔끔한 설정입니다.

src/test/java/leets/weeth/domain/schedule/domain/repository/MeetingRepositoryTest.java (1)

18-150: 저장소 테스트가 포괄적이고 잘 작성되었습니다.

MeetingRepository의 다양한 쿼리 메서드들을 체계적으로 테스트하고 있습니다. AssertJ의 extracting()containsExactly() 등을 활용하여 가독성 높은 검증을 수행하고 있으며, Testcontainers를 통한 실제 데이터베이스 환경 테스트로 신뢰성도 확보하고 있습니다.

Comment on lines +25 to +50
@BeforeEach
void setUp() {
LocalDateTime now = LocalDateTime.now();
Event event1 = Event.builder()
.title("Event 1")
.start(now.plusDays(1))
.end(now.plusDays(2))
.cardinal(1)
.build();

Event event2 = Event.builder()
.title("Event 2")
.start(now.plusDays(3))
.end(now.plusDays(4))
.cardinal(1)
.build();

Event event3 = Event.builder()
.title("Event 3")
.start(now.plusDays(5))
.end(now.plusDays(6))
.cardinal(2)
.build();

eventRepository.saveAll(List.of(event1, event2, event3));
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

테스트 데이터 생성 방식을 개선하세요.

LocalDateTime.now()setUp과 테스트 메서드에서 각각 호출하면 시간 차이로 인해 테스트가 불안정해질 수 있습니다. 또한 PR 설명에서 "공통 테스트 데이터는 fixture로 정의하여 중복을 방지한다"고 명시했지만, 이 테스트는 ScheduleTestFixture를 사용하지 않고 있습니다.

다음과 같이 개선하는 것을 권장합니다:

 @BeforeEach
 void setUp() {
-    LocalDateTime now = LocalDateTime.now();
+    LocalDateTime now = LocalDateTime.of(2024, 1, 1, 0, 0);
     Event event1 = Event.builder()
             .title("Event 1")
             .start(now.plusDays(1))
             .end(now.plusDays(2))
             .cardinal(1)
             .build();

     Event event2 = Event.builder()
             .title("Event 2")
             .start(now.plusDays(3))
             .end(now.plusDays(4))
             .cardinal(1)
             .build();

     Event event3 = Event.builder()
             .title("Event 3")
             .start(now.plusDays(5))
             .end(now.plusDays(6))
             .cardinal(2)
             .build();

     eventRepository.saveAll(List.of(event1, event2, event3));
 }

또한 ScheduleTestFixture를 활용하여 코드 중복을 줄이는 것도 고려해보세요.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@BeforeEach
void setUp() {
LocalDateTime now = LocalDateTime.now();
Event event1 = Event.builder()
.title("Event 1")
.start(now.plusDays(1))
.end(now.plusDays(2))
.cardinal(1)
.build();
Event event2 = Event.builder()
.title("Event 2")
.start(now.plusDays(3))
.end(now.plusDays(4))
.cardinal(1)
.build();
Event event3 = Event.builder()
.title("Event 3")
.start(now.plusDays(5))
.end(now.plusDays(6))
.cardinal(2)
.build();
eventRepository.saveAll(List.of(event1, event2, event3));
}
@BeforeEach
void setUp() {
LocalDateTime now = LocalDateTime.of(2024, 1, 1, 0, 0);
Event event1 = Event.builder()
.title("Event 1")
.start(now.plusDays(1))
.end(now.plusDays(2))
.cardinal(1)
.build();
Event event2 = Event.builder()
.title("Event 2")
.start(now.plusDays(3))
.end(now.plusDays(4))
.cardinal(1)
.build();
Event event3 = Event.builder()
.title("Event 3")
.start(now.plusDays(5))
.end(now.plusDays(6))
.cardinal(2)
.build();
eventRepository.saveAll(List.of(event1, event2, event3));
}
🤖 Prompt for AI Agents
In
src/test/java/leets/weeth/domain/schedule/domain/repository/EventRepositoryTest.java
around lines 25-50, the setup uses LocalDateTime.now() directly which can
produce flaky tests and duplicates fixture logic; replace dynamic now() calls by
obtaining a single fixed reference time (e.g., a constant or from
ScheduleTestFixture) at the start of setUp and use that same reference in all
event creations, and refactor to use ScheduleTestFixture methods to create the
three events (or a single helper that returns prebuilt Event instances) then
saveAll those fixtures so tests share common, deterministic test data and avoid
duplicated construction logic.

Comment on lines +52 to +66
@Test
void testFindByStartLessThanEqualAndEndGreaterThanEqualOrderByStartAsc() {
// given
LocalDateTime start = LocalDateTime.now().plusDays(2);
LocalDateTime end = LocalDateTime.now().plusDays(7);

// when
List<Event> events = eventRepository.findByStartLessThanEqualAndEndGreaterThanEqualOrderByStartAsc(end, start);

// then
assertThat(events)
.hasSize(2)
.extracting(Event::getTitle)
.containsExactly("Event 2", "Event 3"); // 순서와 구성이 모두 일치하는지
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

쿼리 메서드 호출의 파라미터 순서와 테스트 로직을 검증하세요.

메서드 호출 시 (end, start) 순서로 파라미터를 전달하는 것이 직관적이지 않으며, 테스트 예상 결과에 문제가 있을 수 있습니다.

다음 사항들을 확인해주세요:

  1. 파라미터 순서: 메서드명에 StartLessThanEqual이 먼저 나오는데 (end, start) 순서로 전달하는 것은 혼란을 야기합니다. 메서드 시그니처를 확인하여 의도된 순서인지 검증이 필요합니다.

  2. 테스트 로직 오류:

    • Event 1: end=now+2 (정확히 범위의 시작점)
    • 쿼리 범위: [now+2, now+7]
    • end >= start 조건에 따라 Event 1도 포함되어야 하는데, 예상 결과에서 제외되어 있습니다.
  3. LocalDateTime.now() 재사용: setUp에서 사용한 now와 테스트 메서드에서 새로 호출한 LocalDateTime.now()가 달라서 Event 1이 범위에서 벗어날 수도 있습니다. 이는 테스트를 불안정하게 만듭니다.

setUp에서 정의한 고정된 LocalDateTime을 클래스 필드로 저장하여 테스트 메서드에서도 재사용하는 것을 권장합니다.



---

🏁 Script executed:

```shell
#!/bin/bash
# Find EventRepository to check method signature
find . -type f -name "*EventRepository*" | head -20

Length of output: 169


🏁 Script executed:

#!/bin/bash
# Find and display the EventRepositoryTest class, particularly setUp and the test method
rg -n "class EventRepositoryTest" -A 100 src/test/java/leets/weeth/domain/schedule/domain/repository/ | head -150

Length of output: 7338


🏁 Script executed:

#!/bin/bash
# Search for the method definition in repository
rg -n "findByStartLessThanEqualAndEndGreaterThanEqualOrderByStartAsc" . -A 2

Length of output: 3076


메서드 파라미터 순서 불일치와 테스트 로직 오류를 수정하세요.

EventRepository와 MeetingRepository의 메서드 파라미터 순서가 서로 다릅니다:

  • EventRepository: (LocalDateTime end, LocalDateTime start)
  • MeetingRepository: (LocalDateTime start, LocalDateTime end)

그런데 두 서비스 모두 (end, start) 순서로 호출하므로 MeetingGetService에서 잘못된 파라미터가 전달됩니다.

또한 테스트 예상 결과가 잘못되었습니다:

setUp의 Event 1 (start=now+1, end=now+2)은 쿼리 조건 start <= now+7 AND end >= now+2를 만족하므로 결과에 포함되어야 합니다. 현재 예상 결과에서 Event 1을 제외하는 것은 오류입니다.

권장 사항:

  1. 파라미터 순서를 통일하세요. 메서드명의 의미(StartLessThanEqual, EndGreaterThanEqual)에 맞춰 (LocalDateTime start, LocalDateTime end) 순서로 정렬하는 것을 권장합니다.
  2. 테스트 예상 결과를 3개 이벤트 모두 포함하도록 수정하세요: .hasSize(3).containsExactly("Event 1", "Event 2", "Event 3")
  3. LocalDateTime.now()를 setUp에서 클래스 필드로 저장하여 재사용하세요.
🤖 Prompt for AI Agents
In
src/test/java/leets/weeth/domain/schedule/domain/repository/EventRepositoryTest.java
around lines 52-66: the test calls the repository with reversed parameter order
and asserts the wrong expected results; unify repository method signatures to
(LocalDateTime start, LocalDateTime end) for both EventRepository and
MeetingRepository (rename params if necessary), update all call sites (e.g.,
MeetingGetService and the test) to pass (start, end) in that order, modify the
test to use a single LocalDateTime "now" captured in setUp and reuse it (avoid
multiple LocalDateTime.now() calls), and change the assertions to expect all
three events: .hasSize(3) and .containsExactly("Event 1", "Event 2", "Event 3").

Comment on lines +68 to +81
@Test
void testFindAllByCardinal() {
// given
int cardinal = 1;

// when
List<Event> events = eventRepository.findAllByCardinal(cardinal);

// then
assertThat(events)
.hasSize(2)
.extracting(Event::getTitle)
.containsExactly("Event 1", "Event 2");
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

쿼리 결과 순서에 대한 가정을 검증하세요.

findAllByCardinal 메서드는 정렬 순서를 명시하지 않는데, containsExactly를 사용하여 특정 순서를 기대하고 있습니다.

Spring Data JPA는 메서드명에 OrderBy가 없거나 @OrderBy 어노테이션이 없으면 쿼리 결과의 순서를 보장하지 않습니다. 다음 중 하나를 선택하세요:

  1. 순서가 중요하다면: 쿼리 메서드명을 findAllByCardinalOrderByStartAsc 등으로 변경하여 명시적으로 정렬 지정
  2. 순서가 중요하지 않다면: containsExactlyInAnyOrder로 변경하여 순서에 무관하게 검증
 assertThat(events)
         .hasSize(2)
         .extracting(Event::getTitle)
-        .containsExactly("Event 1", "Event 2");
+        .containsExactlyInAnyOrder("Event 1", "Event 2");

쿼리 메서드의 실제 구현을 확인하여 정렬이 보장되는지 검증해주세요.



---

🏁 Script executed:

```shell
#!/bin/bash
# Find EventRepository interface/class definition
fd -e java -e kt | xargs grep -l "findAllByCardinal" | head -20

Length of output: 893


🏁 Script executed:

#!/bin/bash
# Locate the EventRepository file
fd "EventRepository\." -e java -e kt

Length of output: 80


🏁 Script executed:

#!/bin/bash
# Find test setup - look for @BeforeEach or test data initialization in EventRepositoryTest
fd "EventRepositoryTest" -e java

Length of output: 84



---

🏁 Script executed:

```shell
#!/bin/bash
# Get EventRepository method definition
cat src/main/java/leets/weeth/domain/schedule/domain/repository/EventRepository.java

Length of output: 472


🏁 Script executed:

#!/bin/bash
# Get the full test file to see setup and all test methods
cat src/test/java/leets/weeth/domain/schedule/domain/repository/EventRepositoryTest.java

Length of output: 2544



---

🏁 Script executed:

```shell
#!/bin/bash
# Check Event entity for @OrderBy or default ordering
fd "Event\.java" -path "*/schedule/domain/entity/*" -e java

Length of output: 168


🏁 Script executed:

#!/bin/bash
# Get Event entity definition
cat src/main/java/leets/weeth/domain/schedule/domain/entity/Event.java

Length of output: 555



---

🏁 Script executed:

```shell
#!/bin/bash
# Find Schedule parent class
fd "Schedule\.java" -path "*/schedule/domain/entity/*"

Length of output: 168


🏁 Script executed:

#!/bin/bash
# Get Schedule entity definition
cat src/main/java/leets/weeth/domain/schedule/domain/entity/Schedule.java

Length of output: 1406


쿼리 메서드의 정렬 순서 미지정으로 인한 테스트 불안정성 문제 확인됨

findAllByCardinal 메서드에는 정렬을 명시하는 어떤 장치도 없습니다.

문제점:

  • EventRepositoryfindAllByCardinal(int cardinal) 메서드에 OrderBy 절이나 @OrderBy 어노테이션이 없음
  • Event 엔티티와 Schedule 부모 클래스에도 기본 정렬 지정이 없음
  • Spring Data JPA는 명시적 정렬이 없으면 결과 순서를 보장하지 않음
  • containsExactly는 순서와 내용을 모두 검증하므로, 순서 보장 없이 사용 시 테스트가 불안정할 수 있음

해결 방안 (둘 중 선택):

  1. 순서가 중요하지 않다면: 정렬 순서를 명시하지 않고 검증하도록 변경

    assertThat(events)
            .hasSize(2)
            .extracting(Event::getTitle)
    -        .containsExactly("Event 1", "Event 2");
    +        .containsExactlyInAnyOrder("Event 1", "Event 2");
  2. 순서가 중요하다면: 리포지토리 메서드명을 변경하여 정렬 명시

    • findAllByCardinalOrderByIdAsc 또는 다른 정렬 기준 지정
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@Test
void testFindAllByCardinal() {
// given
int cardinal = 1;
// when
List<Event> events = eventRepository.findAllByCardinal(cardinal);
// then
assertThat(events)
.hasSize(2)
.extracting(Event::getTitle)
.containsExactly("Event 1", "Event 2");
}
@Test
void testFindAllByCardinal() {
// given
int cardinal = 1;
// when
List<Event> events = eventRepository.findAllByCardinal(cardinal);
// then
assertThat(events)
.hasSize(2)
.extracting(Event::getTitle)
.containsExactlyInAnyOrder("Event 1", "Event 2");
}

Comment on lines +50 to +56
// when
Meeting meeting = meetingGetService.find(1L);


// when
assertThat(meeting).isEqualTo(testMeeting);
verify(meetingRepository).findById(1L);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

중복된 주석을 수정하세요.

Line 54의 주석이 // when으로 되어 있지만, 실제로는 검증 단계이므로 // then이어야 합니다.

         // when
         Meeting meeting = meetingGetService.find(1L);
 
 
-        // when
+        // then
         assertThat(meeting).isEqualTo(testMeeting);
         verify(meetingRepository).findById(1L);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// when
Meeting meeting = meetingGetService.find(1L);
// when
assertThat(meeting).isEqualTo(testMeeting);
verify(meetingRepository).findById(1L);
// when
Meeting meeting = meetingGetService.find(1L);
// then
assertThat(meeting).isEqualTo(testMeeting);
verify(meetingRepository).findById(1L);
🤖 Prompt for AI Agents
In
src/test/java/leets/weeth/domain/schedule/domain/service/MeetingGetServiceTest.java
around lines 50 to 56, there is a duplicated/misleading comment: the comment on
line 54 is `// when` but this section performs assertions, so change that
comment to `// then` (or remove the duplicate `// when`) so the test clearly
separates the action and verification phases; update the single comment
accordingly.

.hasSize(2)
.extracting(Meeting::getTitle)
.containsExactly("Meeting 1", "Meeting 2");
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

의도나 명확성이 잘 보이게끔 설계된 코드로 보입니다!
허나 메서드명이 긴만큼 .. 추가 설명이나 네이밍을 좀 이해하기 쉽게 해주면 좋을 것 같아요 😊


@Mock
private ScheduleMapper scheduleMapper;

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ScheduleMapper 이녀석은 위 파일에서 사용하지 않는 것 같은데 ... 작성하신 이유가 있을까요!?
필요없다면 제거하는게 좋아보입니다 !😄

@InjectMocks
private EventGetService eventGetService;

private Event testEvent;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

요 친구만 Mock이 아닌 일반 객체로 만든 이유가 있을까요!? 궁금합니다.
아래의 코드들로 짐작해보아 .. 단순 반환용이므로 의도하신걸까요!?

.end(now.plusDays(2))
.cardinal(1)
.build();

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setUp 내부 객체 생성 로직도 , Fixture로 처리할 수 있지 않을까요!? 😲❔

Copy link
Collaborator

@seokjun01 seokjun01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨습니다. 전반적으로 가독성 좋고 이해하기 쉽게 코드 작성해주셔서 문제는 없어보입니다!
궁금한 점 코멘트 남겨두었습니다 😁

Copy link
Collaborator

@jj0526 jj0526 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수고하셨습니다!

Comment on lines +27 to +34
Meeting meeting1 = Meeting.builder()
.title("Meeting 1")
.start(LocalDateTime.now().plusDays(1))
.end(LocalDateTime.now().plusDays(2))
.code(1111)
.cardinal(1)
.meetingStatus(MeetingStatus.OPEN)
.build();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

픽스쳐로 간단하게 할수있을거같아요!

Comment on lines +10 to +28
public static Event createEvent() {
return Event.builder()
.title("Test Meeting")
.location("Test Location")
.start(LocalDateTime.now())
.end(LocalDateTime.now().plusDays(2))
.cardinal(1)
.build();
}

public static Meeting createMeeting() {
return Meeting.builder()
.title("Test Meeting")
.location("Test Location")
.start(LocalDateTime.now())
.end(LocalDateTime.now().plusDays(2))
.code(1234)
.cardinal(1)
.build();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

동일한 객체만 생성하고 있는데, title이나 start 값을 매번 다르게 생성하도록 하면 테스트 데이터가 다양해지고 중복 문제를 줄일수 있을거같은데 어떠신가요?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants