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

[FEAT] playwright E2E 테스트 적용: basic setting #61

Merged
merged 11 commits into from
Feb 5, 2024
Merged

[FEAT] playwright E2E 테스트 적용: basic setting #61

merged 11 commits into from
Feb 5, 2024

Conversation

selfishAltruism
Copy link
Contributor

@selfishAltruism selfishAltruism commented Feb 4, 2024 •
edited
Loading

🚩 관련 이슈
resolve #59

📋 PR Checklist

  • playwright install
  • 로그인 테스트 구현

📌 유의사항

  • 테스트 과정을 시각적으로 안 봐도 된다면 playwright.config.ts에서 headless: false, 지우면 됩니다.

✅ 테스트 결과

  • 예시 테스트 구동 영상
1.mp4

@selfishAltruism selfishAltruism added the feat 새로운 기능, 코드 추가 label Feb 4, 2024
@selfishAltruism selfishAltruism self-assigned this Feb 4, 2024
@netlify Netlify
Copy link

netlify bot commented Feb 4, 2024 •
edited
Loading

✅ Deploy Preview for caucse ready!

Name Link
🔨 Latest commit f7f953e
🔍 Latest deploy log https://app.netlify.com/sites/caucse/deploys/65c05bb9b6df77000848f59b
😎 Deploy Preview https://deploy-preview-61--caucse.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse		 1 paths audited
Performance: 91
Accessibility: 97
Best Practices: 92
SEO: 92
PWA: 80
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

rbgksqkr
rbgksqkr approved these changes Feb 4, 2024
View reviewed changes
Copy link
Contributor

@rbgksqkr rbgksqkr left a comment

Choose a reason for hiding this comment

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

e2e테스트 세팅 고생하셨습니다!! 코멘트에 의견만 주시고 바로 머지해도 될 것 같아요

playwright.config.ts
/* Run tests in files in parallel */
fullyParallel: true,
/* Fail the build on CI if you accidentally left test.only in the source code. */
forbidOnly: !!process.env.CI,
Copy link
Contributor

Choose a reason for hiding this comment

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

여기서는 환경변수 잘 불러와지나요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

저런 식으로 빌드인 값들은 가능한 것 같더라고요. 근데 test 코드 돌리는 컴파일에서는 환경변수가 사용이 안됩니다. 길게 붙잡고 있었는데도 안되더라고요 ㅠㅠ

tests/tester/lockerTester.ts
await page.click(
'#root > div > main > div > div.css-1nqe5pb-Wrapper.egppv972 > a.css-xetqcl-ClearLink.ec7tn3u2 > div.css-1fukgrq-Circle.egppv971',
);
await page.waitForURL(baseURL + PAGE_URL.Locker);
Copy link
Contributor

Choose a reason for hiding this comment

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

테스트 코드니까 해당 화면에서 보여지는 것을 검증하는 expect 문이 있으면 좋을 것 같아요

Copy link
Contributor

Choose a reason for hiding this comment

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

어떠신가요??
근데 초반 틀을 이렇게 잡고 제 것도 추가한다음 나중에 개선하는 것도 괜찮은 것 같아요

Copy link
Contributor Author

Choose a reason for hiding this comment

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

추가할까 했는데, await page.waitForURL(baseURL + PAGE_URL.Locker); 파트 자체가 사실 expect랑 동일한 역활을 하기는 합니다. 그래도 어느 정도의 검증이 필요할 것 같으니까 내일 중으로 조금 더 세밀하게 만들어 보겠습니다.

Copy link
Contributor

Choose a reason for hiding this comment

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

저도 비슷한 생각이긴한데 await page.waitForURL(baseURL + PAGE_URL.Locker); 자체는 페이지 이동 완료하면 성공하는 것 같아서 그 페이지에 뜨는 데이터를 검증하는 것도 괜찮아 보이네요
근데 이번 이슈에선 merge 하고 다음 이슈에서 개선하는 방향이 좋을 것 같아요!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

넵 알겠습니다! 내일 오전 중으로 본 PR 처리할게요!

tests/tester/settingTester.ts
await page.getByText('개인정보 관리').click();
await page.waitForURL(baseURL + PAGE_URL.SettingProfile);
await page.click(settingBackButton);
await page.waitForURL(baseURL + PAGE_URL.Setting);
Copy link
Contributor

Choose a reason for hiding this comment

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

여기도 위와 같이 expect가 추가되면 좋을 것 같아요!

tests/tests.spec.ts

test.beforeEach(signinTester);
test('setting', settingRouterTester);
test('locker', lockerRouterTester);
Copy link
Contributor

Choose a reason for hiding this comment

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

이렇게 분리가 되네요 코드 분리 완전깔끔 👍👍

rbgksqkr
rbgksqkr reviewed Feb 4, 2024
View reviewed changes
.github/workflows/playwright.yml Outdated
push:
branches: [ main, master ]
pull_request:
branches: [ main, master ]
Copy link
Contributor

Choose a reason for hiding this comment

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

기본 세팅같은데 master는 지워도 되겠네요!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

한번 지워볼게요!

@selfishAltruism selfishAltruism merged commit ec1efef into develop Feb 5, 2024
4 checks passed
@selfishAltruism selfishAltruism deleted the feat/#59 branch February 11, 2024 07:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rbgksqkr rbgksqkr rbgksqkr approved these changes

Assignees

@selfishAltruism selfishAltruism

Labels
feat 새로운 기능, 코드 추가
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@selfishAltruism @rbgksqkr