현재 시간, 도메인 로직, 그리고 유닛 테스트
"무엇을 테스트해야 하는지 모르겠어요"
작년(2023) 가을 카카오테크캠퍼스에 멘토로 참여했을 때의 일이다. 프로젝트 로직은 복잡한데, 테스트가 하나도 없어 다음 시간까지 아무거나 좋으니 유닛 테스트를 짜보라고 했다. 특히 database connection이나 스프링 컨텍스트가 필요한 테스트 말고, POJO(Plain Old Java Object)를 테스트 대상으로 삼아보면 좋겠다고 덧붙였다.
일주일 후 다음과 같은 질문을 받았다. "무엇을 테스트해야 하는지 모르겠어요". 신기한 일이다. 프레임워크 다루는 건 거의 현역 개발자만큼 하는 것 같은데, 뭘 테스트해야 하는지 모르겠다는 건 무슨 말일까?
테스트 할 필요가 없는 코드
반대로 생각해보자. 무엇을 테스트하지 말아야 할까? 프레임워크나 라이브러리의 기능을 테스트 할 필요는 없다. 프레임워크나 라이브러리는 이미 테스트되었다. 우리가 테스트해야 하는 것은 우리가 만든 코드다.
물론 프레임워크의 기능이 어떻게 동작하는지 정확하게 알아야 할 필요가 있을 때 테스트를 만들어서 실행시켜 볼 수는 있다. Kent Beck은 이런 테스트를 "학습 테스트"라고 했다. 하지만 이런 테스트 코드를 계속 유지해야할 필요는 없다. 나와 팀 동료들의 머릿속에 동작 방식이 충분히 들어왔다면 지워도 좋다. 모든 코드는 유지관리 비용이 든다. 테스트도 마찬가지여서, 기대한만큼의 효용이 없다면 그 테스트는 지우는 것이 좋다.
가치가 높은 테스트
테스트 피라미드 까지 이야기하지 않더라도, 가장 가치있는 테스트는 대게 다음 두 가지 조건을 충족한다.
그 자체로 훌륭한 문서가 된다.
도메인 로직의 변경이나 리팩터링을 할 때 안전장치가 된다.
학생들이 제출한 실제 프로젝트에서 위 두 가지 조건을 만족하는 테스트를 만들어보자.
문제의 코드
다음 코드는 게시글의 인기도를 업데이트하는 애플리케이션 서비스와 도메인 모델이다.
이 글을 읽는 사람을 위해 로직을 서술하긴 했지만, 리팩터링 하기 전에 로직에 대한 분석은 거의 하지 않는 편이다. 리팩터링을 하면서 로직을 이해하고, 테스트를 만들면서 이해한 것을 문서화 하는 게 더 빠르다.
코드를 보고 어떤 부분이 가장 중요하고 무엇이 문제인지 생각해보자. 그리고 그 부분을 테스트하는 방법을 생각해보자.
// Application Service
@Service
@Transactional
@RequiredArgsConstructor
public class UpdatePostPopularityUseCase {
private final PostRepository postRepository;
public void update() {
final List<Post> posts = postRepository.findAll();
for (final Post post : posts) {
final long likeCount = post.likeCount();
final int postAge = post.measurePostAge();
post.updatePopularity(likeCount, postAge);
postRepository.save(post);
}
}
}
// Domain Model
@Entity
@Getter
@NoArgsConstructor
@Accessors(fluent = true)
class Post {
@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long postId;
@Column(nullable = false)
private Instant createdAt;
@Column(nullable = false)
private Long viewCount = 0L;
@Column(nullable = false)
private Long likeCount = 0L;
@Column(nullable = false)
private Long popularity = 0L;
Post(final Long viewCount,
final Long likeCount,
final Long popularity,
final Instant createdAt) {
this.viewCount = viewCount;
this.likeCount = likeCount;
this.popularity = popularity;
this.createdAt = createdAt;
}
int measurePostAge() {
final Instant now = Instant.now();
final int hours = Duration.between(createdAt, now).toHoursPart();
return 5 > hours ? 1 : hours / 5;
}
void updatePopularity(final long likeCount, final int postAge) {
popularity = (likeCount + viewCount) / postAge;
}
}
무엇이 문제인가?
물론 서비스 운영 경험이 있는 사람이라면 데이터의 규모가 늘어남에 따라 JPA 리파지토리의 findAll() 메서드가 문제가 될 거라고 지적할 수 있다. 동의하지만 이 글의 주제와는 맞지 않는다. 이 코드의 문제는 다음 세 가지이다.
1. Instant.now()를 사용한 현재 시간을 가져오는 코드는 테스트가 불가능하다.
현재 시간은 호출 시점에 따라 계속 변하는 값이다. 바꿔말하면 deterministic 하지 않다(non-deterministic). 테스트 대상 코드가 non-deterministic 한 코드에 의존하면 테스트가 불가능하다.
int measurePostAge() {
final Instant now = Instant.now(); // HERE!
final int hours = Duration.between(createdAt, now).toHoursPart();
return 5 > hours ? 1 : hours / 5;
}
2. 도메인 로직이 도메인 모델에 온전하게 캡슐화되어 있지 않다.
전형적인 Tell Dont Ask 문제이다. Post 클래스 핵심 비즈니스 로직을 위한 데이터를 모두 가지고 있으면서도 자율성이 없고 수동적이다. 애플리케이션 서비스로 로직이 누수되았다.
void updatePopularity(final long likeCount, final int postAge) {
popularity = (likeCount + viewCount) / postAge;
}
```Java
final long likeCount = post.likeCount(); // ask for data
final int postAge = post.measurePostAge(); // ask for data again
post.updatePopularity(likeCount, postAge); // update
3. (Optional) popularity는 다른 필드에 비직교한다. 필드로 만들어서 저장할 필요가 있을까?
popularity를 계산하는 로직은 viewCount, likeCount, createdAt에 의존한다. 이 세 필드가 변경되면 popularity도 변경되어야 한다. 이런 경우 popularity와 다른 필드들의 관계를 비직교(non-orthogonal)하다고 한다. 이런 경우 popularity를 필드로 저장하는 것이 계산해서 리턴하는 메서드를 만드는 편이 복잡도를 줄일 수 있다. 아마 popularity를 필드로 만들고 영속성을 유지한 건, 인기도 순 정렬 등의 조회 로직이 있어서 였을 것 같다. 따라서 이 글에서는 다루지 않는다.
더 알고 싶은 사람은 다음 포스팅을 참고하자.
리팩터링을 해보자!
실제 모든 코드 예제는 다음 PR을 통해 볼 수 있다.
도메인 모델로 보내고자 하는 부분을 메서드로 추출한다. 이때 추출한 updatePopularity의 파라미터가 Post인 것에 주목하자. 로직에 필요한 것은 Post 뿐이라는 것은 다음 리팩터링을 위한 중요한 단서이다.
// class UpdatePostPopularityUseCase
public void update() {
final List<Post> posts = postRepository.findAll();
for (final Post post : posts) {
updatePopularity(post);
postRepository.save(post);
}
}
private void updatePopularity(final Post post) {
final long likeCount = post.likeCount();
final int postAge = post.measurePostAge();
post.updatePopularity(likeCount, postAge);
}
}
2. move instance method
updatePopularity 메서드를 Post 클래스로 옮긴다. 이제 다른 메서드들은 private 가시성으로 변경해도 좋다.
// class UpdatePostPopularityUseCase
public void update() {
final List<Post> posts = postRepository.findAll();
for (final Post post : posts) {
post.updatePopularity(); // HERE!
postRepository.save(post);
}
}
}
// class Post
// moved here
void updatePopularity() {
final long likeCount = likeCount();
final int postAge = measurePostAge();
updatePopularity(likeCount, postAge);
}
// make private
private int measurePostAge() {
final Instant now = Instant.now();
final int hours = Duration.between(createdAt, now).toHoursPart();
return 5 > hours ? 1 : hours / 5;
}
// make private
private void updatePopularity(final long likeCount, final int postAge) {
popularity = (likeCount + viewCount) / postAge;
}
3. introduce parameter
이제 현재 시간을 가져오는 부분을 파라미터로 변경할 때다. Instant.now()를 파라미터로 받도록 변경한다.
// class UpdatePostPopularityUseCase
public void update() {
final List<Post> posts = postRepository.findAll();
for (final Post post : posts) {
post.updatePopularity(Instant.now());
postRepository.save(post);
}
}
}
// class Post
// parameter introduced
void updatePopularity(final Instant now) {
final long likeCount = likeCount();
final int postAge = measurePostAge(now);
updatePopularity(likeCount, postAge);
}
// parameter introduced
private int measurePostAge(final Instant now) {
final int hours = Duration.between(createdAt, now).toHoursPart();
return 5 > hours ? 1 : hours / 5;
}
private void updatePopularity(final long likeCount, final int postAge) {
popularity = (likeCount + viewCount) / postAge;
}
4. add test cases
다음과 같이 테스트를 추가했다. 테스트를 만들기 전까지 IDE 기능만을 사용해 리팩터링했다. 테스트를 만들기 전까지는 가급적 코드를 수정하지 않는 것이 좋다. 리팩터링 중에 실수할 수도 있기 때문이다.
아직 완전히 마음에 들지는 않지만, 이 테스트 케이스들은 문서로서의 가치도 있다.
class PostTest {
private final long likeCount = 10L;
private final long viewCount = 100L;
private final LocalDateTime postCreatedAt = LocalDate.of(2024, 1, 1).atStartOfDay();
@Test
@DisplayName("좋아요 10, 조회수 100, 생성으로부터 9시간 59분 경과했을 때, 인기도는 10 + 100 = 110 이다.")
void updatePopularity_case_0() {
// given
final var now = postCreatedAt.plusHours(9).plusMinutes(59);
final var post = createPostWith(likeCount, viewCount, postCreatedAt);
// when
post.updatePopularity(toInstant(now));
// then
assertThat(post.popularity()).isEqualTo(110L);
}
@Test
@DisplayName("좋아요 10, 조회수 100, 생성으로부터 10시간 경과했을 때, 인기도는 (10 + 100) / (10(h) * 0.5) = 55 이다.")
void updatePopularity_case_1() {
// given
final var now = postCreatedAt.plusHours(10);
final var post = createPostWith(likeCount, viewCount, postCreatedAt);
// when
post.updatePopularity(toInstant(now));
// then
assertThat(post.popularity()).isEqualTo(55L);
}
private Post createPostWith(final long likeCount, final long viewCount, final LocalDateTime createdAt) {
return new Post(likeCount, viewCount, 0L, toInstant(createdAt));
}
private Instant toInstant(final LocalDateTime dateTime) {
return dateTime.atZone(ZoneId.systemDefault()).toInstant();
}
}
5. code simplification
마지막으로 코드를 단순화한다. likeCount를 메서드가 아닌 필드로 대체해도 되고, 아래처럼 메서드를 inline 해서 더 단순하게 만들 수 있다.
// class Post
void updatePopularity(final Instant now) {
final int postAge = measurePostAge(now);
popularity = (likeCount + viewCount) / postAge;
}
private int measurePostAge(final Instant now) {
final int hours = Duration.between(createdAt, now).toHoursPart();
return 5 > hours ? 1 : hours / 5;
}
마무리
간단한 예제를 통해 현재 시간에 의존하는 리팩터링 해봤다. 새 코드는 기존 코드에 비해 어떤 점이 더 좋은가?
시간을 명시적인 파라미터로 바꿔서 테스트 가능하게 변경했다.
이제 인기도를 계산하는 로직이 변경하기가 쉬워졌다.
Last modified: 19 May 2024