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주차 테스트 작성하기 - pangnem #36

Merged
merged 41 commits into from
Aug 30, 2021
Merged

Conversation

jxmen
Copy link

@jxmen jxmen commented Aug 23, 2021

3주차 과제 진행합니다 ~

@jxmen jxmen changed the base branch from main to pangnem August 23, 2021 21:31
Comment on lines 11 to 14
public Task(Long id, String title) {
this.id = id;
this.title = title;
}
Copy link
Author

@jxmen jxmen Aug 24, 2021

Choose a reason for hiding this comment

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

현재는 이 부분을 테스트 코드에서만 사용하고 있습니다.

@jxmen
Copy link
Author

jxmen commented Aug 24, 2021

@johngrib 님께서 계층형 구조로 테스트 코드 작성하기 라는 글을 써주셨네요. 이 방법을 도입해 보아도 괜찮을까요?

@hannut91
Copy link
Contributor

네 ㅎㅎ 좋습니다!

Copy link
Contributor

@hannut91 hannut91 left a comment

Choose a reason for hiding this comment

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

커밋을 작게 나눈것도, 테스트 작성하는 것도 아주 잘해주셨습니다. ㅎㅎ 종립님이 말씀해주신 것도 시도해보면 좋을 것 같네요

}

@Test
@DisplayName("GET /tasks는 모든 할 일 목록을 반환한다")
Copy link
Contributor

Choose a reason for hiding this comment

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

API응답이니 응답한다로 하는 것은 어떨까요?

}

@Test
@DisplayName("GET /tasks/{id}는 존재하는 id일 경우 200을 반환한다")
Copy link
Contributor

Choose a reason for hiding this comment

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

존재하는 할 일 이라고 작성하는 것은 어떨까요?

Copy link
Author

Choose a reason for hiding this comment

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

좋습니다!

Copy link
Contributor

@hannut91 hannut91 left a comment

Choose a reason for hiding this comment

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

잘해주셔서 딱히 코멘트 남길 것은 없어보이네요.

Given 즉 어떤 것이 달라지는지 테스트에서 더 드러나도록 작성해보시면 좋을 것 같습니다

Comment on lines 84 to 87
Long id = TaskList.SECOND.getId();

given(taskService.getTask(id))
.willThrow(new TaskNotFoundException(id));
Copy link
Contributor

Choose a reason for hiding this comment

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

Given에 해당하는 것을 따로 분리하면 더 읽기 좋을거에요. 각 테스트에서 무엇이 달라지는지 더 명확하게 표현할 수 있습니다.

@BeforeEach
void setUp() {
    Long id = TaskList.SECOND.getId();

     given(taskService.getTask(id)).willThrow(new TaskNotFoundException(id));
}

@Test
@DisplayName("404를 응답한다")
void it_response_404() throws Exception {
    // ...
}

@jxmen jxmen changed the title 3주차 테스트 작성하기 3주차 테스트 작성하기 - pangnem Aug 26, 2021
@jxmen
Copy link
Author

jxmen commented Aug 26, 2021

계층형 형태로 테스트를 작성해 본건 처음이라서, 아직 계층형 구조로 테스트 작성은 Describe-Context-It 형태로 나누기 쉬운 것들 먼저 진행하였습니다.

Copy link
Contributor

@hannut91 hannut91 left a comment

Choose a reason for hiding this comment

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

Describe-Context-It을 정확히 이해하신 것 같습니다

}

@Nested
@DisplayName("존재하는 할 일 일경우")
Copy link
Contributor

Choose a reason for hiding this comment

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

실패하는 경우의 테스트보다, 성공하는 경우의 테스트가 먼저 오는게 좋을 것 같아요.

이 함수는 ~ 하다. 그런데 ~할 떄는 ~하다. 같은 느낌이라서요

Comment on lines 169 to 177
mockMvc.perform(put("/tasks/2")
.contentType(MediaType.APPLICATION_JSON)
.content(task.stringify()))
.andExpect(status().isNotFound());

mockMvc.perform(patch("/tasks/2")
.contentType(MediaType.APPLICATION_JSON)
.content(task.stringify()))
.andExpect(status().isNotFound());
Copy link
Contributor

Choose a reason for hiding this comment

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

중복인줄 알았는데, 멱등성을 검사하셨군요. 모킹한 서비스를 사용해서 무조건 성공할 것 같긴하네요.

Copy link
Author

Choose a reason for hiding this comment

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

😵 도움이 필요합니다 😵

멱등성을 검사하였다라는 말씀이 정확히 잘 이해되지 않습니다. 연산을 여러 번 적용하더라도 결과가 달라지지 않는 것으로 이해했는데, 이게 맞는지 잘 모르겠습니다.

그리고 현재 PUTPATCH 메서드가 같이 있는 이유는, 서로 분리하였을 때 한쪽에서 이미 mock으로 에러를 발생시키도록 정의되어 있는데, 이를 다시 mock으로 정의하려고 하니 호출한 곳에서 바로 에러가 발생하였습니다.

Long id = TaskList.FIRST.getId();

Long expectId = TaskList.SECOND.getId();
Task task = TaskList.SECOND.toTask();

// PUT에서 mock을 생성했는데..
given(taskService.updateTask(id, task))
    .willThrow(new TaskNotFoundException(expectId));

// PATCH에서 생성 시
given(taskService.updateTask(id, task)) // => 또 호출 시 여기서 바로 에러 발생
    .willThrow(new TaskNotFoundException(expectId));

지금 이 상태도 두어도 괜찮을까요? 혹은 더 좋은 방법이 있을까요?

Copy link
Contributor

Choose a reason for hiding this comment

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

제가 put 과 patch를 둘 다 테스트하신지 몰랐습니다 ㅎㅎ 멱등성 얘기는 잊어주셔도 좋을 것 같습니다.

저도 예전에 mock을 또 할 경우 에러가 발생하더라구요. 지금은 냅둬도 괜찮을 것 같습니다

class Context_existTask {

@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.

수정된 할 일을 반환한다.는 어떨까요?

테스트의 의도가 더 잘 드러나도록 변경
- 할 일 발급 메서드 구현
- TaskService에서 id 발급 관련 로직을 삭제
- 그에 따라 테스트 코드 변경
@jxmen
Copy link
Author

jxmen commented Aug 27, 2021

image
커버리지를 살펴보니, equals()hashcode(), 그리고 App 클래스의 main 메서드 안 SpringApplication.run() 은 어떻게 테스트 하면 좋을지 궁금해졌네요.

@hannut91
Copy link
Contributor

equalshashcode run은 테스트를 작성해봤자 얻는게 별로 없을 것 같아요. 커버리지는 내가 빠트린 테스트가 있나 확인할 수 있는 좋은 도구로, 과제에 커버리지 목표를 넣은 이유는 커버리지를 보는 습관을 들였으면 해서 넣었습니다.

꼭 테스트하지 않아도 되는 것은 예외로 처리하는게 좋을 것 같아요

Copy link
Contributor

@hannut91 hannut91 left a comment

Choose a reason for hiding this comment

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

고생 많으셨습니다. Given When Then도 금방 적용하셨네요 ㅎㅎ 아샬님의 과제풀이를 통해서 자신의 것과 비교해보면서 복습해보세요

import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;

public class TaskTest {
Copy link
Contributor

Choose a reason for hiding this comment

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

getter setter자체는 테스트를 해봤자 얻는게 별로 없을 것 같아요. 이 객체를 어떻게 사용하는건지만 보여주면 될 것 같습니다.

task = new Task(1L, "Title");

assertThat(task.getId()).isEqualTo(1L);
assertThat(task.getTitle()).isEqualTo("Title");

task.setId(1000L);
task.setTitle("Other title");

assertThat(task.getId()).isEqualTo(1000L);
assertThat(task.getTitle()).isEqualTo("Other title");

@jxmen
Copy link
Author

jxmen commented Aug 29, 2021

TaskController 테스트 같은 경우 TaskService의 특정 메서드가 잘 실행되었는지만 테스트 하면 되는군요!
책임을 떠넘긴다는 내용이 테스트 코드에도 적용이 되네요

@hannut91
Copy link
Contributor

테스트를 작성하기 전에 무엇을 테스트할지 먼저 고민한다면, 명세에 대해서 더 명확해지겠죠 ㅎㅎ 아주 잘해주셨습니다.

이번 한 주도 고생많으셨습니다

@hannut91 hannut91 merged commit 63f05db into CodeSoom:pangnem Aug 30, 2021
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.

None yet

2 participants