[Test] Keychain/NetworkClient 단위 테스트 추가 및 Core 구조 정리#32
Conversation
목표 디렉토리 구조에 맞춰 Config.swift는 Core/Config로, APITargetType.swift는 Core/NetworkAdapter로 이동. Config.xcconfig 이동에 따른 pbxproj 경로도 갱신.
- KeychainTokenStore: 저장/조회/삭제/덮어쓰기, 빈 상태 nil, service 격리, actor 동시 접근 직렬화 (6개) - NetworkClient: Bearer 주입, 401 리프레시 재시도, 동시 401 시 리프레시 1회 보장, 재시도 소진, 로그아웃 토큰 삭제 (9개)
|
Warning Review limit reached
Next review available in: 55 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Walkthrough
Changesxcconfig 경로 변경
테스트 스위트 추가
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
pbxproj가 참조하는 xcconfig 경로 이동에 맞춰 CI placeholder 생성 위치도 Resource → Core/Config로 변경.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
Rephoto_iOSTests/KeychainTokenStoreTests.swift (1)
77-78: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
defer내 정리 Task가 await되지 않아 비결정적으로 실행됩니다.
defer { Task { try? await other.clear() } }는 fire-and-forget이라 테스트 종료 시점까지clear()가 끝난다는 보장이 없습니다. 고유 UUID service를 쓰므로 실해는 없지만, 시뮬레이터 Keychain에 항목이 남을 수 있습니다.addTeardownBlock을 써서 정리를 확실히 await하는 편이 깔끔합니다.♻️ 제안
- let other = KeychainTokenStore(service: "com.rephoto.tests.other.\(UUID().uuidString)") - defer { Task { try? await other.clear() } } + let other = KeychainTokenStore(service: "com.rephoto.tests.other.\(UUID().uuidString)") + addTeardownBlock { try? await other.clear() }🤖 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 `@Rephoto_iOSTests/KeychainTokenStoreTests.swift` around lines 77 - 78, The cleanup for the secondary KeychainTokenStore is being launched inside a non-awaited Task from defer, so the test does not guarantee that other.clear() finishes before teardown. Replace the defer-based fire-and-forget cleanup with a deterministic XCTest teardown approach such as addTeardownBlock, and reference KeychainTokenStore and its clear() call so the cleanup is awaited reliably.Rephoto_iOSTests/NetworkClientTests.swift (1)
54-75: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win핸들러 클로저 내부 단언은 핸들러가 호출되지 않으면 조용히 통과합니다.
test_request_authRequiredPath_injectsBearerToken/test_request_publicPath_doesNotInjectToken의XCTAssert가StubURLProtocol.handler내부에서만 실행됩니다. 요청이 핸들러에 도달하기 전에 처리되거나 핸들러가 호출되지 않으면 단언이 한 번도 실행되지 않아 테스트가 거짓으로 통과합니다.StubURLProtocol이 요청을 기록하므로(recordedRequests), 호출 이후에 기록된 요청에 대해 헤더를 단언하는 편이 더 견고합니다.🤖 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 `@Rephoto_iOSTests/NetworkClientTests.swift` around lines 54 - 75, The Authorization assertions in test_request_authRequiredPath_injectsBearerToken and test_request_publicPath_doesNotInjectToken are only executed inside StubURLProtocol.handler, so the tests can pass even if the handler is never called. Move the header checks to assertions after client.request completes, using StubURLProtocol.recordedRequests to verify the captured request instead. Keep the behavior checks tied to makeClient, request(path:method:), and StubURLProtocol so the tests fail if the request never reaches the stub.
🤖 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 `@Rephoto_iOS.xcodeproj/project.pbxproj`:
- Line 38: The base configuration path has been moved to
Core/Config/Config.xcconfig in the Xcode project, but the CI workflow still
generates the old Rephoto_iOS/Resource/Config.xcconfig placeholder. Update the
placeholder creation path in the iOS workflow so it matches the new
Config.xcconfig location used by project.pbxproj, and verify any related
bootstrap steps reference the same Core/Config path.
---
Nitpick comments:
In `@Rephoto_iOSTests/KeychainTokenStoreTests.swift`:
- Around line 77-78: The cleanup for the secondary KeychainTokenStore is being
launched inside a non-awaited Task from defer, so the test does not guarantee
that other.clear() finishes before teardown. Replace the defer-based
fire-and-forget cleanup with a deterministic XCTest teardown approach such as
addTeardownBlock, and reference KeychainTokenStore and its clear() call so the
cleanup is awaited reliably.
In `@Rephoto_iOSTests/NetworkClientTests.swift`:
- Around line 54-75: The Authorization assertions in
test_request_authRequiredPath_injectsBearerToken and
test_request_publicPath_doesNotInjectToken are only executed inside
StubURLProtocol.handler, so the tests can pass even if the handler is never
called. Move the header checks to assertions after client.request completes,
using StubURLProtocol.recordedRequests to verify the captured request instead.
Keep the behavior checks tied to makeClient, request(path:method:), and
StubURLProtocol so the tests fail if the request never reaches the stub.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d268f106-c00b-4a39-85ef-e50d8ed079c6
📒 Files selected for processing (5)
Rephoto_iOS.xcodeproj/project.pbxprojRephoto_iOS/Core/Config/Config.swiftRephoto_iOS/Core/NetworkAdapter/APITargetType.swiftRephoto_iOSTests/KeychainTokenStoreTests.swiftRephoto_iOSTests/NetworkClientTests.swift
1 participant
✨ PR 유형
어떤 변경 사항이 있나요??
🛠️ 작업내용
리팩토링 계획 Step 5(테스트 강화) 의 일부로 인증/네트워크 레이어 단위 테스트를 추가하고, 목표 디렉토리 구조에 맞춰 일부 파일을 정리했습니다.
1. Core 디렉토리 구조 정리 (
refactor)Config.swift→Core/Config/APITargetType.swift→Core/NetworkAdapter/Config.xcconfig이동에 따른project.pbxproj경로 갱신2. 단위 테스트 추가 (
test) — 총 15개KeychainTokenStoreTests(6개): 저장/조회/삭제/덮어쓰기, 빈 상태 nil 반환, 서로 다른 service 격리, actor 동시 접근 직렬화NetworkClientTests(9개): Bearer 토큰 주입, public 경로 미주입, 401 → 리프레시 후 재시도 성공, 리프레시 실패 시 unauthorized + 콜백, 재시도 소진, 동시 401 발생 시 리프레시 정확히 1회 보장, 로그아웃 시 토큰 삭제📋 추후 진행 상황
📌 리뷰 포인트
NetworkClientTests의 동시성 시나리오(동시 401 → 리프레시 1회) 검증 방식✅ Checklist
Summary by CodeRabbit
버그 수정
테스트