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

[Feature] - BE 테스트 개선 스프링 컨텍스트 캐싱 최적화 #625

Merged
merged 10 commits into from
Dec 30, 2024

Conversation

Libienz
Copy link

@Libienz Libienz commented Dec 23, 2024

✅ 작업 내용

  • 컨트롤러 테스트 스프링 컨텍스트를 1회만 초기화 하도록 환경 통일
  • 서비스 테스트 스프링 컨텍스트 1회만 초기화 하도록 환경 통일

🙈 참고 사항

추상 클래스 계층 구조를 통해 서비스 통합 테스트, 컨트롤러 통합 테스트 환경을 통일하도록 했습니다.
이전에는 10개의 애플리케이션 컨텍스트가 전체 테스트에 소모되었지만 최적화 후 4개로 줄어들었습니다.
테스트 수행시간은 5번씩 측정을 진행해보았을 때 약 39초에서 32초로 18프로 정도 개선되었습니다. (제 로컬 기준)

추상클래스 이름 추천받습니다 인스턴스화가 불가능한 추상클래스라는 점을 명세에 강조하기 위해서 Abstract을 붙여보았어요 다른 좋은 이름 생각나시면 말씀 부탁드리겠습니다.

Copy link

github-actions bot commented Dec 23, 2024

Test Results

 30 files   30 suites   53s ⏱️
296 tests 295 ✅ 1 💤 0 ❌
308 runs  307 ✅ 1 💤 0 ❌

Results for commit e2dc298.

♻️ This comment has been updated with latest results.

Copy link

@eunjungL eunjungL left a comment

Choose a reason for hiding this comment

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

고생하셨습니다 리비~
테스트에 추상화 계층이 하나 더 추가돼서 공통 로직이 잘 모이게 됐네용
덕분에 테스트 작성이 더 쉬워질 것 같슴다! 👍

import org.springframework.boot.test.web.server.LocalServerPort;

@SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT)
public abstract class AbstractControllerIntegrationTest extends AbstractIntegrationTest {

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.

저도 이름 좋다고 생각합니다! 실제로 스프링에서 제공해주는 클래스에도 Abstract로 시작하는 클래스들이 많아서 어색하지 않다고 생각되어요!

Copy link

@slimsha2dy slimsha2dy left a comment

Choose a reason for hiding this comment

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

공통 로직을 추상 클래스로 모아서 훨씬 깔끔해졌고 테스트 성능도 개선되었군요.
수고 많으셨네요 리비. 저도 네이밍은 지금이 명확해서 좋다고 생각합니다 👍

Copy link
Member

@nak-honest nak-honest left a comment

Choose a reason for hiding this comment

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

고생하셨습니다 리비!!
연말이라고 약속이 많이 잡히다보니 리뷰가 많이 늦었네요 ㅠㅠ 죄송합니다

@MockBean 부분 까지 잘 처리한다면 스프링 컨텍스트를 극한으로 재활용할 수 있을 것 같아 의견 드려봤는데요!
해당 부분에 대해서만 결정 된다면 approve 드리도록 하겠습니다!

Comment on lines 28 to 29
@MockBean
private KakaoOauthProvider oauthProvider;
Copy link
Member

@nak-honest nak-honest Dec 29, 2024

Choose a reason for hiding this comment

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

@MockBean 으로 인해 로그인 컨트롤러와 로그인 서비스 테스트는 각각 스프링 컨텍스트를 새로 띄우네요!
혹시 @MockBean 들도 추상 클래스에 protected 로 재정의하여 전체 컨텍스트를 2개로 줄이는 건 어떻게 생각하실까요??

다만 MemberRepository 가 전체 서비스 테스트에서 mock으로 사용시 문제가 발생하는데,
애초에 로그인 서비스 테스트에서 MemberRepository 를 목으로 사용하지 않는 것이 더 좋지 않을까 생각되네요!

Copy link
Author

Choose a reason for hiding this comment

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

우선 운용하는 @MockBean은 KakaoOauthProvider 하나로 좁혀졌습니다.
MemberRepository, JwtTokenProvider 등, 필요없는 MockBean은 전부 제거하였어요

혹시 @MockBean 들도 추상 클래스에 protected 로 재정의하여 전체 컨텍스트를 2개로 줄이는 건 어떻게 생각하실까요??

그런데 @MockBean을 추상 클래스에 위치시키게 되면 MockBean이 관심사가 아닌 테스트들까지 참조가 열리게 됩니다.

  • TravelogueService는 KakaoOauthProvider 목빈이 필요로 하지 않음에도 참조가 열림

그리고 MockBean을 XXXIntegrationTest에 위치시키는 것 자체가 통합 테스트를 부정하는 어폐가 있다고 생각도 드네요

MockBean을 추상클래스안에 위치시키면 스프링 애플리케이션 컨텍스트를 두개로 운용할 수 있음을 확인했는데요~ 선술한 이유를 근거로 추상클래스안에 위치시키지는 않았습니다.

Copy link
Author

Choose a reason for hiding this comment

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

여담으로 통합테스트의 의미를 극한으로 살리기 위해서 MockBean stubbing이 아닌 MockServer를 운용하는 방식도 생각해보았어요. 이 경우 통합테스트의 의미를 살리면서 애플리케이션 컨텍스트도 캐싱하며 사용할 수 있는데요..! 이는 이번 PR의 관심사는 아닌 듯 하군요. 다음 이슈에서 작업되면 매우 좋을 것 같습니다.

MockServer 궁금하시면 다음 링크 한 번 살펴봐주세요

MockServer 공식 사이트

import org.springframework.boot.test.web.server.LocalServerPort;

@SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT)
public abstract class AbstractControllerIntegrationTest extends AbstractIntegrationTest {
Copy link
Member

Choose a reason for hiding this comment

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

저도 이름 좋다고 생각합니다! 실제로 스프링에서 제공해주는 클래스에도 Abstract로 시작하는 클래스들이 많아서 어색하지 않다고 생각되어요!

Copy link
Member

@nak-honest nak-honest left a comment

Choose a reason for hiding this comment

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

제성함다 ㅎㅎ
approve를 이후에 드린다는걸 바로 드렸네요!

@Libienz Libienz requested a review from nak-honest December 29, 2024 16:29
@Libienz
Copy link
Author

Libienz commented Dec 29, 2024

@nak-honest

불필요한 MockBean 제거하는 개선만 수행되었습니다.
코멘트 내용 확인해주세요~

Copy link
Member

@nak-honest nak-honest left a comment

Choose a reason for hiding this comment

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

넵 잘 확인했습니다! 고생하셨습니다 리비!!

@Libienz Libienz merged commit 8a65589 into develop/be Dec 30, 2024
3 checks passed
@Libienz Libienz deleted the feature/be/#624 branch December 30, 2024 02:13
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.

[Feature] - BE 테스트 개선 스프링 컨텍스트 캐싱 최적화
4 participants