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

[TEST] Fixture 관리 객체 생성 및 예시 작성 #135

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

PgmJun
Copy link
Member

@PgmJun PgmJun commented Aug 4, 2024

Issue Number

#114

As-Is

테스트 데이터를 sql로 사용하고 있기 때문에 발생하고 있는 각종 데이터 관련 문제와 불편함이 발생한다.

To-Be

  • Domain별 Fixture 관리 객체를 사용한다.
    • Fixture의 의미는 테스트하고 싶은 데이터 선언의 편의를 위해, 특정 고정값을 사전에 설정해두고 편리하게 사용하는 목적을 가진 테스트 도구 중 하나인데, 제가 구현해둔 Fixture 객체는 그러한 Fixture를 생성하기 쉽게 구현해둔 테스트용 도구입니다.
    • Util로 생성하지 않았는데, 그 이유는 영속화 로직이 담겨있기 때문에 Controller, Service 테스트(+ 필요하다면 Repository 테스트도) 이외에는 필요할 것 같지 않았고, 이 외에 필요하지 않은 곳에서 호출 가능하게 만들어두는 것은 좋지 않은 설계라고 생각했습니다.
    • Repository 테스트에서도 사용할 수 있을 것 같은데 필요하다면 BaseRepositoryTest에 Import하여 사용하면 될 것 같습니다!
  • 사용 예시 제공을 위해 게임 방 정보 조회 기능에 대해 Fixture를 적용해두었습니다 (예시 바로가기)

Check List

  • 테스트가 전부 통과되었나요?
  • 모든 commit이 push 되었나요?
  • merge할 branch를 확인했나요?
  • Assignee를 지정했나요?
  • Label을 지정했나요?

Test Screenshot

스크린샷 2024-08-04 22 25 38
스크린샷 2024-08-04 22 25 50

(Optional) Additional Description

  • Fixture 객체에서 Reflection를 사용합니다.
  • 제가 다 처리해보려고 했는데 Conflict가 어마어마 할 것 같아서 엄두가 안났네요..ㅠ
    때문에 우선 예시를 작성을 해두었으니 PR Merge되면 각자 역할 분담해서 Fixture 사용 로직으로 Refactoring 하면 될 것 같습니다!!
  • Fixture 다수 생성 로직도 예시로 미리 구현해두었는데 이런 식으로 구현하셔서 사용하시면 될 것 같습니다! 예시 보러가기

@PgmJun PgmJun added ♻️ refactor 리팩토링 ✅ test 테스트 관련 🍃 BE back end labels Aug 4, 2024
@PgmJun PgmJun added this to the BE Sprint3 milestone Aug 4, 2024
@PgmJun PgmJun self-assigned this Aug 4, 2024
@PgmJun PgmJun changed the title [Test] Fixture 관리 객체 생성 [Test] Fixture 관리 객체 생성 및 예시 작성 Aug 4, 2024
@PgmJun PgmJun changed the title [Test] Fixture 관리 객체 생성 및 예시 작성 [TEST] Fixture 관리 객체 생성 및 예시 작성 Aug 8, 2024
Copy link
Contributor

@leegwichan leegwichan left a comment

Choose a reason for hiding this comment

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

LGTM~! 리팩토링 해준 이든 고마워~!!

Comment on lines +15 to +18
public BalanceOption createOption(BalanceContent balanceContent) {
BalanceOption balanceOption = new BalanceOption("옵션명", balanceContent);
return balanceOptionRepository.save(balanceOption);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

사소한 제안) 내부에서 저장한다는 것을 알 수 있도록 메서드 이름 바꾸는 건 어떨까요?
(아래는 메서드 예시이고, 적절한 네이밍 사용해주시면 좋을 것 같아요!)

    public BalanceOption getSavedOption(BalanceContent balanceContent) {
        BalanceOption balanceOption = new BalanceOption("옵션명", balanceContent);
        return balanceOptionRepository.save(balanceOption);
    }

Comment on lines +46 to +54
public RoomContent(Room room, BalanceContent balanceContent, int round, LocalDateTime roundEndedAt,
boolean isUsed) {
this.room = room;
this.balanceContent = balanceContent;
this.round = round;
this.roundEndedAt = roundEndedAt;
this.isUsed = isUsed;
}

Copy link
Contributor

@leegwichan leegwichan Aug 9, 2024

Choose a reason for hiding this comment

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

사소한 제안) 생성자에도 "메서드 파라미터 개행 방식"을 적용해보는 건 어떨까요?
https://www.notion.so/ghenmaru/BE-f347e1e9315d46fdb3f066cc55946168?pvs=4#f08ebdc75bc0401b8abe6a9fb8c8d732

    public RoomContent(Room room,
                       BalanceContent balanceContent,
                       int round,
                       LocalDateTime roundEndedAt,
                       boolean isUsed) {
        this.room = room;
        this.balanceContent = balanceContent;
        this.round = round;
        this.roundEndedAt = roundEndedAt;
        this.isUsed = isUsed;
    }

Copy link
Member

@GIVEN53 GIVEN53 left a comment

Choose a reason for hiding this comment

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

fixture 먼저 구현해주신 점 칭찬해용~~

fixture 내에서 repository의 save를 하는 이유가 궁금했는데 이든이 오프라인으로 잘 설명해주었습니다
Repository 테스트는 해당 repository만 autowired해서 단위테스트 성격으로 테스트하는 것이 이상적이지 않을까 생각해요 만약 repository 테스트에서 연관관계의 엔티티가 필요하면 fixture 대신 BaseRepositoryTest에 구현된 save()를 사용하면 어떨지 의견드립니다!

@@ -215,7 +218,7 @@ class 라운드_종료_여부 {
// given
int currentRound = 2;
Room room = roomRepository.save(
new Room(TOTAL_ROUND, currentRound, TIME_LIMIT, STATUS, CATEGORY));
roomFixture.createRoom(TOTAL_ROUND, currentRound, TIME_LIMIT, STATUS, CATEGORY));
Copy link
Member

Choose a reason for hiding this comment

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

의견) createRoom 내에서 save()를 호출하기 때문에 save를 한 번 더 진행하는 것으로 보입니다

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🍃 BE back end ♻️ refactor 리팩토링 ✅ test 테스트 관련
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

3 participants