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

[POM-84] test: 중복된 엔티티 객체 생성 코드 분리 #42

Merged
merged 2 commits into from
Sep 18, 2023

Conversation

hyeon-z
Copy link
Collaborator

@hyeon-z hyeon-z commented Sep 18, 2023

📌 구현 설명

중복된 엔티티 객체 생성 코드 static 메서드로 분리

✅ 피드백 반영사항

이전 PR의 리뷰 사항 반영

@hyeon-z hyeon-z self-assigned this Sep 18, 2023
@hyeon-z hyeon-z added TEST and removed REFACTOR labels Sep 18, 2023
Copy link
Collaborator

@juno-junho juno-junho left a comment

Choose a reason for hiding this comment

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

🏖️

return new Fee(PGType.TOSS, 10000);
}


Copy link
Collaborator

Choose a reason for hiding this comment

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

줄이 한줄 추가되었네요

Copy link
Collaborator Author

Choose a reason for hiding this comment

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


// then
assertThat(sales).isNotNull();
assertThat(sales()).isNotNull();
Copy link
Collaborator

Choose a reason for hiding this comment

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

예외를 던지지 않는다는 메서드로 변경하는게 어떨까요? 생성 자체가 되면 안되는거 아닌가요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

해당 메서드는 Sales가 생성되는 것을 테스트하므로 예외를 던지지 않는다 + Null이 아니다를 합쳐서 isNotNull을 사용했습니다!

Copy link
Collaborator

@choi5798 choi5798 left a comment

Choose a reason for hiding this comment

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

🍶

@hyeon-z hyeon-z merged commit f2ad4be into dev Sep 18, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants