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

Feat: 테스트 격리성 개선 및 WithMockCustomUser 어노테이션 제거 #77

Merged
merged 3 commits into from
Mar 18, 2024

Conversation

Starlight258
Copy link
Collaborator

@Starlight258 Starlight258 commented Mar 17, 2024

🔗 Linked Issue

🛠 개발 기능

  • createTestMeetingcreateTestMember 메소드를 추가
  • @BeforeEach로 테스트 데이터 준비 및 @AfterEach로 테스트 데이터 제거
  • @BeforeEach에 보안 컨텍스트 설정
  • @WithMockCustomUser 제거

🧩 해결 방법

🔍 리뷰 포인트

  • 이전에 테스트시 사용하는 JWT 토큰에 의한 인증 문제가 발생했는데, 보안 컨텍스트를 직접 설정하여 문제를 해결했어요.
  • 테스트 db의 데이터를 조회하는 코드가 있어서 데이터 의존성이 발생했는데, 테스트 데이터를 직접 생성하고 제거하는 식으로 의존성을 해결했어요.
  • 내일 웹소켓 테스트 및 Agenda 테스트 진행해볼게요 ! 생각보다 시간이 길어졌네요 🥹


📋 Code Review Priority Guideline

  • 🚨 P1: Request Change
    • 필수 반영: 꼭 반영해주시고, 적극적으로 고려해주세요 (수용 혹은 토론).
  • 💬 P2: Comment
    • 권장 반영: 웬만하면 반영해주세요.
  • 👍 P3: Approve
    • 선택 반영: 반영해도 좋고 넘어가도 좋습니다. 그냥 사소한 의견입니다.

Copy link
Collaborator

@FacerAin FacerAin left a comment

Choose a reason for hiding this comment

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

바쁘실텐데 고생 많으셨어요 👍 기존 테스트 코드의 문제점을 파악하고 개선하신 점이 인상적이에요 :)

Member member = createTestMember();
UserDetails userDetails = new CustomUserDetails(member);
Authentication auth = new UsernamePasswordAuthenticationToken(userDetails, null, userDetails.getAuthorities());
SecurityContextHolder.getContext().setAuthentication(auth);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Security Context Holder를 통해 JWT 관련 Test를 가능하게 한점 좋습니다 👍👍

@Starlight258 Starlight258 merged commit 1e51dde into dev Mar 18, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants