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

공연 등록 테스트 및 리팩토링 #3

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

Conversation

Jummi10
Copy link
Contributor

@Jummi10 Jummi10 commented Jun 23, 2022

책에서는 테스트 하는 과정에서 use case를 인터페이스로 분리하는 것이 테스트하기 용이하다고 하였는데, 직접 서비스 테스트를 작성해보니 인터페이스가 그렇게 테스트하기에 편리한가? 라는 의문이 들었습니다.
실제 테스트해야하는 클래스는 구현체 클래스의 로직이고, incoming port를 필드로 사용하는 컨트롤러에서 유스케이스를 모킹할 수 있다 정도인 것 같다고 생각이 듭니다. 제가 너무 계층형 아키텍처에 맞게 테스트를 작성한 것인지, 다른 분들은 어떤 식으로 테스트를 작성하셨는지 같이 이야기해보고 싶습니다!

@Jummi10 Jummi10 added bug Something isn't working enhancement New feature or request labels Jun 23, 2022
@Jummi10 Jummi10 self-assigned this Jun 23, 2022
Copy link

@MangKyu MangKyu left a comment

Choose a reason for hiding this comment

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

책을 읽은지 벌써 한달이나 되어버려서 잘 기억은 안나지만...... 간단히 포트와 테스트 관련해서 제가 경험한 내용들을 적어보겠습니다ㅎㅎ

개인적으로는 테스트 작성 혹은 테스터빌리티의 관점보다는 테스트의 관리 및 유지보수 관점이 더 맞지 않을까 싶은데요~ 테스트는 성공 테스트 뿐만 아니라 실패 테스트까지 작성을 해주는게 좋은데, 그러면 1개의 메소드에 대해서도 테스트가 엄청 많아집니당! 메소드가 여러 개라면 테스트가 너무 많아져서 테스트 관리 및 유지보수가 힘들어지는데, 인터페이스를 기준으로 테스트 클래스를 나누면 테스트 클래스가 잘게 쪼개지니까 요게 좋은 것 같습니다ㅎㅎ 추가로 인터페이스에는 @Injectmocks 사용이 불가능하니, 오히려 작성은 구체 클래스가 더 편리하지 않을까 싶습니다!

@@ -15,8 +17,10 @@
@AllArgsConstructor
@NoArgsConstructor(access = AccessLevel.PROTECTED)
@Table(name = "performance")
@Where(clause = "is_deleted = false")
Copy link

Choose a reason for hiding this comment

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

요 부분을 이렇게 작업해두신 이유가 있을까욤???? 어떤 견해이신지 궁금하네요ㅎㅎ

class RegisterPerformanceServiceTest {

@Mock
private PerformanceMapper performanceMapper;
Copy link

Choose a reason for hiding this comment

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

요런 매퍼는 굳이 Mock 처리를 안해도 되지 않을까욤???
(테스트) 프레임워크에 대한 의존도 줄이고, 테스트 작성도 쉬워질 것 같은데용

private final PerformanceMapper mapper;
private final SavePerformancePort savePerformancePort;
private final SavePerformanceCommand savePerformanceCommand;

@Override
public PerformanceId registerPerformance(RegisterPerformanceRequest request) {
Performance performance = mapper.convertRequestToDomainEntity(request);
Copy link

Choose a reason for hiding this comment

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

Performance 도메인 객체에는 List가 존재하는데, 변환 시에는 없네욤...??
이렇게 작업하신 이유가 있을까요~? 도메인 객체에서 빼던지 or 변환 시에 같이 변환해주는게 좋지 않을까 싶은데용
만약 Performance에서 List를 사용하려는 개발자는 의도치 않게 NPE를 마주할 것 같습니당

Copy link
Contributor Author

Choose a reason for hiding this comment

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

수정하겠습니다!

@@ -16,5 +18,8 @@ public class SeriesRequest {
@DateTimeFormat(pattern = "yyyyMMddHHmm")
private final LocalDateTime dateTime;

@NotBlank
Copy link

Choose a reason for hiding this comment

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

@notempty가 아니고 @notblank로도 되나요??
테스트 코드를 잘 작성해두면 요런거 테스트도 금방 해보는 장점이 있을 것 같습니당ㅎㅎ

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants