feat: add BaseTimeEntityTest by minuk8932 · Pull Request #8 · dong149/9zik-server

minuk8932 · 2022-04-07T06:44:49Z

dong149 · 2022-04-07T11:14:28Z

src/test/java/com/goozik/model/entity/BaseTimeEntityTest.java

+    @Mock
+    private Project project;
+
+    private final LocalDateTime NOW = LocalDateTime.of(


https://google.github.io/styleguide/javaguide.html#s5.2.4-constant-names

저희 code style 이거로 맞추기로 해서 해당 내용 참고하면,
상수는 static final 로 정의하는걸로 맞추면 좋을 것 같습니다!

확인했습니다

dong149 · 2022-04-07T11:17:26Z

src/test/java/com/goozik/model/entity/BaseTimeEntityTest.java

+
+        assertAll(
+            () ->
+                assertTrue(user.getCreatedAt().isAfter(REFERENCE_DATE_STARTED_AT)),


이 부분은 한줄로 그냥 연결해주는 게 어떨까요?

() -> assertTrue(user.getCreatedAt().isAfter(REFERENCE_DATE_STARTED_AT)

전에 이 코드로 변경하기 전에 타이핑이 넘어가서 내렸는데요. 코드 통일성 위해서 다 내려뒀었습니다.
말씀하신대로 지금 상태에선 붙이는게 좋을 것 같긴한데요. 향후에 assertAll로 테스트를 진행할 때 타이핑 수가 넘는 경우도 있을 텐데, 그런 경우에만 내리는 걸로 할까요?

일단 큰 상관은 없는 것 같아서 기존에 작업하신 방법대로 가도 무방할 것 같습니다 ㅎㅎ

넵 뭐 어차피 수정이 필요할 것 같으니 수정할 땐 말씀드린대로 진행하겠습니다.

이런 부분을 맞추려고 formatter 를 사용하는거 아닌가요?
저장할 때 자동으로 줄바꿈되도록 설정하면 좋을 것 같아요 ㅎㅎ

@dks301
확인했습니다.

해당 기능은 인텔리제이 버전 업(2021 이상) 필요하네요. 변경하실 분들 참고하세요.
(물론 하위 버전에도 찾아보면 있겠지만.. 찾다 귀찮아서 업데이트 했는데 이게 낫네요.)

dong149 · 2022-04-07T11:21:05Z

src/test/java/com/goozik/model/entity/BaseTimeEntityTest.java

+import org.mockito.MockitoAnnotations;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+@ExtendWith(MockitoExtension.class)


해당 테스트는 통합 테스트로 테스트진행하는게 어떨까 싶은데
다른 분들 의견도 궁금하네요!

@minuk8932
@dks301
@alkwen0996

https://velog.io/@conatuseus/2019-12-06-2212-%EC%9E%91%EC%84%B1%EB%90%A8-1sk3u75zo9

참고해주세요~

안 그래도 초기엔 해당 방식과 동일하게 테스트를 진행했습니다. 데이터를 레포지토리에 저장하고 이후 자동 생성된 시간 값들을 출력해서 정상적으로 동작하는지 확인하는 방식으로요. 물론 @SpringBootTest가 아닌 @DataJpaTest만 이용해서 롤백 진행하도록 했었고, @Autowired로 레포지토리 가져와서 통합테스트로 진행했습니다.
말씀하신대로 통합테스트라 프로젝트 시작시 로딩이 잡히는 거나 간소화되지 않는 점은 조금 아쉽더라구요. 그래서 위와 같은 단위테스트로 진행했는데, 이 코드에도 아쉬운 점은 실제 Auditing을 통해 시간이 생성되는지는 알 수 없다는 것입니다. 즉, 전자와는 다르게 테스트의 의미가 조금 모호해졌다 느꼈습니다. (제가 직접 시간을 지정해서 넣었으니까요.)

만약 단위 테스트로 JpaAuditing을 통해 시간 값이 자동 생성되는 방식이 있다면 제안해주시면 좋겠고, 아니라면 의견 추합해서 조정하도록 하겠습니다~

예전 회사에서, JpaAuditing 테스트를 진행한적이 있는데 그 당시에는 통합테스트로 진행했던 기억이 있습니다
저는 개인적으로 결국 Auditing 테스트에서 중요한 것은 시간이 자동으로 업데이트 되는지를 테스트해보는게 로직상으로 중요한 부분이 아닌가 생각해서, 통합테스트로 진행하는게 맞지 않을까 생각합니다!
@minuk8932

네네 저도 같은 생각이긴 합니다.

auditing test DataJpaTest로도 됩니다~

dong149 · 2022-04-08T05:13:06Z

@minuk8932
민욱님께서 현재, develop브랜치에서 작업하신 것으로 보이는데요!
feature/service-test 에서 새롭게 브랜치 따셔서 feature/service-test로 향하게 하는게 좋지 않을까 제안드립니다!
즉, feature/XXX ( 민욱님께서 새롭게 생성하실 브랜치 ) => feature/service-test 이게 좋지 않을까 생각됩니당

minuk8932 · 2022-04-08T05:29:55Z

뭔가 또 꼬인 것 같네요. feature/service-test 브랜치를 가져와서 브랜치를 새로 따서 하긴 했는데 확인해보고 시간나는대로 수정하겠습니다.

dks301 · 2022-04-08T07:56:37Z

feature/service-test 브랜치

develop에서 따야하는거 아닌가요??

minuk8932 · 2022-04-08T08:08:47Z

원래 develop에서 따는게 맞는데, feature/service-test 브랜치에서 추가되어야할 것이 많다는 것과 현재 제 로컬에 있는 버전이 상당히 낮은 축에( 원본과 머지가 꽤나 부족한 편 ) 속해서 동훈씨랑 저랑 상의해서 우선은 이번 제 테스트 코드를 추가할 때만 해당 브랜치에서 따오기로 했습니다.

dks301

수고하셨습니다~~~~👍

dks301 · 2022-04-08T08:28:57Z

src/test/java/com/goozik/model/entity/BaseTimeEntityTest.java

+    public void initialization() {
+        MockitoAnnotations.openMocks(this);
+    }


이부분은 처음보는데 Mock으로 선언된 User랑 Project를 생성해주는게 맞나요? ㅎㅎ

테스트 기준을 잘못 생각했네요. ㅎㅎ 어제 말씀 나눴던 내용이랑 고려할 것들 정리해서 수정본 올려보겠습니다.

dks301 · 2022-04-08T08:30:36Z

src/test/java/com/goozik/model/entity/BaseTimeEntityTest.java

+
+    @Test
+    @DisplayName("유저 생성시 createAt/updateAt 값 검증")
+    void testUserTimeWhenCreateUser() {


이 테스트 동작할 때도 Project도 생성되는 것 같은데 맞나요?? ㅎㅎ
맞다면 불필요한게 생성되는거라고 생각되서요

네 맞습니다. 이때는 근데 프로젝트가 생성되지 않는걸로 의도했는데, 아마도 위에 걸어주신 리뷰에 'MockitoAnnotations.openMocks(this);' 해당 동작으로 인해 프로젝트 가객체가 동시에 생성될 것 같네요.
사실상 테스트 코드 초기부터 테스트 기준이 잘못 잡혀서 미스난 코드인 것 같고 만약 'MockitoAnnotations.openMocks'를 사용이 필요한 경우라면 각 테스트 메서드 내에서 ''MockitoAnnotations.openMocks(User.class);', ''MockitoAnnotations.openMocks(Project.class);' 이런식으로 선언해서 분리해주는게 좋을 것 같습니다.

dks301 · 2022-04-08T08:33:21Z

src/test/java/com/goozik/model/entity/BaseTimeEntityTest.java

+        assertTrue(user != null);
+        assertTrue(project != null);


프로젝트의 auditing이 정상 동작하는지 확인하는 테스트인데,
user,project가 null인지도 확인하는게 맞을까요??

�그렇네요. 테스트 코드 작성하면서 (아마도 중구난방으로 하다보니) NPE가 자주 발생해서 체크할 겸 써둔거긴한데, 빼두겠습니다.

저 물음표다이아는 뭔가요?

흠.. 글쎄요? 제가 붙여놓은건 아닌데 붙었네요.ㅋㅋ

dks301 · 2022-04-08T08:34:43Z

src/test/java/com/goozik/model/entity/BaseTimeEntityTest.java

+                assertTrue(project.getCreatedAt().isAfter(user.getCreatedAt())),
+
+            () ->
+                assertTrue(project.getUpdatedAt().isAfter(user.getCreatedAt())),
+
+            () ->
+                assertTrue(project.getCreatedAt().isEqual(project.getUpdatedAt()))


assertJ 좋은데 적용해보면 어떨까요??ㅎㅎ

형주님 의견대로, assertJ로 통일하는게 좋아보입니당!

@dks301 @dong149
네, assertJ 확인해보겠습니다.

dks301 · 2022-04-08T08:37:39Z

src/test/java/com/goozik/model/entity/BaseTimeEntityTest.java

+
+        assertAll(
+            () ->
+                assertTrue(user.getCreatedAt().isAfter(REFERENCE_DATE_STARTED_AT)),


이런 부분을 맞추려고 formatter 를 사용하는거 아닌가요?
저장할 때 자동으로 줄바꿈되도록 설정하면 좋을 것 같아요 ㅎㅎ

dks301 · 2022-04-08T08:45:35Z

원래 develop에서 따는게 맞는데, feature/service-test 브랜치에서 추가되어야할 것이 많다는 것과 현재 제 로컬에 있는 버전이 상당히 낮은 축에( 원본과 머지가 꽤나 부족한 편 ) 속해서 동훈씨랑 저랑 상의해서 우선은 이번 제 테스트 코드를 추가할 때만 해당 브랜치에서 따오기로 했습니다.

음 일반적으로는 dong149:feature/service-test 내용을 minuk8932:develop 로 땡겨온 다음에 dong149:develop로 pr을리지 않나요??(질문임)

dong149 · 2022-04-08T13:23:06Z

원래 develop에서 따는게 맞는데, feature/service-test 브랜치에서 추가되어야할 것이 많다는 것과 현재 제 로컬에 있는 버전이 상당히 낮은 축에( 원본과 머지가 꽤나 부족한 편 ) 속해서 동훈씨랑 저랑 상의해서 우선은 이번 제 테스트 코드를 추가할 때만 해당 브랜치에서 따오기로 했습니다.

음 일반적으로는 dong149:feature/service-test 내용을 minuk8932:develop 로 땡겨온 다음에 dong149:develop로 pr을리지 않나요??(질문임)

말씀해주신대로 진행하면, PR확인시에 작업한 내용을 명확하게 확인하기 어려울 것 같다고 생각했습니다.
그래서, 이 경우에는 민욱님이 말씀해주신대로 진행하는게 어떤지 고민했었어용

minuk8932 · 2022-04-09T02:51:17Z

우선 'dong149:feature/service-test 내용을 minuk8932:develop 로 땡겨온 다음에' 여기까진 동일하게 진행했고 저는 dong149:feature/service-test 일로 밀어 넣자는 걸로 이해해서 'dong149:feature/service-test' 이쪽으로 보내려했는데 제가 보낸게 일단은 develop 쪽으로 날아간 것 같네요. ㅎㅎㅎ 근데 의견 조율이 좀 필요할 것 같긴하네요.

feat: add BaseTimeEntityTest

9f42349

minuk8932 requested review from alkwen0996, dks301 and dong149 as code owners April 7, 2022 06:44

dong149 reviewed Apr 7, 2022

View reviewed changes

dong149 assigned minuk8932 Apr 7, 2022

dks301 reviewed Apr 8, 2022

View reviewed changes

minuk8932 added 2 commits April 10, 2022 12:52

feature: add Auditing test

989c442

feat: add Auditing test

55edfc6

Conversation

minuk8932 commented Apr 7, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

minuk8932 Apr 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dong149 commented Apr 8, 2022

Uh oh!

minuk8932 commented Apr 8, 2022

Uh oh!

dks301 commented Apr 8, 2022

Uh oh!

minuk8932 commented Apr 8, 2022

Uh oh!

dks301 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dks301 commented Apr 8, 2022

Uh oh!

dong149 commented Apr 8, 2022

Uh oh!

minuk8932 commented Apr 9, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

minuk8932 Apr 8, 2022 •

edited

Loading