[2단계 - 웹 자동차 경주] 도기(김동호) 미션 제출합니다. #104

kdkdhoho · 2023-04-15T12:19:58Z

안녕하세요 조앤! 좋은 하루 보내고 계신가요?
생각보다 2단계가 빠르게 마무리되었네요.
그럼 이번에도 잘 부탁드립니다 🙇‍♂️

궁금한 점

인텔리제이 설정에서 build tools를 gradle에서 intellj로 변경하고 WebApplication을 실행하면 에러가 발생합니다.
에러 메시지로는, PlayRequest에 기본 생성자가 존재하지 않아 발생하는 에러로 판단됩니다.
다시 build tools를 gradle로 변경하면 문제 없이 잘 실행됩니다.

PlayRequest 내의 필드들을 모두 불변으로 하고 싶은데, 방법이 없나요??
무엇이 문제인지 관련된 키워드를 알려주시면 찾아보겠습니다 !

- 자동차가 최소 2개 이상 생성되었는가? - 자동차 이름에 중복이 존재하지 않는가?

seovalue

안녕하세요, 도기! 조앤이에요ㅎㅎ
리팩터링까지 깔끔하게 해주셔서 이정도면 머지해도 될 것 같아요ㅎㅎ
하지만~ 이전 코멘트에 대해 답변도 몇가지 달아보았고, 추가로 몇가지 질문도 드려봤는데 답변이 궁금해서 마지막 Request Changes 남겨요! 👍

seovalue · 2023-04-17T11:28:09Z

src/main/java/racingcar/controller/RacingController.java

@@ -18,6 +19,6 @@ public void run() {

        racingGame.playRacing(cars, trialCount);

-        outputView.printResult(cars.getCars(), cars.winnerNames());
+        outputView.printResult(PlayResponse.from(cars));


리뷰 반영 👍

seovalue · 2023-04-17T11:28:44Z

src/main/java/racingcar/repository/GameDao.java

    public static final String TABLE_NAME = "game";
    public static final String KEY_COLUMN_NAME = "id";

    private final JdbcTemplate jdbcTemplate;
    private final SimpleJdbcInsert insertActor;

+    @Autowired


@Autowired를 붙여주신 이유가 있나요? 붙였을 때와 그렇지 않을 때의 동작이 달라지나요?

개인적인 취향으로써, 암시적으로 되어있더라도 명시적으로 적어주는 것을 선호하다 보니 붙이게 되었습니다.

현재는 생성자가 1개이므로 붙였을 때와 붙이지 않았을 때 동작은 같습니다.
하지만 생성자가 2개 이상인 경우에는 하나의 메서드에만 @Autowired를 붙여주어 의존성 주입 할 방식을 선택해야 합니다.

혹시 실무에서는 생성자가 1개일 때 @Autowired를 붙여주나요? 혹은 그렇지 않나요??

오홍!ㅎㅎ 그건 팀의 컨벤션에 따라 다를 것 같아요!

seovalue · 2023-04-17T11:29:30Z

src/main/java/racingcar/repository/GameDao.java

@@ -31,9 +35,9 @@ public long insert(final int trial_count) {
        return insertActor.executeAndReturnKey(params).longValue();
    }

-    public int countAll() {
+    public Optional<Integer> countAll() {


count는 Int를 반환하는 값인데 optional을 반환하기로 결정하신 이유가 궁금해요. 실제 쿼리에서도 count() 함수를 실행했을 때 조건에 해당하는 값이 없다면 null이 아닌 0을 뱉는데, 동일하게 동작하도록 하는건 어떨까요?

인텔리제이에서 Integer에서 언박싱할 때 NPE가 발생할 수 있다고 경고하기에 Optional로 반환했습니다.
하지만 실제로 null이 아닌 0을 반환하는 것을 확인했고, 쿼리 실행 결과를 바로 return 하도록 수정하였습니다!

seovalue · 2023-04-17T11:30:05Z

src/main/java/racingcar/response/PlayResponse.java

-    public PlayResponse(final List<String> winners, final List<CarDto> racingCars) {
-        this.winners = convertToString(winners);
+    private PlayResponse(final List<String> winnerNames, final List<CarDto> racingCars) {
+        this.winners = String.join(",", winnerNames);


개인적으로 다시 메서드를 풀어서 적어주신 게 좀 더 직관적인 것 같네요 👍

seovalue · 2023-04-17T11:32:05Z

src/main/java/racingcar/response/PlayResponse.java

+        return new PlayResponse(winners, carDtos);
+    }
+
+    private static void decideWinner(final List<String> winners, final RecordDto recordDto) {


내부에서 파라미터로 들어온 리스트의 값을 변경시켜주는 것보다 recordDto.getPlayerName()을 반환해 외부에서 그 값을 컬렉션에 넣어주는 건 어떨까요? (의견이 궁금한 거예요!)

죄송하지만 이해가 잘 되지 않습니다 😭

decideWinner 메서드의 역할이 애매해 보여서 남겼는데 제가 헷갈리게 코멘트를 남겼군요!!

public static PlayResponse from(final List<RecordDto> recordDtos) { List<String> winnerNames = recordDtos.stream() .filter(RecordDto::isWinner) .map(RecordDto::getPlayerName) .collect(Collectors.toList()); List<CarDto> carDtos = recordDtos.stream() .map(CarDto::from) .collect(Collectors.toList()); return new PlayResponse(winnerNames, carDtos); }

헉.. 이렇게 좋은 방법이 있었네요 😅

seovalue · 2023-04-17T11:32:49Z

src/test/java/racingcar/repository/GameDaoTest.java

 import java.util.Arrays;
 import java.util.List;

 @TestPropertySource(locations = "/application.properties")
 @SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.NONE)
+@Transactional
+@TestInstance(TestInstance.Lifecycle.PER_CLASS)


seovalue · 2023-04-17T11:35:53Z

src/main/java/racingcar/controller/WebController.java

-    @PostMapping("/plays")
-    public PlayResponse plays(@RequestBody final PlayRequest playRequest) {
-        return gameService.playRacing(playRequest.getNames(), playRequest.getCount());
+    @PostMapping(value = "/plays", consumes = MediaType.APPLICATION_JSON_VALUE, produces = MediaType.APPLICATION_JSON_VALUE)


도기! 너무너무 잘 구현해주셨는데 이번에 딱 POST /plays와 GET /plays에 대해 RestAssured 혹은 mockMvc를 활용해 E2E 테스트를 도전해보시는 것은 어떨까요?

kdkdhoho · 2023-04-19T07:56:01Z

안녕하세요 조앤! 2단계 첫 리뷰요청 보내고 꽤 많은 날이 지났네요.
사실 2단계 빠르게 끝날 줄 알았는데, E2E 테스트라는 난관에 봉착과 동시에 여러 일정이 몰려서 이제야 재리뷰 요청 보냅니다.
E2E 테스트.... 너.......무............ 어렵네요 😭

이번에 리뷰 반영하면서 궁금했던 점들 적어보겠습니다.

1. E2E 테스트의 목적과 범위?

이번에 처음으로 E2E 테스트를 접했고 도전해봤습니다.
처음엔 String requestJson과 String responseJson을 만들어, MockMvc를 통해 Input과 Output을 검증하려고 했습니다. 하지만 자동차 경주를 하는 로직에 랜덤성이 존재하기에 이를 제어하고자 테스트용 Service와 RacingGame을 추가하려고 했습니다.
하지만 마음처럼 잘 되지 않아 주변 크루들에게 물어보니, 대부분 응답되는 json의 key값이 2개인지? key에 'winners', 'racingCars'가 포함되는지만 테스트하더라구요. 여기서 많은 고민이 생겼습니다.

E2E 테스트의 목적은 무엇인가요? 그리고 범위가 어떻게 되나요?

2. 테스트 시, 자동으로 테이블 초기화해주는 방법은 없나요?

이번에 Dao 테스트와 Controller 테스트를 하면서 테스트를 하나씩 실행하면 통과되지만, 전체 테스트를 실행하면 많이 깨지더라구요.
주로 깨지는 이유가 Game 테이블의 id가 AUTO_INCREMENT로 되어 있어서 @Transactional로 데이터를 돌려놓아도, id 값은 다시 낮은 값부터 들어가는 것이 아닌, 이전 id의 +1 값이 들어가서 제가 기대하던 경우와 달라지기 때문이었습니다.
그래서 beforeEach에서 항상 모든 테이블을 동일한 환경으로 세팅해주어야겠다고 생각했습니다.

그런데 현재는 코드에서 쿼리문을 실행시키는 방식으로 하였는데 코드가 굉장히 길어질뿐더러 유지보수 측면에서도 매우 안좋다고 생각합니다. 혹시 스프링에게 schema.sql에 있는 쿼리문을 실행시켜달라고 하는 방법이 없을까요??

seovalue

안녕하세요, 도기! 조앤이에요ㅎㅎ
controller 테스트까지 작성해주시느라 고생 많으셨어요~ 앞서 말씀드렸듯이 요구사항은 모두 만족하기에, 이제 머지해도 될 것 같아요!! 👏
다만 남겨주신 질문에 대해 제 의견 남겨두었으니, 꼭 확인하시고 도기도 의견 남겨주세요! (댓글 남기시면 저(@seovalue) 멘션해주세요.. 그냥 댓글달면 알림이 안와서 확인이 안되더라구요 😢)
고생 많으셨어요!!

seovalue · 2023-04-19T14:28:18Z

build.gradle

@@ -22,6 +22,7 @@ dependencies {

 	// test
 	testImplementation 'org.springframework.boot:spring-boot-starter-test'
+	testImplementation 'io.rest-assured:rest-assured:4.4.0'


버전을 선택하는 기준이 무엇이었나요?

어.. 솔직히 말씀드리면 기억이 나지 않습니다 😭 아마 RestAssured 사용법 관련 포스팅을 보다가 무의식적으로 설정한 것 같아요.

특별한 이유가 있는 게 아니면 따로 버전을 명시하지 않는 것이 좋은가요?

아뇨! 버전 명시는 필요한데, 특정 버전을 선택하는 기준이 궁금했어요! 버전마다 제공되는 기준도 다르고 하니깐요~

seovalue · 2023-04-19T14:31:23Z

src/test/java/racingcar/repository/GameDaoTest.java

@@ -17,16 +16,48 @@
 @TestPropertySource(locations = "/application.properties")
 @SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.NONE)
 @Transactional
-@TestInstance(TestInstance.Lifecycle.PER_CLASS)
 class GameDaoTest {


도기~ 2번째 질문에 대한 답 여기서 할게요!
@Sql 애노테이션에 대해 알아보시면 좋을 것 같아요ㅎㅎ

@TestPropertySource(locations = "/application.properties") @SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.NONE) @Transactional @Sql("/schema.sql") class GameDaoTest { @Autowired private GameDao gameDao; @Autowired private JdbcTemplate jdbcTemplate; @BeforeEach void beforeEach() { initGameTable(); initGameTableData(); } // 기존 코드 }

https://docs.spring.io/spring-framework/docs/current/reference/html/testing.html#testcontext-executing-sql-declaratively

seovalue · 2023-04-19T14:52:30Z

src/test/java/racingcar/controller/WebControllerTest.java

+class WebControllerTest {
+
+    @Autowired
+    private MockMvc mockMvc;


우선 mockMvc를 사용하기로 선택하신 이유가 궁금해요. mockMvc는 RestAssured와 어떤 차이가 있나요?

하지만 열심히 적용해주셨으니 참고 자료도 남겨요ㅎㅎ

[도기 질문] e2e 테스트의 목적은 무엇인가요? 범위가 어떻게 되나요?

앞으로 다양한 테스트를 할테니, 우선 참고 자료를 먼저 링크해요.
단위 테스트 vs 통합 테스트 vs 인수 테스트
인수 테스트와 E2E 테스트 차이

E2E 테스트의 목적은 테스트하고자 하는 기능이 필요로 하는 모든 구성 요소를 거쳐 최종적으로 우리가 원하는 결과를 반환하는지를 확인하는 것이에요. [목적]
즉, 지금 구조로 보자면 Controller - Service - Dao 까지를 거쳐 최종적으로 Controller 에서 원하는 json 응답을 뱉어내는지를 검증하는거죠. [범위]

지금 테스트해주신 것을 보자면, 저는 얼추 E2E 테스트에 근접하다고 생각해요. 하지만 이런 고민을 해주신 이유가 결과적으로 winners에 누가 들어있는지, racingCars에는 어떤 값이 담겨있는지까지 검증해야 완벽한 검증이라고 생각하셔서 그런 것 같아요.

우리가 컨트롤할 수 없는 부분이 비즈니스 로직에 담겼을 때 이러한 것은 테스트 하기 어려운 코드 라고 생각해요.

관련된 글

1. 테스트하기 좋은 코드 - 테스트하기 어려운 코드

2. 테스트하기 좋은 코드 - 제어할 수 없는 코드 개선

[아래는 좀 더 나아간 내용이라.. 남길까 고민되었지만 그래도 남겨보아요!]

이럴 때엔 어쩔 수 없지만 위에 jojoldu 님이 작업해주신 것처럼 리팩터링해볼 수도 있고, 아니면 랜덤에 대해서만 mocking을 사용하는 수도 있어요.

mocking이라 함은 실제 객체가 아닌 가짜 객체를 만들어 우리가 원하는 응답을 뱉도록 하는 객체예요. 테스트가 특정 케이스에 의존하거나 우리가 값을 컨트롤하지 못하는 경우에 사용할 수 있어요. 즉 지금과 같은 케이스에는 GameService를 모킹해서 playRacing을 호출했을 때 이런 값을 반환해달라고 모킹을 해야할거예요.

하지만 모킹을 사용하게 된다면 실제 동작을 테스트하는 것이 아니기 때문에 신뢰도가 떨어질 수도 있겠죠. 이런 부분을 보완하기 위해서 단위 테스트가 있다는 생각도 들구요ㅎㅎ 테스트에 대해서 어떻게 해야 신뢰도를 높일 수 있을지 많이 고민해보시면 좋을 것 같아요~

처음엔 ~~스프링 학습 테스트에 예제 코드가 존재하여 그나마~~ 익숙한 RestAssured를 사용했습니다.
하지만 E2E 테스트를 위해 결과적으로 winners에 누가 들어있는지, racingCars에는 어떤 값이 담겨있는지까지 검증하고 싶었고,
따라서 GameService와 RacingGame 객체를 mocking하기 위해 결과적으로 MockMvc를 사용했습니다.

하지만 이렇게 Service를 mocking하면 WebController와 GameService와의 의존성이 끊어지기에, 결과적으로 E2E 테스트가 아닌, controller의 단위 테스트가 된다는 의견을 들었습니다. 따라서 기존 생각을 내려놓고 단순히 json의 key값으로만 테스트했습니다.
이 과정에서 익숙해진 MockMvc를 사용했습니다.

추가로 RestAssured도 사용해보고 싶었지만, 현재 웹 자동차 경주미션에서 스프링 테스트에 집중하기보단 Spring MVC, Spring JDBC, Spring Core에 집중하고 싶었습니다.

MockMvc와 RestAssured의 차이로는 원하는 Controller와 Mocking한 객체를 이용할 것이냐 혹은, 실제 프로덕션 코드를 전부 사용하냐인 것 같아요.

Spring 테스트 정말 공부할 것도 많고 사용하기 어렵군요 ,,
상세하고 친절히 답변달아주셔서 너무 감사합니다 🥲
열심히 공부해볼게요..!

seovalue · 2023-04-19T14:52:48Z

src/test/java/racingcar/controller/WebControllerTest.java

+
+@SpringBootTest
+@AutoConfigureMockMvc
+class WebControllerTest {


@seovalue 안녕하세요 조앤!
남겨주신 코멘트 모두 확인 후 답변 남겨두었습니다. 특히, 테스트 관련 답변에서 너무 감동이었어요 🥲

이전에 남겨주신 코멘트에 대한 추가적인 질문이 있었는데, 따로 남겨주시지 않은 것들이 있습니다!
아래에 링크 남겨드리겠습니다 :)
#104 (comment)
#104 (comment)
#27 (comment) // 요거는 답변에 대한 피드백을 듣고 싶어서 남깁니다!
#27 (comment)

항상 감사합니다.

kdkdhoho added 30 commits April 14, 2023 13:35

docs: 요구사항 재정리

b959f9d

chore: DB 연결 설정 수정

9925d70

rename: 디렉터리 구조 변경

576820a

refactor: Cars가 List<Car>를 갖도록 수정

bf704ef

rename: Application 이름 수정

e6cf884

feat(Cars): 예외 검증 기능 구현

84eba7b

- 자동차가 최소 2개 이상 생성되었는가? - 자동차 이름에 중복이 존재하지 않는가?

refactor: validator 객체 삭제 및 도메인 내에서 검증하도록 변경

f697e12

refactor(TrialCount): 변수명 수정

c12a4b0

refactor(PlayResponse): 우승자 이름 타입 변경

4837a0d

refactor: GameService에서 모든 로직 수행

5cb5555

chore: ddl 수정

c23c678

refactor: Vehicle 객체 제거

b920ac6

test: RecordDaoTest 테스트 통과안되는 내용 수정

f52b9c5

test(CarTest): 테스트 깨지는 것 수정

5383de8

refactor: 최소 시도 횟수를 1로 기능 변경

2c48461

feat: WebController에 @Valid 추가

e5e7492

docs: 2단계 요구사항 정리

a26ce7c

refactor: WebController 디테일한 요소 추가

9a167f5

feat: gameId로 모든 행을 조회하는 기능 구현

e47cf42

feat: game 테이블의 모든 행의 수를 반환하는 기능 구현

a1ef663

feat: 자동차 경주 게임 이력 조회 기능 구현

bcbf766

refactor: Console Application에서 플레이의 중간 과정을 출력하는 로직 제거

8d92221

refactor: Console Application에서 우승자와 player 별 최종 이동거리를 출력하도록 변경

0be0d40

refactor: RacingController, OutputView, ConsoleApplication 코드 개선

2dacc0d

feat(Car): 같은 거리인지 반환하는 기능 구현

06a4269

feat(Cars): 우승자 리스트 구하는 메서드 개선

8d9f906

feat(Cars): 우승자의 이름을 반환하는 기능 구현

2eefa6d

refactor: 중복 코드 제거

f9fb4f9

feat(PlayRequest): 예외 메시지 추가

9af3ceb

feat: BindExceptionHandler, RuntimeExceptionHandler 구현

25e3daf

kdkdhoho added 15 commits April 16, 2023 14:44

chore: Test DB 연결 정보 수정

60ba600

refactor: GameDao, RecordDao 생성자에 @Autowired 추가

9695388

refactor: Game 테이블 내 데이터 개수 조회 메서드, 반환 타입을 Optional<Integer>로 수정

0c8f3d3

test: 실패하는 테스트 수정 및 DB 테스트 코드 개선

9501f2f

refactor: 메서드명 변경

9514009

feat: CarDto 정적 팩터리 메서드 추가

e59588a

feat: CarDtos 구현

695a188

refactor: 중복 로직 제거

33752da

feat: PlayResponse 정적 팩터리 메서드 추가

fc58377

refactor: 코드 중복 제거

565b478

remove: CarDtos 제거

bb1f018

refactor: PlayResponse @tostring 제거

5035114

chore: ddl NOT NULL 키워드 추가

0e0e9b8

refactor: @tostring 제거

f77c785

refactor(PlayRequest): 기본 생성자 만들어주는 롬복 애노테이션 제거

f49a1b8

seovalue requested changes Apr 17, 2023

View reviewed changes

kdkdhoho added 11 commits April 18, 2023 13:57

refactor(GameService): 메서드 접근제어자 수정

4f1283b

refactor(WebController): @RequestMapping value 중복 제거

0ea81ce

refactor: GameDao의 countAll 메서드의 반환타입을 Optional에서 int형으로 수정

62cd740

refactor: RacingGame 객체를 Component로 설정하여 GameService에서 의존성 주입받도록 수정

9c82fcf

refactor(GameService): 메서드명, 변수명 수정

9909909

refactor(GameDao): 변수명 수정

39179eb

chore: ddl문 수정

fa2e995

test: 모든 테스트가 동일한 환경에서 수행되도록 beforeEach 작성

06c7de6

refactor(RecordDto): 필요없는 애노테이션 제거

8235671

chore: rest-assured 의존성 추가

1a3ea6d

test: E2E 테스트 추가

a359d63

seovalue approved these changes Apr 19, 2023

View reviewed changes

seovalue merged commit 69c194f into woowacourse:kdkdhoho Apr 19, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[2단계 - 웹 자동차 경주] 도기(김동호) 미션 제출합니다. #104

[2단계 - 웹 자동차 경주] 도기(김동호) 미션 제출합니다. #104

kdkdhoho commented Apr 15, 2023 •

edited

Loading

seovalue left a comment

seovalue Apr 17, 2023

seovalue Apr 17, 2023

kdkdhoho Apr 18, 2023

seovalue Apr 24, 2023

seovalue Apr 17, 2023

kdkdhoho Apr 18, 2023

seovalue Apr 17, 2023

seovalue Apr 17, 2023

kdkdhoho Apr 18, 2023

seovalue Apr 24, 2023

kdkdhoho Apr 28, 2023

seovalue Apr 17, 2023

seovalue Apr 17, 2023 •

edited

Loading

kdkdhoho commented Apr 19, 2023

seovalue left a comment

seovalue Apr 19, 2023

kdkdhoho Apr 20, 2023 •

edited

Loading

seovalue Apr 24, 2023

seovalue Apr 19, 2023

seovalue Apr 19, 2023

kdkdhoho Apr 20, 2023 •

edited

Loading

seovalue Apr 19, 2023

kdkdhoho Apr 20, 2023

[2단계 - 웹 자동차 경주] 도기(김동호) 미션 제출합니다. #104

[2단계 - 웹 자동차 경주] 도기(김동호) 미션 제출합니다. #104

Conversation

kdkdhoho commented Apr 15, 2023 • edited Loading

궁금한 점

seovalue left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seovalue Apr 17, 2023 • edited Loading

Choose a reason for hiding this comment

kdkdhoho commented Apr 19, 2023

1. E2E 테스트의 목적과 범위?

2. 테스트 시, 자동으로 테이블 초기화해주는 방법은 없나요?

seovalue left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kdkdhoho Apr 20, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kdkdhoho Apr 20, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kdkdhoho commented Apr 15, 2023 •

edited

Loading

seovalue Apr 17, 2023 •

edited

Loading

kdkdhoho Apr 20, 2023 •

edited

Loading

kdkdhoho Apr 20, 2023 •

edited

Loading