버그를 고쳤다.
패치도 작았다.
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
조금 길지만 괜찮다.
테스트 이름은 짧은 별명이 아니라, 실패했을 때 읽는 에러 메시지이기도 하니까.
테스트 이름을 보면 리뷰 비용이 줄어든다
코드 리뷰에서 리뷰어가 계속 질문해야 하는 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이 문제였는지, 어떤 리뷰 코멘트가 있었는지 다 기억한다.
하지만 미래의 동료는 모른다.
미래의 나도 모른다.
그래서 테스트 이름은 지도여야 한다.
폴더는 목차여야 한다.
그리고 좋은 테스트는 이렇게 말해야 한다.
내가 지키는 동작은 이것입니다.
내가 여기 있는 이유는 이것입니다.
내가 깨지면 이 흐름을 다시 보세요.
그 정도로 친절한 테스트라면, 버그 하나를 막는 것 이상을 한다.
다음 디버깅의 시간을 줄이고, 다음 리뷰의 질문을 줄이고, 다음 사람의 머릿속에 구조를 남긴다.
테스트 이름은 그냥 이름이 아니다.
작은 지도다.