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주차 과제 - Spring 테스트 작성하기 #83

Merged
merged 27 commits into from
Aug 21, 2022
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
d3341a1
생성자 추가 , equals , hashCode 추가
jdalma Aug 15, 2022
77d34df
테스트 코드 추가
jdalma Aug 15, 2022
9fb83c8
Task equals메소드 수정
jdalma Aug 16, 2022
b357afb
out 폴더 무시
jdalma Aug 16, 2022
30e07c6
TaskController 계층형 테스트 코드 추가
jdalma Aug 16, 2022
45263d4
toString 메소드 추가
jdalma Aug 16, 2022
4caf49c
import 패키지 * 자동 제거
jdalma Aug 16, 2022
5c8484e
TaskControllerWebTest 계층 테스트 코드 작성
jdalma Aug 16, 2022
8aebc01
Update app/src/test/java/com/codesoom/assignment/controllers/TaskCont…
jdalma Aug 17, 2022
3503251
TaskControllerNestedTest 준비영역과 실행영역을 구분
jdalma Aug 17, 2022
3089b94
TaskControllerWebTest 테스트 코드 작성
jdalma Aug 17, 2022
9815368
Update app/src/test/java/com/codesoom/assignment/controllers/TaskCont…
jdalma Aug 18, 2022
7d25cc2
DEFAULT_SIZE 적용
jdalma Aug 18, 2022
7fd1735
Revert "DEFAULT_SIZE 적용"
jdalma Aug 18, 2022
8e03669
Merge remote-tracking branch 'origin/HEAD'
jdalma Aug 18, 2022
23c6e66
DEFAULT_SIZE 적용
jdalma Aug 18, 2022
d7cdfba
clearTasks 메소드 추가
jdalma Aug 18, 2022
9c7d9e6
TaskControllerNestedTest 주석 수정 및 준비 단계 확인
jdalma Aug 18, 2022
4a9d718
MockMVC 테스트 Context단계 수정
jdalma Aug 18, 2022
b86f1b5
clearTasks 메소드 id도 초기화하게 수정
jdalma Aug 18, 2022
8138d60
TaskServiceTest 테스트 코드 추가
jdalma Aug 18, 2022
f4ae790
Task 모델 유닛 테스트 추가
jdalma Aug 18, 2022
067bf9b
mock MVC 테스트 준비 단계 수정
jdalma Aug 19, 2022
a8f92e7
TaskController mock을 사용하지 않은 MVC 테스트 코드 추가
jdalma Aug 19, 2022
c12316e
Update app/src/test/java/com/codesoom/assignment/controllers/TaskCont…
jdalma Aug 21, 2022
ad0b279
Context를 Describe로 수정
jdalma Aug 21, 2022
28f14e5
verify 추가
jdalma Aug 21, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,4 @@ Thumbs.db
Thumbs.db:encryptable
ehthumbs.db
ehthumbs_vista.db
/app/out/
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,17 @@
import com.codesoom.assignment.application.TaskService;
import com.codesoom.assignment.models.Task;
import org.springframework.http.HttpStatus;
import org.springframework.web.bind.annotation.*;
import org.springframework.web.bind.annotation.CrossOrigin;
import org.springframework.web.bind.annotation.DeleteMapping;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PatchMapping;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.PutMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.ResponseStatus;
import org.springframework.web.bind.annotation.RestController;

import java.util.List;

Expand Down
34 changes: 34 additions & 0 deletions app/src/main/java/com/codesoom/assignment/models/Task.java
Original file line number Diff line number Diff line change
@@ -1,10 +1,20 @@
package com.codesoom.assignment.models;

import java.util.Objects;

public class Task {
private Long id;

private String title;

public Task() {
}

public Task(Long id, String title) {
this.id = id;
this.title = title;
}

public Long getId() {
return id;
}
Expand All @@ -20,4 +30,28 @@ public String getTitle() {
public void setTitle(String title) {
this.title = title;
}

@Override
public boolean equals(Object o) {
if (this == o)
return true;
if (o == null || this.getClass() != o.getClass())
return false;
Task task = (Task) o;
return Objects.equals(this.id, task.getId()) && this.title.equals(task.getTitle());
}
Comment on lines +35 to +42
Copy link
Collaborator

Choose a reason for hiding this comment

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

좋습니다! 다만 if 문은 한 줄짜리라도 꼭 중괄호를 생략하지 않는 습관을 들이는 것이 좋습니다. 비싸지 않으면서도 치명적인 실수를 방어해주는 좋은 습관이죠.

이와 관련된 유명하고 재미있는 버그 이야기가 있습니다. Apple의 SSL/TLS 프로토콜에서 발견된 끔찍한 실수입니다. if 중괄호가 습관화되어 있었다면 하지 않았을 실수죠.
https://embeddedgurus.com/barr-code/2014/03/apples-gotofail-ssl-security-bug-was-easily-preventable/

Copy link
Author

Choose a reason for hiding this comment

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

이런 일이 있었군요 ㅎㅎ 감사합니다 !!
수정하겠습니다


@Override
public int hashCode() {
return Objects.hash(id, title);
}

@Override
public String toString() {
final StringBuilder sb = new StringBuilder("Task{");
sb.append("id=").append(id);
sb.append(", title='").append(title).append('\'');
sb.append('}');
return sb.toString();
Comment on lines +51 to +55
Copy link
Contributor

Choose a reason for hiding this comment

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

String.format을 사용해서 표현하는 건 어떨까요? 다음과 같이 표현할 수 있을거에요

return String.format("{ id = %s, title = %s }", id, title);

더 좋은건 Template literal이지만 자바에서는 지원하질 않네요 ㅠ 만약 이런게 지원하는 코틀린을 사용했다면 다음과 같이 사용할 수 있을거에요

return "{ id = $id, title = $title }"

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,302 @@
package com.codesoom.assignment.controllers;

import com.codesoom.assignment.TaskNotFoundException;
import com.codesoom.assignment.application.TaskService;
import com.codesoom.assignment.models.Task;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;

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

class TaskControllerNestedTest {

private final String DEFAULT_TITLE = "TEST";
private final long DEFAULT_SIZE = 3;
private TaskController controller;

@BeforeEach
void setUp() {
// subject
controller = new TaskController(new TaskService());

// fixtures
}

void setDefaultSizeTasks(){
for(long i = 1 ; i <= DEFAULT_SIZE ; i++){
Task newTask = new Task();
newTask.setTitle(DEFAULT_TITLE + i);
controller.create(newTask);
}
}

@Nested
@DisplayName("list 메소드는")
class Describe_List{

@Nested
@DisplayName("Task가 존재한다면")
class Context_ExistsTask{

@BeforeEach
void setUp(){
setDefaultSizeTasks();
}

@Test
@DisplayName("모든 Task를 반환한다")
void It_ReturnAllTask(){
assertThat(controller.list()).hasSize((int) DEFAULT_SIZE);
for(long i = 1 ; i <= DEFAULT_SIZE ; i++){
assertThat(controller.detail(i)).isEqualTo(new Task(i , DEFAULT_TITLE + i));
}
}
}

@Nested
@DisplayName("Task가 존재하지 않는다면")
class Context_NotExistsTask{
private final int EMPTY = 0;

@Test
@DisplayName("비어있는 List를 반환한다")
void It_ReturnEmptyList(){
assertThat(controller.list()).hasSize(EMPTY);
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

우리는 지금 스코프가 있는 테스트 코드 스타일을 선택해서 작업을 하고 있어요. 이렇게 테스트별로 스코프가 있는 방식으로 작업할 때 열심히 고민해야 하는 주제는 바로 테스트 코드 스코프에서 논리적인 인과를 잘 마련해주는 것입니다.

여기에서 controller.list()를 호출하는 상황에 대해 생각해 보죠. 조금 올라가보면 @BeforeEach에서 new TaskController를 하고 있습니다. 그리고 이 테스트에서는 "자신 있게" "Task가 존재하지 않는다면"이라고 이야기하고 있죠. 이 자신감의 근거는 "TaskController를 new 했으니까 당연히 list 는 비어있을거야"라는 생각이라 할 수 있습니다. 하지만 이것은 일종의 상식에 근거한 비약입니다. 만약 컨트롤러를 새로 만들었을 때 디폴트 할 일이 리스트에 들어가있게 어느 동료가 수정한다면 이 테스트는 실패할 겁니다.

정말 논리적으로 튼튼한 테스트코드를 작성하려면 몇 가지 조치가 필요해요. 가장 쉬운 방법은 이 두 가지입니다.

  • 방법 1. "Task가 존재하지 않는다면"새로 생성한 컨트롤러를 통해 호출한다면으로 수정한다.
  • 방법 2. "Task가 존재하지 않는다면" 컨텍스트 스코프에서 모든 할 일을 삭제한다.

위의 두 방법이 각자 어떤 장점을 갖고 있는지에 대해 생각해 보세요.

Copy link
Author

@jdalma jdalma Aug 18, 2022

Choose a reason for hiding this comment

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

하나의 사건 (원인)이 다른 사건 (결과)을 일으킬 때 둘의 관계 출처

스코프가 있는 테스트 코드 스타일논리적인 인과 기억하겠습니다 ㅎㅎ


방법 1. "Task가 존재하지 않는다면"을 "새로 생성한 컨트롤러를 통해 호출한다면"으로 수정한다.

글로 명시하는게 읽는 사람에겐 더 정확하게 전달될 것 같습니다. 하지만 "새로 생성한 컨트롤러"라는 글은 테스트 코드에 뭔가 맞지 않는 느낌이네요. 글로 명시 하여도 @BeforeEach가 수정되면 해당 테스트 부분도 신경 써줘야 할 것 같습니다

방법 2. "Task가 존재하지 않는다면" 컨텍스트 스코프에서 모든 할 일을 삭제한다.

Context단계에서 테스트에 대한 준비를 더 철저하게 해주는게 더 좋을 것 같습니다

전체적으로 다시 확인 해보겠습니다!


}

@Nested
@DisplayName("detail 메소드는")
class Describe_Detail{

@Nested
@DisplayName("id가 null이라면")
class Context_NullPathVariable{
private final Long givenId = null;

@Test
@DisplayName("TaskNotFoundException를 던진다")
Copy link
Collaborator

Choose a reason for hiding this comment

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

잘 수정하셨습니다. 그런데 여기에서 한 가지 더 생각해볼만한 문제가 있습니다. 바로 TaskNotFoundException 클래스의 이름 변경입니다. 누가 Task 클래스의 이름을 Todo로 변경한다고 상상해 봅시다. IntelliJ 에서 rename 해서 클래스 이름을 변경하겠죠? 그리고 TaskTodo로 바뀌었으니까, 작업한 사람은 TaskNotFoundException 클래스가 있는 걸 보고 "이것도 바꿔야겠다"라고 생각하고 TodoNotFoundException으로 이름을 바꿀 거에요. 그리고 Task가 들어간 다른 클래스들도 몇 개 더 바꿨다고 상상해 보세요.

아마 그 분은 이 테스트의 @DisplayName의 문자열도 수정해야 한다는 걸 놓칠 수도 있을 겁니다. IntelliJ가 문자열까지 rename해주지는 않으니까요. 그래서 rename하는 사람이 이 설명을 수정하지 않고 넘어갈 수 있겠죠. 거짓된 설명이 남게 되는 것입니다.

Suggested change
@DisplayName("TaskNotFoundException를 던진다")
@DisplayName("찾지 못했다는 내용의 예외를 던진다")

하지만 위와 같이 설명을 작성해 두면 클래스 이름 변경에 취약했던 설명이 이름 변경에도 유연하게 대응하게 됩니다. 즉 클래스 이름에 테스트 설명이 강하게 의존하고 있었던 문제를 약하게 의존하게 만들어 해소한거죠. 잘 생각해보면 객체지향 원칙이 주석이나 설명에도 작동한다는 사실을 알 수 있습니다.

Copy link
Author

Choose a reason for hiding this comment

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

아 중요한 걸 놓치고 있었네요..
개발을 할 때만 유연하게가 아니라 말씀하신 주석이나 설명 , 문서 작성에도 의존에 대한 고민이 필요하네요
감사합니다 ㅎㅎ

void It_ThrowException(){
assertThatThrownBy(() -> controller.detail(givenId))
.isInstanceOf(TaskNotFoundException.class);
}
}

@Nested
@DisplayName("존재하지 않는 id로 조회한다면")
class Context_SearchInvalidId{
private final Long invalidId = 100L;
Copy link
Collaborator

Choose a reason for hiding this comment

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

이 아이디가 정말로 존재하지 않는다는 논리적인 근거를 이 스코프에서 마련해 보세요. @BeforeEach에서 100L에 해당하는 할 일을 지워버리는 것도 방법이죠.


@Test
@DisplayName("TaskNotFoundException을 던진다")
void It_ThrowException(){
assertThatThrownBy(() -> controller.detail(invalidId))
.isInstanceOf(TaskNotFoundException.class);
}
}

@Nested
@DisplayName("존재하는 id로 조회한다면")
class Context_SearchValidId{

@BeforeEach
void setUp(){
setDefaultSizeTasks();
}

@Test
@DisplayName("Task를 반환한다")
void It_ReturnTask(){
assertThat(controller.detail(DEFAULT_SIZE))
.isEqualTo(new Task(DEFAULT_SIZE , DEFAULT_TITLE + DEFAULT_SIZE));
}
}
}

@Nested
@DisplayName("create 메소드는")
class Describe_Create{

@Nested
@DisplayName("id가 null이 아닌 Task가 주어져도")
class Context_NotNullIdTask{
private final long beforeId = DEFAULT_SIZE + 1;
private final String title = DEFAULT_TITLE + beforeId;
private final Task beforeTask = new Task(beforeId , title);

@Test
@DisplayName("generate된 id가 세팅된다")
void It_GenerateId(){
Task afterTask = controller.create(beforeTask);
assertThat(beforeTask.getId()).isNotEqualTo(afterTask.getId());
assertThat(afterTask.getTitle()).isEqualTo(title);
}
}

@Nested
@DisplayName("title이 null이면")
class Context_NullTitle{
private final Task titleNullTask = new Task();
private final String title = null;

@Test
@DisplayName("Task의 제목은 null로 저장된다")
void It_SaveTitleNull(){
Task saveTask = controller.create(titleNullTask);
assertThat(saveTask.getTitle()).isEqualTo(title);
}
}

@Nested
@DisplayName("title이 존재하는 Task를 전달하면")
class Context_ExistsTitleTask{
private final String title = DEFAULT_TITLE + DEFAULT_SIZE;
private final Task beforeTask = new Task(null , title);

@Test
@DisplayName("저장된다")
void It_SaveTask(){
int oldSize = controller.list().size();
Task afterTask = controller.create(beforeTask);
assertThat(beforeTask.getTitle()).isEqualTo(afterTask.getTitle());
assertThat(controller.list()).hasSize(oldSize + 1);
}
}
}

@Nested
@DisplayName("update 메소드는")
class Describe_Update{

@Nested
@DisplayName("id가 null이거나 id에 해당하는 Task가 없다면")
class Context_IdIsNullAndInvalidId{
private final Long nullId = null;
private final Long invalidId = 100L;
private final Task updateTask = new Task();

@Test
@DisplayName("TaskNotFouneException이 발생한다")
void It_ThrowException(){
assertThatThrownBy(() -> controller.update(nullId , updateTask))
.isInstanceOf(TaskNotFoundException.class);
assertThatThrownBy(() -> controller.update(invalidId , updateTask))
.isInstanceOf(TaskNotFoundException.class);
}
}

@Nested
@DisplayName("id에 해당하는 Task가 있으면")
class Context_ValidId{

private final String title = DEFAULT_TITLE + (DEFAULT_SIZE + 1);
private final Task beforeTask = new Task(null , title);

@BeforeEach
void setUp(){
setDefaultSizeTasks();
}

@Test
@DisplayName("Task의 title을 업데이트한다")
void It_UpdateTitle(){
int oldSize = controller.list().size();
Task afterTask = controller.update(DEFAULT_SIZE , beforeTask);
assertThat(beforeTask.getTitle()).isEqualTo(afterTask.getTitle());
assertThat(controller.list()).hasSize(oldSize);
}
}
}

@Nested
@DisplayName("patch 메소드는")
class Describe_Patch{

@Nested
@DisplayName("id가 null이거나 id에 해당하는 Task가 없다면")
class Context_IdIsNullAndInvalidId{

private final Long nullId = null;
private final Long invalidId = 100L;
private final Task updateTask = new Task();

@Test
@DisplayName("TaskNotFouneException이 발생한다")
void It_ThrowException(){
assertThatThrownBy(() -> controller.patch(nullId , updateTask))
.isInstanceOf(TaskNotFoundException.class);
assertThatThrownBy(() -> controller.patch(invalidId , updateTask))
.isInstanceOf(TaskNotFoundException.class);
}
}

@Nested
@DisplayName("id에 해당하는 Task가 있으면")
class Context_ValidId{

private final String title = DEFAULT_TITLE + (DEFAULT_SIZE + 1);
private final Task beforeTask = new Task(null , title);

@BeforeEach
void setUp(){
setDefaultSizeTasks();
}

@Test
@DisplayName("Task의 title을 업데이트한다")
void It_UpdateTitle(){
int oldSize = controller.list().size();
Task afterTask = controller.patch(DEFAULT_SIZE , beforeTask);
assertThat(beforeTask.getTitle()).isEqualTo(afterTask.getTitle());
assertThat(controller.list()).hasSize(oldSize);
}
}
}

@Nested
@DisplayName("delete 메소드는")
class Describe_Delete{

@Nested
@DisplayName("id가 null이거나 id에 해당하는 Task가 없다면")
class Context_IdIsNullAndInvalidId{

private final Long nullId = null;
private final Long invalidId = 100L;

@Test
@DisplayName("TaskNotFouneException이 발생한다")
void It_ThrowException(){
assertThatThrownBy(() -> controller.delete(nullId))
.isInstanceOf(TaskNotFoundException.class);
assertThatThrownBy(() -> controller.delete(invalidId))
.isInstanceOf(TaskNotFoundException.class);
}
}

@Nested
@DisplayName("id에 해당하는 Task가 있다면")
class Context_ValidId{

private final Long validId = DEFAULT_SIZE;

@BeforeEach
void setUp(){
setDefaultSizeTasks();
}

@Test
@DisplayName("해당 Task를 삭제한다")
void It_DeleteTask(){
int beforeSize = controller.list().size();
controller.delete(validId);
assertThat(controller.list()).hasSize(beforeSize - 1);
}
}
}
}