Skip to content

Conversation

@cousim46
Copy link
Member

PR 체크리스트

PR을 올렸다면 아래 사항은 반드시 지켜주세요.

  • Jira에 맞는 적절한 브랜치를 생성했습니다
  • PR 우측 Labels에서 적절한 라벨을 부착하였습니다.
  • Commit 메세지 규칙에 맞게 작성했습니다.
  • 제가 의도한 파일들과 수정 사항들만 커밋이 된 것을 확인 하였습니다.
  • git rebase와 squash를 했고 커밋 수가 하나인 것을 확인 했습니다.
  • 기능에대한 오류가 없는것을 확인했습니다
  • 테스트로 구현한 화면 혹은 기능을 확인했습니다
  • 코드 리뷰 사항을 모두 반영하였습니다.
  • 리뷰어에 1명 할당했습니다

마크다운에서 체크를 하기 위해서는 [x] 이렇게 수정하시면 됩니다. [x ], [ x ] 가 아니라 정확하게 [x]여야 합니다. 혹은 마우스 클릭으로 체크해주세요.

구현한것들 및 간단한 설명

  • 이름, 이메일, 주민번호로 해당 아이디 찾기
  • [ ]

@cousim46 cousim46 added the Status : peer 리뷰요청 New feature or request label Apr 27, 2022
@cousim46 cousim46 requested review from loadkrnis and syb1231 April 27, 2022 15:55
@cousim46 cousim46 self-assigned this Apr 27, 2022
@github-actions
Copy link

github-actions bot commented Apr 27, 2022

Unit Test Results

4 files  4 suites   28s ⏱️
6 tests 6 ✔️ 0 💤 0

Results for commit 0c71d1b.

♻️ This comment has been updated with latest results.

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.

리뷰 늦어서 죄송합니다 🙏

String userName = "정회운";
String birthDate = "9812111";
String email = "cousim55@gmail.com";
//when
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.

넵 알겠습니다!

Comment on lines 48 to 54
String loginId = webTestClient.post().uri("/login/id")
.contentType(MediaType.APPLICATION_JSON_UTF8)
.bodyValue(new UserLoginIdFindRequest(userName,birthDate,email))
.exchange().expectBody(String.class).returnResult().getResponseBody();
JSONParser jsonParser = new JSONParser();
JSONObject jsonObject = (JSONObject) jsonParser.parse(loginId);
String userLoginId = (String)jsonObject.get("loginId");
Copy link
Collaborator

Choose a reason for hiding this comment

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

when 절은 한줄로 써주셔야합니당! :)

Copy link
Member Author

Choose a reason for hiding this comment

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

넵 알겠습니다!

"message : 사용자 이름, 생년월일, 이메일을 잘못입력하셨습니다.', " +
"'status : 404'," +
"'Error : Not_Found'")
void 이름_이메일_주민번호_잘못작성() throws Exception {
Copy link
Collaborator

Choose a reason for hiding this comment

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

이 이름의 테스트는 총 3개가 만들어져야할 것 같은데 어떻게 생각하실까요?
이름이 틀렸을때 ~~한다
이메일이 틀렸을때 ~~한다
생년월일이 틀렸을때 ~~한다

Copy link
Member Author

Choose a reason for hiding this comment

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

이름, 이메일, 생년월일 각각 틀렸을경우 똑같은 결과값을 반환합니다.
그래서 테스트를 하나로 만들었습니다!

Copy link
Collaborator

@loadkrnis loadkrnis May 15, 2022

Choose a reason for hiding this comment

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

지금 작성해주신 테스트는 이름이 틀렸을 때 ~~를 반환한다. 테스트라고 생각이 드는데요.
생년월일이 틀였을 때 ~~에러를 반환한다 테스트로 검증이 안된것 같아요.

테스트의 DisplayName은 분명히 세가지 중 하나라도 올바르지 않을 경우라고 적혀있는데,
실제 테스트코드는 사용자의 name만 틀린 상태로 검증하는 것 같아요.

테스트는 한 가지 목적의 검증을 수행하도록 작성되어야 할 것 같아요.
그래서 제 생각은 같은 에러메세지를 확인해도 위에서 말씀드린 세가지의 테스트가 나와야할 것 같습니다.
이유는 아래와 같은데 혹시 어떻게 생각하실까요?

  • 하나의 검증이 실패하면 후속 검증은 평가되지 않으므로 전체 테스트에 대한 현황을 확인할 수 없습니다.
  • 여러 가지 목적이 검증되는 테스트는 여러가지 테스트 의도를 포함하는 테스트이므로, 테스트 의도가 명확하지 않습니다.

@loadkrnis
Copy link
Collaborator

@cousim46
�궁금한 점 코멘트 남겼습니다~!
그리고 이전 리뷰가 반영이 안된 것들이 조금 있어서 그것도 확인 좀 부탁드릴게요!

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