테스트 코드를 작성하며 설계의 문제를 발견한 이야기 - 무심코 작성한 코드의 숨은 위험을 찾아내다

개발을 하면서 우리는 흔히 예상 가능한 예외 상황만을 고려하고, "이 정도면 잘 동작하겠지"라고 넘어가는 경우가 많다.

하지만 이런 무심코 작성한 코드가 실서비스에서 치명적인 버그를 초래할 수 있다.

 

이번 글에서는 테스트 코드를 작성하면서 발견한 설계적 문제와, 이를 어떻게 개선했는지를 공유하고자 한다.

문제의 코드

처음 작성한 코드는 다음과 같았다. ProductGroupContextCommandRequestDto의 각 필드를 읽어와 적절한 DomainMapper를 찾아 매핑하는 역할을 한다.

 

public ProductGroupContextCommand createCommand(Long productGroupId, ProductGroupContextCommandRequestDto dto) {
    ProductGroupContextCommandBuilder builder = createBuilder();
    if (productGroupId != null) {
        builder.withProductGroupId(productGroupId);
    }

    for (Field field : getDeclaredFields(dto.getClass())) {
        field.setAccessible(true);
        try {
            Object fieldValue = field.get(dto);
            if (fieldValue != null) {
                DomainMapper<Object> mapper = findMapperOrThrow(fieldValue);
                mapper.map(fieldValue, builder);
            }
        } catch (IllegalAccessException e) {
            throw new CoreException(ErrorType.UNEXPECTED_ERROR, e);
        }
    }
    return builder.build();
}

 

 

처음에는 큰 문제가 없어 보였다. 하지만 테스트 코드를 작성하면서 여러 엣지 케이스에 대해 생각하니 치명적인 문제를 발견했다.

 

 

테스트 코드가 드러낸 문제점

(1) dto의 필드 값이 null이면 매핑이 누락된다

위 코드에서 fieldValue가 null이면 DomainMapper를 찾지 않고 넘어가도록 되어 있다. 즉, 필드가 null이면 아무런 조치 없이 건너뛰기 때문에 완전하지 않은 객체가 반환될 가능성이 높다.

(2) dto에 필드가 없는 경우에도 동작이 진행된다

만약 ProductGroupContextCommandRequestDto가 아무 필드도 갖고 있지 않다면, builder.build()가 호출되면서 의미 없는 객체가 반환될 가능성이 크다. 하지만 이 경우 아무런 예외가 발생하지 않기 때문에 디버깅이 어렵다.

(3) IllegalAccessException이 발생할 경우 디버깅이 어렵다

테스트를 진행하면서 IllegalAccessException이 발생했을 때, 어떤 필드에서 문제가 발생했는지 알기 어려웠다. 기존 예외 메시지에는 필드 이름이 포함되지 않았기 때문에, 디버깅이 힘들었다.

테스트를 통해 문제를 해결하다

위 문제들을 해결하기 위해 다음과 같이 코드를 개선했다.

public ProductGroupContextCommand createCommand(Long productGroupId, ProductGroupContextCommandRequestDto dto) {
		ProductGroupContextCommandBuilder builder = createBuilder();

		if (productGroupId != null) {
			builder.withProductGroupId(productGroupId);
		}

		Field[] declaredFields = getDeclaredFields(dto.getClass());

		if(declaredFields.length == 0) {
			throw new CoreException(ErrorType.UNEXPECTED_ERROR, "ProductGroupContextCommandRequestDto class declaredFields cannot be empty");
		}
		Arrays.stream(declaredFields)
			.forEach(field -> {
				field.setAccessible(true);
				try {
					Object fieldValue = field.get(dto);

					if (fieldValue == null) {
						throw new CoreException(ErrorType.INVALID_INPUT_ERROR, "Field " + field.getName() + " cannot be null.");
					}

					DomainMapper<Object> mapper = findMapperOrThrow(fieldValue);
					mapper.map(fieldValue, builder);

				} catch (IllegalAccessException e) {
					throw new CoreException(ErrorType.UNEXPECTED_ERROR, "Access to field " + field.getName() + " failed", e);
				} catch (NullPointerException e) {
					throw new CoreException(ErrorType.INVALID_INPUT_ERROR, "Field " + field.getName() + " is null", e);
				}
			});

		return builder.build();
	}

 

수정된 코드의 주요 개선점

- dto의 필드가 하나도 없는 경우 예외를 발생시킴 (declaredFields.length == 0 체크).
- fieldValue가 null이면 예외를 발생시켜 불완전한 객체가 반환되지 않도록 변경.
- IllegalAccessException 발생 시 어떤 필드에서 문제가 발생했는지 명확히 알 수 있도록 예외 메시지를 보완.
- NullPointerException을 별도로 처리하여 예외 발생 원인을 명확하게 표시.

결론: 테스트 코드가 없었다면?

만약 테스트 코드를 작성하지 않았다면, 우리는 이 문제를 쉽게 발견하지 못했을 것이다. 특히 null 값이 들어왔을 때 객체가 불완전한 상태로 반환되는 문제는, 실제 서비스에서 예상치 못한 버그로 이어졌을 가능성이 크다.

하지만 테스트 코드를 작성함으로써, 다음과 같은 교훈을 얻을 수 있었다.

무심코 작성한 코드가 안전한지 검증하는 과정이 필요하다.

예외 상황을 명확하게 정의하지 않으면 디버깅이 어려워진다.

테스트 코드가 없었다면 실서비스에서 치명적인 버그를 유발할 수도 있었다.

 

앞으로도 테스트 코드를 적극적으로 활용하여 무심코 작성한 코드가 만드는 위험을 사전에 방지하는 습관을 기르는 것이 중요하다고 생각한다.