-
Notifications
You must be signed in to change notification settings - Fork 47
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
경북대 FE_이효은 5주차 과제 Step1 #25
base: hyoeunkh
Are you sure you want to change the base?
Conversation
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.
고생하셨습니다~!
발생하는 워닝들은 @testing-library/react의 버전이 낮아서 발생하는 것 같아요. 최신 버전으로 올리고 실행해보시겠어요?
그리고 fireEvent보다는 userEvent를 사용하시면 실제 사용자와의 상호작용과 더욱 가깝게 테스트가 가능합니다. 사용해보시면 좋을 것 같아요!
이렇게 짜는게 맞는지 모르겠다고 하셨는데 충분히 잘 작성하신 것 같습니다.
현재 페이지 단위의 e2e테스트를 작성해 주셨는데 단위 테스트도 기회가 되시면 한 번 작성해 보시길 추천드립니다.
개인적으로 저는 통합테스트나 e2e테스트에 그리 긍정적이진 않습니다.. ㅎㅎ
뷰는 언제든 바뀌고, 기획도 언제든 바뀝니다. 오히려 단위 테스트를 열심히 작성한다면 신뢰할 수 있는 모듈들의 조합으로 만들어진 앱은 당연히 신뢰할 수 있지 않을까 생각해요.
그래서 저는 e2e는 큰 의미를 두지 않는 편이고, 엄청나게 잘 작성할 필요도 없다고 생각하는 주의입니다.
render( | ||
<ChakraProvider> | ||
<QueryClientProvider client={queryClient}> | ||
<GoodsDetail productId={productId} /> | ||
</QueryClientProvider> | ||
</ChakraProvider>, | ||
); |
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.
서비스 전체적으로 ChakraProvider와 QueryClientProvider는 반드시 필요할 것 같네요.
매 render마다 프로바이더를 작성해주지 않아도 되도록 renderWithProvider같은 함수로 추상화 해보면 어떨까요?
안녕하세요 멘토님!
test 코드 작성이 생소해서 세팅부터 많은 에러를 겪고
현재도 겨우 테스트를 통과한 것이라(자잘한 에러가 뜨는데 왜 통과인지도 모르겠어요) 사실 아직 잘 모르겠습니다..
뭔가 이렇게 하면 안 될 것 같다는 생각이 드는 코드입니다 ㅎㅎ,,
피드백을 받고 수정하는 것이 나을 것 같아 제출합니다. 잘 부탁드립니다!
아 그리고 5주차부터 MSW를 사용한 목데이터로 구현하기 때문에 이전코드 사용하지않고 제공된 코드로 구현했습니다!