버그를 고쳤다.
패치도 작았다.
MySQL 서버가 처음 인사해야 할 순간에, 인사 대신 에러를 보낼 수 있다는 걸 먼저 알아차리게 만든 수정이었다.
그런데 코드 리뷰에서 진짜 재미있는 이야기는 패치 본문이 아니라 테스트 이름에서 시작됐다.
“이 테스트, 이름만 보고 뭘 검증하는지 알 수 있을까요?”
처음엔 사소해 보인다.
테스트는 통과한다. 버그도 막는다. 그러면 된 거 아닌가?
하지만 리뷰어의 질문은 꽤 중요했다.
테스트는 단순한 안전벨트가 아니라, 나중에 코드를 읽는 사람을 위한 지도이기도 하니까.
티켓 번호는 주소가 아니다
버그를 고치다 보면 테스트 이름을 이렇게 짓고 싶어진다.
QCP_5385
BUG_1234
ISSUE_9876
당장에는 편하다.
“아, 이 테스트는 그 티켓 때문에 만든 거야.”
그런데 3개월 뒤에 누군가가 MySQL handshake 코드를 고치다가 이 테스트를 만나면 어떨까?
QCP_5385
이 이름은 아무 말도 해주지 않는다.
티켓 시스템을 열어야 한다. 링크를 찾아야 한다. 맥락을 복원해야 한다.
테스트 이름이 문 앞에서 이렇게 말하는 셈이다.
“들어오고 싶으면 먼저 암호표를 찾아오세요.”
좋은 테스트 이름은 반대로 말한다.
MySqlInitialHandshakeErrPacketTests
이름만 봐도 대충 감이 온다.
- MySQL 관련 테스트구나
- initial handshake 단계구나
- ERR packet 케이스구나
티켓 번호는 사라지지 않아도 된다.
그건 클래스 상단 주석이나 테스트 설명에 남기면 된다.
이 케이스는 특정 장애 티켓에서 발견됐다.
Target MySQL 서버가 connection을 더 받을 수 없을 때,
정상 handshake 대신 ERR packet을 먼저 반환할 수 있다.
즉, 티켓 번호는 “출처”로 남기고, 테스트 이름은 “행동”을 설명해야 한다.
폴더는 목차다
이번 리뷰에서 더 좋았던 지적은 위치였다.
테스트 파일이 Issues 같은 티켓 보관함에 들어가 있으면, 나중에 찾는 사람이 질문을 거꾸로 해야 한다.
“이 기능의 테스트가 어디 있지?”
가 아니라,
“이 티켓 번호가 뭐였지?”
가 되어버린다.
대부분의 코드베이스에서는 테스트도 대상 코드의 구조를 따라가는 편이 읽기 쉽다.
예를 들어 production code가 이렇게 생겼다면:
Modules/ARiSA.MySql/Protocol/Payloads/ErrorPayload.cs
Modules/ARiSA.MySql/Protocol/Payloads/InitialHandshakePayload.cs
테스트도 이런 식이면 자연스럽다.
ARiSA.Tests/Modules/MySql/Protocol/Payloads/MySqlInitialHandshakeErrPacketTests.cs
폴더 구조가 이렇게 말해준다.
이 테스트는 MySQL module의 Protocol/Payloads 동작을 검증한다.
이건 작은 차이처럼 보이지만, 유지보수에서는 꽤 크다.
좋은 폴더는 목차다.
코드를 읽는 사람이 검색창을 열기 전에, 어디를 봐야 하는지 알려준다.
“부품 테스트”와 “흐름 테스트”는 다르다
이번 케이스에서 테스트는 MySQL ERR packet payload를 만들고 세 가지를 확인했다.
1. 이 packet이 ERR packet으로 감지되는가?
2. ErrorCode / SQL State / Message가 보존되는가?
3. 이 packet을 정상 handshake로 파싱하면 실패하는가?
좋은 테스트다.
작고 빠르다. 실제 MySQL 서버를 띄우지 않아도 된다.
하지만 리뷰어는 한 가지를 더 짚었다.
“이번 변경의 핵심은 initial handshake 단계에서 ERR packet을 받았을 때 Local로 전달하고 handshake를 false로 종료하는 것 아닌가요?”
맞다.
payload parser 테스트는 부품을 확인한다.
하지만 이번 패치의 핵심 흐름은 이거다.
Target DB에서 첫 packet 수신
↓
ERR packet인지 확인
↓
원본 ERR packet을 client로 전달
↓
handshake 종료
부품 테스트만 있으면 이런 상태다.
ERR packet 판별기: 정상
ERR packet parser: 정상
handshake 조립 흐름: 직접 검증 안 됨
그래서 가장 표준적인 개선은 테스트를 두 층으로 나누는 것이다.
첫 번째 층은 payload/parser 테스트.
MySqlInitialHandshakeErrPacketTests
두 번째 층은 session behavior 테스트.
MySqlProxySessionTests
그리고 session 테스트는 이런 질문에 답해야 한다.
initial ERR packet을 받으면,
정말 client로 그대로 전달하고,
handshake를 멈추는가?
테스트 이름도 이렇게 가면 좋다.
InitialServerErrPacket_IsForwardedToLocalAndStopsHandshake
조금 길지만 괜찮다.
테스트 이름은 짧은 별명이 아니라, 실패했을 때 읽는 에러 메시지이기도 하니까.
작은 flush 한 줄이 말해준 것
그리고 실제로 session behavior 테스트를 붙이자, 더 흥미로운 일이 생겼다.
처음에는 코드가 이렇게 보였다.
Target DB
│
│ initial ERR packet
▼
Proxy session
├─ ERR packet인지 확인
├─ client로 전달
└─ handshake 종료
말로만 보면 충분해 보인다.
“client로 전달했으니 됐겠지.”
그런데 테스트는 그렇게 너그럽지 않았다.
가짜 Target DB와 가짜 Local client를 stream으로 연결해놓고, 정말 client 쪽에서 같은 packet을 읽을 수 있는지 확인했다.
┌──────────────────┐ ┌──────────────────┐ ┌──────────────────┐
│ Fake Target DB │ │ Proxy Session │ │ Fake Client │
└────────┬─────────┘ └────────┬─────────┘ └────────┬─────────┘
│ │ │
│ 1. ERR packet │ │
├─────────────────────────▶│ │
│ │ 2. error로 감지 │
│ │ 3. client로 write │
│ ├─────────────────────────▶│
│ │ 4. return false │
│ │ │ 5. packet read?
여기서 테스트가 멈췄다.
이유는 간단했다.
작은 packet은 내부 write buffer에 잠깐 머무를 수 있다. 보통 흐름에서는 다음 read나 flush 타이밍에 자연스럽게 밀려나간다. 하지만 이번 경로는 다르다.
ERR packet 처리
↓
client로 write
↓
바로 return false
↓
흐름 종료
즉, “보냈다”와 “상대가 실제로 읽을 수 있다” 사이에 작은 틈이 있었다.
그래서 패치는 결국 이런 모양이 됐다.
ERR packet 수신
↓
client로 write
↓
write buffer flush
↓
handshake false 종료
코드로 보면 아주 작다.
SendAsync(Local, errPacket)
FlushWrite()
return false
하지만 의미는 꽤 크다.
이 한 줄은 “내부적으로 write를 호출했다”가 아니라 “client가 실제로 에러를 받을 수 있다”를 보장한다.
그리고 이건 parser 테스트만으로는 잡기 어렵다.
parser 테스트는 이렇게 말한다.
이 bytes는 ERR packet입니다.
session behavior 테스트는 이렇게 말한다.
그 ERR packet이 실제 proxy 흐름에서 client에게 도착합니다.
둘은 비슷해 보이지만 다르다.
하나는 부품의 진실이고, 다른 하나는 제품의 진실이다.
코드 변경도 지도가 될 수 있다
이번 개선을 한 장으로 그리면 이렇게 된다.
Before
ARiSA.Tests
└── Issues
└── QCP_5385.cs
└── payload/parser만 확인
Proxy flow
Target DB ──ERR packet──▶ Proxy ──write?──▶ Client
└─ return false
buffer flush 보장 약함
After
ARiSA.Tests
└── Modules
└── MySql
├── Protocol/Payloads
│ └── MySqlInitialHandshakeErrPacketTests.cs
│ └── ERR packet 자체를 이해하는 테스트
│
└── MySqlProxySessionTests.cs
└── 실제 proxy 흐름을 검증하는 테스트
Proxy flow
Target DB ──ERR packet──▶ Proxy ──write + flush──▶ Client
└─ return false
이 도식에서 중요한 건 파일이 예쁘게 정리됐다는 사실만이 아니다.
테스트의 위치와 계층이, 코드 변경의 의도를 같이 설명한다는 점이다.
Payload 테스트
└─ 이 packet은 무엇인가?
Session 테스트
└─ 이 packet을 받으면 proxy는 어떻게 행동해야 하는가?
이렇게 나누면 다음 사람이 더 빨리 이해한다.
“아, 이건 단순 parser 버그가 아니라 handshake 초입의 예외 흐름이구나.”
“아, 그래서 session 테스트가 따로 있구나.”
“아, flush는 테스트를 위한 꼼수가 아니라 client 관측 가능성을 보장하는 동작이구나.”
좋은 테스트는 이렇게 코드 변경의 이유까지 설명한다.
테스트 이름을 보면 리뷰 비용이 줄어든다
코드 리뷰에서 리뷰어가 계속 질문해야 하는 PR은 힘들다.
이 테스트는 왜 여기 있죠?
이 클래스명은 왜 티켓 번호죠?
이 namespace는 왜 폴더랑 다르죠?
이 테스트가 실제 변경 경로를 막나요?
반대로 좋은 테스트는 PR을 읽는 사람에게 계속 힌트를 준다.
파일 위치: Modules/MySql/Protocol/Payloads
클래스명: MySqlInitialHandshakeErrPacketTests
메서드명: InitialServerErrPacket_PreservesMysqlErrorDetails
이 정도면 리뷰어는 많은 걸 묻지 않아도 된다.
“아, MySQL initial handshake에서 ERR packet을 먼저 감지하는 회귀 테스트구나.”
바로 이해된다.
그게 좋은 이름의 힘이다.
내가 얻은 체크리스트
이번 리뷰에서 얻은 테스트 정리 체크리스트는 이렇다.
1. 테스트 파일은 대상 모듈 구조를 따라가는가?
2. namespace는 폴더 구조와 맞는가?
3. 클래스명은 티켓 번호가 아니라 동작을 설명하는가?
4. 티켓 번호는 주석이나 링크로만 남겼는가?
5. parser/helper 테스트와 실제 behavior 테스트를 구분했는가?
6. PR 제목은 repo 관례에 맞게 컴포넌트를 드러내는가?
특히 마지막 두 개가 중요하다.
작은 helper 테스트는 빠르고 좋다.
하지만 버그를 실제로 막는 흐름이 있다면, 적어도 하나는 그 흐름을 직접 겨냥해야 한다.
그리고 PR 제목은 리뷰어가 가장 먼저 보는 문장이다.
fix(apps/arisa): Handle MySQL initial ERR packet
이런 제목은 짧지만 꽤 많은 정보를 준다.
- fix다
- apps/arisa 변경이다
- MySQL initial ERR packet 처리다
리뷰어는 제목에서 이미 지도 한 장을 받는다.
이름 짓기는 사소한 일이 아니다
테스트가 통과하는 것은 중요하다.
하지만 테스트가 어디에 있고, 무엇을 말하고, 어떤 흐름을 보호하는지도 중요하다.
버그 수정 직후의 우리는 모든 맥락을 알고 있다.
왜 이 테스트가 생겼는지, 어떤 packet이 문제였는지, 어떤 리뷰 코멘트가 있었는지 다 기억한다.
하지만 미래의 동료는 모른다.
미래의 나도 모른다.
그래서 테스트 이름은 지도여야 한다.
폴더는 목차여야 한다.
그리고 좋은 테스트는 이렇게 말해야 한다.
내가 지키는 동작은 이것입니다.
내가 여기 있는 이유는 이것입니다.
내가 깨지면 이 흐름을 다시 보세요.
그 정도로 친절한 테스트라면, 버그 하나를 막는 것 이상을 한다.
다음 디버깅의 시간을 줄이고, 다음 리뷰의 질문을 줄이고, 다음 사람의 머릿속에 구조를 남긴다.
테스트 이름은 그냥 이름이 아니다.
작은 지도다.