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.

Sign up for GitHub

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

Jump to bottom

Enhance Chromatic UI snapshot test #1000

Open
sungik-choi opened this issue Nov 10, 2022 · 3 comments
Open

Enhance Chromatic UI snapshot test #1000

sungik-choi opened this issue Nov 10, 2022 · 3 comments
Assignees
@yangwooseong @sungik-choi
Labels
enhancement Issues or PR related to making existing features better priority:B Issue that important but not urgent test Issue or PR that related to test

Comments

@sungik-choi
Copy link
Contributor

Description

  • 크로마틱의 스냅샷 테스트가 유의미하게 동작하도록 수정합니다.
  • 스냅샷 리뷰 과정에 대해 점검합니다.

Reasons for suggestion

현재 랜덤으로 이미지를 가져오는 등의 로직이 스토리에 포함되어있어 스냅샷 비교가 유의미하게 동작하고 있지 않습니다. 크로마틱의 스토리북 자동 배포 기능뿐만 아니라 UI 리뷰 기능도 잘 활용할 수 있도록 해야 합니다.

Proposed solution

  • 스토리 내부의 랜덤으로 스타일을 변경하는 로직을 모두 제거합니다.
  • Chromatic GitHub action에서 리뷰 관련 옵션 중 팀의 상황에 가장 적절한 옵션을 적용합니다.

References
@sungik-choi sungik-choi added enhancement Issues or PR related to making existing features better dx labels Nov 10, 2022
@sungik-choi sungik-choi self-assigned this Nov 10, 2022
@sungik-choi sungik-choi moved this to Q4 2022 in Bezier React Nov 10, 2022
@sungik-choi sungik-choi moved this from Q4 2022 to Future in Bezier React Nov 11, 2022
@sungik-choi
Copy link
Contributor Author

  • 현재 크로마틱 무료 플랜은 한달에 5000 snapshot, story 하나당 1snapshot만 제공하는데, PR 업데이트마다 빌드를 하니 월초에 바로 제한을 초과해버리는 상황.
  • 이슈의 우선순위를 낮추고, 일단 스토리북 자동 배포만 사용하는 방향으로.

@sungik-choi sungik-choi mentioned this issue Nov 30, 2022
Merged
6 tasks
@sungik-choi sungik-choi removed the status in Bezier React Jan 17, 2023
@sungik-choi sungik-choi moved this to 📌 Backlog in Bezier React Jan 17, 2023
@sungik-choi sungik-choi added the test Issue or PR that related to test label Apr 3, 2023
@sungik-choi sungik-choi moved this from 📌 Backlog to 🧊 Pending in Bezier React Apr 3, 2023
@sungik-choi sungik-choi moved this from 🧊 Pending to 🏃‍♀️ In progress in Bezier React Apr 3, 2023
@sungik-choi sungik-choi moved this from 🏃‍♀️ In progress to 📌 Backlog in Bezier React Apr 3, 2023
@sungik-choi
Copy link
Contributor Author

sungik-choi commented May 12, 2023 •
edited
Loading

@sungik-choi sungik-choi mentioned this issue May 12, 2023
Open
@sungik-choi sungik-choi removed the dx label Jun 21, 2023
@sungik-choi sungik-choi added the priority:B Issue that important but not urgent label Jul 6, 2023
@yangwooseong yangwooseong mentioned this issue Oct 4, 2023
Merged
6 tasks
yangwooseong added a commit that referenced this issue Oct 27, 2023
@yangwooseong
Bump @storybook/react to ^7.4.2 (#1656)
572ac77 
<!--
  How to write a good PR title:
- Follow [the Conventional Commits
specification](https://www.conventionalcommits.org/en/v1.0.0/).
  - Give as much context as necessary and as little as possible
  - Prefix it with [WIP] while it’s a work in progress
-->

## Self Checklist

- [x] I wrote a PR title in **English** and added an appropriate
**label** to the PR.
- [x] I wrote the commit message in **English** and to follow [**the
Conventional Commits
specification**](https://www.conventionalcommits.org/en/v1.0.0/).
- [x] I [added the
**changeset**](https://github.com/changesets/changesets/blob/main/docs/adding-a-changeset.md)
about the changes that needed to be released. (or didn't have to)
- [x] I wrote or updated **documentation** related to the changes. (or
didn't have to)
- [x] I wrote or updated **tests** related to the changes. (or didn't
have to)
- [x] 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
<!-- Please link to issue if one exists -->

<!-- Fixes #0000 -->

Fixes #1255 

## Summary
<!-- Please brief explanation of the changes made -->

- 스토리북 메이저 버전을 7로 올리고 브레이킹 체인지에 대응합니다.

## Details
<!-- Please elaborate description of the changes -->

버전 업에 따른 변경 사항은 다음과 같습니다.
- `main` 파일 마이그레이션 (codemod 사용), ts 마이그레이션하여 스토리북에서 제공하는 타입 활용
([#](https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#esm-format-in-mainjs)).
`preview` 파일도 themeProvider 분리하고 ts 마이그레이션
- `getTitle` 유틸을 활용해서 `component/.../lastFolderName`형식으로 타이틀 얻어오던 것을 제거.
스토리북 7버전 부터는 동적으로 타이틀을 계산하는 것을 허용하지 않고 있고, 명시적으로 지정하지 않으면 알아서 경로를 잘 보여주게
됩니다.
([#](https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#titles-are-statically-computed))
  - CSF2 -> CSF3으로 변경 (codemod 사용)
- `control` 의 `option` 필드가 `control` 아래에 있었던 것을 하나 올려서 같은 depth 가 되도록 변경
([#](https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#deprecated-controloptions))
  - default export 되는 meta 의 타입 단언을 타입 선언으로 변경
  - 이제 docs 가 스토리마다 있는 것이 아니라 컴포넌트 단위로 존재하게 됩니다.

이후 pr 에서 아래 이슈들을 차례대로 다루면 될 것 같습니다.

#1083
#1000

### Breaking change? (Yes/No)
<!-- If Yes, please describe the impact and migration path for users -->

- No

## References
<!-- Please list any other resources or points the reviewer should be
aware of -->

- https://storybook.js.org/docs/7.0/react/migration-guide#page-top
-
https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#migration
@yangwooseong yangwooseong mentioned this issue Jan 10, 2024
Merged
6 tasks
yangwooseong added a commit that referenced this issue Jan 10, 2024
@yangwooseong
Fix ArrowRightUp icon broken bug by upgrading svgo library (#1896)
1093c94 
<!--
  How to write a good PR title:
- Follow [the Conventional Commits
specification](https://www.conventionalcommits.org/en/v1.0.0/).
  - Give as much context as necessary and as little as possible
  - Prefix it with [WIP] while it’s a work in progress
-->

## Self Checklist

- [x] I wrote a PR title in **English** and added an appropriate
**label** to the PR.
- [x] I wrote the commit message in **English** and to follow [**the
Conventional Commits
specification**](https://www.conventionalcommits.org/en/v1.0.0/).
- [x] I [added the
**changeset**](https://github.com/changesets/changesets/blob/main/docs/adding-a-changeset.md)
about the changes that needed to be released. (or didn't have to)
- [x] I wrote or updated **documentation** related to the changes. (or
didn't have to)
- [x] I wrote or updated **tests** related to the changes. (or didn't
have to)
- [x] 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
<!-- Please link to issue if one exists -->

<!-- Fixes #0000 -->

- None

## Summary
<!-- Please brief explanation of the changes made -->

- `ArrowRightUp` icon 이 깨지는 버그를 수정합니다. (**target branch: main**)


![image](https://github.com/channel-io/bezier-react/assets/28595102/31902bd8-d706-4ea0-bdbb-a5d4c3da7f0c)


## Details
<!-- Please elaborate description of the changes -->

- svgo 3.0.3 에서 빌드했을 때는 안깨지다가 3.0.4에서 빌드하면 깨지는 것으로 보아 svgo 라이브러리의 문제라고
추정됩니다.
- 최신 버전(3.2.0)에서 빌드하면 재현이 안되는 것을 확인했기 때문에 최신 버전으로 업그레이드 합니다. 빌드 결과는 커멘트로
달리는 스토리북 프리뷰를 참고해주세요.
- 시각 회귀 테스트가 잘 동작했다면 사전에 방지할 수 있었을 것으로 보입니다
(#1000)

### Breaking change? (Yes/No)
<!-- If Yes, please describe the impact and migration path for users -->

- No

## References
<!-- Please list any other resources or points the reviewer should be
aware of -->

- svgo 3.0.3로 빌드한 스토리북 :
#1725 (comment)
- svgo 3.0.4로 빌드한 스토리북:
#1739 (comment)
@yangwooseong yangwooseong mentioned this issue Apr 30, 2024
Closed
@yangwooseong yangwooseong self-assigned this Apr 30, 2024
@channel-io channel-io deleted a comment from channeltalk bot Apr 30, 2024
@yangwooseong
Copy link
Collaborator

yangwooseong commented Apr 30, 2024 •
edited
Loading

2가지를 시도해볼 수 있을 것 같습니다

  • 컴포넌트마다 chromatic visual test 전용의 스토리북을 하나씩 만들어서 스토리북 갯수를 줄이기
  • 스토리북에서 제공하는 turbosnap 옵션을 활용해서 변화가 없는 스토리는 카운트를 20% 로 줄이기 (https://www.chromatic.com/docs/setup-turbosnap/). 만약 turbosnap을 활성화해서 사용량을 절감한다면 chromatic 전용 스토리북을 만들지 않아도 될 수도 있습니다

이거에 앞서서 UI 테스트가 checks에 포함되어 있지 않아서 이 설정을 우선 해야할 것 같습니다

@channel-io channel-io deleted a comment from channeltalk bot Apr 30, 2024
This was referenced Apr 30, 2024
Closed
Merged
yangwooseong added a commit that referenced this issue May 2, 2024
@yangwooseong
Remove random value in storybook (#2187)
8334164 
<!--
  How to write a good PR title:
- Follow [the Conventional Commits
specification](https://www.conventionalcommits.org/en/v1.0.0/).
  - Give as much context as necessary and as little as possible
  - Prefix it with [WIP] while it’s a work in progress
-->

## Self Checklist

- [x] I wrote a PR title in **English** and added an appropriate
**label** to the PR.
- [x] I wrote the commit message in **English** and to follow [**the
Conventional Commits
specification**](https://www.conventionalcommits.org/en/v1.0.0/).
- [x] I [added the
**changeset**](https://github.com/changesets/changesets/blob/main/docs/adding-a-changeset.md)
about the changes that needed to be released. (or didn't have to)
- [x] I wrote or updated **documentation** related to the changes. (or
didn't have to)
- [x] I wrote or updated **tests** related to the changes. (or didn't
have to)
- [x] 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

<!-- Please link to issue if one exists -->

<!-- Fixes #0000 -->

- #1000

## Summary

<!-- Please brief explanation of the changes made -->

- 시각회귀테스트가 잘 동작하려면 스토리북에서 랜덤한 이미지나 값을 사용하지 않아야 합니다. 고정된 값을 사용하는 것으로
변경합니다.

## Details

<!-- Please elaborate description of the changes -->

- 생략

### Breaking change? (Yes/No)

<!-- If Yes, please describe the impact and migration path for users -->

- No

## References

<!-- Please list any other resources or points the reviewer should be
aware of -->

- None
yangwooseong added a commit to yangwooseong/bezier-react that referenced this issue May 2, 2024
@yangwooseong
Remove random value in storybook (channel-io#2187)
2552ef9 
<!--
  How to write a good PR title:
- Follow [the Conventional Commits
specification](https://www.conventionalcommits.org/en/v1.0.0/).
  - Give as much context as necessary and as little as possible
  - Prefix it with [WIP] while it’s a work in progress
-->

## Self Checklist

- [x] I wrote a PR title in **English** and added an appropriate
**label** to the PR.
- [x] I wrote the commit message in **English** and to follow [**the
Conventional Commits
specification**](https://www.conventionalcommits.org/en/v1.0.0/).
- [x] I [added the
**changeset**](https://github.com/changesets/changesets/blob/main/docs/adding-a-changeset.md)
about the changes that needed to be released. (or didn't have to)
- [x] I wrote or updated **documentation** related to the changes. (or
didn't have to)
- [x] I wrote or updated **tests** related to the changes. (or didn't
have to)
- [x] 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

<!-- Please link to issue if one exists -->

<!-- Fixes #0000 -->

- channel-io#1000

## Summary

<!-- Please brief explanation of the changes made -->

- 시각회귀테스트가 잘 동작하려면 스토리북에서 랜덤한 이미지나 값을 사용하지 않아야 합니다. 고정된 값을 사용하는 것으로
변경합니다.

## Details

<!-- Please elaborate description of the changes -->

- 생략

### Breaking change? (Yes/No)

<!-- If Yes, please describe the impact and migration path for users -->

- No

## References

<!-- Please list any other resources or points the reviewer should be
aware of -->

- None
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@yangwooseong yangwooseong

@sungik-choi sungik-choi

Labels
enhancement Issues or PR related to making existing features better priority:B Issue that important but not urgent test Issue or PR that related to test
Projects
No open projects
Bezier React
Status: 📌 Backlog
Milestone
No milestone
Development

No branches or pull requests
2 participants
@yangwooseong @sungik-choi