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

[step2] 2단계 - 인수 테스트 리팩터링 #235

Merged
merged 5 commits into from
Jan 25, 2022

Conversation

DaehunGwak
Copy link

2단계가 완료되어 리뷰 요청드립니다 :)

지난번 Response 상속 구조 피드백도 같이 반영하였습니다~

Copy link

@junwoochoi junwoochoi left a comment

Choose a reason for hiding this comment

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

2단계 미션도 잘 진행 해주셨습니다 👍 👍

3단계는 기능 구현이 추가된 요구사항이 있는 미션입니다.
인수 테스트 사이클을 지키면서 미션을 진행하시면 좋은 연습이 될 것 같습니다!

이어서 미션 진행해주세요! 😄

Comment on lines +11 to +18
public class CommonExceptionAdvice {

@ExceptionHandler(DataIntegrityViolationException.class)
public ResponseEntity<CommonExceptionResponse> handleDataIntegrityViolationException(Exception e) {
return ResponseEntity.status(HttpStatus.CONFLICT)
.body(new CommonExceptionResponse(e));
}
}

Choose a reason for hiding this comment

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

익셉션 핸들러를 사용해서 예외처리 하셨군요 👍
에러 응답용 리스폰스까지 만들어주셔서 깔끔하고 좋은 것 같습니다!

@@ -9,7 +9,10 @@
@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;

@Column(unique = true)

Choose a reason for hiding this comment

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

데이터상에 바로 unique 필드를 활용해서 간단히 구현하셨네요~ 👍

Comment on lines +37 to +54
/**
* Given 중복이름으로 지하철역 생성
* When 같은 이름으로 지하철역 생성 요청을 하면
* Then 지하철역 생성이 실패한다.
*/
@DisplayName("중복이름으로 지하철역 생성")
@Test
void createDuplicatedNameStation() {
// given
String 강남역_이름 = "강남역";
StationTestStep.지하철역_생성하기(강남역_이름);

// when
ExtractableResponse<Response> response = StationTestStep.지하철역_생성하기(강남역_이름);

// then
assertThat(response.statusCode()).isEqualTo(HttpStatus.CONFLICT.value());
}

Choose a reason for hiding this comment

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

리팩토링을 잘 해주셔서, 깔끔하게 중복 이름 생성을 테스트해주셨네요 💯

ExtractableResponse<Response> response = LineTestStep.지하철_노선을_생성한다(color, name);

// then
assertThat(response.statusCode()).isEqualTo(HttpStatus.CONFLICT.value());

Choose a reason for hiding this comment

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

assert하는 부분도 Step메소드에 분리해보면 어떨까요?
ex )

생성_요청이_실패(response); 
// 혹은 다른 케이스의 assertion도 
생성_요청이_성공한다(response);

해당 부분도 메소드로 분리된다면 가독성이 올라가면서

/**
     * Given 지하철 노선 생성을 요청 하고
     * When 같은 이름으로 지하철 노선 생성을 요청 하면
     * Then 지하철 노선 생성이 실패한다
     */
  /**
     * Given 지하철 노선 생성을 요청 하고
     * When 생성한 지하철 노선 조회를 요청 하면
     * Then 생성한 지하철 노선을 응답받는다
     */

같은 주석들을 제거해도 좋을 것 같은데 대훈님 의견은 어떠신 지 궁금합니다! 😃

Copy link
Author

Choose a reason for hiding this comment

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

확실히 코멘트 제거되면서, 코드만 한눈에 보이는게 좋겠네요!
그렇게 리팩터링 해보겠습니다 :)

@junwoochoi junwoochoi merged commit 86505a6 into next-step:daehungwak Jan 25, 2022
DaehunGwak added a commit to DaehunGwak/atdd-subway-map that referenced this pull request Jan 30, 2022
junwoochoi pushed a commit that referenced this pull request Feb 2, 2022
* refactor: 테스트 검증 스텝 분리 후 리팩터링

#235 (comment)

* refactor: 지하철역_생성_후_아이디_추출하기 스텝 추가 및 리팩터링

* feat: 지하철 노선 생성 시 필요한 인자 추가하기

- 테스팅용 DTO LineTestRequest 를 추가
- 위 클래스 관련된 LineTestStep, LineAcceptanceTest 부분 수정

* refactor: test step 패키지 변경

* feat: 지하철 구간 등록 기능 성공 시나리오 추가

* fix: 노선 생성 시 구간 초기화 안되어 수정

* fix: 지하노선 삭제 시 Section 기본 생성자 없어 실패 문제 수정

* refactor: Station 엔티티 불필요한 노선 필드 삭제

* feat: 구간 등록 시 상행선역이 노선의 하행 종점역이 아닐 때 실패 기능 추가

* feat: 구간 등록 시 하행역이 해당 노선에 등록되어 있을 때 실패 기능 추가

* feat: 구간 제거 기능 추가

* refactor: 구간 삭제 시 orphanRemoval 활용하도록 리팩터링

* feat: 등록된 구간을 통해 역 목록 조회 기능 추가

* refactor: Section 생성 책임을 Line 에 위임

#284 (comment)

* refactor: SectionService 메서드를 LineService 메서드로 이동

#284 (comment)

* refactor: LineService 내 중복되는 조회 메서드 추출

#284 (comment)

* refactor: Section 인수테스트 공통 부분 BeforeEach 로 추출

#284 (comment)

* refactor: Sections 추출 및 리팩터링

#284 (comment)

* refactor: Section의 자식 엔티티는 영속성 전이 사용하지 않도록 수정

- 영속성 전이: 부모의 연관관계를 가지 자식을 함께 저장하고 싶을 때 사용
- 내가 사용하는 Section은 이미 Line을 주체로 작성 된 상태
- 따라서, Section에서는 자식 엔티티들을 관리하고 있는 상태가 아니므로 CascadeType.PERSIST를 사용할 필요가 없음
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants