효과적인 코드 리뷰를 위해서

종종 팀 내에서 코드 품질이 이슈가 됩니다. 그리고 유닛 테스트와 코드 커버리지를 향상시키는 방법에 대해 모두가 한 마디씩 던집니다. 하지만 그리 오래가진 못합니다. 모두들 다시 바빠지면서, 코드 품질에 대한 관심은 금세 식어버리기 때문입니다. 하지만 아마도 대부분은 1년 이내에 데자뷰를 겪게 될 것입니다. 내년이 되면 또 코드 품질이 이슈화 될 것이고, 이전에 나왔던 아이디어와 똑같은 것들을 또 테이블 위에서 보게 될 것이니까요.

안녕하세요. 저는 LINE NOW에서 테스트 자동화 관련 업무를 맡고 있는 QA(Quality Assurance) 엔지니어 Bryan Liu입니다. 이번 글에선 LINE 대만에서 유닛 테스트와 코드 리뷰 프로세스를 활성화시키기 위해 제가 했던 일들을 소개해드릴까 합니다.

유닛 테스트와 코드 리뷰

LINE OJT(on-the-job training)에서 CTO가 설명하는 것처럼, 상호(peer) 코드 리뷰는 LINE 엔지니어 문화의 한 축입니다. Facebook에선 개발에서 가장 중요한 세 가지 요소로 코드 리뷰와 코드 리뷰, 그리고 코드 리뷰를 꼽기도 했습니다. 맞습니다. 유닛 테스트를 통해 코드 품질을 향상시킬 수 있는 유일한 길은 그것을 우리 엔니지어 문화의 한 부분으로 만드는 것인데요. 바로 코드 리뷰가 그 일을 도와줄 수 있습니다.

(보이 스카웃 규칙 – 항상 네가 처음 만난 것보다 더 나은 상태로 코드를 만들어놓고 떠나라)
코딩을 위한 보이 스카웃 규칙, {codemotion} 발췌.

‘코드 리뷰를 할 때, 새롭게 추가된 코드나 버그 수정에 대한 유닛 테스트가 충분한 지 확인하는 것을 권장한다.’ 리뷰어는 이 보이 스카웃 규칙을 따라야 합니다. 그리고 지속적으로 이걸 수행한다면, 코드 커버리지는 확장되거나 최소한 현재 상태를 유지할 수 있을 것입니다. 예를 들어, 코드 커버리지가 떨어졌다면, 리뷰 받는 사람은 자신이 겪고 있는 어려움과 테스트를 더 추가하지 않은 이유에 대해 모두에게 설명해야 합니다. 모든 사람들이 이 설명에 동의하고, 다른 이슈가 없다면, 리뷰 받는 사람은 이후 처리를 진행할 수 있습니다. 그러나 그렇지 않다면 반드시 문제를 고쳐야 합니다.

효과적인 코드 리뷰를 위한 팁들

가장 효과적으로 코드 리뷰할 수 있는 방법은 페어 프로그래밍입니다만, 팀의 성향에 따라 GitHub에서 PR(Pull Request)를 이용하는 것도 좋은 방법입니다. 코드 리뷰를 ‘제대로’ 하려면, 우선 코드 리뷰 프로세스를 효율적으로 만들어야 합니다. 한 가지 아이디어는 리뷰어를 희귀한 자원으로 다루는 것입니다. 우리 중 누구도 코드 리뷰를 주 업무로 하고 있진 않다는 것이 그 이유입니다.

그리고 여기 코드 리뷰를 효과적이고 효율적으로 하기 위한 몇 가지 팁이 더 있습니다.

변화를 작게 유지하자

Cisco 시스템 프로그래밍 팀의 연구에 따르면 300줄에서 400줄 정도의 코드를 60분에서 90분 동안 리뷰하면 70~90%의 결함을 발견할 수 있다고 합니다. 각각의 PR을 릴리스 가능한 단위(기능(feature), 버그 수정), 혹은 의미있는 PR이 될 수 있는 하나의 응집된 아이디어로 다뤄야 합니다. 너무 많은 수정사항이 포함된 커다란 PR이 왜 좋지 않은지, 적합한 PR 사이즈는 무엇인지 여기서 확인해봅시다.

코드 리뷰, @iamdeveloper, Twitter 발췌 
Defect density(결함 밀집도) vs LoC(라인 수), Cisco study case발췌

리뷰는 자주, 짧은 세션으로 진행하자

코드 리뷰는 적절한 양을, 천천히, 정해진 시간 동안에 진행할 때 가장 효과적인 결과가 나옵니다. 리뷰하면서 코드가 400줄을 넘어가면 참석한 리뷰어는 결함을 찾아낼 수 있는 능력을 잃어버리게 됩니다. 결함이 발견되는 비율(inspection rates)은 1시간에 300줄 정도일 때가 가장 좋습니다.

Defect density(결함 밀집도) vs inspection rate(결함이 발견되는 비율), Cisco study case발췌

리뷰를 위해 최대한 빨리 PR을 보내자

가치있는 코드 리뷰가 되기 위해선, 세부사항을 구현하기 전에 논의를 시작해서 커다란 차이점(diffs) 뭉텅이를 보내지 않도록 노력해야 합니다. 큰 문제가 있다면 이를 작은 문제로 나누어서 한 번에 작은 문제 하나씩 해결하는 방법을 이용합니다. 서로 다른 아이디어는 서로 다른 PR로 분리하고, 필요하다면 서로 다른 리뷰어를 할당합니다.

코드 리뷰 중 마지막 PR에서 구조나 설계에 관한 문제가 발견된다면, 위 그림과 같이 팬을 설치하기 위해 벽에 구멍을 내는 것 같은 엉뚱한 해결책이 적용될 수 밖에 없습니다. @isoiphone on Twitter 발췌.

의미있는 PR을 만들기에 충분한 정보를 제공하자

Reviewer resource is very limited, treat it wisely! (리뷰어는 매우 제한되어 있으니 그들을 현명하게 다룹시다!)

리뷰어가 코드의 문맥을 빨리 파악할 수 있도록 충분한 정보를 제공하는 것이 중요합니다. 무슨 이유로 어떻게 코드를 변경했는지, 어떤 위험이나 우려가 발견되었는지에 대한 충분한 정보를 리뷰어에게 제공해야 합니다. 이런 정보들은 후에 수준 높은 논의를 이끌어내는 훌륭한 촉매 역할을 합니다. 또한 리뷰가 시작되자마자 리뷰 받는 본인이 스스로 다른 에러들을 발견하게 되는 추가적인 혜택도 종종 누릴 수 있습니다. 모든 PR에 매우 상세한 정보를 쓰는 건 필요없겠지만, 적어도 무엇을 완료하여 테스트했고 어떤 부분에 리뷰어가 집중해야 하는 지 명확하게 덧붙이는 건 가능할 것입니다.

이런 작업을 할 때 GitHub에서 제공하는 이슈와 PR 템플릿을 사용하면 도움이 될 것입니다. 무슨 일을 하려는지 설명하는 스크린 숏을 첨부하는 것도 매우 좋은 아이디어입니다. 아래 PR 템플릿을 사용한 두 가지 예시가 있습니다. 이렇게 템플릿을 잘 활용하면 코드 리뷰는 물론 QA 확인에도 도움이 되는 충분한 정보를 제공할 수 있습니다.

GitHub PR 템플릿 사용 예시 – 1
GitHub PR 템플릿 사용 예시 – 2

코드 분석 및 코드 스타일 확인을 거치자

우리의 눈은 중요한 비즈니스 로직과 알고리즘 확인을 위해 아껴두고, 정적 코드 분석과 코딩 스타일 확인은 SonarQube나 ESLint와 같은 툴에게 맡깁시다. 이러한 코드 스캐닝 툴, 타입 체킹 툴, 코드 분석 툴들은 버그와 code smells, 취약점(vulnerabilities)들을 리포팅 해 줄 수 있는데요. 여기에 좋은 테스트 세트까지 함께 한다면 확실하게 신뢰도를 높일 수 있습니다.

SonarQube 이슈 탐지 SonarQube website 발췌

코드 리뷰에서 가장 중요한 부분 중의 하나는 개발자들의 성장과 노력에 대해 보상하는 것입니다. 따라서 최대한 많이 그들을 칭찬해주도록 노력해야 합니다. 

마지막으로, 코드를 제대로 이해할 수 없다면 적절한 리뷰가 될 수 없습니다. 진행되고 있는 논의가 지지부진한 것 같다면, 좀 더 효과적으로 리뷰할 수 있는 사람을 찾아서 논의가 진전될 수 있도록 합니다.

이것을 엔지니어 문화의 일부로 만듭니다

‘문화는 아무도 보지 않을 때도 사람들이 하게 만드는 것이다.’ 라고 합니다. 코드 리뷰 프로세스가 생략되었다고 가정해봅시다. 그래도 여전히 적절한 테스트를 만들겠습니까? 쉽지 않겠죠? 하지만 여전히 시도해 볼 만한 가치가 있습니다. 애자일이 도입된 프로젝트라면, 아래 항목들을 고려하여 팀 문화를 자기 주도적으로 만들어봅시다. 그리고 끊임없이 배우고 향상시켜봅시다.

  • 자치권(autonomy): 팀원은 각자 책임을 갖고 각자가 선호하는 방식으로 일합니다(예시: 스크럼, 페어 프로그래밍).
  • 숙달(mastery): 끊임없이 좋은 코딩 사례를 만들고, 코드 리뷰를 하면서 서로가 서로에게 배우게 하여, 결과적으로 각 개인의 코딩 스킬을 향상시킵니다.
  • 목적(purpose): 코드 품질은 궁극적인 목표입니다. 버그를 조기에 발견하여 생산이 지연되는 것을 막습니다.

팀 문화 형성을 촉진시키기 위해, 저는 아래 두 가지 항목에 노력을 기울였습니다.

스킬 향상

팀 문화 형성의 근본적인 토대를 마련하기 위해선, 개발자들이 일상 업무속에서 점점 커져나가는 팀의 합의점(모범 사례)에 이를 수 있도록 올바른 개념과 완전한 지식을 갖고 있어야 합니다. 저희는 개발자들을 돕기 위해서 현장 컨설턴트의 도움을 받아 유닛 테스트와 리팩토링, TDD(Test-Driven Development) 워크샵을 열었습니다. 

저희가 워크샵에서 다룬 주제들은 아래와 같습니다(꼭 아래와 똑같이 해야 하는 건 아닙니다). 

  1. 유닛 테스트
    • 코드의 구현보다 코드의 의도를 밝히는 데 초점을 둔 테스트 케이스 설계
    • 의존관계 정의 및 고립
    • 추출과 덮어쓰기와 의존성 주입 메소드 소개
    • stub과 mock 프레임워크, assertion 라이브러리 설명
    • 메소드 추출과 변수 inline화 등 여러 가지 리팩토링 스킬 연습
  2. Kata 수행 
    • 요구사항 분석, 시나리오 정제, 주요 예시 찾기
    • 코드 설계 및 구현
  3. TDD와 리팩토링
    • 데모 리팩토링, code smells와 관련된 메소드 정의 및 제거
    • TDD 접근 방법으로 라이브 코딩(예제: baby steps, red light green light)
    • 예제 수행
워크샵 중 찍은 사진

진척 사항 측정

If you can’t see it, you can’t measure it, you can’t improve it! (볼 수 없는 건, 측정할 수 없고, 향상시킬 수 없다!)

공용 대시보드의 시각적 효과와 메시지 알림 기능을 이용하여 우리가 추구하는 목표를 모두에게 지속적으로 상기시키기 위해, 입구 옆에 설치된 커다란 스크린에 아래 대시보드들을 차례로 표시했습니다.

SonarQube 프로젝트 대시보드

모든 정적 코드 분석의 통계는 SonarQube에서 가져옵니다. 생산 서비스와 직접 연결된 코드 저장소들은 여기에 리포트를 게시해야 합니다.

팀 단위 코드 커버리지

팀 단위 코드 커버리지 차트는 팀 내 각 코드 저장소의 코드 커버리지 추세를 보여줍니다. 이 차트가 있다면 굳이 개개인이 SonarQube 프로젝트 페이지를 헤매고 다닐 필요가 없습니다. 이런 종류의 차트를 줄지어 세워놓으면, 각 팀들이 어떻게 하고 있는지 한눈에 비교하기 쉽습니다.

PR 크기와 해결 시간

DevOps의 핵심 아이디어는 ‘어떻게 하면 소프트웨어 변경사항을 자주, 좋은 품질로 릴리스할 수 있을까’ 입니다. 각 배포 유닛을 작게 만드는 것이 바로 여기서 쓰인 트릭입니다. 너무 큰 PR은 좋은 코드 리뷰를 이끌어내는 게 불가능할 뿐 아니라 코드 품질과 릴리스 주기에도 영향을 미칩니다. 따라서 과제와 변경사항을 작게 만드는 게 DevOps에선 중요한 스킬입니다. 이 아이디어를 알리기 위해 아래와 같이 ‘Resolution time(해결에 걸린 시간) vs. PR size(PR 크기)’ 차트를 만들었습니다.

  • 동그라미 크기와 색상: 변경되는 세트의 크기(코드 라인 수)
  • 해결에 걸린 시간(Resolution time): PR 생성 후 머지(merge)될 때까지 걸린 시간 
  • #번호: PR 번호

이런 차트들은 지속적으로 모두의 관심을 끌어서, 모범 사례 채택의 진척 상황과 함께 우리가 추구하는 목표를 상기시켜 줍니다. 이것들은 우리가 만든 몇 가지 예시에 불과합니다. 다른 사람에게 당신의 의도를 시각적으로 전달할 수 있는 방법으로 어떤 게 있을 지 잘 생각해봐야 합니다. 이런 차트들은 월간 회의에서 진행 상황을 요약해 보여줄 때도 유용합니다.

PR 댓글 알림

각 커밋이 PR에 제출되면 웹훅이 작동하면서 아래처럼 github 커멘트가 생성됩니다. 이 커멘트를 통해 PR을 만든 사람은 테스트를 추가하고 새롭게 발견된 몇 가지 취약점을 수정해야 한다는 점을 상기할 수 있습니다. 이걸 바로 상기시키는 이유는 이런 종류의 작업은 변경사항이 릴리스되는 2주 후에 하는 것보다 지금 바로 하는 게 좀 더 효율적이기 때문입니다. 또한 품질 지수를 개선하려면, 리뷰어들은 리뷰 받는 사람이 왜 문제를 겪고 있는지 그 이유를 찾아내는 데에도 도움을 줄 수 있어야 합니다.

  • Last n Avg.(최근 n회 평균): 각 항목 별 최근 추세
  • xxx_violations(xxx_위반사항): 발견된 버그와 취약점, code smell의 개수
  • line_coverage(라인 커버리지): 전체 대비 유닛 테스트가 수행된 코드 라인 수의 %

요약 및 향후 계획

클린 코드를 작성하는 방법과 code smell을 정의하여 제거하는 방법은 코드 리뷰 시 좋은 논의 거리가 될 수 있습니다. 그리고 팀이 이러한 공통 이슈 해결에 진정으로 시간을 사용할 때, 그 과정에서 문화가 자라날 수 있습니다.

다른 측면에서, 별도로 추적되지 않는 항목들은 크게 쓸모가 없습니다. 중요한 건 시간에 따른 데이터의 추세를 보여주는 것인데요. 이런 추세들은 우리가 그에 상응해서 움직일 수 있는 직접적인 원인이 되기 때문입니다. 위 차트에 나타난 추세 선을 살펴보면 모든 것들이 진척 상황과 함께 움직이고 있습니다. 현재 저희는 아래 대시보드들을 추가하는 것을 고려하고 있습니다.

  • 품질: 버그의 open, close 누적 개수와 심각도, 결함 밀집도 정보 
  • 속도: 배포 주기, 생산 리드 타임, 변경 실패율과 MTTR(mean time to repair, 평균 수정 시간)

Reference