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

[HIMIN-222] 회원 테스트 구현 #114

Merged
merged 4 commits into from
Sep 16, 2023

Conversation

BeommoKoo-dev
Copy link
Collaborator

PR 확인 항목

PR 보내기전에 아래 항목들을 만족 하였는지 체크 해주세요

  • 곤모슬 팀에서 정한 커밋 메시지 규칙과 일치하는가?
  • 곤모슬 팀에서 정한 코딩 컨벤션과 일치하는가?

PR 종류

어떤 종류의 PR인지 아래 항목중에 체크 해주세요

  • 버그 수정
  • 기능 추가
  • 코드 스타일 변경 (formatting, local variables)
  • 리팩토링 (기능 변경 X)
  • 빌드 관련 수정
  • 문서 내용 수정
  • 테스트 추가
  • 그 외, 어떤 종류인지 기입 바람:

어떤 기능이 추가 되었나요?

기존 테스트와 비슷한 로직이라 나머지 회원 테스트 작성해서 한번에 PR 올립니다.

Issue Number: HIMIN-222

기존에 있던 기능에 영향을 주나요?

  • 아니요

기타

UpdateMember테스트에서 usingRecursiveComparsion호출할 때, assertThat()에 actual을 먼저 넣어주지 않고 request를 넣어 준 이유는, actual을 assertThat()의 인자로 넣어주면 type비교를 해버려서 테스트가 실패해서 입니다.(이유는 아직 정확하게 파악하지 못했습니다.)

Copy link
Collaborator

@Curry4182 Curry4182 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다 ~!

.map(AddressResponse::from)
.toList();

MemberResponse expected = MemberResponse.of(member, addressResponses);
Copy link
Member

Choose a reason for hiding this comment

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

p2;
expected를 구하는 건 given이 아니라고 생각하는데 then으로 빼는 건 어떤가요??

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

저도 이 부분에 대해서 원래는 then이라고 생각했었는데, then의 정의를 다시 찾아보니 'when절에서 정의한 결과물을 묘사한다'
라고 정의돼 있습니다! (출처 :https://en.wikipedia.org/wiki/Given-When-Then)
따라서 Response를 예측하는 부분은 'Service의 로직을 타지 않는 부분'이므로, preCondition이라고 생각이 되어 일부러 given에 넣어주었는데 이슬님 생각이 궁금합니다 !

Copy link
Member

Choose a reason for hiding this comment

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

expected도 어떻게보면 결과를 묘사한것으로 볼 수 있지 않을까 싶어요!
제가 생각한 gwt는 'given이 주어진 상황에서 when을 했을 때 then이라는 결과가 나온다' 인데, expected가 주어진 데이터,,?로 보기에는 살짝 힘들지 않을까 싶은데 범모님은 어떻게 생각하세요??

// when
memberService.updateMember(member.getId(), request);
Member actual = memberRepository
.findById(member.getId()).get();
Copy link
Member

Choose a reason for hiding this comment

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

p2;
actual 구하는 것도 then이 맞을 것 같아요..! when은 실제 행위가 일어나는 거니까 updateMember()만 들어가야한다고 생각해요! findById로 멤버를 조회하는 건 테스트의 결과를 확인하기 위한 부가적인 메서드 같은데 범모님 의견은 어떠신가요 ??

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

actual 또한 preCondition에 속한다고 생각되어 given에 넣어주려 했으나, 순서상 updateMember의 호출 이후에 필요하다 생각되어 then으로 빼주었습니다 !

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

이 부분은 다시 보니, when절에서 어떤 행동을 가한 후, 해당 엔티티를 조회하는 로직이므로 then에 더 적합하다는 생각이 드네요 !

Copy link
Member

@Yiseull Yiseull left a comment

Choose a reason for hiding this comment

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

테스트 작성하느라 고생하셨습니다!!

@BeommoKoo-dev BeommoKoo-dev merged commit 9e0efb3 into main Sep 16, 2023
1 check passed
@BeommoKoo-dev BeommoKoo-dev deleted the HIMIN-222--beommo-member-find-test branch September 18, 2023 06:43
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.

None yet

3 participants