Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

3주차 테스트 작성하기 #100

Open
wants to merge 10 commits into
base: tmxhsk99
Choose a base branch
from

Conversation

tmxhsk99
Copy link

안녕하세요
테스트를 다 작성하지 못했지만
일단 올리고 틈틈히 작성하겠습니다
감사합니다.

Comment on lines 21 to 28
@BeforeEach
void setUp() {
for (int i = 0; i < 10; i++) {
Task task = new Task();
task.setTitle("task" + i);
taskService.createTask(task);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

할 일의 10개를 만들어 놓고 테스트를 작성해 주셨네요. 아마도 좀 더 진짜 같은 테트스를 작성하고 싶으셨을 것 같아요. 할 일이 1개이든 10개이든 같은 부류의 입력값이기 때문에 10개를 추가한다고 해서 테스트의 품질이 좋아지지는 않아요.

테스트를 작성할 때는 입력이나 출력이 어떤 부류에 속하는지 생각해 보면 좋습니다. getTasks() 가 반환할 수 있는 부류가 빈 배열, 하나 이상의 배열을 생각해 볼 수 있는데요. 하나 이상의 배열 안에 10개의 배열이 포함되어 있기 때문에 10개 까지는 테스트해보지 않아도 좋을 것 같아요.

참고

  • 이펙티브 소프트웨어 테스팅 - Chapter 2 명세 기반 테스트



@Test
@DisplayName("할일 상세 조회 성공 테스트")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

만약에 이 테스트가 실패해서 이런 메시지가 나왔다고 가정해 보겠습니다.

X 할일 상세 조회 성공 테스트

위에 테스트는 실패했다라는 피드백을 받을 수 있습니다. 하지만 원래 기대했던 바가 무엇이었는지 모르기 때문에 테스트 코드를 보고 원래 기대했던 바와 지금 현재 상황이 왜 틀린지 살펴본 후 문제를 수정해야 합니다.

테스트를 명확하게 잘 짜는 것도 중요하지만 테스트가 실패했을 때 빠르게 알아차릴 수 있는 것도 중요합니다. 따라서 이 테스트는 무엇을 테스트하고 있고, 어떠한 경우에 무엇을 기대하는지 작성해주는게 좋을 것 같습니다.

이럴 때 좋은 프레임워크가 있는데요. 바로 Given When Then입니다.

~것이 주어졌을 때(Given) ~하면(When) ~그래서 어떤 상태가 된다. 혹은 결과가 나온다.(Then)

taskService = new TaskService();
}
@Test
@DisplayName("최초 할일 리스트 요청시 빈 리스트 반환 테스트")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getTasks는 할 일이 비어있으면 빈 리스트를 반환한다.

요렇게 쓰는 것은 어떨까요? 테스트 대상과 주어진 상황 그래서 기대하는 결과를 적어보았어요

@Test
@DisplayName("최초 할일 리스트 요청시 빈 리스트 반환 테스트")
void getFirstTasksIsEmpty() {
Assertions.assertThat(taskService.getTasks()).hasSize(0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이렇게 임포트해서 assertThat만 사용할 수 있겠네요

import static org.assertj.core.api.Assertions.assertThat;

assertThat(taskService.getTasks()).hasSize(0);

Comment on lines 110 to 123
@Test
@DisplayName("할일을 삭제 후 삭제 되었는지 확인하는 테스트")
void deleteTaskSuccess() {
// given
Task task = new Task();
task.setTitle("task0");
taskService.createTask(task);

// when
taskService.deleteTask(1L);

// then
Assertions.assertThat(taskService.getTasks()).hasSize(0);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

좋은 테스트를 작성하고 싶을 때 생각해보면 좋은 것이 하나가 있는데, 바로 어떤 변화가 일어나는지에 대해서 집중해서 테스트를 작성하는 것입니다. 그러면 테스트 대상을 사용함으로써 우리 애플리케이션에 어떤 변화가 일어나는지, 왜 테스트 대상을 사용해야 하는지를 더 드러낼 수 있습니다.

    @Test
    @DisplayName("할일을 삭제 후 삭제 되었는지 확인하는 테스트")
    void deleteTaskSuccess() {
        // given        
        Task task = new Task();
        task.setTitle("task0");
        taskService.createTask(task);

        int oldSize = taskService.getTasks();

        // when
        taskService.deleteTask(1L);
        
        int newSize = taskService.getTasks();

        // then
        assertThat(newSize - oldSize).isEqualTo(-1);
    }

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

뭔가 하면서도 약간 방향성 없이 했는데
확실히 이해가 가는것 같숩나더
변화하는 것을 집중해 테스트하는것 생각하면서 테스트를 작성하니까
작성하기 쉬워지네요!

@tmxhsk99
Copy link
Author

안녕하세요 질문이 있습니다!
mock을 사용해서 테스트사용해서 테스트를 작성하다보니
개인의 선호도에 따라 mock 사용을 선호하시지 않는 분 계시고
mock 을 선호하시는 분도 계시고
책(구글 소프트웨어 엔지니어링)에서는 상황에따라 mock을 어느정도 적용할지는
그때그때마다 다르다고 합니다
보통 어느 정도 수준으로 mock을 사용하시나요
외부 API를 사용하는경우는 mock을 써야 한다던가
이 정도는 그냥 실제 인터페이스를 구현해서 사용한다던가
항상 장단점이 존재해서 약간 그부분이 고민인것 같습니다

@hannut91
Copy link
Contributor

모킹에 대해서 어느정도 써야하는지, 언제 쓰는게 적절한지 고민이 되셨군요.

테스트 주도 개발로 배우는 객체 지향 설계와 실천책을 보면 객체를 발견해내는 과정에서 모킹을 사용합니다.
어떤 기능을 만들기 위해서 테스트를 정의하는데, 이 기능을 할 객체가 없다. 그러면 그러한 객체가 있다는 것을 가정하고
모킹을 사용해서 테스트를 완성합니다. 이 테스트를 작성할 때 객체의 책임과 역할에 대해서 고민하고 정의하고 있는거죠.
이런 경우에 모킹이 없다면 기능을 구현하다가 다른 객체를 구현하게 되고 그러면 하나의 기능에 집중하기 어렵습니다.
그래서 이런 경우에 모킹이 유용하고 복잡성을 줄여줄 수 있어서 사용하는 편입니다.

그런데 "구글 엔지니어는 이렇게 일한다"라는 책에서도 모킹은 결국에 우리가 원하는 것을 찾아내지 못할 가능성이 높기 때문에
사용을 지양하고 실제처럼 테스트를 작성하라고 하는데요.
모든 테스트에 모킹을 사용하면 이 기능이 무얼 하는건지 알기 어렵게 만들고, 테스트가 통과하더라도 기능이 올바르게 동작하지 않는 경우가
발생할 수 있어요.

그래서 저는 MockMvc로 작성하는 컨트롤러 부분은 서비스들을 모킹해서 작성하는 편이고, 비즈니스 레이어는 모킹하는 코드와 통합테스트를 둘다 작성할 때가 많습니다.

See also

import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.BDDMockito.given;

@SpringBootTest
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SpringBootTest는 스프링 부트 애플리케이션의 모든 빈을 로드하는 매우 포괄적인 테스트를 위한 어노테이션인데, 이로 인해 테스트 실행 시간이 길어질 수 있습니다. 또한, 모든 빈을 로드하는 것은 불필요한 의존성 문제를 일으킬 수 있습니다.

만약 Mock을 이용해서 테스트를 진행하면 빈을 로드하지 않아도 될 수도 있고 혹은 통합 테스트를 작성해서 레이어 간의 동작하는 것을 테스트하고 싶으시다면 @DataJpaTest를 사용할 수 있어요.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

최초에 빈으로 로드하는 방식으로 했다가
그냥 테스트마다 Service를 생성하는 방식으로 변경했는데
지우는걸 잊었던 것 같습니다
제거하겠습니다.

Comment on lines +83 to +84
Task task = new Task();
task.setTitle("task0");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

할 일을 만드는 것이 반복되고 있네요. 테스트를 위해서 필요한 데이터를 미리 만들어놓고 사용하는 것은 어떨까요?
테스트를 위한 데이터 모음을 fixture라고 하는데, 믿고 사용할 수 있는 데이터를 미리 만들어 놓거나 쉽게 만들 수 있는 것을 만들어놓고 테스트에서는 테스트를 위한 코드들만 보이도록해서 의도를 더 명확하게 할 수 있을거에요

Copy link
Author

@tmxhsk99 tmxhsk99 Jul 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요
질문이 있습니다, 그런 할일 만드는 중복을 없애면서 테스트코드를 작성하다가
구글엔지니어는 이렇게 일한다 책에 12.4 테스트공유: DRY가 아니라 DAMP! 단락을 보고
서술적으로 테스트를 작성하는 것에 대해 혹시 어떻게 생각하시나요?
요정도 간단한 비즈니스로직에서는 사실 그렇게 차이는 없을 것 같지만...
혹시 제가 fixture를 잘못 이해하고 있는것인지 헷갈리기도 합니다...(fixture DRY관점이랑 상관이 없다던가...)
아니면 이정도는 간단하니 fixture를 써도 상관이 없다던가...뭔가 상반되는 내용같아서 헷갈립니다...

Copy link
Contributor

@hannut91 hannut91 Jul 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"책에서는 중복을 무조건 제거하는 것이 좋지 않을 수 있다." 라고 이야기하면서 단순하고 명료하게만 만들어준다면 테스트에서의 다소의 중복은 괜찮다고 이야기하고 있네요.

이부분에 대해서는 저도 적극 동의합니다. 효율적으로 작성된 테스트보다는 이해하기 쉬운 테스트여야 하죠.

책에도 예를들긴 했지만 이런 경우를 생각해볼 수 있을 것 같습니다.

@Nested
@DisplayName("findProduct")
class Describe_findProduct {

	@Nested
	@DisplayName("할일이 존재하면")
	class Context_when_product_exists {
		@Test
		@DisplayName("할 일을 반환합니다.")
		void it_returns_empty_list() {
			var product = productFinder.find(ID);

			assertThat(product.getId()).isEqualTo(ID);
		}
	}

	@Nested
	@DisplayName("할일을 찾을 수 없으면")
	class Context_when_product_does_not_exist {
		@Test
		@DisplayName("할 일을 찾을 수 없다는 예외를 던집니다.")
		void it_returns_empty_list() {
			assertThatThrownBy(() -> productFinder.find(9999L)) .isInstanceOf(ExpectedException.class)
		}
	}
}

하지만 그냥 할 일을 찾을 수 없다고 에러를 던지는 것보다 언제 저걸 던지는지 보여주고 싶다면, 혹은 할 일이 존재하는 경우를 보여주고 싶다면

@Nested
@DisplayName("findProduct")
class Describe_findProduct {

	@Nested
	@DisplayName("할일이 존재하면")
	class Context_when_product_exists {
		@BeforeEach
		void setUp() {
			productCreator.create(SOME_PRODUCT);
		}

		@Test
		@DisplayName("할 일을 반환합니다.")
		void it_returns_empty_list() {
			var product = productFinder.find(SOME_PRODUCT.getId());

			assertThat(product.getId()).isEqualTo(SOME_PRODUCT.getId());
		}
	}

	@Nested
	@DisplayName("할일을 찾을 수 없으면")
	class Context_when_product_does_not_exist {
		@BeforeEach
		void setUp() {
			productRemove.remove(SOME_PRODUCT.getId());
		}

		@Test
		@DisplayName("할 일을 찾을 수 없다는 예외를 던집니다.")
		void it_returns_empty_list() {
			assertThatThrownBy(() -> productFinder.find(SOME_PRODUCT.getId())) .isInstanceOf(ExpectedException.class)
		}
	}
}

이런식으로 이전에 어떤 상황이었는지를 명시적으로 드러낼 수 있습니다. 여기서 SOME_PRODUCT같이 테스트에서는 별로 중요하지 않은 데이터를 미리 만들어 놓고 사용하는 것을 fixture라고 합니다.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

말씀해 주신 대로 하면 어떤 행위를 했는지도 같은 블록에 있어서
이 테스트를 보는 다른 사람에게 알아볼 수 있을 것 같네요
fixture가 before에 전부 섞여 있는 게 불편해서 (코드를 왔다 갔다 해야 되고 뭐가 이 테스트에 쓰이는지 알기 힘든 )
뭔가 답답한 감이 있었는데 해결된 것 같습니다.
BDD 형식으로 fixture도 사용하면서 하면 괜찮을 것 같습니다
감사합니다

Task task = createTask();
int beforeSize = taskController.list().size();
// when
Task insertTask = taskController.create(task);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

insertTask라는 이름은 할 일을 insert하는 메서드 처럼 보이네요. insertedTask라는 이름을 사용할 수 있을 것 같아요.

그런데 여기서 이변수가 사용되고 있진 않네요.

@hannut91
Copy link
Contributor

커밋 메시지를 작성할 때 @가 들어갈 때는 이렇게 백틱(`)으로 감싸주는게 좋아요. 그렇지 않으면 다음과 같이 의도하지 않게 다른 사람을 언급할 수가 있어요.

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants