5주차: 유효성검사 추가 및 회원관리 #88
Conversation
| import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest; | ||
| import org.springframework.test.context.junit.jupiter.SpringExtension; | ||
|
|
||
| @ExtendWith(SpringExtension.class) |
There was a problem hiding this comment.
안녕하세요!
주신 참고용으로 주신 종립님 블로그에 적혀져 있어서
무심코 적은 것 같습니다
확인해보니 unit5의 라이프사이클에 Test에서 사용할 기능을 확장할때 사용된다고 하는데
현재 DataJpaTest 어노테이션 안에는 이미 해당 어노테이션이 선언되어있어서
사용 할 이유가 없는것 같습니다.
과거 스프링 부트 버전에서는 2.0부터는 있고 1.5은 없는것 보니 그 사이에 추가된 것 같습니다
해당 어노테이션은 삭제하겠습니다
| @Service | ||
| public class UserCreator { | ||
| private final UserRepository userRepository; | ||
| public UserCreator(UserRepository userRepository) { |
There was a problem hiding this comment.
위에 빈 줄 하나를 추가해서 생성자와 속성을 분리하면 좋을 것 같아요
| User user = userRepository.findById(id) | ||
| .orElseThrow(() -> new IllegalArgumentException("해당하는 유저가 없습니다.")); |
There was a problem hiding this comment.
위에서 만든 userReader의 getUser()를 사용해도 좋을 것 같아요
| User updatedUser = userRepository.save(user); | ||
| return updatedUser; |
There was a problem hiding this comment.
우변이 이미 충분히 설명해주고 있는 것 같아서, 변수 선언 없이 바로 반환해줘도 좋을 것 같아요
| @DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class) | ||
| class 유저_정보가_주어지면 { | ||
| private UserRequest USER_REQUEST; | ||
| private UserCreator userCreator; |
There was a problem hiding this comment.
Describe - Context - It에서 Context는 주어진 상황이 달라지는 경우를 명시합니다. 테스트에 필요한 준비는 여기가 아니라 Describe 밑에 혹은 바깥에 있는게 좋을 것 같아요. 오히려 여기에 이런 것들이 있으면 테스트 case마다 달라지나? 생각이 들 것 같아요
| this.userReader = userReader; | ||
| } | ||
| public void deleteUser(Long id) { | ||
| User findeduser = userReader.getUser(id); |
There was a problem hiding this comment.
foundUser라고 할 수 있겠네요. 혹은 단순히 user라고 해도 될 것 같아요. 왜냐하면 변수의 스코프가 짧아서 단순해도 괜찮을 것 같아요.
| import org.springframework.beans.factory.annotation.Autowired; | ||
| import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest; | ||
|
|
||
| @DataJpaTest |
There was a problem hiding this comment.
@DataJpaTest를 쓰면 테스트를 위한 in memory로 동작할거에요. 이렇게 해도 대부분 처리되지만 실제 DB랑은 차이가 있을 수 있어요. 그래서 정말 제대로 테스트하려면 실제 DB랑 테스트하도록 하는 것이 좋을 것 같아요.
| @Nested | ||
| @DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class) | ||
| class updateUser_메서드는 { | ||
| private UserUpdater userUpdater = new UserUpdater(userRepository, new UserReader(userRepository)); | ||
| @Nested | ||
| @DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class) | ||
| class 수정할_유저가_존재한다면 { | ||
| private UserData USER_REQUEST; | ||
| private Long savedUserId; | ||
| @BeforeEach | ||
| void setUp() { | ||
| userRepository.deleteAll(); | ||
| USER_REQUEST = createUserRequest(); | ||
| savedUserId = userRepository.save(USER_REQUEST.toUser()).getId(); | ||
| } | ||
|
|
||
| @DisplayName("해당 유저정보를 수정 후 수정한 유저정보를 리턴한다") | ||
| @Test | ||
| void it_updates_and_returns_user() { | ||
| UserData userUpdateRequest = UserData.builder() | ||
| .name("newName") | ||
| .email("newEmail") | ||
| .password("newPassword") | ||
| .build(); | ||
|
|
||
| User user = userUpdater.updateUser(savedUserId, userUpdateRequest); | ||
|
|
||
| Assertions.assertThat(user.getName()).isEqualTo("newName"); | ||
| Assertions.assertThat(user.getEmail()).isEqualTo("newEmail"); | ||
| Assertions.assertThat(user.getPassword()).isEqualTo("newPassword"); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
@Nested
@DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class)
class updateUser_메서드는 {
private UserUpdater userUpdater = new UserUpdater(userRepository, new UserReader(userRepository));
private Long savedUserId;
@Nested
@DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class)
class 수정할_유저가_존재한다면 {
@BeforeEach
void setUp() {
userRepository.deleteAll();
UserData userData = createUserRequest();
savedUserId = userRepository.save(userData.toUser()).getId();
}
@DisplayName("해당 유저정보를 수정 후 수정한 유저정보를 리턴한다")
@Test
void it_updates_and_returns_user() {
UserData userUpdateRequest = UserData.builder()
.name("newName")
.email("newEmail")
.password("newPassword")
.build();
User user = userUpdater.updateUser(savedUserId, userUpdateRequest);
Assertions.assertThat(user.getName()).isEqualTo("newName");
Assertions.assertThat(user.getEmail()).isEqualTo("newEmail");
Assertions.assertThat(user.getPassword()).isEqualTo("newPassword");
}
}
}USER_REQUEST는 다른 곳에서 쓰이지는 않아서 내부 변수로 변경했습니다.
User user = userUpdater.updateUser(savedUserId, userUpdateRequest);이 문장을 보니 update라는 말이 3번이나 나오기도 하고 user라는 말이 거의 6번 정도 나오네요.
User user= userUpdater.update(id, request); 이런 모양이면 좀 더 읽기는 쉬울 것 같아요.
There was a problem hiding this comment.
항상 설명이 부족한 변수명만 보다 보니까
약간 이 문맥에서 무엇을 의미하는 거지
생각하게되는 불편함이 있어서
최대한 자세히 적는게 저도 모르게 습관이 된것 같네요 ;;
너무 중복되거나 많은 설명은 오히려 가독성이 안좋아 질 수 있다는 말씀이시군요
차후에는 문맥상 적절하게 변수명이나 메서드명을 짓도록 신경쓰면서 해보겠습니다 !
There was a problem hiding this comment.
아니에요. 잘 작성해주셨어요. 암식적인 맥락보다는 명시적으로 표현하는게 좋죠. 엔터프라이즈 레벨에서는 누가 내 코드를 수정할지 모르기 때문에, 나만 알고 있는 맥락이 있으면 안됩니다 ㅎㅎ 다만 조금 과해보였다는 생각이 들었어요
| import org.springframework.data.jpa.repository.JpaRepository; | ||
| import org.springframework.data.repository.CrudRepository; | ||
|
|
||
| @Primary |
There was a problem hiding this comment.
@Primary를 사용하셨군요.
이번 기회에 https://tech.kakaopay.com/post/martin-dev-honey-tip-2/ 이 글을 한 번 읽어봅시다
| public ProductController(ProductCreator productCreator, ProductUpdater productUpdater, ProductReader productReader, ProductDeleter productDeleter) { | ||
| this.productCreator = productCreator; | ||
| this.productUpdater = productUpdater; | ||
| this.productReader = productReader; | ||
| this.productDeleter = productDeleter; | ||
| } |
There was a problem hiding this comment.
컨트롤러도 마찬가지로 하나의 일만 하도록 나눠보는 것도 좋아요
| @Nested | ||
| @DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class) | ||
| class 패스워드가_없는_유저_정보_요청이_오면 { | ||
| @BeforeEach | ||
| void setUp() { | ||
| USER_REQUEST = UserData.builder() | ||
| .name(TEST_NAME) | ||
| .email(TEST_EMAIL) | ||
| .build(); | ||
| } |
There was a problem hiding this comment.
경우에 따라 자세히 테스트를 작성해 주셨네요. ParameterizedTest로도 작성해도 좋을 것 같네요
There was a problem hiding this comment.
안녕하세요
최하단의 코드처럼 처음에 작성했는데 확인해보니 @Nested 애너테이션을 사용하는 내부 클래스에 static 키워드를 사용하는 것은 허용되지않아
@MethodSource("provideInvalidUserRequests")를 활용할수가 없더라구요...
혹시 다른 제가 모르는 방법이 있는지 궁금합니다!
[최초 시도한 코드]
@Nested
@DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class)
class 올바르지_않은_유저정보_요청이_오면 {
@ParameterizedTest
@DisplayName("상품 생성시 올바르지 않은 요청 케이스별 테스트 요청인 경우 에러응답을 반환한다.")
@MethodSource("provideInvalidUserRequests")
void createProductInvalidRequestCase(UserData user) throws Exception {
mockMvc.perform(post("/users")
.contentType(APPLICATION_JSON)
.content(objectMapper.writeValueAsString(user)))
.andExpect(status().isBadRequest())
.andDo(print());
}
private static Stream<Arguments> provideInvalidUserRequests() {
return Stream.of(
Arguments.of(UserData.builder().name("").email("").password("").build()),
Arguments.of(UserData.builder().name("testName").email("").password("").build()),
Arguments.of(UserData.builder().name("").email("test@Email").password("").build()),
Arguments.of(UserData.builder().name("").email("").password("testPassword").build())
);
}
}There was a problem hiding this comment.
@MethodSource에는 static 메서드만 올 수 있다.@Nested아래에는 static 메서드를 사용할 수 없다.
첫 번째 든 생각은 @Nested외부에 static 메서드를 쓸 수 있지 않을까 해서 외부에 선언했습니다.
그랬더니 method를 찾을 수 없다고 했습니다. 그래서 @MethodSource의 Javadoc을 보니 다음과 같이 써있네요.
The names of factory methods within the test class or in external classes to use as sources for arguments.
Factory methods in external classes must be referenced by fully qualified method name — for example, com.example.StringsProviders#blankStrings. ... 이하 생략
test클래스 안에 있으면 스태틱 메서드 이름만 적어줘도 되는데 외부에 있으면 위와 같이 전체 다 써줘야 하네요.
따라서 static method를 외부에두고
@MethodSource("com.codesoom.assignment.controllers.user.UserControllerTest#provideInvalidUserRequests")이렇게 풀네임을 적어줬더니 되네요
There was a problem hiding this comment.
이것 때문에 엄청 삽질했는데 ...
공식문서안에 있었군요....ㅠㅠ 먼가 부끄럽네요
감사합니다!
안녕하세요
일단 요청 드리고 시간 날 때 점차 진행하겠습니다
감사합니다.