-
Notifications
You must be signed in to change notification settings - Fork 177
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
[Spring 체스 - 2단계] 오리(최진영) 미션 제출합니다. #425
Conversation
네 아주 좋은 고민이네요. 저도 테스트 관련한 부분이 아직도 가장 어렵고 고민되는 부분인데요.
위와 같은 고민은 항상 하고 있고 그에 따라 현재는 아래와 같은 선택을 하고 있습니다.
그런데 서비스 테스트에서 저장소를 Mocking 할지에 대한건 더 깊이 고민을 해봐야 할 것 같습니다. |
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.
오리 안녕하세요.
2단계 미션 잘 구현해주셨네요!
테스트 코드도 꼼꼼하게 잘 작성해주신게 인상적이었습니다!
그리고 테스트 코드를 작성하며 생긴 고민과 오리의 생각을 잘 정리해주셔서 좋았습니다.
해당 부분에 대해 코멘트 남겼으니 확인해주세요~ (테스트에 대한 오리의 생각을 듣고 싶어 아직 merge는 하지 않을게요!😁)
(+ 현재 체스말을 움직이면 화면이 깜박이는데 왜 그런걸까요? 움직일때마다 화면을 재로드 해오는건가요?)
long chessGameId = chessGameRoomService.createNewChessGame(request.toNewChessGameRoom()); | ||
return ResponseEntity.created(URI.create("/chessgames/" + chessGameId)).build(); | ||
} |
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.
HTTP 규약을 꼼꼼히 보신게 느껴지네요! 201 응답 👍
|
||
public class ChessGameRoom { | ||
|
||
private final Long id; | ||
private final String title; | ||
private final String password; | ||
private final Turn turn; | ||
|
||
public ChessGameRoom(Long id, String title, String password, Turn turn) { | ||
Objects.requireNonNull(title, "title은 null이 들어올 수 없습니다."); | ||
Objects.requireNonNull(password, "password는 null이 들어올 수 없습니다."); | ||
Objects.requireNonNull(turn, "turn은 null이 들어올 수 없습니다."); | ||
this.id = id; | ||
this.title = title; | ||
this.password = password; | ||
this.turn = turn; | ||
} | ||
|
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.
DB에서 ChessGameRoom을 조회해서 도메인 로직을 수행하는 객체를 만드셨네요 👍
|
||
@SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT) | ||
class ChessGameRoomControllerTest { | ||
|
||
@Autowired | ||
private ChessGameDao chessGameDao; | ||
|
||
@LocalServerPort | ||
private int port; | ||
|
||
@BeforeEach | ||
void setUp() { | ||
RestAssured.port = port; | ||
} | ||
|
||
@Test | ||
@DisplayName("모든 체스방의 리스트 반환") | ||
void findAllChessGameRoom() { | ||
RestAssured.given().log().all() | ||
.contentType(MediaType.APPLICATION_JSON_VALUE) | ||
.when().get("/chessgames") | ||
.then().log().all() | ||
.statusCode(HttpStatus.OK.value()); | ||
} |
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.
컨트롤러 테스트까지 꼼꼼하게 작성해주신 것 좋네요💯
안녕하세요 소니 :) 주셨던 코멘트에 대한 제 생각 comment 더해서 풀리퀘 드려요 😄 피드백 주셨던 내용과 공유해주신 글, 그리고 관련해서 여러 글을 보고 고민했던 부분들을 조금이나마 다시 정리할 수 있었어요 :) 사실 아래 부분에 대해서는 이견이 없는 것 같아요.
그럼 문제 상황은 다시 총 4가지로 분류할 수 있을 것 같아요.
1. 통합 테스트
2. 슬라이싱 테스트
3. Mock vs Fake그럼 필요한 의존성 주입을 어떤 객체로 하느냐에 대한 건 이 두 방법에 갈리게 되었어요. Mock의 경우 사실 예전에 한참 단위테스트에 미쳐(?)있을 때 모든 테스트를 단위테스트로 커버리지를 채우고자했을 때 사용했던 방법이었어요 ㅎㅎ….
남은 방법은 이제 Fake객체 한가지 입니다. 실제 환경은 아니지만 실제 환경과 최대한 맞게 구현할 수 있는 테스트 방법이에요. 개발자가 구현한 내용이 실제 환경과 최대한 맞다면 mock과는 다르게 믿을 수는 있거든요. 정리를 하자면 이런 내용들때문에 대부분의 상황에서는 Fake를 고려하게 될 것 같아요. 테스트는 “속도”보다는 “실패 케이스에 대한 보장”이 먼저이긴 하지만 Fake도 충분히 이야기한것을 바탕으로 실패 케이스에 대한 보장을 해주고, Service는 연결과 로직에 대한 테스트만 해주면되기 때문에 통합테스트하기 아깝기도 하구요. 다만 위에도 이야기한 만약 Fake 객체도 만들기 힘든 상황이라면 어쩔수 없이(ㅜㅜ) 해당 객체에 대해서만 Mocking을 할 것 같아요. 감사합니다 🙇♂️ |
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.
오리 안녕하세요.
테스트 관련해서 오리의 생각을 잘 정리해주셨네요!💯
앞으로의 미션에서 오리가 생각한 기준대로 테스트를 작성해보면 좋을 것 같아요~
이번 미션은 여기서 Approve 하겠습니다. 하나 더 선택 미션을 드리자면, 현재 구현해주신 api 이외의 경로로 조회 요청이 들어왔을 때에 대한 처리는 안 되어 있는 것 같은데요. 404 ERROR에 대한 처리를 어떻게 할지, 커스텀 페이지를 어떻게 보여줄지에 대한 고민과 적용도 한 번 해보시면 좋을 것 같아요 :) 이 부분은 다음 미션을 진행할 때 적용하셔도 됩니다. 이번 미션도 수고 하셨어요! 👍
안녕하세요 소니 :)
해보고 싶은 부분들은 많지만 욕심에 비해서 공부를 하는 속도가 느리다보니 아직까지 부족한게 많아보이는 이번 미션인 것 같아요 😢
스프링 사용해보기가 이번 미션의 목적인만큼 그래도 기본에 충실하자라는 마음으로 미션을 진행했습니다 :)
개발하면서 느꼈던 내용 comment 더해서 풀리퀘 드려요 😄
1. 이전 피드백
2. 통합 ? 단위?
@SpringBootTest
를 사용해서 스프링 컨텍스트를 모두 올리기 때문에 실제 환경과 완전히 비슷한 환경에서 진행하는 방식이긴해요.Controller
,Service
,Dao
에 대해서 어떤 테스트를 진행해야하는지 고민을 했었어요. 테스트를 조금 더 효율적으로 할 수 있는 방법이 없을까하구요.Dao의 경우 다른 기능이 전부 필요없이 Jdbc에 관련된 빈들만 필요하다라고 생각했어요. 또한 DB와 가장 밀접하여 존재하는 객체이기 때문에 DB에 대한 의존성없이 단위테스트만 진행하는 것에 대한 의미를 찾지 못했어요. 실제 DB에 접속해서 테스트가 잘되는지를 테스트해야 Dao에 대한 안정성을 보장한다라고 생각했었기 때문이에요.
Controller의 경우 어떻게보면 Dao랑 마찬가지로
@WebMvcTest
슬라이싱 테스트를 고민을 했었는데요. 결국은 아무리 슬라이싱테스트를 진행한다라고하더라도 실제 환경이랑 완전히 유사한 통합테스트가 필요하다라고 생각했고, 거기에 가장 적합한 것이 외부와 가장 밀접하게 기능이 추상화된 controller라고 생각했어요. 실제 사용자가 사용하는 관점에서 테스트할 수 있는 계층인거죠.아직 인수테스트나 통합테스트의 범위가 어디까지인지는 미션을 진행하면서 또 고민해봐야할 문제이고, 컨텍스트 로딩이 많아질수록 더 느려진다는걸 감안하면 슬라이싱 테스트를 생각하겠지만 지금 범위 내에서는
@SrpingBootTest
까지도 충분하다고 생각했어요.Service는 지금도 많이 고민되는데요. 결국 각각의 계층에 비해서 Service는 연결다리로써의 역할만 한다라고 생각했어요.
Controller는 웹과의 통신, Dao는 DB와의 연결, 그리고 도메인은 각각의 비즈니스 로직을 갖는데에 비해 Service는 단순히 필요한 도메인 객체들을 호출해서 기능을 사용했었거든요. 그래서 Service는 Spring에 종속적이기보다는 객체들을 에러없이 잘 호출하고 있는지만 테스트하면 되지 않을까라는 생각이에요.
현재는 아직 익숙치 않아서
@JdbcTest
를 사용해서 의존관계를 맺어두었는데 앞서 말한 내용대로라면 Service는 Spring에 대한 의존성 없이 단위테스트로 두고, Dao에 대한 의존성 주입은 실제 Dao가 아닌 Fake 혹은 Mock Dao를 두어서 Service에 대한 연결 기능에 대한 테스트만 진행하는게 좋을 것 같다라는게 제가 내린 결론이었어요.조금 장황하게 이야기한 것 같은데... 소니는 어떤식으로 통합, 단위테스트를 진행하시는편이신지 궁금합니다 :)
감사합니다 🙇♂️