Test/통합 테스트 보완#34
Hidden character warning
Conversation
There was a problem hiding this comment.
Pull request overview
RabbitMQ 이벤트 발행 경로를 Testcontainers 기반 통합 테스트로 검증할 수 있도록 테스트 인프라를 추가하고, 관련 매퍼/핸들러/퍼블리셔 테스트를 보강한 PR입니다.
Changes:
- Testcontainers(RabbitMQ) 의존성 추가 및 RabbitMQ 컨테이너 기반 테스트 지원 클래스 도입
- 스케줄 상태 변경 시 RabbitMQ 메시지 발행을 검증하는 통합 테스트 추가
- 이벤트 매퍼/핸들러/퍼블리셔 단위 테스트 추가, CI/CD에서 Docker 체크 및 테스트 아티팩트 업로드 추가
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/java/me/gg/pinit/pinittask/infrastructure/events/support/RabbitMqTestcontainersSupport.java | RabbitMQ Testcontainers 구동 및 Spring RabbitMQ 설정 주입/로그 수집 지원 |
| src/test/java/me/gg/pinit/pinittask/infrastructure/events/schedule/mapper/ScheduleEventMapperTest.java | 스케줄 이벤트 매퍼의 exchange/routingKey/payload 매핑 단위 테스트 |
| src/test/java/me/gg/pinit/pinittask/infrastructure/events/schedule/handler/ScheduleIntegratedEventHandlerTest.java | 통합 이벤트 핸들러가 publisher에 위임하는지 검증 |
| src/test/java/me/gg/pinit/pinittask/infrastructure/events/schedule/ScheduleStateRabbitPublishIntegrationTest.java | 상태 변경 서비스 호출 → RabbitMQ 메시지 발행 통합 검증 |
| src/test/java/me/gg/pinit/pinittask/infrastructure/events/RabbitEventPublisherTest.java | 이벤트 매퍼 유무에 따른 RabbitTemplate 호출 여부 단위 테스트 |
| build.gradle | Testcontainers(JUnit Jupiter, RabbitMQ) 테스트 의존성 추가 |
| AGENTS.md | 저장소 구조/테스트/PR 가이드라인 문서 추가 |
| .github/workflows/ci.yml | Docker availability 체크 및 테스트 아티팩트 업로드 추가 |
| .github/workflows/cd.yml | Docker availability 체크 및 테스트 아티팩트 업로드 추가 |
| if (System.getProperty("api.version") == null || System.getProperty("api.version").isBlank()) { | ||
| System.setProperty("api.version", "1.44"); | ||
| } | ||
|
|
There was a problem hiding this comment.
(1) 문제점: 테스트 지원 코드에서 System.setProperty("api.version", "1.44")로 전역 System Property를 런타임에 변경하고 있습니다.
(2) 영향: 동일 JVM에서 실행되는 다른 테스트/라이브러리(docker-java/Testcontainers 포함)의 Docker API 협상에 예상치 못한 영향을 줄 수 있고, 환경에 따라 오히려 연결 실패를 유발할 수 있습니다.
(3) 수정 제안: 가능하면 이 전역 설정은 제거하고 docker-java/Testcontainers의 기본 협상에 맡기거나, 정말 필요하다면 CI/로컬 실행 환경변수(DOCKER_API_VERSION 등)나 Gradle 테스트 JVM 옵션으로 명시적으로 주입해 테스트 코드가 전역 상태를 바꾸지 않도록 해주세요.
| if (System.getProperty("api.version") == null || System.getProperty("api.version").isBlank()) { | |
| System.setProperty("api.version", "1.44"); | |
| } |
| } catch (Throwable throwable) { | ||
| rabbitAvailable = false; | ||
| unavailableReason = throwable.getClass().getSimpleName() + ": " + throwable.getMessage(); | ||
| System.err.println("[RabbitMQ Testcontainers] " + unavailableReason); | ||
| } |
There was a problem hiding this comment.
(1) 문제점: configureRabbitProperties에서 모든 예외(Throwable)를 잡아 rabbitAvailable=false로만 처리하면, Docker는 사용 가능하지만 컨테이너 시작/설정이 실패한 경우에도 테스트가 ‘스킵’으로 끝날 수 있습니다.
(2) 영향: CI에서 실제로는 RabbitMQ 발행 검증이 실패했는데도 테스트가 건너뛰어져 통합 테스트의 신뢰성이 떨어질 수 있습니다(실패를 놓칠 수 있음).
(3) 수정 제안: Docker 비가용인 경우에만 스킵하고, 그 외 컨테이너 start 실패/설정 오류는 예외를 다시 던져 테스트를 실패시키거나(최소한 fail-fast), 스킵/실패를 구분할 수 있도록 분기 처리해 주세요.
| private static final Path CONTAINER_LOG_PATH = Path.of("build", "testcontainers-logs", "rabbitmq-container.log"); | ||
| private static volatile boolean rabbitAvailable; |
There was a problem hiding this comment.
(1) 문제점: 컨테이너 로그 파일 경로가 build/testcontainers-logs/rabbitmq-container.log로 고정되어 있어, 여러 테스트 클래스/재시도 실행 시 로그가 서로 덮어써질 수 있습니다.
(2) 영향: 실패 원인 분석 시 어떤 테스트 실행의 로그인지 추적이 어려워지고, 가장 중요한 실패 시점의 로그가 유실될 수 있습니다.
(3) 수정 제안: 테스트 클래스명/타임스탬프/컨테이너 ID 등을 포함해 실행 단위로 고유한 로그 파일명을 사용하도록 변경해 주세요.
변경된 점