-
Notifications
You must be signed in to change notification settings - Fork 1
[25.07.26 / TASK-229] Feature - FE Git Action E2E CI 적용 #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
base: main
Are you sure you want to change the base?
Conversation
랭크 순위는 무조건 표시됩니다
Warning Rate limit exceeded@six-standard has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 9 minutes and 33 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
""" Walkthrough프로젝트에 새로운 GitHub Actions E2E 테스트 워크플로우가 추가되었습니다. .gitignore에 Cypress 스크린샷 폴더가 무시되도록 수정되었고, Cypress 환경 변수 값이 샘플 값으로 변경되었습니다. 리더보드 E2E 테스트에서 랭킹 관련 검증 코드가 삭제되었습니다. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
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.
Actionable comments posted: 1
🧹 Nitpick comments (4)
.gitignore (1)
51-53
: 스크린샷 외에도 비디오·결과 디렉터리도 무시하도록 확대 제안E2E 실패 시 Cypress 가
cypress/videos/
또는cypress/results/
같은 폴더에 결과물을 남길 수 있습니다..gitignore
에 스크린샷만 추가하면 일부 아티팩트가 커밋될 가능성이 있습니다. 동일한 이유로 다음 항목도 함께 무시하는 것이 깔끔합니다./cypress/screenshots +/cypress/videos +/cypress/resultscypress.config.ts (1)
15-21
: 환경변수 중복 정의로 유지보수 부담 발생
cypress.config.ts
의env
블록과 GitHub Actions 워크플로우에서 동일한 값을 두 번 선언하고 있습니다. 실제 값이 변경되면 두 곳을 모두 수정해야 하므로 드리프트 위험이 있습니다. 아래처럼.env
파일(또는 CIenv:
)만 신뢰하도록 구성 파일에서 삭제하거나process.env
참조로 대체하는 방식을 추천합니다.- env: { - NEXT_PUBLIC_BASE_URL: 'http://localhost:3000', - NEXT_PUBLIC_CHANNELTALK_PLUGIN_KEY: 'sample_key', - NEXT_PUBLIC_GA_ID: 'sample_id', - NEXT_PUBLIC_SENTRY_AUTH_TOKEN: 'sample_token', - NEXT_PUBLIC_SENTRY_DSN: 'sample_dsn', - }, + // 환경변수는 .env 또는 CI 설정에서 주입합니다..github/workflows/process-e2e.yaml (2)
27-34
: CI 내부 .env 생성 단계는 시크릿과 분리하면 더 안전플레인 텍스트로 토큰 샘플을 커밋/로그에 남기고 있습니다. 실제 시크릿이 필요해질 때를 대비해 GitHub
secrets
를 사용하여 주입하도록 미리 구조를 잡아두면 변경량이 최소화됩니다.- echo "NEXT_PUBLIC_SENTRY_AUTH_TOKEN=sample_token" >> .env + echo "NEXT_PUBLIC_SENTRY_AUTH_TOKEN=${{ secrets.SENTRY_AUTH_TOKEN }}" >> .env물론 현재는 더미 값이라 문제가 없지만, 습관적으로 시크릿 관리를 분리해 두는 편이 안전합니다.
61-66
: 스크린샷 업로드 경로가.gitignore
와 맞지 않을 때를 대비한 확인CI에서는
cypress/screenshots
만 업로드하지만, 테스트 스펙에서cy.screenshot({ capture: "runner" })
등으로 서브폴더가 생길 수 있습니다. 패턴을 와일드카드로 넓혀 두면 누락 방지에 도움이 됩니다.- path: cypress/screenshots + path: cypress/screenshots/**
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/process-e2e.yaml
(1 hunks).gitignore
(1 hunks)cypress.config.ts
(1 hunks)cypress/e2e/leaderboards.cy.ts
(0 hunks)
💤 Files with no reviewable changes (1)
- cypress/e2e/leaderboards.cy.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: process-e2e-test
🔇 Additional comments (1)
.github/workflows/process-e2e.yaml (1)
41-43
:wait-on
타임아웃 불일치앞 단계에서는 60 초(60000 ms)를 사용하고, Cypress Action에서는 120 초를 사용합니다. 빌드 시간이 길어지면 첫 번째
wait-on
에서 실패할 수 있습니다. 두 값을 동일하게 맞추거나, 첫wait-on
을 생략하고 Action의wait-on
만 활용해 중복을 제거하세요.
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.
고생하셨습니다! 👍 🔥 코멘트 확인 한 번 부탁드릴게요!
좋았던 점
- 테스트 실패시에 결과물(스크린샷) 남기는 것 좋을 것 같습니다!
- name: Set up Node.js | ||
uses: actions/setup-node@v4 | ||
with: | ||
node-version: 23 |
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.
코드래빗 코멘트처럼 LTS로 바꾸거나, api 리포지토리의 CI 처럼 여러 노드 버전에서 돌리는 게 좋을 것 같네요!
그런데 기존 FE 워크플로우를 보니 동일하게 23 버전으로 돌리긴 하네요. 기준님 생각은 어떠신가요?
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의 초반 코드를 해당 플로우의 코드에서 가져와서 23 버전으로 맞춘 것 같네요.
그래도 지금까지 플로우에서 큰 문제가 없었던 만큼, 우선 23 버전으로 두는 게 괜찮을 것 같습니다!
.gitignore
Outdated
# dependencies | ||
/node_modules | ||
/cypress/screenshots |
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.
이건 진짜 별 거 아닌데, 이게 56번째줄 testing 섹션에 있는게 어떨까 싶네요?!
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.
반영했습니다!
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.
실패시 slack 오도록 하는건 어떻게 되었나요? 노션에 내용 있길래 여쭤봅니다~~
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.
반영했습니다!
NEXT_PUBLIC_CHANNELTALK_PLUGIN_KEY: 'sample_key', | ||
NEXT_PUBLIC_GA_ID: 'sample_id', | ||
NEXT_PUBLIC_SENTRY_AUTH_TOKEN: 'sample_token', | ||
NEXT_PUBLIC_SENTRY_DSN: 'sample_dsn', |
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.
저번에 물어 본다는 것을 놓쳤는데
센트리 DSN 경우 node run-time 에서 모듈 올릴때 유효하지 않은 경우 뻑나거든요!
근데 cypress 는 이게 skip 이 되나봐요?!
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.
it('랭킹 순위가 표시되어야 한다', () => { | ||
cy.get('[data-testid="rank"], [class*="rank"]').should('be.visible'); | ||
cy.contains('1').should('be.visible'); | ||
cy.contains('2').should('be.visible'); | ||
}); | ||
|
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.
사용자랑 게시글 모두 보이니까 굳이 랭킹은 스킵한거죠?
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.
네 맞습니다!
랭킹 값이 렌더되면 유저 랭킹 순위는 무조건 표시되어서 (map의 인덱스 값 사용) 큰 문제가 없을 것 같아 제거했습니다
- name: Upload screenshots | ||
uses: actions/upload-artifact@v4 | ||
if: failure() | ||
with: | ||
name: cypress-screenshots | ||
path: cypress/screenshots |
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.
"artifact" 까진 너무 좋은데 이걸 저장해서 어디에 사용하나요?! next step 이 없는게 아쉽네요.
ps) artifact 링크를 slack hook 으로 전송 가능함!
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.
반영했습니다!
(실패 메세지에 artifact 링크가 포함되도록 수정했어요)
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.
좋았던 점
- cypress 가 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.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
.github/workflows/process-e2e.yaml (1)
68-82
: Slack 알림 Job이 테스트 성공 시에도 실행되어 불필요한 알림이 발생합니다
slackNotify
Job이needs
또는if: failure()
조건 없이 정의되어 있어,process-e2e-test
가 성공해도 실행됩니다. 실패 시에만 알림을 보내려면 다음과 같이 수정하세요.-slackNotify: - runs-on: ubuntu-latest +slackNotify: + needs: process-e2e-test # 테스트 Job 결과를 참조 + if: failure() # 실패한 경우에만 실행 + runs-on: ubuntu-latest
♻️ Duplicate comments (1)
.github/workflows/process-e2e.yaml (1)
16-20
: Node.js 23 버전은 GitHub Actions에서 아직 지원되지 않아 워크플로가 실패합니다
actions/setup-node
가 GA/LTS 로는 22.x까지밖에 제공되지 않습니다. LTS(20 또는 22)로 변경하거나, 다중 버전 매트릭스를 사용해 호환성을 확보해주세요.
🧹 Nitpick comments (1)
.github/workflows/process-e2e.yaml (1)
41-52
:wait-on
을 두 번 사용하여 대기 시간이 불필요하게 길어질 수 있습니다
라인 41에서 직접npx wait-on
을 호출한 뒤, Cypress Action(라인 50)에서도 동일 URL에 대해wait-on
을 수행합니다. 한 곳만 남겨도 충분하므로 중복을 제거해 Job 실행 시간을 단축할 수 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/process-e2e.yaml
(1 hunks)Dockerfile
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- Dockerfile
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.
반영해주신 부분 확인 했씁니다! 고생하셨습니다!! 🚀
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.
이전에 올려주셨던 Cypress E2E 테스트를 위한 CI군요!
고생하셨습니다~!!🔥
좋았던 점
- 실패 시 Slack 알림에 아티팩트 링크까지 포함된 부분 좋은 것 같아요! 실제로 실패 이유를 파악할 때 유용할 것 같습니다!
- wait-on으로 서버 기동 기다리는 부분도 좋았습니다!
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.
굿굿굿~ 빠른 코멘트 흡수 감사해요~
🔥 변경 사항
FE에서 풀 리퀘스트 실행 시 E2E 테스트가 실행되도록 개선하였습니다!
우선은 실제 동작하도록 완성 후, 성능 개선을 위해 캐싱 등을 도입할 계획입니다!
🏷 관련 이슈
📸 스크린샷 (UI 변경 시 필수)
X
📌 체크리스트
Summary by CodeRabbit
Chores
.gitignore
에 Cypress 스크린샷 디렉토리가 추가되어 해당 파일이 git에 포함되지 않습니다.Tests