Replies: 3 comments
-
|
친절한 discussion 덕분에 이해가 잘 되네요 !! 🚀 예시@Test("ViewModel 해제 시 Coordinator 해제")
func coordinatorDeallocatesWithViewModel() {
weak var weakCoordinator: MockSplashCoordinator?
autoreleasepool {
let coordinator = MockSplashCoordinator()
weakCoordinator = coordinator
let vm = SplashViewModel(
useCase: MockSplashUseCase(),
coordinator: coordinator
)
}
// 스코프 종료 → vm 해제 → coordinator 해제
#expect(weakCoordinator == nil)
}🔍 weak로 선언한 이유테스트에서 만약 아래와 같이 작성한다면, var coordinator = MockSplashCoordinator()테스트 코드가 coordinator를 계속 소유하게 되어 🔍 autoreleasepool을 사용한 이유해제 시점을 명확하게 보장하기 위해 사용했습니다. 실제로 테스트를 돌려보지는 못해서 오류나 다른 제안이 있다면 편하게 말씀해주세요 !! |
Beta Was this translation helpful? Give feedback.
-
|
친절한 설명 감사합니다! Coordinator 부분이 헷갈리는게 많았는데, 이전 히스토리까지 한번에 정리를 해주셔서 많은 도움이 되었어요. 테스트코드의 필요성과 말씀하신 점진적인 테스트 -> 레거시 제거 방식은 너무나 좋다고 생각합니다. 다만 조금 염려되는 부분은 대부분이 반응형프로그래밍 방식으로 이루어져 있어서 테스트코드 또한 그에맞게 작성이 될텐데,
말이 조금 복잡해졌는데 정리하자면
레거시제거 작업이 우선이라고 생각합니다. 다만 해당 작업에대한 리소스가 꽤나 클것으로 예상되어 |
Beta Was this translation helpful? Give feedback.
-
|
@juri123123 @kwonseokki 두 분 다 좋은 답변 감사해요! 현재 패턴의 핵심이 두번째로 석기님이 말씀해주신 우려에 충분히 공감합니다! 다만 저는 순서를 반대로 진행하는게 좋을 것 같아요. 테스트 없이 Swift Concurrency 마이그레이션을 먼저 진행하면, 그 과정에서 기존 동작이 깨져도 확인할 방법이 없어서 오히려 더 큰 리스크가 생길 것 같습니다. 또한 그래서 아래 순서로 가는 게 현실적이라고 생각해요.
두 분 의견도 남겨주시면 확인 후 작업 진행하겠습니다! 👀 |
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
-
Coordinator 리팩토링을 검증하기 위한 테스트 코드 도입에 대해 논의하기 위해 Discussion을 열었습니다!🙇♂️
📍 현재까지의 리팩토링 과정
Coordinator는 두 단계에 걸친 리팩토링을 진행해왔습니다.
1단계: Router 제거 (Discussion #514)
Router 추상화의 이점이 크게 활용되지 않고 네비게이션 불일치 문제, rootViewController 전환 등의 문제가 생긴다는 논의를 바탕으로,
UINavigationController를 직접 주입하는 방식으로 변경했습니다.2단계: Coordinator 인스턴스 해제 방식 변경
기존에는 모든 Coordinator가
finishFlow()호출과addDependency()등록을 수동으로 관리해야 했는데요.하나라도 누락되면 메모리 누수가 발생했고, 실제로 이전 PR에서 누락된 경우가 확인되기도 했습니다.
이에 강한참조를 활용하여 Coordinator의 생명주기를 관리하는 방식으로 리팩토링을 진행했습니다.
[현재 구조]
graph LR NC["NavigationController"] C["Coordinator"] VM["ViewModel"] VC["ViewController"] NC -. "weak" .-> C VC -- "strong" --> VM VM -- "strong" --> C style NC fill:#f0f4f8,stroke:#94a3b8,color:#334155 style C fill:#dbeafe,stroke:#3b82f6,color:#1e3a5f style VM fill:#dbeafe,stroke:#3b82f6,color:#1e3a5f style VC fill:#dbeafe,stroke:#3b82f6,color:#1e3a5fCoordinatorFinishOutput프로토콜을 통해finished클로저를 외부에 노출함으로써, 부모 Coordinator는 자식 종료 시점을 알 수 있으면서도 자식 Coordinator를 강하게 참조하지 않는 구조로 설계했습니다.(아직
TabBar나MyPage등에addDependency나finishFlow등이 잔존하고 있어 순차적으로 제거가 필요합니다.)💡 테스트 코드의 필요성
레거시 코드를 본격적으로 걷어내기 전에 테스트 코드가 먼저 필요하다고 생각한 이유는 크게 세 가지입니다.
1. 현재 패턴이 의도대로 동작하는지 보장
리팩토링 이후에도 기존과 같이 기능이 잘 동작하는지 보장이 되어야 했는데요, ViewModel이 UseCase 이벤트를 받아 콜백 클로저를 올바른 타이밍에 호출하는 것이 핵심이라고 생각했습니다. Coordinator는 그 콜백을 클로저와 연결할 뿐이기 때문에, ViewModel의 transform 로직이 제대로 동작하는지 확인하는 것이 첫 번째 목적이었어요.
sequenceDiagram participant C as Coordinator participant VM as ViewModel participant UC as UseCase C ->> VM: 콜백 클로저 와이어링<br/>(onNoticeSkipped, onNoticeExist 등) C ->> VM: transform() 호출 VM ->> UC: getAppNotice() UC -->> VM: needUpdate 이벤트 발행<br/>(.none / .forcedUpdate / .optionalUpdate / .networkError) VM ->> C: 매핑된 콜백 클로저 호출2. 레거시 제거 시 회귀 방지
테스트가 통과하는 상태를 유지하면서 레거시 코드를 제거하면, 기존 동작이 깨지는 것을 방지할 수 있고, 문제가 생기는 경우 원인을 빠르게 찾아낼 수 있습니다.
3. 기수제 환경에서의 지속 가능성
메이커스 특성상 6개월마다 인원이 바뀔 수 있는 기수제이다보니, 구두로 전달되던 컨텍스트가 사라지는 일이 종종 있었어요. 테스트 케이스 자체에 명세처럼 담겨 있으면, 히스토리를 모르는 분도 코드 의도를 빠르게 파악할 수 있을 거라 생각했습니다.
SplashFeature 테스트 코드 작성 과정
SplashFeature가 다른 기능 모듈과의 의존성도 없고, 구조가 간단하기 때문에 샘플로 정했습니다.
SUT로 ViewModel을 선택한 이유
테스트 대상(SUT)으로 ViewModel을 선택한 이유는 두 가지입니다.
UINavigationController와 직접 상호작용하기 때문에 Unit Test에 적합하지 않습니다.bindOutput내부에 있습니다. 적절한 콜백이 잘 실행되는지 검증해야하므로 ViewModel을 검증하는 것이 정확하다고 판단했습니다.graph TB subgraph legacy["기존 패턴 (수동 해제)"] direction TB L1["Coordinator 생성"] --> L2["addDependency()로 등록"] L2 --> L3["화면 전환 로직 실행"] L3 --> L4["finishFlow() 수동 호출"] L4 --> L5["부모가 의존성 제거"] L4 -. "누락 시" .-> L6["⚠️ 메모리 누수"] end subgraph current["신규 패턴 (자동 해제)"] direction TB N1["Coordinator 생성"] --> N2["VC → VM → Coordinator 체인 형성"] N2 --> N3["콜백 클로저 와이어링"] N3 --> N4["화면 dismiss → VC 해제"] N4 --> N5["VM 해제 → Coordinator 자동 해제"] end style legacy fill:#fef2f2,stroke:#ef4444 style current fill:#f0fdf4,stroke:#22c55e style L6 fill:#fee2e2,stroke:#ef4444,color:#991b1bTestDouble 구성
MockSplashUseCaseXCTest 대신 Swift Testing을 선택한 이유
이번 테스트에서 Swift Testing(
import Testing)을 선택했는데요, 이유는 다음과 같습니다.@Test("설명")어노테이션으로 테스트 의도를 바로 드러낼 수 있고,#expect매크로가 XCTAssert 계열보다 실패 시 diff가 명확합니다.@Suite로 테스트를 논리적으로 그룹화할 수 있어, 케이스가 늘어나도 관리하기 편합니다.테스트 코드의 비동기 처리 고민
작성하면서 가장 고민됐던 부분은 비동기 처리였습니다.
ViewModel의
bindOutput이receive(on: DispatchQueue.main)으로 메인 스레드에서 콜백을 실행하는데, 테스트에서 이걸 동기적으로 기다리기 위해 아래 헬퍼를 작성했습니다.메인 큐에 빈 태스크를 하나 올려서, 그 앞에 적재된 ViewModel 콜백이 먼저 실행되는 걸 보장하는 방식입니다. 더 나은 방법이 있으면 의견 남겨주세요!
현재까지 작성된 테스트 케이스
긍정 케이스(호출됨)와 부정 케이스(호출되지 않음)를 함께 검증하여, 의도한 콜백만 실행되는지 확인했습니다.
transformCallsGetAppNoticetransform호출 시useCase.getAppNotice()실행 여부noneEmitsOnNoticeSkipped.none→onNoticeSkipped호출forceUpdateEmitsOnNoticeExist.forcedUpdate→onNoticeExist+ 모델 전달optionalUpdateEmitsOnOptionalNoticeExist.optionalUpdate→onOptionalNoticeExist+ 모델 전달networkErrorEmitsOnNetworkError.networkError→onNetworkError+ retry 클로저 전달 및 재호출 검증forcedUpdateDoesNotEmitOnNoticeSkipped.forcedUpdate시onNoticeSkipped미호출 확인optionalUpdateDoesNotEmitOnNoticeSkipped.optionalUpdate시onNoticeSkipped미호출 확인networkErrorDoesNotEmitOnNoticeSkipped.networkError시onNoticeSkipped미호출 확인🚀 향후 계획
레거시 Coordinator 코드를 플로우별로 순차 제거할 예정입니다.
테스트 코드 작성이나 리팩토링 방향에 대해 의견 있으면 편하게 남겨주세요! 👀
@juri123123 @kwonseokki
Beta Was this translation helpful? Give feedback.
All reactions