[Spring, Test] 가독성 좋은 테스트 코드로 리팩터링하기

어제 프로젝트가 0.1.0 릴리즈가 되었는데, 그동안 전체적인 테스트 코드 리팩터링을 담당했습니다.

 

제가 중요하게 생각했던 내용은 메인 테스트 메서드 코드가 읽기 쉽게 되어 있는지에 대한 것이었습니다.

최종적인 목표는 메인 테스트 메서드에는 핵심 내용만 볼 수 있도록 만드는 것입니다.

 

 

 

1. 1차 테스트 코드 리팩터링

---

지난 달에 프로젝트가 많이 시작되지 않은 상태에서 리팩터링을 1차적으로 진행되었습니다.

 

[1] var 사용하기

https://hello70825.tistory.com/537

[Java] var는 사용해도 되는 것일까?

 

거의 6개월 전에 글을 작성한 적이 있는데, 실제로 테스트에서 사용해본 것은 팀프로젝트가 처음이었습니다.

직접 경험해보니 글에 나와있는 장점중에 하나인 변수명과 변수 값에만 집중한다는 장점이 매우 크게 다가왔습니다.

 

var를 사용하게 된다면 코드 읽는 흐름이 [타입] → [변수명] → [변수 값]에서 [변수명] → [변수 값]으로 줄어들었습니다.

변경점은 읽는 흐름을 1개만 제거한 것이지만, 현재 테스트 코드는 수천줄로 코드 읽는 피로감이 효과적으로 줄어들게 됩니다.

---

[2] @BeforeEach 삭제

읽기 좋은 테스트 코드를 만들기 위해서는 하나의 테스트 메서드 안에 중요한 내용은 다 있어야 한다고 생각합니다.

위 이유도 있었고, 나중에 새롭게 생기게될 테스트 메서드에서는 @BeforeEach 데이터를 사용하지 않을 수도 있다고 판단하여 @BeforeEach를 삭제하게 되었습니다.

 

나중에 생각해보니 페이징 테스트에서는 1순위 조건과 2순위 조건에서 서로 다른 데이터를 사용해야하기 때문에 결국엔 삭제할 수 밖에 없었습니다.

1순위 조건 데이터와 2순위 조건 데이터를 합치면 되는거 아닌가라고 생각할 수 있지만, 그러면 테스트 데이터를 만들기도 어려워지고, 특히 유지보수하기 매우 어려워집니다.

 

@BeforeEach를 삭제한 결과 테스트 메서드 로직이 길어진 부분도 있었지만, 불필요한 이동 없이 모든 테스트 메서드에서 핵심 로직을 파악할 수 있게 되었습니다.

---

[3] 통일된 테스트 격리 설정

테스트 격리는 2차 리팩터링에도 진행했지만, 여기에 전부 언급하겠습니다.

당시 제 팀은 JUnit5의 BeforeEachCallback, @Transactional을 사용하여 테스트 격리를 사용하고 있었습니다.
두 개를 사용하신 분은 에러를 마주쳤을 확률이 높겠지만, 둘의 테스트 격리 방식은 다르게 동작합니다.

BeforeEachCallback은 테스트 코드를 수행하기 직전에 데이터 초기화를 진행하고, @Transactional은 테스트 코드가 수행된 이후에 롤백이 됩니다.

그래서 인수 테스트에만 사용하던 BeforeEachCallback을 서비스 테스트와 리포지토리 테스트에도 적용 했습니다.

 

테스트 격리 방식에 대한 트러블 슈팅 글은 아래를 참조하면 됩니다.

https://hello70825.tistory.com/572

[테스트 격리] 일관된 테스트 격리를 적용하는 트러블 슈팅

 

참고로.. @SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT)로 한다면 @Transactional로는 격리가 되지 않습니다.

 

If your test is @Transactional, it will rollback the transaction at the end of each test method by default. If you’re using this arrangement in combination with either RANDOM_PORT or DEFINED_PORT, any transaction initiated on the server won’t rollback as the test is running in a different thread than the server processing.

 

왜냐하면 스프링 테스트 공식 문서를 살펴보면 서로 다른 스레드에서 서버 역할 - 테스트 역할이 나누어져 있어서 롤백이 되지 않는다고 나와있습니다.

현재 @Transactional은 서버 코드가 아닌 테스트 코드에 있기 때문에 적용되지 않습니다. 그래서 인수테스트는 보통 @Sql 어노테이션을 통해 격리하는 경우가 많습니다.

 

왜 스레드가 나뉘어지면 @Transactional이 적용되지 않을까.. 라는 생각이 드는 분은 @Transactional 내부 코드를 끝까지 내려가보면 Connection은 ThreadLocal에 저장한다는 것을 알 수 있습니다.

ThreadLocal과 연결된 스레드는 1 대 1 관계이기 때문에 다른 스레드가 접근을 하지 못하게 됩니다.

 

이건 @Transactional 내부 코드를 보게 된 계기인데, 이것도 결국엔 ThreadLocal과 관련이 있는거라 참고하면 좋을 것 같습니다.

https://hello70825.tistory.com/555

@Transaction를 적용하면 어떻게 하나의 Connection을 사용할까?

---

 

 

 

2. 2차 테스트 코드 리팩터링

---

거의 모든 기능들이 만들어지고, 릴리즈되기 직전 대규모로 2차적인 테스트 코드 리팩터링을 진행했습니다.

자잘자잘하게 고친 점이 많아서 중요한 부분만 다루겠습니다.

 

[1] @Nested를 사용하여 테스트 코드 계층화

일반적으로 테스트 코드를 작성하게 된다면 @Test만 사용하게 될 것입니다.

하지만 @Test만 사용하게 된다면 테스트 코드를 유지보수하기 어려워진다는 단점이 존재합니다.

만약 서비스에 있는 메서드들이 많을 경우, 다양한 코드가 계층화 없이 모여있기 때문에 고쳐야할 테스트 메서드를 찾는데 오랜 시간이 걸릴 것으로 예상됩니다.

 

하지만 @Nested를 적용한다면..??

다음과 같이 효과적으로 각 테스트의 역할을 구분할 수 있게 됩니다.

 

* 너무 과도한 @Nested는 피하기

 

처음 적용한 @Nested의 컨벤션은 다음과 같이 적용했습니다.

[메서드이름_테스트] → [필요하면 다양한 조건의 옵션 테스트 종류] → [성공테스트/실패테스트]

최대 depth가 3번이 되는 테스트 코드도 존재했습니다.

 

같은 팀의 크루인 우가의 리뷰로 [메서드이름_테스트]를 [메서드이름_성공_테스트], [메서드이름_실패_테스트]로 나누어서 구분지어도 효과적일 것이라는 피드백을 받았습니다. depth가 3인 테스트 코드 경우에는 너무 오른쪽으로 이동되어 있다는 문제점도 있었구요. 이렇게 고쳐보니 굳이 depth를 하나 더 추가할 필요는 없어보여서 [메서드이름_성공/실패_테스트]로 수정하게 되었습니다.

 

그래서 팀마다 적당한 @Nested 타협점을 찾아서 적용하시길 바랍니다.

---

[2] 2개 이상의 검증 로직이 있는 테스트에는 SoftAssertions 적용하기

이거는 지하철 미션에서 코드 리뷰를 받았을 때인데, 아래 사진으로 보여드리겠습니다.

어디 부분이 틀렸는지 가독성이 더 좋아집니다.

---

[3] 각 테스트의 부모 클래스 만들기

1) 중복 코드 제거

인수 테스트(Acceptance Test), 서비스 테스트(Service Test), 리포지토리 테스트(Repository Test)라는 부모 클래스를 만들게 된다면 클래스에 붙이는 어노테이션과 @Autowired를 제거할 수 있습니다.

그리고 바로 아래 4번에 설명할 제 팀에 남아있는 매우 큰 중복 코드였던 save(), saveAll()을 부모 클래스에 두어 중복 코드를 효과적으로 제거할 수 있었습니다.

---

2) 컨텍스트 캐싱 재사용

일반적으로 테스트 코드를 작성하면 전체 테스트 수행시 클래스마다 아래 Spring 로고가 한번씩 나오는 것을 확인할 수 있습니다.

Spring TestContext Framework는 매 테스트 코드마다 Context를 새로 생성하는게 매우 비효율적이기 때문에 캐싱을 사용하여 속도를 최적화할 수 있습니다.

 

The Spring TestContext framework stores application contexts in a static cache. This means that the context is literally stored in a static variable. In other words, if tests run in separate processes, the static cache is cleared between each test execution, which effectively disables the caching mechanism.

 

스프링 Context Caching 공식 문서를 살펴보면 위와 같은 내용이 나와있는데, TestContext가 static 변수로 저장되는 정적 변수로 저장한다고 합니다.

 

공식 문서을 더 살펴보면 추가적인 중요한 부분을 살펴보면 TestContext 최대 저장 개수는 32개이고, 이걸 넘어가면 가장 오래된 TestContext부터 제거한다고 나와있습니다.

 

그리고 이 캐싱 값을 설정하는건 공식 문서에 나와있으니 참고하시길 바랍니다.

---

[4] save(), saveAll() 중복 코드 효과적으로 제거하기

 

1) 어차피 부모 클래스를 사용하는데, 부모 클래스에 두기

→ 수백 줄의 중복 코드 제거

 

2) save() → 단일_@@_저장(), saveAll() → 복수_@@_저장()으로 컨벤션 통일

→ 테스트 코드 읽기 더 쉬워짐

 

하지만.. 중복 코드가 한가지 더 있었습니다.

빨간줄은 이미 수정된 코드라서 그렇습니다

saveAll()을 수행할 때마다 리스트로 감싸주고 보낸다는 점입니다.

리스트가 재사용되면 문제 없겠지만, 대다수의 테스트에서는 더 이상 쓰이지 않았고, 그런 코드가 100줄은 넘어갔습니다.

---

[5] varargs 적용하기

가변 인자는 레벨 1 마지막 미션에서 처음으로 한 번 사용한 것 같은데, 제대로 적용해본건 이번 팀프로젝트가 처음이네요.

핵심 테스트 로직에 List.of()로 감싸주는 로직은 굳이 알아야할 필요가 없기 때문에 varargs를 적용했습니다.

 

그러면 메인 테스트 메서드 로직에는 List가 사라지고...

서브 메서드 로직에 들어가게 됩니다.

---

[6] Fixture 사용하기

[에러 발생]

별 생각없이 평소대로 public static final을 사용하여 Fixture를 만드려고 했지만, 문제가 발생했습니다.

아무 이유도 없이 테스트 데이터가 바뀌는 것입니다.

참 신기하게도 편의점 상품 카테고리중에 하나인 아이스크림이 편의점 종류 카테고리인 CU로 바뀌는 것이었습니다.

오류가 나는 해당 테스트의 로그를 살펴보니 find 메서드에서 update문이 나가고, 이후에 select 문을 호출하게 됩니다.

find 메서드 호출전에 데이터가 바뀌었다는 것이니 처음엔 save 메서드에 문제가 발생하는줄 알았습니다.

하지만 진짜 문제는 한참 전에 이미 실행된 인수 테스트에서 발생했었습니다. 테스트 격리가 안됐던 것이죠

 

카테고리와 관련된 테스트 코드들을 다 확인하니 아래와 같은 문제점이 있었습니다.

위 인수 테스트에서 4번째 카테고리로 CU가 들어갑니다. 그래서 public static final CU는 id = 4로 저장이 되버렸습니다.

이게 한참 지난 후에야 CategoryRepository를 테스트할 때 문제가 발생하는데, 여기서는 4번째 값이 아이스크림입니다.

여기서 아이스크림이 저장되어야 하는데, CU로 바꿔치기 당하는 것입니다.

여기서 디버깅 모드로 확인해보면 아이스크림은 제대로 저장이 됩니다.

saveAll이 save를 반복문으로 돌린다는 것은 다들 아실겁니다.

그래서 순차적으로 저장하다가 CU를 저장하게 될 때, CU는 이미 id = 4이므로 아이스크림 자리에 저장을 하게 됩니다.

그 이유는 public staic final은 프로그램이 실행될 때 static 메모리에 저장하고, 프로그램 종료 전까지 남아있습니다.

그래서 Fixture를 호출할 때마다 같은 데이터를 가져오는 것입니다. 인수 테스트에서 한 번 써먹은 데이터니 인수 테스트에서 변경된 id값이 존재하겠네요.

 

[그러면 JPA에서 Fixture를 사용하려면??]

위의 문제가 결국엔 데이터를 재사용하게 된다는 것입니다.

해결 방법은 데이터를 재사용하지 않으면 됩니다. 그러면 간단하게 매 호출마다 새로운 인스턴스를 생성해서 반환하게 만들면 됩니다.

메서드를 호출할 때마다 새로운 인스턴스를 생성해서 반환한다..??

이건 제가 꾸준히 미션에서 사용해왔던 정적 팩토리 메서드가 하는 일과 완벽히 일치합니다.

그래서 JPA에서는 Fixture를 사용할 때, 정적 팩토리 메서드를 만들어서 사용하면 됩니다.

---

 

 

 

3. 추가로 고려해볼점

---

아직 팀프로젝트에 적용은 안했지만, 고려해볼만한 점을 남깁니다.

 

1. 변수명도 한글로 작성하기

어차피 메서드 명도 한글로 작성하는데, 변수명도 한글로 작성하면 더 빠르게 읽을 수 있을 것 같습니다.

하지만 이 부분은 고민하다가 적용을 하지 않았는데요.

이유는 변수명을 하나하나 바꿔줘야하기 때문에 현재로는 노가다가 너무 많습니다.

그래서 바로 아래 2번을 적용해서 코드양이 줄어들면 그때 적용하는 것으로 생각하고 있습니다.

---

2. 객체 참조 끊기

객체지향의 사실과 오해, 오브젝트 저자이신 조영호님의 우아한 객체지향 세미나 영상을 살펴보면 아래와 같은 내용을 설명하고 있습니다.

 

- 객체 참조는 객체간 강한 결합도를 가지고 있게 되는데 이게 꼭 필요할지..

- 객체들의 라이프사이클을 생각해본적이 있는지..

- 객체들의 결합도, 응집도 생각하기..

- 때로는 객체지향보다는 절차지향이 나을수도..

 

이 부분은 레벨 4에 다시 이야기를 해보고, 모두 동의를 하면 리팩터링을 진행할 예정입니다.

 

테스트 코드 관점에서 객체 참조를 끊으면 좋은 점은 현재 테스트하는 것에만 집중할 수 있게 된다는 점입니다.

예를 들어서 어떤 유저가 리뷰에 좋아요를 누르는 테스트 코드를 만든다고 해봅시다.

이때 리뷰 이미지, 리뷰 태그(맛있어요, 갓성비, ..), ... 이런 내용들이 다른 유저가 좋아요를 할 때 필요할까요?

해당 부분은 리뷰 엔티티를 만들 때 필요한 데이터이지, 리뷰 좋아요 테스트에서는 전혀 필요 없는 내용입니다.

그래서 객체 참조를 끊게 된다면 메인 테스트 코드에서는 정말 필요한 코드만 남을 것으로 예상됩니다.

---