#21 Answer 패키지 테스트 리팩토링 #28

LeeJaeYun7 · 2023-09-21T13:30:19Z

Answer 패키지의 테스트를 리팩토링하였습니다.

JwtUtil -> JwtService 리팩토링

AnswerDTOTest -> AnswerRequestTest 수정 AnswerServiceTest 클래스 read, update, delete 메소드 추가

f-lab-moony

고생 많으셨습니다 ~
코멘트를 좀 남겨 뒀는데, 확인 후 리뷰 재요청 부탁드려요 ~

f-lab-moony · 2023-09-26T15:01:00Z

src/main/java/com/example/interviewPrep/quiz/answer/domain/Answer.java

        this.version = version;
    }

+    public void setContent(String content){


내용 수정 시 사용되는 메서드 같은데, updateContent로 이름을 바꾸는 건 어떨까요 ?

넵 updateContent로 하는 편이 좋을 것 같네요
수정하겠습니다

f-lab-moony · 2023-09-26T15:02:11Z

src/main/java/com/example/interviewPrep/quiz/answer/service/AnswerService.java


-        Optional<Member> member = memberRepository.findById(memberId);
-        Optional<Question> question = questionRepository.findById(answerRequest.getQuestionId());
+        Member answerMember = memberRepository.findById(memberId).orElse(null);


Answer의 member와 question이 null이어도 되는건가요 ?

orElseThrow로 값이 없을 시 Exception을 리턴하도록 하는 편이 맞을 것 같습니다
수정했습니다

f-lab-moony · 2023-09-26T15:03:12Z

src/main/java/com/example/interviewPrep/quiz/answer/service/AnswerService.java

+
+        Optional<Answer> answer = answerRepository.findById(id);
+
+        if(answer.isEmpty()){


이부분 orElseThrow로 합칠 수 있을 것 같아요 ~

넵 그렇게 하는 편이 가독성 측면에서 나을 것 같습니다

f-lab-moony · 2023-09-26T15:04:48Z

src/main/java/com/example/interviewPrep/quiz/email/controller/EmailController.java

    public void emailChangeConfirm(@RequestBody MemberRequest memberRequest) throws Exception {
        logger.info("post emailConfirm");
-        Long memberId = JwtUtil.getMemberId();
+        Long memberId = jwtService.getMemberId();


이건 어디에 쓰이고 있나요 ?

memberId 변수가 따로 쓰이는 곳은 없어서, 저 변수는 불필요한 것 같습니다.
jwtService.getMemberId는 해당 메소드(이 경우 emailChangeConfirm)를
멤버가 로그인한 상태에서만 사용할 수 있도록 제한하는 역할을 합니다.

f-lab-moony · 2023-09-26T15:16:25Z

...est/java/com/example/interviewPrep/quiz/Answer/controller/AnswerCreateWebControllerTest.java

+    CustomAuthenticationEntryPoint customAuthenticationEntryPoint;
+
+    @MockBean
+    JwtUtil jwtUtil;


이거 사라지지 않았나요 ?

넵 피드백 반영했습니다

f-lab-moony · 2023-09-26T15:25:13Z

...est/java/com/example/interviewPrep/quiz/Answer/controller/AnswerCreateWebControllerTest.java

+    AnswerService answerService;
+
+    @MockBean
+    CustomOAuth2UserService customOAuth2UserService;


다른 서비스들은 안쓰이는 거 아닌가요 ?

이거 안 넣어주면 에러가 나더라고요
아마 Spring Security 적용한 부분과 관련이 있는 것 같습니다

f-lab-moony · 2023-09-26T15:25:49Z

...est/java/com/example/interviewPrep/quiz/Answer/controller/AnswerCreateWebControllerTest.java

+                .andDo(print())
+                .andExpect(status().isCreated());
+
+        verify(answerService).createAnswer(any(AnswerRequest.class));


이런 경우에는 any보다는 실제 요청이 잘 반환되었는지 파라미터 값도 검증하는게 좋을 것 같아요~

해당 요청을 AnswerService 테스트 클래스에서 검증해서 여기서는 따로 필요 없을 것 같습니다!
verify문 자체를 삭제하는쪽으로 생각했습니다

f-lab-moony · 2023-09-26T15:26:00Z

...est/java/com/example/interviewPrep/quiz/Answer/controller/AnswerCreateWebControllerTest.java

+    @BeforeEach
+    void setUp() throws Exception {
+
+        AnswerRequest validAnswerRequest = AnswerRequest.builder()


valid랑 invalid랑 차이가 뭐에요 ?

valid는 유효한 경우, invalid는 유효하지 않은 경우인데,
AnswerRequest DTO에서 Objects.requireNonNull로 검사한다고 해서
invalidRequest에 대한 테스트는 작성하지 않았습니다.
이건 빼도 괜찮을 것 같습니다

f-lab-moony · 2023-09-26T15:26:33Z

src/test/java/com/example/interviewPrep/quiz/Answer/dto/AnswerRequestTest.java

+                .content("답안입니다.")
+                .build();
+
+        assertThat(answerRequest.getContent()).isEqualTo("답안입니다.");


content만 확인하는 이유가 있을까요 ?

questionId도 검증하도록 추가했습니다

AnswerRequest 클래스 수정

AnswerRequest 클래스 수정2

AnswerReadWebControllerTest, AnswerServiceTest 수정

#28 QuestionController getAllQuestions 메소드 추가

LeeJongbokz added 6 commits September 21, 2023 22:29

Answer 패키지 테스트 AnswerCreateWebControllerTest 리팩토링

fe4a3e2

JwtService, JwtServiceTest 클래스 개발

df7df72

AnswerServiceTest 클래스 리팩토링, JwtService 클래스에 getMemberId 메소드 추가

0a0b431

remove unused imports

1a425a7

JwtUtil, JwtUtilTest 클래스 삭제,

bf20043

JwtUtil -> JwtService 리팩토링

AnswerContrller 클래스 updateAnswer 메소드 추가,

74714a9

AnswerDTOTest -> AnswerRequestTest 수정 AnswerServiceTest 클래스 read, update, delete 메소드 추가

f-lab-moony requested changes Sep 26, 2023

View reviewed changes

LeeJongbokz and others added 11 commits September 28, 2023 15:33

AnswerReadWebControllerTest 추가, 피드백 반영

43c8cbf

AnswerServiceTest, AnswerReadWebControllerTest 수정

81260f7

AnswerRequest 클래스 수정

AnswerServiceTest, AnswerReadWebControllerTest 수정

3e8b26c

AnswerRequest 클래스 수정2

Test 깨지는 버그 수정

a9172aa

피드백 반영

672c61a

remove unused imports

70888af

AnswerCreateWebControllerTest,

53ac422

AnswerReadWebControllerTest, AnswerServiceTest 수정

테스트 수정

e763005

remove unused imports

e84ba0f

merge conflict 해결

8d3efeb

Merge branch 'main' into fix/AnswerTest

1a807db

LeeJaeYun7 merged commit 3380852 into main Oct 3, 2023

LeeJaeYun7 added a commit that referenced this pull request Nov 1, 2023

Merge pull request #36 from f-lab-edu/feat/getAllQuestions

7fd154f

#28 QuestionController getAllQuestions 메소드 추가


		Optional<Answer> answer = answerRepository.findById(id);

		if(answer.isEmpty()){

#21 Answer 패키지 테스트 리팩토링 #28

#21 Answer 패키지 테스트 리팩토링 #28

Uh oh!

Conversation

LeeJaeYun7 commented Sep 21, 2023

Uh oh!

f-lab-moony left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LeeJaeYun7 Oct 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

LeeJaeYun7 Oct 2, 2023 •

edited

Loading