Showing Posts From

코드

개발자 코드 리뷰처럼 테스트 코드 리뷰받기

개발자 코드 리뷰처럼 테스트 코드 리뷰받기

개발자 코드 리뷰처럼 테스트 코드 리뷰받기 테스트 코드도 코드다 오늘 개발팀 코드 리뷰에 참석했다. PR 하나에 댓글이 37개. 네이밍부터 로직까지 전부 뜯어본다. 근데 내 테스트 코드 PR은 댓글 2개. "LGTM", "Approved". 뭔가 이상하다고 느꼈다. 테스트 코드도 코드인데 왜 이렇게 대충 봐줄까. 프로덕션 코드는 30분 검토하고, 테스트 코드는 3분. 이해는 간다. 테스트 코드는 "그냥 돌아가면 되는 거" 아닌가. 근데 정말 그럴까. 지난주 배포 장애가 터졌다. E2E 테스트는 전부 통과했는데 실제론 버그. 알고 보니 테스트가 잘못 짜여 있었다. 그걸 아무도 몰랐다. 리뷰 때 대충 봤으니까.그날 깨달았다. 테스트 코드 리뷰를 개발 코드만큼 엄격하게 해야 한다고. 아니, 어쩌면 더 엄격해야 할 수도. 왜냐면 테스트 코드가 틀리면 버그를 못 잡으니까. 처음엔 팀원들이 이해 못 했다. "그냥 테스트인데 뭘 그렇게 까다롭게 봐요?" "기능 개발도 바쁜데 테스트 코드까지 리뷰해요?" 설득이 필요했다. 데이터를 모았다. 지난 3개월 프로덕션 버그 분석. 결과는 명확했다. 버그 42건 중 18건이 테스트 코드 문제. 테스트가 잘못 짜여 있거나, 엣지 케이스를 놓쳤거나, 아예 테스트가 없었거나. 팀 회의에서 공유했다. 분위기가 바뀌었다. 리뷰 체크리스트 만들기 막상 테스트 코드 리뷰를 시작하니 또 문제. 뭘 봐야 하는지 모르겠다는 거다. 개발 코드 리뷰는 레퍼런스가 많다. 클린 코드, SOLID 원칙, 디자인 패턴. 근데 테스트 코드는? "테스트가 통과하면 되는 거 아니에요?" 아니다. 통과하는 건 최소한이다. 제대로 테스트하고 있는지 봐야 한다. 체크리스트를 만들었다. 처음엔 10개 항목으로 시작. 지금은 25개까지 늘었다. 기본 항목테스트 이름이 명확한가 한 테스트에서 하나만 검증하는가 Given-When-Then 구조가 명확한가 하드코딩된 값이 있는가 Sleep이나 고정 대기 시간이 있는가신뢰성 항목Flaky할 가능성은 없는가 외부 의존성을 제대로 모킹했는가 테스트 순서에 의존하지 않는가 실패 시 원인을 바로 알 수 있는가 타임아웃 설정이 적절한가유지보수 항목중복 코드가 없는가 픽스처/헬퍼 함수를 재사용하는가 UI 변경 시 영향도가 최소화되는가 테스트 데이터 생성이 명확한가 실패 메시지가 구체적인가체크리스트를 팀 위키에 올렸다. PR 템플릿에도 추가했다. "테스트 코드 리뷰 체크리스트 확인 완료" 처음엔 귀찮아했다. 리뷰 시간이 두 배로 늘었으니까. 근데 효과는 바로 나타났다. 2주 만에 Flaky 테스트가 40%에서 15%로 줄었다. 리뷰에서 미리 잡았기 때문이다. "여기 waitForElement 대신 explicit wait 쓰세요." "이 assertion은 너무 느슨한데요. 구체적으로 값 확인하세요." 한 달 후엔 테스트 실패 원인 파악 시간이 절반으로 줄었다. 실패 메시지가 명확해졌으니까. Before: "Test failed" After: "Login button should be enabled after valid email input, but was disabled" 리뷰 문화 만드는 법 체크리스트만으론 안 된다. 문화가 필요하다. 처음엔 내가 모든 테스트 코드를 리뷰했다. PR마다 최소 5개 이상 댓글. "이건 왜 이렇게 짰어요?" "여기 엣지 케이스 빠졌는데요?" "이 테스트 이름은 의도가 안 보여요." 팀원들 반응은 두 가지. 절반은 짜증. "너무 깐깐한 거 아니에요?" 절반은 감사. "이런 거까지 봐주시네요." 3주 정도 지나니 변화가 보였다. 팀원들이 서로 테스트 코드를 리뷰하기 시작했다. 내가 지적했던 포인트를 다른 사람 PR에서도 잡는 거다. "여기 assertion이 너무 일반적인데, 구체적으로 바꾸면 어때요?" "이 테스트 100줄인데 헬퍼 함수로 분리하는 게?" "waitFor 조건이 애매해서 Flaky할 것 같아요." 문화가 생기기 시작했다. 월간 "테스트 코드 리뷰 챔피언"을 뽑았다. 가장 의미 있는 리뷰 댓글을 단 사람. 상품은 스타벅스 기프티콘 5만원. 별거 아닌데 효과는 있었다. 사람들이 리뷰에 더 신경 쓰기 시작했다. "이번 달은 내가 챔피언 할 거야." 약간 게임처럼 됐다. 그리고 규칙을 하나 더 만들었다. "테스트 코드 approve 없이는 머지 불가" 개발 코드는 2명 approve 필요. 이제 테스트 코드도 마찬가지. 최소 1명은 체크리스트 기반으로 꼼꼼히 봐야 한다. 처음엔 병목이 됐다. 리뷰 대기 시간이 길어졌다. 근데 2주 정도 지나니 적응됐다. 오히려 배포 전 발견되는 버그가 줄어서 전체 속도는 빨라졌다. 실제 리뷰 사례 지난주 후배 J가 올린 PR. 로그인 E2E 테스트 추가. 코드는 이랬다. def test_login(): driver.get("https://example.com") driver.find_element(By.ID, "email").send_keys("test@test.com") driver.find_element(By.ID, "password").send_keys("password123") driver.find_element(By.ID, "submit").click() time.sleep(3) assert "Dashboard" in driver.title테스트는 통과했다. 근데 문제가 많았다. 내 리뷰 댓글: 1. 테스트 이름이 너무 일반적 ❌ test_login ✅ test_login_with_valid_credentials_shows_dashboard2. 하드코딩된 URL과 credentials # Before driver.get("https://example.com")# After driver.get(config.BASE_URL) email = test_data.get_valid_user()["email"]3. time.sleep(3) 사용 # Before time.sleep(3)# After WebDriverWait(driver, 10).until( EC.title_contains("Dashboard") )4. assertion이 너무 느슨함 # Before assert "Dashboard" in driver.title# After assert driver.title == "Dashboard - Welcome" assert driver.find_element(By.CLASS_NAME, "user-name").text == "Test User"5. Given-When-Then 구조가 불명확 def test_login_with_valid_credentials_shows_dashboard(): # Given login_page = LoginPage(driver) valid_user = test_data.get_valid_user() # When login_page.enter_email(valid_user["email"]) login_page.enter_password(valid_user["password"]) login_page.click_submit() # Then dashboard_page = DashboardPage(driver) assert dashboard_page.is_displayed() assert dashboard_page.get_welcome_message() == f"Welcome, {valid_user['name']}"J가 처음엔 당황했다. "테스트 통과했는데 왜 이렇게 고쳐야 해요?" 설명했다. "지금은 통과해. 근데 내일 프론트가 타이틀 바꾸면 깨져. 모달 하나 더 뜨면 타이밍 꼬여. 누가 이 테스트 보면 뭘 검증하는지 모르겠어." "이건 지금은 테스트고, 3개월 후엔 레거시야. 6개월 후엔 아무도 안 만지는 코드. 그때 가서 고치려면 2시간 걸려. 지금 30분 투자하면 그걸 막을 수 있어." J가 수정해서 다시 올렸다. 완전히 달라졌다. Page Object Pattern 적용, 명확한 wait 조건, 구체적인 assertion. 머지 후 Slack에 메시지가 왔다. "이제 왜 리뷰가 중요한지 알겠어요. 제 코드가 훨씬 나아졌어요." 이게 문화다. 리뷰에서 자주 잡히는 것들 3개월간 테스트 코드 리뷰 데이터를 정리했다. 총 PR 85개, 리뷰 코멘트 428개. TOP 5 지적 사항 1위: 불명확한 wait 조건 (78회) # Bad time.sleep(2) time.sleep(5) implicit_wait(10)# Good WebDriverWait(driver, 10).until( EC.presence_of_element_located((By.ID, "result")) )대부분 "그냥 돌아가게" 하려고 sleep을 넣는다. 근데 이게 Flaky의 90%를 차지한다. 로컬에선 되는데 CI에서 실패하는 이유. 2위: 너무 일반적인 assertion (62회) # Bad assert response.status_code == 200 assert element.is_displayed() assert len(results) > 0# Good assert response.status_code == 200 assert response.json()["status"] == "success" assert element.is_displayed() and element.is_enabled() assert len(results) == 3 assert results[0]["title"] == "Expected Title""일단 통과하게" 만들려다 보면 이렇게 된다. 근데 이런 테스트는 버그를 못 잡는다. 버그가 있어도 통과하니까. 3위: 테스트 간 의존성 (54회) # Bad def test_1_create_user(): global user_id user_id = create_user()def test_2_update_user(): update_user(user_id) # test_1에 의존# Good @pytest.fixture def created_user(): user_id = create_user() yield user_id delete_user(user_id)def test_update_user(created_user): update_user(created_user)테스트 순서에 의존하면 안 된다. pytest는 순서 보장 안 한다. parallel 실행하면 무조건 깨진다. 4위: 하드코딩 (47회) # Bad driver.get("https://dev.example.com") login("admin@example.com", "password123") assert element.text == "John Doe"# Good driver.get(config.BASE_URL) login(test_user.email, test_user.password) assert element.text == test_user.full_name환경 바뀌면 바로 깨진다. dev에서 staging으로, staging에서 prod로. 5위: 불명확한 테스트 이름 (43회) # Bad test_user() test_api() test_button_click() test_scenario_1()# Good test_user_registration_with_valid_email_creates_account() test_api_returns_404_when_resource_not_found() test_submit_button_disabled_when_form_invalid() test_checkout_flow_with_multiple_items_calculates_total_correctly()테스트 이름은 문서다. 실패했을 때 이름만 봐도 뭐가 문제인지 알아야 한다. 리뷰어 가이드 리뷰하는 것도 배워야 한다. 처음엔 다들 뭘 어떻게 봐야 할지 몰랐다. 가이드를 만들었다. 리뷰 순서테스트 이름부터 읽기 - 의도 파악 Given-When-Then 구조 확인 - 논리 흐름 Assertion 체크 - 실제로 뭘 검증하는지 Wait/Sleep 확인 - Flaky 가능성 하드코딩 찾기 - 유지보수성 중복 코드 확인 - 리팩토링 필요성좋은 리뷰 코멘트 예시 ❌ "이거 이상한데요?" ✅ "이 assertion은 element가 존재하는지만 확인하는데, 실제로는 올바른 값을 가지고 있는지도 검증해야 할 것 같습니다. expected_value와 비교하는 건 어떨까요?"❌ "바꾸세요" ✅ "time.sleep(2)는 CI 환경에서 불안정할 수 있어요. WebDriverWait로 특정 조건을 기다리는 게 더 안정적일 것 같은데, 어떤 조건을 기다려야 할까요?"❌ "이해가 안 가요" ✅ "test_user()라는 이름만으로는 이 테스트가 정확히 뭘 하는지 알기 어려운데, 더 구체적인 이름으로 바꾸면 좋을 것 같아요. 예: test_user_login_with_invalid_password_shows_error_message()"질문형으로 쓴다. 명령형보다 부드럽다. "왜 이렇게 했어요?" 보다 "이렇게 하면 어떨까요?" 그리고 대안을 제시한다. 문제만 지적하지 말고 해결책도. 개발자와 동등한 리뷰 제일 중요한 건 이거다. 테스트 코드 리뷰를 개발 코드와 똑같이 대한다. 처음엔 개발자들이 내 리뷰를 우습게 봤다. "QA가 코드를 뭘 알아?" "테스트는 그냥 돌아가면 되는 거 아니에요?" 바꿔야 했다. 내 리뷰 스탠다드를 올렸다. 개발 코드 리뷰만큼 디테일하게. 때로는 더 깐깐하게. 시니어 개발자 K의 PR. API 테스트 코드 추가. def test_api(): response = requests.get("/api/users") assert response.status_code == 200내 댓글: "이 테스트는 API가 200을 리턴하는지만 확인하는데, 실제 응답 데이터 구조나 내용은 검증하지 않네요. 스키마 검증이나 특정 필드 값 확인을 추가하면 어떨까요? 또한 에러 케이스(404, 500 등)에 대한 테스트도 필요해 보입니다." K가 처음엔 어이없어했다. "테스트인데 이 정도면 충분한 거 아니에요?" 대답했다. "K님이 짠 프로덕션 코드에서 이런 PR 올리면 approve 하실 건가요? 'return 200'만 있고 실제 비즈니스 로직은 없는 코드요. 테스트 코드도 똑같아요. 제대로 된 검증이 있어야 버그를 잡죠." 침묵. 그리고 수정된 코드. def test_get_users_returns_valid_user_list(): # Given expected_users = test_data.get_sample_users(3) # When response = requests.get(f"{config.API_BASE_URL}/api/users") # Then assert response.status_code == 200 assert response.headers["Content-Type"] == "application/json" data = response.json() assert "users" in data assert len(data["users"]) == 3 for idx, user in enumerate(data["users"]): assert "id" in user assert "email" in user assert user["email"] == expected_users[idx]["email"]def test_get_users_with_invalid_token_returns_401(): response = requests.get( f"{config.API_BASE_URL}/api/users", headers={"Authorization": "Bearer invalid_token"} ) assert response.status_code == 401 assert response.json()["error"] == "Unauthorized"완전히 달라졌다. 이제 이게 버그를 잡을 수 있는 테스트다. K가 Slack으로 메시지 보냈다. "리뷰 감사합니다. 테스트를 너무 만만하게 봤네요." 이후로 K는 내 리뷰를 진지하게 받아들였다. 다른 개발자들도 마찬가지. 지금은 내가 "Changes requested" 하면 다들 제대로 고친다. 개발 코드 리뷰어가 요청한 것처럼. 동등해졌다. 3개월 후 결과 데이터로 말한다. Flaky 테스트Before: 전체의 40% After: 12%프로덕션 버그 (테스트 코드 원인)Before: 월평균 6건 After: 월평균 1.5건테스트 실패 원인 파악 시간Before: 평균 25분 After: 평균 8분테스트 코드 가독성 점수 (팀 자체 평가)Before: 10점 만점에 5.2점 After: 8.7점테스트 커버리지Before: 62% After: 78%커버리지가 오른 이유가 재밌다. 테스트를 더 많이 짠 게 아니라, 기존 테스트가 제대로 된 검증을 하게 됐기 때문. 하나의 테스트가 여러 케이스를 제대로 커버하니 자연스럽게 올랐다. 그리고 부수 효과. 테스트 코드를 보고 기능을 이해하는 개발자가 늘었다. "이 API 어떻게 쓰는지 테스트 코드 보면 알겠네요." 테스트가 문서가 됐다. 지금도 계속 배운다 완벽하지 않다. 여전히 놓치는 게 있다. 지난주에도 리뷰를 통과한 테스트가 CI에서 깨졌다. race condition을 못 잡았다. 병렬 실행하니 DB 트랜잭션 충돌. 리뷰 체크리스트에 항목을 추가했다. "병렬 실행 시 안전한가?" 매달 회고를 한다. "이번 달 리뷰에서 놓친 것들" "새로 추가할 체크리스트 항목" "더 나은 리뷰 방법" 테스트 코드 리뷰는 계속 진화한다. 기술이 바뀌니까. 툴이 바뀌니까. 팀이 배우니까. 근데 핵심은 하나다. 테스트 코드를 프로덕션 코드만큼 진지하게 대하기. 이게 전부다.테스트 코드 리뷰 문화, 3개월 걸렸지만 이제는 당연하다. 코드는 코드니까.