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

[HOTFIX-134] 이메일 발송 E2E 테스트 작성 #45

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

cousim46
Copy link
Member

E2E Test - 이메일 발송

@cousim46 cousim46 added the Status : peer 리뷰요청 New feature or request label Apr 14, 2022
@cousim46 cousim46 requested review from loadkrnis and syb1231 April 14, 2022 17:20
@cousim46 cousim46 self-assigned this Apr 14, 2022
Copy link
Collaborator

@loadkrnis loadkrnis left a comment

Choose a reason for hiding this comment

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

고생하셨습니다!
리뷰 반영 후 다시 리뷰 요청주시면 감사하겠습니다 🙏

앗 그리고 왜 브랜치 이름이 HOTFIX-134 일까용?
이건 급한 버그수정이 아니니까 HOTFIX는 아닌 것 같아요!
이번 PR만 hotfix로 하시고 다음부터는 새로운 티켓 넘버로 올려주세용

@@ -16,7 +16,7 @@ spring:
# 데이터베이스 로그인 비밀번호
password: lossleaderBack12!
# 데이터베이스 url
url: jdbc:mysql://218.55.129.211:10000/lossleader
url: jdbc:mysql://127.0.0.1:3306/lossleader
Copy link
Collaborator

Choose a reason for hiding this comment

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

요거 일부러 바꾸신걸까요?

Copy link
Member Author

Choose a reason for hiding this comment

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

다시 재변경했습니다!

@@ -1,165 +0,0 @@
//package lossleaderproject.back.user.service;
Copy link
Collaborator

Choose a reason for hiding this comment

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

이건 기존에 존재하던 테스트같은데 왜 삭제하셨을까용?

Copy link
Member Author

Choose a reason for hiding this comment

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

e2e 테스트가 아니여서 e2e 테스트부터 차근차근 할려는 생각으로 기존에 있던 테스트들을 삭제하였습니다.

.exchange().expectBody(String.class).isEqualTo("메일 발송성공");
}

}
Copy link
Collaborator

Choose a reason for hiding this comment

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

이거 intellij 에서 eof 설정이 안되어있으신 것 같아요!
이 글 참조하셔서 설정 권장드립니다. :)

Comment on lines 24 to 48
@Test
@DisplayName("메일 발송")
public void 메일발송() throws Exception {
webTestClient.post().uri("/lossleader-user/email").contentType(MediaType.APPLICATION_JSON_UTF8)
.syncBody(new SendEmail("[email protected]"))
.exchange().expectBody(String.class).isEqualTo("메일 발송성공");
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

E2E 테스트도 service 테스트처럼
아래와 같은 방법으로 given when then 구성으로 작성해주세요!

@Test
    @DisplayName("가입되어 있는 유저의 이메일 주소를 받았을 때 메일 발송을 성공한다.")
    public void 메일발송() throws Exception {
        // given
        String targetEmail = "[email protected]";

        // when
        ResponseEntity result = webTestClient.post()
                .uri("/lossleader-user/email")
                .bodyValue(new SendEmail("[email protected]"))
                .retrieve()
                .bodyToMono(ResponseEntity.class);

        // then
        Assertions.assertThat(result.getStatusCode()).isEqualTo(HttpStatus.OK); // 예시
        Assertions.assertThat(result.getBody()).isEqualTo("메일 발송성공");
    }

위처럼 작성할 수 있는건 이 포스팅 참고해서 작성 해주세요! 그리고 이 엔드포인트에 테스트 케이스가 조금 부족한 것 같아요!
제가 생각했을 때는 아래와 같은 케이스들이 존재할 것 같습니다.
아래 번호마다 DisplayName이라고 생각해주시면 좋을 것 같아요.

  1. 가입되어 있는 유저의 이메일 주소를 받았을 때 메일 발송을 성공한다.
  2. 미가입 유저의 이메일 주소를 받았을 때 ~~를 응답한다.
  3. 이메일 주소가 유효하지 않으면 ~~를 응답한다.

혹시 더 있으시면 작성해주시면 좋을 것 같아요!

@DisplayName("메일 발송")
public void 메일발송() throws Exception {
webTestClient.post().uri("/lossleader-user/email").contentType(MediaType.APPLICATION_JSON_UTF8)
.syncBody(new SendEmail("[email protected]"))
Copy link
Collaborator

Choose a reason for hiding this comment

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

SendEmail 이라는 class는 어떤 역할을 하는 class인지 잘 확인이 어려운 것 같아요!
해당 class는 서버에서 request body로 받는 값이니까 SendEmailRequest 로 바꾸면 좋을 것 같은데 어떻게 생각하세용?

Copy link
Member Author

Choose a reason for hiding this comment

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

네 좋은거 같아서 수정하겠습니다!

@loadkrnis
Copy link
Collaborator

loadkrnis commented Apr 15, 2022

그리고 현재 이 PR [HOTFIX-134] 이메일 발송 E2E 테스트 작성 으로 바꾸시는건 어떨까용?
PR 제목만봐도 티켓이름을 알 수 있고. 여러모로 유리할 것 같아요.

그리고 commit message는 Prefix를 붙여주세요. 실제 메세지 내용은 작업에 대한 간략한 설명이 있으면 좋습니다.
기존. Add : E2E Test - 이메일 발송
이후. [HOTFIX-134] 이메일 발송 E2E 테스트 작성
위 방식대로 했을 경우 코드 롤백할 때 유리한 점이 많습니당.

@syb1231 syb1231 requested review from syb1231 and removed request for syb1231 April 18, 2022 14:07
@syb1231 syb1231 added Status: Conflict 해결중 Status: Conflict 해결중 and removed Status: Conflict 해결중 Status: Conflict 해결중 labels Apr 18, 2022
@cousim46 cousim46 changed the title Add : E2E Test - 이메일 발송 [HOTFIX-134] 이메일 발송 E2E 테스트 작성 Apr 20, 2022
@github-actions
Copy link

github-actions bot commented Apr 20, 2022

Unit Test Results

3 files  3 suites   4s ⏱️
3 tests 3 ✔️ 0 💤 0

Results for commit 6cf3bdd.

♻️ This comment has been updated with latest results.

@cousim46 cousim46 force-pushed the HOTFIX-134 branch 7 times, most recently from e170ce2 to 88cd73a Compare April 22, 2022 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status : peer 리뷰요청 New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants