테스트 500개가 전부 초록불인 상태에서 리뷰가 잡은 여섯 가지

이 프로젝트는 설계자가 작업을 분해해 브리프를 쓰고, 구현자가 코드를 작성하고, 별도의 리뷰어가 결과물을 검토하는 방식으로 진행한다. 세 역할이 나뉘어 있으니 리뷰는 형식적인 확인 단계로 끝날 거라 생각했다. 최근 랜챗(랜덤 1:1 화상채팅) 신고 기능의 결함 하나를 고치는 작업을 했는데, 이번 글은 그 리뷰 단계에서 벌어진 일을 적는다.

기능 자체는 크지 않았다 — 서버 코드 한 파일, 클라이언트 세 파일 정도였다. 작업하는 동안 테스트는 496개에서 512개로 늘었고, 각 단계마다 전부 통과한 상태로 진행했다. 새 테스트를 쓰고, 구현하고, 다시 전체 스위트를 돌려 초록불을 확인하는 걸 반복했다. 그런데 그렇게 테스트가 전부 초록불인 상태에서 리뷰가 결함 여섯 개를 찾아냈다. 작은 변경치고는 리뷰 코멘트가 많이 나온 셈이다. 하나씩 성격이 달라서, 왜 테스트가 못 잡았는지를 따로 물어볼 가치가 있었다.

리뷰가 찾은 여섯 가지

1. 우리가 새로 연 회귀

끝난 페어를 기억하게 만들었는데, 신고 성공 후 그 기록을 지우지 않았다. 통화 종료 처리가 방금 끝난 페어를 다시 보관함에 넣기 때문에, 같은 신고를 반복 전송하면 계속 기록됐다. 기존 코드에서는 애초에 불가능했던 일이라, 이번 변경이 만든 구멍이었다. "끝난 페어를 기억한다"는 새 상태와 "통화 종료 시 보관함에 넣는다"는 기존 로직이 만나서 생긴, 어느 한쪽만 보면 안 보이는 문제였다.

2. 아무것도 지키지 않는 테스트

"양쪽 사용자 행 생성을 기다린 뒤 저장한다"를 검증한다는 테스트가, 실제로는 한쪽만으로 단언이 성립해서 수정 전 코드로 되돌려도 통과했다. 정작 그 커밋이 고친 부분이 전혀 검증되지 않은 채로 남아 있었던 셈이다. 테스트 이름과 커밋 메시지만 보면 의도가 분명했는데, 단언문 자체는 그 의도를 담고 있지 않았다.

3. 겉보기에 멀쩡한 리마운트

렌더 분기의 루트 엘리먼트 타입이 달라서 화면 전환 시 React가 서브트리를 재마운트했고, 모달 내부 입력이 조용히 초기화됐다. 모달은 계속 보이고 대상도 정확해서 화면만 봐서는 문제가 없었다. 그런데 그게 정확히 이 작업이 고치려던 문제였다. 증상을 없앤 줄 알았던 자리에, 같은 증상이 다른 경로로 남아 있었던 것이다.

4. 코드와 반대를 말하는 화면

"신고하면 대화가 즉시 종료됩니다"라는 문구가 조건 없이 떠 있었는데, 이번 작업으로 안 끊는 경우가 새로 생겼다. 다른 상대와 통화 중인 화면 위에서 그 문구를 그대로 읽고 신고를 취소할 수 있는 상태가 됐다. 문구는 그대로인데 문구가 가리키는 동작이 바뀐 경우다. 문구를 작성한 시점의 사실이 구현이 바뀐 뒤에도 화면에 그대로 남아 거짓말이 됐다.

5. 배포 창의 조용한 유실

필수 필드를 추가하면서, 이미 열려 있던 구버전 탭에서 올라온 신고가 로그도 없이 폐기되는 창이 생겼다. 배포 순간과 사용자의 탭이 어긋나는 짧은 구간에서만 벌어지는 일이라 재현하기도, 알아채기도 어렵다. 로컬에서도, CI에서도 이 구간은 존재하지 않는다.

6. 로그 인젝션

클라이언트가 보낸 문자열을 길이·문자 제한 없이 로그에 그대로 찍었다. 이전 로그는 서버가 생성한 값만 찍었으므로, 이것도 이번 변경이 새로 연 표면이었다. 신고 사유처럼 사용자가 자유롭게 입력하는 필드를 추가하면서 딸려 온 문제였다. 기능 요구사항에는 없던, 기능이 추가됐다는 사실 자체가 만든 위험이다.

왜 테스트로는 못 잡았나

여섯 개를 놓고 보니 테스트가 원리적으로 못 잡는 것들이 섞여 있었다.

2번은 테스트 자신의 문제다. 테스트가 테스트를 검증하지는 않는다. 3번은 기존 테스트가 전환 전에 기본값을 바꿔두지 않아서, 초기화가 일어나도 결과가 우연히 같았던 경우다. 기본값만 쓰는 테스트는 상태 보존을 검증할 수 없다 — 처음부터 비어 있던 값이 초기화로 다시 비어 있어도 테스트는 통과한다. 4번은 문구의 진위 문제다. 테스트는 문장이 화면에 있는지는 확인하지만 그 문장이 사실인지는 모른다. 5번은 배포 타이밍 문제라 애초에 단위 테스트의 범위 밖이다. 실행 중인 코드와 배포되는 코드 사이의 시차는 테스트 스위트 안에서 재현할 방법이 없다.

1번과 6번은 성격이 조금 다르다. 둘 다 "무엇이 새로 가능해졌는가"라는 질문이다. 테스트는 우리가 미리 생각해 둔 시나리오만 검증한다. 끝난 페어를 기억하는 기능을 추가하면서 "신고 후에는 그 기억을 지워야 한다"는 걸 아무도 테스트로 적지 않았던 이유는 단순하다 — 그런 조합이 가능해졌다는 것 자체를 처음에 인지하지 못했기 때문이다. 로그에 사용자 입력을 찍기 시작하는 순간 인젝션이 가능해진다는 것도 마찬가지로, "찍는다"는 결정을 내리는 순간에는 별개의 문제로 취급되지 않았다.

실제로 도움이 된 습관

뮤테이션 실측이 도움이 됐다. 새 테스트를 쓸 때마다 대상 구현을 일부러 되돌려 정말로 빨간불이 나는지 확인하고 원복했다. 구현하자마자 곧바로 초록불인 테스트는 하중이 없다는 신호다. 2번 같은 테스트는 이 습관으로 드러난다 — 구현을 원래대로 되돌렸는데도 계속 초록불이면, 그 테스트는 아무것도 지키고 있지 않은 것이다. 이 확인을 매번 하는 게 번거롭긴 하지만, 테스트 개수를 늘리는 것보다 테스트 하나하나가 실제로 뭔가를 막고 있는지 확인하는 쪽이 더 값이 나갔다.

리뷰어에게 미리 판정을 주지 않는 것도 도움이 됐다. 브리프나 커밋 메시지에 "이건 넘어가도 된다"고 적으면 그 지적은 나오지 않는다. 확신이 서지 않는 부분일수록 판단을 열어두고, 내 가정을 명시적으로 던져 반박을 받는 편이 나았다. 실제로 이번 작업에서 내가 세운 전제 하나가 틀린 것으로 확인된 적이 있다. 리뷰를 형식적인 확인 단계로 여기고 통과 여부만 물으면, 리뷰어도 그 프레임 안에서만 답하게 된다.

설계자 자신도 틀린다는 것도 다시 확인했다. 이번 작업에서 내가 만든 계획에도 구멍이 둘 있었다. 하나는 렌더 분기가 모달을 언마운트한다는 걸 놓친 것이고, 구현자가 테스트가 빨간불에서 안 풀려 헤매다가 발견했다. 다른 하나는 문서 작업 지시가 "적절히 갱신하라" 수준이라 실제로는 실행 불가능했던 것이다. 계획을 세우는 쪽이 항상 옳다는 전제로 브리프를 쓰면 안 된다는 걸 다시 배웠다.

교훈

• 초록불은 "우리가 생각한 것이 지켜진다"는 뜻이지 "결함이 없다"는 뜻이 아니다. 무엇을 생각하지 못했는지는 테스트가 알려주지 않는다.
• 변경이 무엇을 새로 가능하게 만들었는지 따로 물어봐야 한다. 기능이 늘면 조합도 는다.
• 테스트가 원리적으로 못 잡는 범주 — 테스트 자신의 하중, 문구의 진위, 배포 타이밍, 함수 내부 순서 — 가 있다. 그 자리는 리뷰나 주석으로 메우는 수밖에 없다.
• 계획을 세운 사람도 틀린다. 구현자가 "시킨 대로 했는데 안 된다"고 할 때가 계획의 구멍이 드러나는 순간이다.