test: 콘텐츠 통합 테스트 작성#272
Conversation
📝 WalkthroughWalkthroughContentController, ContentCollectionController, ContentFacade, ContentRepository에 대한 통합 테스트가 추가되었다. ContentControllerIntegrationTest와 ContentCollectionControllerIntegrationTest는 인증/인가, 요청 검증, 응답 코드, DB 반영과 배치 실행 경로를 확인한다. ContentFacadeIntegrationTest는 썸네일 업로드와 롤백 흐름을 검증한다. ContentRepositoryTest는 QueryDSL 필터와 커서 페이지네이션 테스트를 확장한다. 기존 ContentCollectionControllerTest는 삭제되었다. Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/test/java/com/codeit/team5/mopl/content/controller/ContentCollectionControllerIntegrationTest.java`:
- Around line 60-72: The authentication helper logic in
ContentCollectionControllerIntegrationTest is duplicated elsewhere, so extract
adminAuth, userAuth, and authOf into a shared test support class such as
IntegrationTestSecuritySupport. Keep the helper behavior centralized by moving
the common UsernamePasswordAuthenticationToken/MoplUserDetails/AuthUser
construction there, then update both integration tests to call the shared helper
instead of maintaining separate copies.
- Around line 136-161: The current tests around
ContentCollectionControllerIntegrationTest only verify status codes and do not
confirm that ContentCollectionController correctly maps request params into
JobParameters. Update the success path test for the async job launch in the
controller flow by capturing the JobParameters passed to asyncJobLauncher.run
and asserting that startPage and endPage are populated with the expected values
(for example 1 and 5). Keep the existing failure test for
JobParametersInvalidException, but add the parameter-mapping assertion in a
successful case to ensure the controller does not ignore or mis-map the request
inputs.
In
`@src/test/java/com/codeit/team5/mopl/content/controller/ContentControllerIntegrationTest.java`:
- Around line 216-276: Add missing unauthenticated-request coverage for the
PATCH and DELETE content integration tests: in the PatchContent and
DeleteContent nested classes of ContentControllerIntegrationTest, add tests that
call the corresponding endpoints without authentication and assert 401, matching
the existing pattern used in PostContent, GetContent, and GetContents. Reuse the
same MockMvc setup with csrf but omit authentication, and verify the security
filter chain is enforcing access consistently for the patch/delete paths.
In
`@src/test/java/com/codeit/team5/mopl/content/facade/ContentFacadeIntegrationTest.java`:
- Around line 159-173: The `update_notFound_rollsBackStorage` test is asserting
too broadly with `RuntimeException.class`, so tighten it to the specific
not-found exception expected by the update flow. Update the `assertThatThrownBy`
assertion in `ContentFacadeIntegrationTest.update_notFound_rollsBackStorage` to
verify the same exception type used by `ContentControllerIntegrationTest` and
the `contentFacade.update(...)` path, likely `ContentNotFoundException`, so
unrelated runtime bugs won’t pass the test.
In
`@src/test/java/com/codeit/team5/mopl/content/repository/ContentRepositoryTest.java`:
- Around line 293-349: The watcherCount and rate cursor tests only cover unique
values, so they do not verify the id tie-break behavior used by cursor
pagination. Update findContents_watcherCountDescCursor and
findContents_rateDescCursor in ContentRepositoryTest to include at least two
contents with the same watcherCount and the same averageRating, then assert that
ContentRepository.findContents orders and paginates them correctly using the
secondary id sort key. Use the existing request(...) setup and
ContentSortByType.WATCHER_COUNT / ContentSortByType.RATE paths to confirm no
duplicates or omissions across pages.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 40660755-48ec-4e51-8a25-9a7e75bcc6ab
📒 Files selected for processing (5)
src/test/java/com/codeit/team5/mopl/content/controller/ContentCollectionControllerIntegrationTest.javasrc/test/java/com/codeit/team5/mopl/content/controller/ContentCollectionControllerTest.javasrc/test/java/com/codeit/team5/mopl/content/controller/ContentControllerIntegrationTest.javasrc/test/java/com/codeit/team5/mopl/content/facade/ContentFacadeIntegrationTest.javasrc/test/java/com/codeit/team5/mopl/content/repository/ContentRepositoryTest.java
💤 Files with no reviewable changes (1)
- src/test/java/com/codeit/team5/mopl/content/controller/ContentCollectionControllerTest.java
2fe72ba to
4e457df
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/test/java/com/codeit/team5/mopl/content/controller/ContentCollectionControllerIntegrationTest.java`:
- Around line 241-259: The test in ContentCollectionControllerIntegrationTest
hardcodes the expected leagueId as a magic number, which should instead come
from the enum used by the controller mapping. Update the assertion in
success_returnsAccepted to compare params.getString("leagueId") against
SportsDbLeague.EPL.getLeagueId() so the test stays aligned with the enum
definition while still verifying that the request is mapped correctly.
In
`@src/test/java/com/codeit/team5/mopl/content/facade/ContentFacadeIntegrationTest.java`:
- Line 154: The async rollback delete verification in
ContentFacadeIntegrationTest hardcodes the same timeout value in multiple
places. Extract the shared timeout used by verify(binaryContentStorage,
timeout(3000)) into a class-level constant and update both verification calls to
use that constant, keeping the change aligned with the
ContentFacadeIntegrationTest test methods that check rollback deletion.
- Around line 152-154: The rollback verification is too loose because
binaryContentStorage.delete is only checked with anyString(), so the test never
confirms the deleted key matches the one returned from
binaryContentStorage.store. Update the affected ContentFacadeIntegrationTest
cases to capture the argument passed to store and delete (for example with an
ArgumentCaptor) and assert they are the same, using the
binaryContentStorage.store and binaryContentStorage.delete calls as the
reference points.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 671d5dc4-941a-47c0-93ac-64af26c77033
📒 Files selected for processing (6)
src/test/java/com/codeit/team5/mopl/content/controller/ContentCollectionControllerIntegrationTest.javasrc/test/java/com/codeit/team5/mopl/content/controller/ContentCollectionControllerTest.javasrc/test/java/com/codeit/team5/mopl/content/controller/ContentControllerIntegrationTest.javasrc/test/java/com/codeit/team5/mopl/content/facade/ContentFacadeIntegrationTest.javasrc/test/java/com/codeit/team5/mopl/content/repository/ContentRepositoryTest.javasrc/test/java/com/codeit/team5/mopl/global/support/security/IntegrationTestSecuritySupport.java
💤 Files with no reviewable changes (1)
- src/test/java/com/codeit/team5/mopl/content/controller/ContentCollectionControllerTest.java
shong9124
left a comment
There was a problem hiding this comment.
테스트 실행 로직상 문제 없는 것 확인하고 승인하겠습니다!
추후에 커버리지에서 걸리면 실패 케이스 누락된 부분 몇 개 더 추가하면 될 것 같습니다!
주말까지 고생하셨습니다~
- 인증 헬퍼 IntegrationTestSecuritySupport로 추출 - 수집 Job 파라미터 전달 검증 추가 (ArgumentCaptor) - PATCH/DELETE 미인증 401 테스트 추가 - watcherCount/rate 커서 동률 tie-break 테스트 추가 - 롤백 예외 검증을 ContentNotFoundException으로 구체화
4e457df to
c878529
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/test/java/com/codeit/team5/mopl/content/controller/ContentCollectionControllerIntegrationTest.java (1)
241-259: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winleagueId 매직넘버 "4328", enum 참조로 바꾸는 게 더 안전합니다.
"4328"은SportsDbLeagueenum의 내부 매핑에 전적으로 의존하는 값이라, 나중에 매핑 데이터가 갱신되면 이 테스트는 "왜 실패했는지" 바로 알기 어려운 상태로 깨지게 됩니다. 실패 메시지만 봐서는 원인 추적에 시간이 걸릴 수 있어요.
SportsDbLeague.EPL.getLeagueId()를 직접 참조하면 enum 정의와 항상 동기화되면서, 컨트롤러가 요청 파라미터를 올바르게 매핑하는지 검증한다는 테스트 본연의 목적도 그대로 유지됩니다.♻️ 제안: enum 참조로 변경
verify(asyncJobLauncher).run(any(), paramsCaptor.capture()); JobParameters params = paramsCaptor.getValue(); - assertThat(params.getString("leagueId")).isEqualTo("4328"); + assertThat(params.getString("leagueId")).isEqualTo(SportsDbLeague.EPL.getLeagueId()); assertThat(params.getString("season")).isEqualTo("2023-2024");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/codeit/team5/mopl/content/controller/ContentCollectionControllerIntegrationTest.java` around lines 241 - 259, The test in ContentCollectionControllerIntegrationTest is asserting a hardcoded leagueId magic number, which is brittle against future mapping changes. Replace the fixed "4328" expectation with the corresponding SportsDbLeague enum reference used by the production mapping, so the assertion stays aligned with the enum definition. Update the success_returnsAccepted test to verify the captured JobParameters against SportsDbLeague.EPL.getLeagueId() while keeping the existing league and season checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/test/java/com/codeit/team5/mopl/content/controller/ContentCollectionControllerIntegrationTest.java`:
- Around line 167-235: The TV/Sports/SportsDay controller test sections are
missing the failure-path coverage that already exists in CollectTmdbMovies for
async job launch exceptions. Add matching 500-response tests in
CollectTmdbTvSeries, CollectSportsEvents, and CollectSportsEventsByDay by
stubbing asyncJobLauncher.run(...) to throw the same exception and asserting the
controller’s run() endpoint returns Internal Server Error, following the
existing CollectTmdbMovies pattern and using the same mockMvc/verify setup.
---
Duplicate comments:
In
`@src/test/java/com/codeit/team5/mopl/content/controller/ContentCollectionControllerIntegrationTest.java`:
- Around line 241-259: The test in ContentCollectionControllerIntegrationTest is
asserting a hardcoded leagueId magic number, which is brittle against future
mapping changes. Replace the fixed "4328" expectation with the corresponding
SportsDbLeague enum reference used by the production mapping, so the assertion
stays aligned with the enum definition. Update the success_returnsAccepted test
to verify the captured JobParameters against SportsDbLeague.EPL.getLeagueId()
while keeping the existing league and season checks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5eed84d6-fec7-48af-92d3-d16f4854003a
📒 Files selected for processing (6)
src/test/java/com/codeit/team5/mopl/content/controller/ContentCollectionControllerIntegrationTest.javasrc/test/java/com/codeit/team5/mopl/content/controller/ContentCollectionControllerTest.javasrc/test/java/com/codeit/team5/mopl/content/controller/ContentControllerIntegrationTest.javasrc/test/java/com/codeit/team5/mopl/content/facade/ContentFacadeIntegrationTest.javasrc/test/java/com/codeit/team5/mopl/content/repository/ContentRepositoryTest.javasrc/test/java/com/codeit/team5/mopl/global/support/security/IntegrationTestSecuritySupport.java
💤 Files with no reviewable changes (1)
- src/test/java/com/codeit/team5/mopl/content/controller/ContentCollectionControllerTest.java
관련 이슈
작업 내용
변경 사항
테스트 내용
체크리스트
리뷰 포인트
스크린샷 / 참고 자료
🔗 관련 이슈
#측정하지 않음📝 작업 내용
이 PR은 content 도메인에서 핵심 로직이 단위 테스트 중심으로만 검증되던 한계를 보완하기 위해, 실제 Spring 요청 흐름(인증/인가/예외 전파)과 DB 반영까지 포함하는 통합 테스트를 추가한 작업입니다. 특히 컨트롤러–퍼사드–레포지토리(및 QueryDsl) 경계를 “단일 컴포넌트”가 아니라 “실제 호출 흐름”에 가깝게 검증하도록 구성해, 기존에 놓치기 쉬웠던 입력 파라미터 검증, 권한별 응답 코드, 비동기 배치 실행 경로, 썸네일 상태 전이/롤백 같은 회귀 리스크를 줄이는 데 목적이 있습니다.
기존에는
ContentCollectionControllerTest가 컨텐츠 수집 관리자 API를 중심으로 테스트하고 있었으나, 이번 변경에서는 이를 통합 테스트로 대체해 보안 필터 및 요청 검증 로직까지 포함한 엔드포인트 단위 검증으로 전환했습니다. 또한 외부/비동기 실행 의존성은 테스트 안정성을 위해 부분적으로 목킹(asyncJobLauncher, 외부BinaryContentStorage)하되, 그 외에는 실제 컴포넌트 조합으로 동작을 확인하도록 선택했습니다.주요 변경사항
컨트롤러 레벨 통합 테스트 추가로 “권한/입력/예외” 흐름을 실제로 검증
ContentCollectionControllerIntegrationTest와ContentControllerIntegrationTest를 추가해 MockMvc 기반으로 401/403, 400 요청 파라미터 검증 실패, 202/500 비동기/배치 경로 및 예외 케이스를 엔드포인트별로 검증했습니다.asyncJobLauncher만@MockitoBean으로 대체하고,ArgumentCaptor로JobParameters전달 값까지 확인하도록 구성했습니다.퍼사드 통합 테스트로 썸네일 업로드/연결 및 상태 전이/롤백 보장
ContentFacadeIntegrationTest에서 썸네일 미업로드/업로드/업데이트 시나리오를 포괄적으로 검증했습니다.DELETED로 전환되고 신규 썸네일이 연결되는지 확인했으며, 업로드 후 후속 작업 예외 발생 시BinaryContentStorage.delete가 롤백 경로로 비동기 호출되는지까지 검증했습니다(외부 저장소는 목 처리).QueryDsl(필터/정렬/커서 페이지네이션) 검증을 통합 테스트 범위로 확장
ContentRepositoryTest에findContents/countContents의 QueryDsl 분기별 테스트(타입/키워드/태그 조합, 커서 기반 페이지네이션의 누락·중복 방지, 정렬 타이브레이크)를 보강해 “단일 쿼리 결과의 정확성”을 더 두텁게 확인할 수 있게 했습니다.📊 변경 효과 요약 (가능한 경우에만)
ContentCollectionControllerIntegrationTest,ContentControllerIntegrationTest,ContentFacadeIntegrationTest,IntegrationTestSecuritySupportContentCollectionControllerTest+1419 / -224