Skip to content

Conversation

@Gyuhyeok99
Copy link
Contributor

@Gyuhyeok99 Gyuhyeok99 commented Jan 20, 2025

관련 이슈

작업 내용

유저 관련 통합 테스트 코드 추가하였습니다.

유저통합서비스테스트
  • 마이페이지 조회
  • 관심대학교 조회
  • 프로필 수정
  • 닉네임 수정

특이 사항

BaseIntegrationTest 및 TestDataSetUpHelper를 추가하였습니다.
기존에는 UniversityDataSetUpIntegrationTest으로 특정 도메인(University)에 한정된 테스트 데이터 설정 클래스를 사용했습니다.

이러한 접근 방식은:

대학교 서비스에서만 재사용이 가능하고 다른 도메인에서 대학교 데이터가 필요할 경우 중복 코드가 발생하며 클래스 이름과 패키지 구조가 특정 도메인에 종속되어 유지보수성이 떨어지는 문제가 있었습니다.

이러한 문제를 해결하기 위해:

BaseIntegrationTest: 모든 도메인의 통합 테스트가 공통으로 사용할 수 있는 기본 설정 클래스
TestDataSetUpHelper: 여러 도메인에서 재사용 가능한 테스트 데이터 설정을 담당하는 헬퍼 클래스

를 추가하여 통합 테스트 관련 데이터와 설정을 관리하도록 하였습니다.

리뷰 요구사항 (선택)

S3Service를 통합 테스트로 진행하기엔 부담이 조금 있는 거 같아서 s3Service의 경우만 @MockBean을 사용해서 진행하였는데 이 접근 방식이 적절한지, 혹은 더 나은 대안이 있는지 의견 부탁드립니다

@Gyuhyeok99 Gyuhyeok99 self-assigned this Jan 20, 2025
@Gyuhyeok99 Gyuhyeok99 linked an issue Jan 20, 2025 that may be closed by this pull request
1 task
@nayonsoso nayonsoso added 테스트 Added tests and removed 기능 labels Jan 20, 2025
Comment on lines 9 to 20
@TestContainerSpringBootTest
@ExtendWith(DatabaseClearExtension.class)
public abstract class BaseIntegrationTest {

@Autowired
protected TestDataSetUpHelper testDataSetUpHelper;

@BeforeEach
public void setUpBaseData() {
testDataSetUpHelper.setUpBasicData();
}
}
Copy link
Collaborator

@nayonsoso nayonsoso Jan 20, 2025

Choose a reason for hiding this comment

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

BaseIntegrationTest: 모든 도메인의 통합 테스트가 공통으로 사용할 수 있는 기본 설정 클래스
TestDataSetUpHelper: 여러 도메인에서 재사용 가능한 테스트 데이터 설정을 담당하는 헬퍼 클래스

이 둘의 차이가 무엇인지 구체적으로 설명 부탁드립니다🥺
코드를 읽어봐도 왜 TestDataSetUpHelper를 직접 쓰지 않고,
BaseIntegrationTest가 필요했는지 모르겠는 부분들이 있네요.!

Copy link
Contributor Author

@Gyuhyeok99 Gyuhyeok99 Jan 21, 2025

Choose a reason for hiding this comment

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

저는 테스트 환경 설정(@TestContainerSpringBootTest, @ExtendWith 등)이랑 테스트 데이터 관리가 서로 다른 관심사라고 생각해서 분리했었는데 결국엔 계속 같이 사용하게 될테니 굳이인 거 같다는 생각도 들긴 하네요 😂

Copy link
Collaborator

@nayonsoso nayonsoso Jan 22, 2025

Choose a reason for hiding this comment

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

그리고 BaseIntegrationTest 를 상속하고 있는 각각의 통합테스트에서
'어떻게 테스트 데이터가 세팅되어있는지'를 확인하기 위해서
xxxTest → BaseIntegrationTest → TestDataSetUpHelper 를 거쳐야 하네요.
테스트 데이터를 확인하기 위한 depth 가 깊은 느낌입니다😓

테스트 환경 설정이랑 테스트 데이터 관리가 서로 다른 관심사라고 생각해서

말씀하신 이유 때문이라면 하나로 합치는게 더 응집도 있을 것 같습니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

이해했습니다! 저도 그게 좋은 거 같습니다!

Comment on lines 22 to 23
@Component
public class TestDataSetUpHelper {
Copy link
Collaborator

@nayonsoso nayonsoso Jan 20, 2025

Choose a reason for hiding this comment

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

여기에 @TestComponent가 아니라 @Component를 붙인 이유는
@TestComponent을 사용한다면 @TestConfiguration에서 빈으로 등록할 때
주입해줘야 하는게 너무 많아서인가요?

Copy link
Collaborator

@nayonsoso nayonsoso left a comment

Choose a reason for hiding this comment

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

수고하셨습니다😆
테스트코드가 잘 작성되어서, 명세서처럼 읽히네요!🥹👍

@Gyuhyeok99 Gyuhyeok99 merged commit 35bc2ac into main Jan 23, 2025
@Gyuhyeok99 Gyuhyeok99 deleted the test/155-add-siteuser-integration-test branch January 25, 2025 04:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

테스트 Added tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test: siteUser 관련 통합테스트 코드 추가

3 participants