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

[MVC 구현하기 - 3단계] 배럴(김나은) 미션 제출합니다. #75

Merged
merged 9 commits into from
Sep 16, 2021

Conversation

knae11
Copy link

@knae11 knae11 commented Sep 15, 2021

안녕하세요 에드!

테스트 코드도 짜보려고 시도했는데 쉽지가 않네요ㅠㅠ
스프링테스트할 때 빈을 가상으로 띄우는 것처럼 application이 실행되야 restAssuredTest도 실행이 되더라구요!
혹시 테스트 할 수 있는 방법이나 부분이 있다면 힌트 주시면 감사드려요!ㅎㅎㅎ

지난 피드백 도움이 많이 되었어요ㅎㅎㅎ 이번에도 잘 부탁드릴게요! :)

Copy link

@sjpark-dev sjpark-dev left a comment

Choose a reason for hiding this comment

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

안녕하세요 배럴~!!

View 구현과 레거시 코드 변환 작업을 너무 잘하셨네요! 👍👍👍
덕분에 JsonView를 구현한 부분에서 많이 배울 수 있었습니다.

질문하신 부분에 대한 답변을 남겼습니다.

- [x] HTML 이외에 REST API를 지원할 수 있도록 JSON 뷰를 추가
- [x] UserController 추가해서 정상동작하는지 확인
- [x] Legacy MVC 제거
- [ ] 테스트코드 작성

Choose a reason for hiding this comment

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

흐흐흐 테스트 코드 작성 어렵죠 😉

말씀하신 것 처럼 RestAssured는 사용하기 어렵죠...
그래서 저는 Adapter 단에서 테스트를 했습니다.
예를 들면 특정 경로의 컨트롤러 메소드가 ModelAndView를 반환할 때,
그 ModelAndView는 내가 의도한 값 또는 뷰를 갖고 있는지 테스트 했습니다.

테스트에는 정답이 없으니 편하신대로 하는게 제일입니다. 👍

Copy link

@sjpark-dev sjpark-dev Sep 16, 2021

Choose a reason for hiding this comment

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

PR의 빌드가 실패하고 있어요!

image

원인을 파악 해봅시다!

Choose a reason for hiding this comment

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

ControllerScannerTest도 실패하고 있어요!

Copy link
Author

Choose a reason for hiding this comment

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

앗!!ㅋㅋㅋㅋ 컨트롤러를 수정하면서 기존 테스트가 깨지는 것을 확인하지 못했네요! 😱수정하였습니다! :)

Copy link
Author

Choose a reason for hiding this comment

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

testImplement 에 모듈을 넣어줘야 하는군요!!ㅋㅋㅋㅋㅋ 오 에드덕분에 해결했어요~

Comment on lines 66 to 69
@RequestMapping(value = "/logout", method = RequestMethod.GET)
public ModelAndView logout(HttpServletRequest request, HttpServletResponse response) {
return new ModelAndView(new JspView("redirect:/"));
}
Copy link

@sjpark-dev sjpark-dev Sep 16, 2021

Choose a reason for hiding this comment

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

로그아웃 버튼을 누르면 index 페이지로 리다이텍트만 됩니다!
실제로 로그아웃이 되지 않아요!

Copy link
Author

Choose a reason for hiding this comment

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

어머... 이것도 제가 옮기다가 놓친 부분을 잘 잡아주셨네요!! 역시 에드!!

public void render(Map<String, ?> model, HttpServletRequest request, HttpServletResponse response) throws Exception {
public void render(Map<String, ?> model, HttpServletRequest request, HttpServletResponse response)
throws Exception {
ObjectMapper objectMapper = new ObjectMapper();

Choose a reason for hiding this comment

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

매 요청마다 ObjectMapper를 만들면 성능상 좋지 않다고 합니다!

Copy link
Author

Choose a reason for hiding this comment

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

오!! 그렇군요!! private static final 로 빼주었어요 :) 👍

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

Copy link

@sjpark-dev sjpark-dev left a comment

Choose a reason for hiding this comment

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

안녕하세요 배럴~!!

작성하신 코드 잘 읽었습니다.
기능과 테스트 코드 모두 잘 돌아가네요. ✔
더 이상 드릴 피드백이 없으니 이만 머지 하겠습니다. 👍

미션 수고하셨습니다.

ModelAndView modelAndView = restController.logout(request, response);

assertThat(modelAndView.getView()).isInstanceOf(JspView.class);
assertThat(session.getAttribute(UserSession.SESSION_KEY)).isNull();

Choose a reason for hiding this comment

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

session을 mock 객체로 선언해서 이 assert는 로그아웃을 하든 안하든 항상 참입니다. 😆

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

class RestControllerTest {

Choose a reason for hiding this comment

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

제가 위에서 테스트에 대해서 너무 축약해서 설명한 것 같아요. 🙇‍♂️

배럴이 작성한 컨트롤러 테스트 코드들은 프레임워크를 전혀 타지 않아요.
컨트롤러 테스트도 좋지만 이번 미션은 mvc 프레임워크를 만드는 미션인 만큼,
직접 구현한 프레임워크의 컴포넌트들(handlermapping, handleradapter 등) 을 테스트하면 더 좋았을 것 같아요. 😆

Dispatcher servlet을 테스트하기는 힘들지만 그 하위 컴포넌트 들은 테스트 할 수 있어요!
Request -> HandlerMapping -> HandlerAdapter -> ModelAndView의 데이터 흐름을 말이죠.

무엇을 대상으로 테스트할지 생각하는 것도 굉장히 중요하다고 생각합니다!

@sjpark-dev sjpark-dev merged commit cc460bb into woowacourse:knae11 Sep 16, 2021
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.

2 participants