-
Notifications
You must be signed in to change notification settings - Fork 2
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
chore/#38 playwright 테스트 설정 #61
Conversation
🐱 스토리북이 배포되었습니다: https://www.chromatic.com/build?appId=667edcfe9b923ba1892556f4&number=22 🐱 |
VRT 테스트 성공VRT 테스트가 성공적으로 완료되었습니다. |
- name: VRT 레이블 제거 | ||
uses: actions/github-script@v6 | ||
with: | ||
github-token: ${{secrets.GITHUB_TOKEN}} | ||
script: | | ||
github.rest.issues.removeLabel({ | ||
issue_number: context.issue.number, | ||
owner: context.repo.owner, | ||
repo: context.repo.repo, | ||
name: 'VRT' | ||
}) |
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.
VRT 레이블을 제거해야 하는 이유가 뭔가요?
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.
VRT 테스트가 완료됐다는 의미랑 트리거용으로 사용했기 때문에 PR을 원상복귀하기 위해서 추가했는데, 없어도 문제가 있진 않습니다.
"unexpected": 0, | ||
"flaky": 0 | ||
} | ||
} |
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.
PR 날릴 때 EOL 한 번씩 확인해주세요~!
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.
playwright action에서 자동으로 생성되는 파일인데, 현재 formatting 실행할 수 있는 방법이 있나용?
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.
혹시 결과 파일의 위치를 옮길 수 있나요? 컴포넌트가 많아질 수록(themed가 추가될 수록) 결과를 확인하기 힘들어질 것 같아서 각 컴포넌트 폴더 내에 함께 위치하면 어떨까 해서요!
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.
잘 이해가 안됐는데 다시 설명해줄 수 있나요?
현재 .playwright/components/snapshots/Button.test.ts-snapshots 처럼 존재하는데
각 컴포넌트 폴더 내에 함께 위치한다는게, packages/primitive/components/Button에 두자는 말씀인지 헷갈리네용
e2e/components/Button.test.ts
Outdated
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.
위 comment와 같은 맥락에서 각 컴포넌트의 e2e 테스트 코드는 각 컴포넌트 폴더 내에 위치하면 좋을 것 같습니다! (jest 테스트와 같은 depth에서 테스트하면 어떨까요?)
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.
다만 이 comment를 반영하게 된다면 현재 @ghdtjgus76 가 작업중인 code gen과의 병합도 필요할 것 같습니다!
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.
넵 codegen에도 반영해야 할 거 같네요
다만, 디바이더 컴포넌트 작업을 위해서 기존 codegen PR은 머지한 후에 별도 이슈로 파서 반영해야 할 거 같습니다!
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.
@ghdtjgus76 @ShinYoung-Kim
CDD로 개발한다는 관점에서 유닛 테스트까지는 컴포넌트 폴더에 위치하는게 맞는 것 같은데, e2e 테스트는 독립적인 것 같아서 지금의 폴더 구조를 유지했습니다. 추후에는 컴포넌트끼리 조합해서 사용할텐데 컴포넌트 폴더에 엮이기는 어렵지 않을까요?
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.
음 저도 근데 다른 디자인 시스템들도 e2e 폴더는 가장 루트로 빼는 거 같아서 지금이 좋은 거 같습니다
다만 코드젠에 e2e 관련 템플릿 및 자동화도 추가해야할 거 같다는 의미였습니다~
type Value = | ||
| string | ||
| boolean | ||
| number | ||
| { | ||
[Key: string]: Value; | ||
}; | ||
|
||
interface Options { | ||
id: string; | ||
args?: Record<string, string | boolean>; | ||
globals?: Record<string, Value>; | ||
} |
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.
사람마다 type, interface 중 선호하는 방법이 다르던데 저희 팀의 컨벤션을 정하면 좋을 것 같습니다!
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.
@gs0428 @ShinYoung-Kim @ghdtjgus76
라이브러리 개발엔 interface 사용이 권장된다고 하네요. (선언병합 때문에)
근데 결국에 primitive한 값을 표현하는 부분에서 type을 사용할 수 밖에 없는데, 혼합 형식의 지금 구조로 다 같이 이야기 해보는게 좋을 것 같아요.
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.
객체처럼 표현할 때는 interface로 사용하되 원시값들은 type 사용하는 방식 좋을 거 같습니다~!
- name: 의존성 설치 | ||
run: pnpm install |
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.
pnpm install --no-frozen-lockfile로 지정해줘야 할 거 같습니다!
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.
--no-frozen-lockfile 지정하는 이유가 있나요? lockfile에 문제 없는 경우엔 지정 안해도 될 것 같아서 비워뒀긴 합니다.
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.
음 ci 환경에서 관련 에러가 발생한 적이 있어서 추가하면 좋을 거 같다고 생각했습니다!
디벨롭 브랜치에 있는 다른 워크 플로우 파일은 --no-frozen-lockfile 속성 지정되어 있긴 하네요
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.
--frozen-lockfile은 CI 환경에서 설치 과정의 일관성과 재현성을 보장하기 위해 사용된다고 하는데 그럼 다른 워크 플로우 파일을 수정하는게 맞겠네요
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.
--frozen-lockfile은 CI 환경에서 설치 과정의 일관성과 재현성을 보장하기 위해 사용된다고 하는데 그럼 다른 워크 플로우 파일을 수정하는게 맞겠네요
넵, --no-frozen-lockfile을 지정하면 lockfile을 무시한다해서 그냥 두겠습니당.
e2e/components/Button.test.ts
Outdated
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.
넵 codegen에도 반영해야 할 거 같네요
다만, 디바이더 컴포넌트 작업을 위해서 기존 codegen PR은 머지한 후에 별도 이슈로 파서 반영해야 할 거 같습니다!
🐱 스토리북이 배포되었습니다: https://www.chromatic.com/build?appId=667edcfe9b923ba1892556f4&number=30 🐱 |
VRT 테스트 성공VRT 테스트가 성공적으로 완료되었습니다. |
Caution Review failedThe pull request is closed. Walkthrough변경 사항은 GitHub Actions 워크플로우를 업데이트하여 Visual Regression Testing(VRT) 자동화 및 Playwright 통합을 포함합니다. 새로운 워크플로우 파일이 추가되었으며, 기존 파일의 구성도 개선되었습니다. Playwright 및 접근성 테스트를 위한 새로운 스크립트와 종속성이 Changes
Sequence Diagram(s)sequenceDiagram
participant PR as Pull Request
participant Workflow as GitHub Actions Workflow
participant Playwright as Playwright
participant Storybook as Storybook Server
PR->>Workflow: PR labeled with "VRT"
Workflow->>Playwright: Run VRT tests
Playwright->>Storybook: Start Storybook server
Playwright->>Playwright: Execute VRT tests
Playwright->>Storybook: Terminate server
Playwright->>Workflow: Update snapshots
Workflow->>PR: Comment on VRT results
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
요약 (Summary)
컴포넌트의 시각적 회귀 테스트와 접근성 테스트를 위해 playwright를 도입하였습니다.
컴포넌트 수정이 스타일의 변화와 같은 시각적인 변경이 있는지 확인합니다. 단위 테스트로 확인할 수 없는 UI 변경을 감지합니다.
사용자 사용 경험 향상을 위해 접근성을 위배하지 않았는지 확인합니다. 단위 테스트로는 실제 UI 동작을 확인할 수 없어 playwright에서의 테스트가 유의미하다고 생각했습니다.
배경 (Background)
UI 컴포넌트 개발 과정에서 의도하지 않은 시각적 변경이나 접근성 문제가 발생할 수 있습니다. 이러한 문제들은 단순한 단위 테스트로는 포착하기 어렵고, 수동 테스트는 시간이 많이 소요되며 일관성이 떨어질 수 있습니다. Playwright를 도입함으로써 자동화된 방식으로 이러한 문제들을 효과적으로 감지하고 해결할 수 있습니다.
목표 (Goals)
이외 고려 사항들 (Other Considerations)
컴포넌트를 수정할 때, 스타일을 변경해야 하는 경우가 존재합니다.
만일, 합의된 변경 사항이라면 PR에서 VRT 레이블을 추가하여 스냅샷을 업데이트하여 테스트가 통과될 수 있도록 해야 합니다.
테스트 환경이 ci 환경이기 때문에 로컬 환경이 아님을 유의해야 합니다.
필요한 경우 폰트 설치와 같은 설정을 추가로 해주어야 합니다.
관련 이슈
Summary by CodeRabbit
.gitignore
파일에 테스트 및 보고서 관련 파일 추가.package.json
에 E2E 테스트 관련 스크립트 및 종속성 추가.