-
Notifications
You must be signed in to change notification settings - Fork 3
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
refactor: 테스트 코드 내 로그인 중복 제거 #453
Conversation
* feat: id를 사용하는 entity는 BaseEntity를 상속하도록 수정 * refactor: flyway 사용 규칙에 따라 스크립트 수정
* refactor: 패키지 및 클래스 명 수정 * feat: 카카오 소셜로그인 idToken 검증 및 payload 추출 기능 구현 * feat: 카카오 소셜로그인 기능 구현 * refactor: jwkProvider를 static 변수로 가지도록 수정 * refactor: 구글 소셜로그인 검증 방식 통일 * refactor: 구글 소셜로그인 검증 방식 추상화 * feat: 멤버 객체 내 nickname 필드 추가 * feat: 닉네임 객체 구현 * refactor: 클래스 네이밍 변경 * feat: JwkProvider를 찾지 못할 때 예외 구현 및 record -> class로 변경 * refactor: final 키워드 추가 * chore: 닉네임 추가 sql 버전 변경 5.3 -> 5.4 --------- Co-authored-by: thdwoqor <celebarte01@gmail.com>
* feat: IdTokenResolver가 Provider에 맞게 동작한다 * test: 지원하지 않는 Issuer 예외 테스트 추가 * refactor: 유지보수가 용이하도록 구조 개선 * refactor: 누락된 final 추가
* feat: 게스트로 로그인한다 * feat: 빈 패스워드는 어떤 문자열과도 일치하지 않는다 * refactor: Member 클래스 내 생성자를 정적 팩토리 메소드로 변경 * refactor: final 키워드 추가 * fix: 머지 중 누락 --------- Co-authored-by: Kong-hana01 <gksqlsl11@khu.ac.kr>
8fe4cad
to
c619ef4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드가 100만줄은 줄어든 것 같네요!
고생하셨고, 남긴 커멘트 한번 읽어주십쇼!
|
||
@GetMapping("/feedbacks") | ||
@GetMapping |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
제가 싼 💩을 치워주셨군요..👍
public DatabaseCleaner getAuthDatabaseCleanerBean() { | ||
return new DatabaseCleaner(Set.of(MEMBER, MEMBER_GENRE, SPRING_SESSION, SPRING_SESSION_ATTRIBUTES)); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
명확해서 좋습니다!
하나 마음에 걸리는 건 테이블 이름에 종속적이라는 것이네요.
하지만 그런 일은 잘 안생기니까 당장은 괜찮다고 생각합니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
인정합니다!
flyway로 관리하니까 잘하면 어찌저찌해볼 순 있겠지만 어떻게 해야할지는 잘 모르겠네요 ㅎㅎ..
this.mockLogins.add(webClientBuilder.baseUrl(HOST + port + PATH) | ||
.build() | ||
.post() | ||
.exchangeToMono(response -> Mono.just(new MockSessionLogin(response))) | ||
.block()); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
예전에 파워가 작업해 주셨던 거랑 마찬가지로 WebClient 및 Mono가 쓰였네요!
당시엔 익숙하지 않아서 좀 어려웠지만 지금은 무슨일이 일어나는지 대략 알겠네요 ㅎㅎ
this.memberRepository = memberRepository; | ||
this.roomRepository = roomRepository; | ||
this.mockLoginServer = mockLoginServerObjectProvider.getObject(); | ||
System.out.println(123123); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
123123 👍
디버깅의 흔적인가요 😂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋ 제가 싼 💩 을 알려주셨군요
|
||
@ActiveProfiles("default") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
테스트들이 기본적으로 'default' 프로파일로 동작하기를 원하시는 것 같네요.
최근에 찾아낸 꿀팁 하나 공유해볼게요.
build.gradle 에 다음과 같은 설정을 하면 테스트들에 기본 프로파일을 지정해 줄 수 있어 편리하더라구요.!
필요하다면 이렇게 적용해봐도 괜찮을 것 같습니다.
...
tasks.named('test') {
useJUnitPlatform()
systemProperty("spring.profiles.active", "test")
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오 좋은데요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아! 이 방식 적용하려고 했었는데 auth 프로필인 경우에는 default 프로필을 내려야하는데, 그 설정하는게 안되더라고요 ㅠ
아무튼 꿀팁 감사합니다 ㅎㅎ
private String cookie; | ||
|
||
@Autowired | ||
public FeedbackControllerTest(FeedbackRepository feedbackRepository, | ||
ObjectProvider<MockLoginServer> mockLoginServerObjectProvider |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
최근에 비슷한 코드를 리뷰한 적이 있는데, 생성자에 Autowired가 없어도 주입이 잘 된다라는 리뷰를 남겼었거든요.
제가 잘못된 지식을 알려준건지 의심되네요 ㄷㄷ
어떻게 붙이게 되었는지 계기가 궁금합니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저두 확인해봤었는데요.
운영 환경에서는 땡칠이 말씀해주신 내용이 맞아요!
근데 테스트 환경에서는 spring이 빈 주입을 담당하는게 아니라 JUnit 패키지의 ParameterResolver
가 빈 주입을 담당하더라고요. 그리고 SpringExtension
이 ParameterResolver를 상속받아서 빈 주입을 시켜주는데, 이 친구는 autowired
를 직접 명시해야지 resolve할 수 있다고 판단하더라고요. 그래서 autowired를 직접 작성해야지 잘 동작합니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아 저번에 말씀해주신게 이 내용이었군요.!
테스트 클래스는 스프링의 의존성 관리 대상이 아니므로 자동으로 의존성이 주입되지 않는다.
따라서 @Autowired
어노테이션을 통해 명시해줘야 SpringExtension이 동작하며 빈을 생성, 주입시킬 수 있다.
라고 이해했는데 맞을까요?
관련 이슈번호
작업 사항
테스트용 profile 분리
모든 테이블 정보 초기화
멤버/세션 관련 테이블을 제외한 모든 테이블 정보 초기화
MockServer 구현
다른 테스트에서 로그인 요청시 캐싱한 정보 반환
세션 로그인
->쿠키 로그인
방식으로 변환될 때 구현체만 만들고 갈아끼울 수 있도록 구현TestInstance를 method -> class로 변경
매 테스트 마다 db를 비워주기에 테스트 간의 격리가 보장
되기에 TestInstance를 class 단위로 변경기타 사항
조금 더 속도 개선이 있을 줄 알았지만...! 기껏해야 1~2초 정도 개선되었습니다. 😢
개선할 수 있는 지점이 더 있어보이긴 하지만 댓글 <-> 멤버 간의 격리가 이뤄지고나서 개선해볼 수 있을 것 같아요.