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

Resolve some warning message during test #1895

Conversation

yangwooseong
Copy link
Collaborator

@yangwooseong yangwooseong commented Jan 10, 2024

Self Checklist

  • I wrote a PR title in English and added an appropriate label to the PR.
  • I wrote the commit message in English and to follow the Conventional Commits specification.
  • I added the changeset about the changes that needed to be released. (or didn't have to)
  • I wrote or updated documentation related to the changes. (or didn't have to)
  • I wrote or updated tests related to the changes. (or didn't have to)
  • I tested the changes in various browsers. (or didn't have to)
    • Windows: Chrome, Edge, (Optional) Firefox
    • macOS: Chrome, Edge, Safari, (Optional) Firefox

Related Issue

Summary

  • yarn test실행 시 테스트 코드에서 나오는 warning message의 일부를 resolve합니다.
  • https://app.circleci.com/pipelines/github/channel-io/bezier-react/5804/workflows/c0aae4cd-38de-4423-a3fc-9480443ef7f9/jobs/22311 여기서 실행 결과를 보면, 성공한 테스트임에도 불구하고 굉장히 많은 경고 메시지가 테스트 결과에 포함되는 것을 볼 수 있습니다. 이런 불필요한 메시지가 로그에 포함되면 테스트 코드 결과를 파악하기도 어렵고, 경고 메시지 자체가 잠재적인 버그의 원인이기 때문에 결코 무시할 수 없습니다.
  • 이번 작업으로 95%이상 경고 메시지를 줄였고, scss 마이그레이션 작업을 하면서 완전히 없애면 좋을 것 같습니다

Details

  • 에러 메시지의 원인은 아래와 같았습니다.

  • jest test callback 함수 안에서 비동기 작업이 수행됨 -> 대부분의 로그를 차지하고 있는 문제로, FeatureProvider 안에서 오타 수정하면 Promise 실행 안하게 됨
    image

  • active="false" -> 정확한 원인은 아직 파악 안된 상태..
    image

  • Icon컴포넌트의 forwardRef -> 아마 styled(Icon) 으로 감싸고 나서 asChild로 인한 ref forwarding이 안되는 이슈인 듯하여 scss로 마이그레이션 하면 해결 됫 것으로 예상
    image

  • ref forwarding 이 누락됨
    image

  • hasError 속성을 잘못 넘김 (Enhance handling of improper hasError return value for useFormFieldProps #1876 참고)
    image

Breaking change? (Yes/No)

  • No

References

@yangwooseong yangwooseong added the test Issue or PR that related to test label Jan 10, 2024
@yangwooseong yangwooseong self-assigned this Jan 10, 2024
Copy link

changeset-bot bot commented Jan 10, 2024

⚠️ No Changeset found

Latest commit: 2ebfd12

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

Copy link
Contributor

github-actions bot commented Jan 10, 2024

Chromatic Report

🚀 Congratulations! Your build was successful!

Copy link

codecov bot commented Jan 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (66923cc) 86.48% compared to head (2ebfd12) 86.45%.

Additional details and impacted files
@@            Coverage Diff             @@
##            alpha    #1895      +/-   ##
==========================================
- Coverage   86.48%   86.45%   -0.03%     
==========================================
  Files         251      251              
  Lines        3484     3485       +1     
  Branches      755      755              
==========================================
  Hits         3013     3013              
- Misses        387      388       +1     
  Partials       84       84              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sungik-choi
Copy link
Contributor

👍

active="false" -> 정확한 원인은 아직 파악 안된 상태..

-> scss로 마이그레이션하고, html element에 active attribute를 전달하지 않으면 해결될 거 같아요

Icon컴포넌트의 forwardRef -> 아마 styled(Icon) 으로 감싸고 나서 asChild로 인한 ref forwarding이 안되는 이슈인 듯하여 scss로 마이그레이션 하면 해결 됫 것으로 예상

-> #1313 을 해결해야함

hasError 속성을 잘못 넘김

-> #1876 에도 적었듯이 어떻게 invalid한 attribute인지 아닌지 잘 구별할지 고민되네요

@yangwooseong yangwooseong force-pushed the feat/resolve-warning-message-during-test branch from 9438837 to 2ebfd12 Compare January 11, 2024 07:38
@yangwooseong yangwooseong merged commit 3f781aa into channel-io:alpha Jan 11, 2024
10 of 11 checks passed
@yangwooseong yangwooseong deleted the feat/resolve-warning-message-during-test branch January 11, 2024 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Issue or PR that related to test
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants