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

회원가입 서비스 테스트 코드 작성 #43

Merged
merged 6 commits into from
Feb 15, 2024
Merged

Conversation

uijin-j
Copy link
Contributor

@uijin-j uijin-j commented Feb 14, 2024

🎫 관련 이슈

Resolves #42

✅ 구현 내용

  • Faker 의존성 추가
  • 회원가입 테스트 코드 작성
  • Checkstyle 한글 클래스명 허용하도록 변경

@uijin-j uijin-j added the 🚀 더 만들어봤슈 기능 구현 label Feb 14, 2024
@uijin-j uijin-j added this to the 🚀 1차 스프린트 milestone Feb 14, 2024
@uijin-j uijin-j self-assigned this Feb 14, 2024
Copy link
Contributor

@yenzip yenzip left a comment

Choose a reason for hiding this comment

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

정말 고생 많으셨습니다.. 👍

저희 테스트 코드는 서비스 코드만 짜는 게 맞는지 한 번 더 확인차 여쭤봅니다..! (도메인 테스트 코드는 작성하지 않은 것이 맞나요?)

전반적으로 테스트 코드가 정말 깔끔하네요 ❤️‍🔥

@@ -161,6 +161,8 @@
<module name="TypeName">
<property name="tokens" value="CLASS_DEF, INTERFACE_DEF, ENUM_DEF,
ANNOTATION_DEF, RECORD_DEF"/>
<!-- 클래스명 한글 허용 -->s
<property name="format" value="^[a-zA-Z_$\p{IsHangul}][a-zA-Z\d_$\p{IsHangul}]*$"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

테스트 이름에 한글 적용하는 거 저는 정말 좋습니다..! (사심 x 100 🔥 )

Copy link
Member

Choose a reason for hiding this comment

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

저도 적극 찬성합니다!


private User createUser(String email, String password, String nickname) {
return User.builder()
.email(isBlank(email) ? this.email : email)
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
Member

Choose a reason for hiding this comment

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

역시 깔끔 코드...👍

.withMessage("이미 사용 중인 닉네임입니다.");
}

@Test
Copy link
Contributor

@yenzip yenzip Feb 14, 2024

Choose a reason for hiding this comment

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

개인적인 생각으로는 이메일이나 비밀번호 형식 관련 테스트는 @ParameterizedTest 로 다양한 형식을 테스트 해보는게 좋다고 생각하는데, 의진님은 어떻게 생각하시나요?
물론 기능 구현이 저희의 최우선의 목표이기 때문에 추후, 리팩토링데이에 적용해주셔도 될 것 같습니다 👍

Copy link
Member

Choose a reason for hiding this comment

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

실패 테스트 경우는 ParameterizedTest 적용하는 것도 좋겠네요! 저도 우선 지금 코드도 너무 잘 짜주셔서 리팩토링 데이 때 해주셔도 될 것 같습니다!!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

넹! 저도 넘넘 좋습니다🥰 일단 내일이 스프린트 마감이니..
리팩토링 Task 백로그 페이지를 만들어 넣어 놓겠습니당!

Copy link
Member

@Sehee-Lee-01 Sehee-Lee-01 left a comment

Choose a reason for hiding this comment

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

확실히 service만 작성하니 마음도 편해지고 더 리뷰에 집중할 수 있게되네요! 전반적으로 코드 너무 깔끔하게 짜주셔서 편안하게 리뷰했습니다!👍
보시고 더 수정할 것이 없으시다면 머지해주셔도 됩니다!

@@ -161,6 +161,8 @@
<module name="TypeName">
<property name="tokens" value="CLASS_DEF, INTERFACE_DEF, ENUM_DEF,
ANNOTATION_DEF, RECORD_DEF"/>
<!-- 클래스명 한글 허용 -->s
<property name="format" value="^[a-zA-Z_$\p{IsHangul}][a-zA-Z\d_$\p{IsHangul}]*$"/>
Copy link
Member

Choose a reason for hiding this comment

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

저도 적극 찬성합니다!


@SpringBootTest
@Transactional
@DisplayNameGeneration(ReplaceUnderscores.class)
Copy link
Member

Choose a reason for hiding this comment

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

새로 배워갑니다!👍👍

.withMessage("이미 사용 중인 닉네임입니다.");
}

@Test
Copy link
Member

Choose a reason for hiding this comment

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

실패 테스트 경우는 ParameterizedTest 적용하는 것도 좋겠네요! 저도 우선 지금 코드도 너무 잘 짜주셔서 리팩토링 데이 때 해주셔도 될 것 같습니다!!


private User createUser(String email, String password, String nickname) {
return User.builder()
.email(isBlank(email) ? this.email : email)
Copy link
Member

Choose a reason for hiding this comment

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

역시 깔끔 코드...👍

// then
User actual = userRepository.findById(saved).get();
Password encodedPassword = actual.getPassword();
assertThat(actual).extracting("email", "nickname", "provider")
Copy link
Member

Choose a reason for hiding this comment

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

extracting 하나 더 배워갑니다!👍

SignUpRequest request = new SignUpRequest(newEmail, password, duplicatedNickname);

// when
ThrowingCallable signup = () -> userService.signUp(request, Provider.BASIC);
Copy link
Member

Choose a reason for hiding this comment

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

👍

@Sehee-Lee-01 Sehee-Lee-01 added the ✅ 테스트혀봤슈 빌드&테스트가 필요할 때 태그 label Feb 15, 2024
Copy link

📝 Jacoco Test Coverage

Overall Project 58.88%

There is no coverage information present for the Files changed

@uijin-j uijin-j merged commit 34d52d8 into dev Feb 15, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✅ 테스트혀봤슈 빌드&테스트가 필요할 때 태그 🚀 더 만들어봤슈 기능 구현
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants