-
Notifications
You must be signed in to change notification settings - Fork 303
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
Step1 - 지하철역 인수 테스트 작성 #1051
Step1 - 지하철역 인수 테스트 작성 #1051
Conversation
저도 추상화 수준을 맞추어 분리하는 방향이 좋다고 생각해요! :) 그래야 코드를 편하게 읽을 수 있더라구요. 👍 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수민님! 1단계 미션 수고하셨습니다! 👍
코멘트 남겨두었으니, 다음 단계 진행하기 전에 확인해주세요.
이만 머지하겠습니다.
@Test | ||
void accessGoogle() { | ||
// TODO: 구글 페이지 요청 구현 | ||
ExtractableResponse<Response> response = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0단계 리뷰사이클 연습 없이 1단계 미션 진행하셨군요! ㅎㅎ
여기도 작성 부탁드려요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
제가 이 PR 리뷰 요청을 안 드렸네요...!
@DisplayName("지하철역을 생성한다.") | ||
@Test | ||
void create_station() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
given, when, then 테스트 시나리오가 사라졌어요 😭
/**
* When 지하철역을 생성하면
* Then 지하철역이 생성된다
* Then 지하철역 목록 조회 시 생성한 역을 찾을 수 있다
*/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
다음 PR 부턴 살려오겠습니다..!
// given | ||
createStation(createBody("강남역1")); | ||
createStation(createBody("강남역2")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
반복해서 쓰이는 매직리터럴은 상수로 선언하면 어떨까요?
} | ||
|
||
private ExtractableResponse<Response> createStation(Map<String, String> params) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
반복되는 api 요청부를 private 메소드로 추출! 💯
assertThat(stationNames).containsAnyOf("강남역2"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
containsAnyOf는 파라미터를 array로 받을 수 있어요.
assertThat(stationNames).containsAnyOf("강남역1", "강남역2");
안녕하세요!
인수 테스트를 작성하다보니, 반복되는 부분(생성, 조회) 이 발생해서 이 부분을 별도의 메서드로 분리했습니다.
삭제같은 경우는 한 번밖에 나오지는 않았지만, 추상화 수준(..?)을 맞춰야지(생성이 메서드로 분리되어 있어) 읽기가 더 편한 것 같아 삭제도 별도의 메서드로 분리하였습니다.