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

feat(Control): Control 테스트코드 작성 #135

Merged
merged 11 commits into from
Sep 19, 2024
Merged

Conversation

Brokyeom
Copy link
Member

@Brokyeom Brokyeom commented Sep 16, 2024

변경사항

  • Control(Checkbox, Radio, Toggle) 컴포넌트의 테스트코드를 작성합니다.

링크

시급한 정도

🏃‍♂️ 보통 : 최대한 빠르게 리뷰 부탁드립니다.

기타 사항

image
  • 간단한 check, uncheck 여부만 판단하는 테스트 코드를 작성 해 보았습니다. 추가했으면 좋겠는 테스트가 있다면 코멘트 남겨주세요!(onClick 이벤트 테스트 추가 후에 머지하는게 좋아보여요)
  • expect api에 대한 lint 에러가 발생하더라구요? 타입 설정을 다 해준 것 같은데 문제가 발생해서 파악중에 있습니다.

@Brokyeom Brokyeom self-assigned this Sep 16, 2024
Copy link

height bot commented Sep 16, 2024

Link Height tasks by mentioning a task ID in the pull request title or commit messages, or description and comments with the keyword link (e.g. "Link T-123").

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

Copy link

changeset-bot bot commented Sep 16, 2024

🦋 Changeset detected

Latest commit: af75ed9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@sopt-makers/ui Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pull-request-size pull-request-size bot added size/L and removed size/M labels Sep 16, 2024
@Brokyeom Brokyeom marked this pull request as ready for review September 16, 2024 11:11
Copy link
Member

@jungwoo3490 jungwoo3490 left a comment

Choose a reason for hiding this comment

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

좋습니다!!! PR에 언급하신 대로 클릭 이벤트 테스트만 추가해주면 될 것 같아요.
RTL의 user-event를 활용하여 클릭 이벤트를 발생시키고, 스파이 함수를 활용하여 이벤트 핸들러 호출 여부 확인하는 부분만 추가하면 될 것 같습니다!!

lint 에러 확인해보니 타입 추론 관련한 이슈 같아요. 실제로 테스트 파일 상단에서 명시적으로 expect를 import헤주면 lint 에러가 해결되긴 합니다만, 그렇게 하면 config 파일에서 globals 옵션을 준 이유가 사라져서... 저도 스마트하게 해결하는 방안을 한번 찾아보도록 하겠습니다!! 일단은 임시로 lint 오류 우회하기 위해 명시적으로 import 하는 방법을 택하는 것도 좋을 것 같다는 생각이 듭니다!!

고생하셨습니다!!!!! 🚀🚀🚀

@@ -0,0 +1,30 @@
import { render, screen } from '@testing-library/react';
import { describe } from 'vitest';
Copy link
Member

Choose a reason for hiding this comment

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

Comment에 남겼듯이 여기에 expect를 추가해서 임시로 lint 오류를 우회하는 것도 좋아보입니다!!

Copy link
Member Author

Choose a reason for hiding this comment

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

40fe12b에서 expect import 하는 방식으로 일단 우회했습니다.
onClick 테스트 추가 후에 rerequest 드릴게요~

@Brokyeom
Copy link
Member Author

image onClick 이벤트에 대한 테스트코드를 추가했어요.

Copy link
Member

@jungwoo3490 jungwoo3490 left a comment

Choose a reason for hiding this comment

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

테스트가 컴포넌트의 기본적인 기능들은 어느 정도 잘 커버하고 있는 것 같아요.
너무 과하게 커버리지에 집착하면 오히려 역효과가 나기 때문에 이 정도면 충분하다는 생각이 드네요!!
고생 많으셨습니다아!!! 👍🏻🚀

@Brokyeom Brokyeom merged commit 15fb35b into main Sep 19, 2024
1 check passed
@Brokyeom Brokyeom deleted the feat/ui-control-test branch September 19, 2024 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants