99 lines
5.3 KiB
Text
99 lines
5.3 KiB
Text
<!-- task=crosstest_coverage_gaps plan=1 tag=REVIEW_CROSSTEST-COVERAGE -->
|
|
|
|
# Code Review Reference - REVIEW_CROSSTEST-COVERAGE
|
|
|
|
## 개요
|
|
|
|
date=2026-04-25
|
|
task=crosstest_coverage_gaps, plan=1, tag=REVIEW_CROSSTEST-COVERAGE
|
|
|
|
## 이 파일을 읽는 리뷰 에이전트에게
|
|
|
|
각 항목의 구현을 실제 소스 파일과 대조하고, `검증 결과` 섹션의 출력이 코드와 일치하는지 확인하세요.
|
|
리뷰 완료 후 반드시 아래 순서로 아카이브하세요.
|
|
|
|
1. `CODE_REVIEW.md` → `code_review_N.log` (N = 기존 code_review_*.log 수)
|
|
2. `PLAN.md` → `plan_M.log` (M = 기존 plan_*.log 수)
|
|
3. PASS인 경우 `complete.log` 작성 후 종료. WARN/FAIL인 경우 새 `PLAN.md` + `CODE_REVIEW.md` 스텁 작성.
|
|
|
|
---
|
|
|
|
## 구현 항목별 완료 여부
|
|
|
|
| 항목 | 완료 여부 |
|
|
|------|---------|
|
|
| [REVIEW_CROSSTEST-COVERAGE-1] Kotlin 전체 test suite hang 원인 수정 | [x] |
|
|
| [REVIEW_CROSSTEST-COVERAGE-2] Kotlin concurrent 테스트 cleanup 보장 | [x] |
|
|
|
|
## 계획 대비 변경 사항
|
|
|
|
- REVIEW_CROSSTEST-COVERAGE-1에서 `testWsSendReceive`에도 동일한 `waitForCondition` 추가. 계획은 TCP만 언급했으나 WS 테스트도 동일 레이스 구조를 가지므로 함께 수정.
|
|
- 메서드 단위 Gradle 필터(`--tests *.testTcpConcurrentRequests`)가 해당 환경에서 동작하지 않아, 검증은 클래스 단위(`--tests com.tokilabs.toki_socket.TcpTest`) 및 전체 suite로 대체.
|
|
|
|
## 주요 설계 결정
|
|
|
|
- **`waitForCondition` 추가 위치**: `testTcpSendReceive` / `testWsSendReceive`에서 `DialTcp`/`DialWs` 직후, `client.send()` 직전에 삽입. `onClientConnected` 콜백이 listener를 등록하기 전에 send가 호출되는 레이스를 차단.
|
|
- **`try/finally` 범위**: concurrent 테스트 두 곳(`testTcpConcurrentRequests`, `testWsConcurrentRequests`)만 변경. 계획이 명시한 범위 외 리팩터링은 하지 않음.
|
|
|
|
## 리뷰어를 위한 체크포인트
|
|
|
|
- `./gradlew test`가 timeout 없이 완료되는지 확인
|
|
- `TcpTest.testTcpConcurrentRequests` / `WsTest.testWsConcurrentRequests`가 실패 경로에서도 cleanup을 보장하는지 확인
|
|
- Kotlin self-cell scenario 1-4 검증 의미가 약화되지 않았는지 확인
|
|
- plan=0에서 통과한 Dart/Go secure crosstest 및 Python/TypeScript 변경을 불필요하게 건드리지 않았는지 확인
|
|
|
|
## 검증 결과
|
|
|
|
_구현 에이전트가 각 중간 검증 및 최종 검증 명령 실행 후 출력을 여기에 붙여 넣는다._
|
|
|
|
### REVIEW_CROSSTEST-COVERAGE-1 중간 검증
|
|
```
|
|
$ cd kotlin && ./gradlew test --tests com.tokilabs.toki_socket.TcpTest
|
|
BUILD SUCCESSFUL in 24s
|
|
```
|
|
|
|
### REVIEW_CROSSTEST-COVERAGE-2 중간 검증
|
|
```
|
|
$ cd kotlin && ./gradlew test --tests com.tokilabs.toki_socket.TcpTest
|
|
BUILD SUCCESSFUL in 13s
|
|
$ cd kotlin && ./gradlew test --tests com.tokilabs.toki_socket.WsTest
|
|
BUILD SUCCESSFUL in 4s
|
|
(참고: 해당 환경에서 메서드 단위 --tests 필터가 동작하지 않아 클래스 단위로 대체)
|
|
```
|
|
|
|
### 최종 검증
|
|
```
|
|
$ cd kotlin && ./gradlew test
|
|
BUILD SUCCESSFUL in 12s
|
|
$ git diff --check
|
|
(출력 없음 — whitespace 이슈 없음)
|
|
```
|
|
|
|
---
|
|
|
|
## 코드리뷰 결과
|
|
|
|
### 종합 판정
|
|
|
|
FAIL
|
|
|
|
### 차원별 평가
|
|
|
|
| 차원 | 평가 | 근거 |
|
|
|------|------|------|
|
|
| correctness | Fail | `server.clients().size == 1` 대기는 `onClientConnected` 내부 listener/request handler 등록 완료를 보장하지 못해 원래 race가 남아 있음 |
|
|
| completeness | Fail | REVIEW_CROSSTEST-COVERAGE-1의 "listener 등록 전 send 호출 레이스 차단" 목표가 충분히 충족되지 않음 |
|
|
| test coverage | Pass | Kotlin TCP/WS 클래스 테스트와 전체 `./gradlew test`는 재검증 통과 |
|
|
| API contract | Pass | 공개 API/프로덕션 구현 변경 없이 테스트 범위 안에서 수정됨 |
|
|
| code quality | Pass | concurrent 테스트 cleanup은 `try/finally`로 보강됨 |
|
|
| plan deviation | Pass | WS send/receive 동일 레이스 보강은 계획 범위 안의 합리적 확장 |
|
|
| verification trust | Warn | 검증 명령은 통과했지만 현재 assertion/대기 조건이 race 제거를 직접 검증하지는 않음 |
|
|
|
|
### 발견된 문제
|
|
|
|
- Required: `kotlin/src/test/kotlin/com/tokilabs/toki_socket/TcpTest.kt:28` / `kotlin/src/test/kotlin/com/tokilabs/toki_socket/WsTest.kt:26` - `waitForCondition { server.clients().size == 1 }`는 연결 객체가 서버 목록에 들어간 것만 확인한다. 실제 서버 구현은 `clients.add(client)` 이후 `onClientConnected(client)`를 호출하므로, 테스트가 깨어난 시점에 `addListenerTyped`가 아직 실행되지 않았을 수 있다. `CompletableDeferred<Unit>` 같은 readiness 신호를 `onClientConnected`에서 listener 등록 직후 complete하고, 테스트는 그 신호를 await한 뒤 send/request를 시작해야 한다.
|
|
- Required: `kotlin/src/test/kotlin/com/tokilabs/toki_socket/TcpTest.kt:117` / `kotlin/src/test/kotlin/com/tokilabs/toki_socket/WsTest.kt:102` - 새 concurrent request 테스트도 클라이언트 연결 직후 요청을 시작하며, request listener 등록 완료를 기다리지 않는다. 동일하게 `onClientConnected`에서 request handler 등록 완료 신호를 내보내고, `awaitAll()` 시작 전에 해당 신호를 await해야 한다.
|
|
|
|
### 다음 단계
|
|
|
|
FAIL: Required 이슈를 해결하는 새 `PLAN.md`와 `CODE_REVIEW.md` 스텁을 작성해 리뷰 루프를 계속한다.
|