Skip to content

[review] PR #179 리뷰 코멘트 반영 — 테스트 seam 격리·진단·컨벤션 보강#180

Merged
geeksbaek merged 1 commit into
codex/fix-icloud-sync-resetfrom
claude/stoic-dirac-DCSE9
Jun 10, 2026
Merged

[review] PR #179 리뷰 코멘트 반영 — 테스트 seam 격리·진단·컨벤션 보강#180
geeksbaek merged 1 commit into
codex/fix-icloud-sync-resetfrom
claude/stoic-dirac-DCSE9

Conversation

@geeksbaek

Copy link
Copy Markdown
Collaborator

개요

PR #179에 달린 14개 리뷰 코멘트를 검토하고, 타당한 12개를 패치했습니다. 이 PR은 codex/fix-icloud-sync-reset(=#179 head)을 base로 하여, 리뷰 반영 diff만 포함합니다. 머지 시 #179에 그대로 흡수됩니다.

⚠️ iOS 빌드/테스트는 macOS·Xcode 환경이 필요해 이 작업 환경(Linux)에서는 make test/tuist build를 실행하지 못했습니다. make validate(architecture/pattern/docs)는 통과했고, 변경은 컴파일 관점에서 정적 검토했습니다. 머지 전 CI의 Check Build 결과를 확인해 주세요.

반영한 항목 (12)

# 위치 내용
SE/CO AppEnvironment.swift isUnitTestingNSClassFromString("XCTestCase") fallback 제거 — 테스트 스킴이 IS_UNIT_TESTING=1을 주입하므로 환경 변수만으로 판정(릴리스 텔레메트리 무력화/향후 UI 테스트 부팅 실패 리스크 차단)
격리 SafetySnapshotService.swift 테스트 전용 beforeApplyingSnapshot#if DEBUG seam(beforeApplyingSnapshotForTesting)으로 분리 — production restore 죽은 코드 제거
Sec SafetySnapshotService.swift isExcludedFromBackup 적용 실패를 os_logLog.error 비치명 이벤트로 보고(백업 제외 회귀 관측성)
격리 CoreDataStorage.swift / 테스트 sync 영속 플래그 저장소를 주입 가능한 userDefaults로 분리 → 테스트가 격리 suite 사용, .standard(실제 앱) 오염 방지. 정적 resetPersistentSyncFlagsForTesting 제거
동시성 CoreDataStorage.swift hasEverHadUserCollection setter의 UserDefaults I/O를 lock 임계영역 밖으로 이동(재진입 트랩 회피)
동시성 CoreDataStorage.swift completeFirstImportWait에서 timedOutwaiting 해제보다 먼저 설정 → TOCTOU 윈도우 제거(억제 항상 fail-safe)
진단 Log.swift / CoreDataStorage.swift Log.SnapshotisFirstImportTimedOut 추가 + Crashlytics custom key 기록(진단 공백 보완)
명료성 CoreDataStorage.swift 실패 import 경로에 _exportRetryCount 비대칭 사유 주석
컨벤션 Log.swift event/clickisFirebaseConfigured 가드 스타일 통일(os_log 후 단일 guard)
컨벤션 1.1.A/2.3 CoreDataStorage.swift logSyncDiagnostics 클로저 owner 네이밍 + guard 뒤 빈 줄
컨벤션 8 CoreDataStorage.swift / 테스트 #if DEBUG extension·테스트 helper extension에 // MARK: 추가
헤더 AppEnvironment.swift Created by CodexAri(프로젝트 표준 작성자)

반영하지 않은 항목 (2) — 사유

  • recovery grace 자동 UC 생성 제거 (CoreDataStorage.swift:295): 리뷰 본문에서 의도된 trade-off로 명시했고 코드 변경 요청이 아닙니다(빈 UC export로 인한 데이터 오염 차단이 PR 목적). "degraded 상태 UI 안내 추가"는 제품 결정이라 범위 밖으로 두었습니다.
  • Log isFirebaseConfigured 가드 5개 메서드 전면 통일 (Log.swift:84): os_log(로컬 로깅)는 Firebase 미구성·단위 테스트에서도 항상 실행되어야 하므로 제안된 "단일 top-level guard isFirebaseConfigured"는 그대로 적용하면 로컬 로깅이 사라집니다. 안전하게 적용 가능한 event/click만 통일하고, emit/error/setContext는 현행 유지(동작은 모두 올바름).

Generated by Claude Code

리뷰에서 타당하다고 판단한 항목을 패치한다.

- AppEnvironment: isUnitTesting의 NSClassFromString("XCTestCase") fallback 제거.
  테스트 스킴이 IS_UNIT_TESTING=1을 주입하므로 환경 변수만으로 판정해 릴리스
  텔레메트리 무력화/향후 UI 테스트 부팅 실패 리스크를 차단.
- SafetySnapshotService: 테스트 전용 beforeApplyingSnapshot seam을 #if DEBUG로
  격리(production restore 죽은 코드 제거). backup 제외 실패를 Log.error로 보고.
- CoreDataStorage: 동기화 영속 플래그 저장소를 주입 가능한 userDefaults로 분리해
  테스트가 .standard(실제 앱 suite)를 오염시키지 않도록 함. hasEverHadUserCollection
  setter의 UserDefaults I/O를 lock 임계영역 밖으로 이동. completeFirstImportWait의
  timedOut/waiting 갱신 순서를 fail-safe하게 조정해 TOCTOU 윈도우 제거.
- Log.Snapshot에 isFirstImportTimedOut 추가(Crashlytics 진단 공백 보완),
  event/click의 isFirebaseConfigured 가드 스타일 통일.
- logSyncDiagnostics 클로저 owner 네이밍/guard 뒤 빈 줄, #if DEBUG 및 테스트 helper
  extension MARK 추가, 신규 파일 작성자 헤더 정렬.
- 테스트: 격리된 UserDefaults suite 주입으로 전환하고 정적 reset helper 제거.

https://claude.ai/code/session_01L4kUZhiUNUtnr18SabrHzR
@coderabbitai

coderabbitai Bot commented Jun 4, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: fe7e1bcd-ad5a-436e-9139-13e2aa663625

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/stoic-dirac-DCSE9

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.

@geeksbaek
geeksbaek marked this pull request as ready for review June 10, 2026 13:46
@geeksbaek
geeksbaek merged commit ca2391e into codex/fix-icloud-sync-reset Jun 10, 2026
1 of 2 checks passed
@geeksbaek
geeksbaek deleted the claude/stoic-dirac-DCSE9 branch June 10, 2026 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants