-
Notifications
You must be signed in to change notification settings - Fork 0
Test#151 Playwright e2e 테스트 환경 구축 #153
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
Summary of Changes
Hello @toothlessdev, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request significantly enhances the project's testing capabilities by establishing a dedicated End-to-End (E2E) testing environment using Playwright. It separates E2E test concerns into a new workspace, provides initial test coverage for a user flow, and refines the existing unit test coverage reporting for better developer experience.
Highlights
- E2E Test Environment Setup: A new
mosu-e2eworkspace has been established to house Playwright End-to-End tests. This includes a dedicatedpackage.json,.gitignore, andplaywright.config.tsfor the E2E suite, separating it from the main application. - Playwright Configuration & Execution: The Playwright configuration has been moved to the new
mosu-e2eworkspace, withbaseURLset tohttp://localhost:3000and awebServercommand configured to build and start themosu-appbefore E2E tests run. A roote2escript has been added for easy execution. - Sample E2E Test: A new E2E test (
signup.spec.ts) has been added to themosu-e2e/testsdirectory, demonstrating a basic user flow by navigating to the signup page and asserting the correct URL. - Vitest Coverage Enhancement: The
mosu-app'spackage.jsonnow includes an updatedtest:coveragescript that automatically opens the generated HTML coverage report. Additionally,vitest.config.tshas been configured to generate coverage reports in multiple formats (text, JSON, HTML). - Dependency Management: Playwright-related dependencies have been updated and moved to the new
mosu-e2eworkspace'spackage.json, and the rootyarn.lockhas been updated accordingly, including the addition of@types/node.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
📦 Package 변경사항
📝 추가 및 변경된 파일총 11개 파일 변경 |
kimgho
left a comment
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 테스트도 중요하죠!
앞으로 테스트 코드 계속 추가해봅시다 👍
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.
Code Review
This pull request sets up a new Playwright E2E testing environment in a dedicated mosu-e2e workspace, which is a great structural improvement. The changes include configuration for Playwright, a new test for the signup flow, and updates to the Vitest coverage script.
I've identified a few areas for improvement:
- Maintainability: The new E2E test uses hardcoded URLs and a brittle XPath selector. These should be updated to use Playwright's
baseURLand more robust locators to make the tests more resilient. - Cross-platform compatibility: The
test:coveragescript uses a macOS-specific command. - Developer Experience: The
webServercommand in the Playwright config could be optimized to speed up local development. - Correctness: There's a minor typo in one of the new npm scripts.
Overall, this is a solid foundation for E2E testing. Addressing these points will make the test suite more robust and easier for the whole team to work with.
| "scripts": { | ||
| "test": "playwright test", | ||
| "test:ui": "playwright test --ui", | ||
| "report" :"playwright show-report" |
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.
| import { test, expect } from "@playwright/test"; | ||
|
|
||
| test("회원가입 페이지로 이동합니다", async ({ page }) => { | ||
| await page.goto("http://localhost:3000/"); |
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.
The test contains a hardcoded URL. It's a best practice to use the baseURL configured in playwright.config.ts to make tests more maintainable and portable across different environments. You can use a relative path for page.goto().
| await page.goto("http://localhost:3000/"); | |
| await page.goto("/"); |
|
|
||
| test("회원가입 페이지로 이동합니다", async ({ page }) => { | ||
| await page.goto("http://localhost:3000/"); | ||
| await page.locator(`xpath=//*[@id="__next"]/header/nav/ul/li[8]/a`).click(); |
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.
Using XPath locators like this is highly discouraged as they are brittle and can easily break with small changes to the DOM structure. It's better to use user-facing locators that are more resilient to changes.
Playwright recommends using locators like page.getByRole(), page.getByText(), or page.getByTestId(). Please consider adding a data-testid attribute to the element or using a more robust locator.
For example, if the link has text "로그인", you could use:
await page.getByRole('link', { name: '로그인' }).click();This makes the test much more readable and less likely to break.
| await page.locator(`xpath=//*[@id="__next"]/header/nav/ul/li[8]/a`).click(); | ||
| await page.getByText("회원가입").click(); | ||
|
|
||
| await expect(page).toHaveURL("http://localhost:3000/auth/signup"); |
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.
The URL in the assertion is hardcoded. To make the test more robust and reusable across different environments, you should use a relative path. This will be combined with the baseURL from your Playwright configuration.
| await expect(page).toHaveURL("http://localhost:3000/auth/signup"); | |
| await expect(page).toHaveURL("/auth/signup"); |
| "test": "vitest", | ||
| "test:watch": "vitest --watch", | ||
| "test:coverage": "vitest run --coverage" | ||
| "test:coverage": "vitest run --coverage && open coverage/index.html" |
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.
The open command is specific to macOS. This will cause the test:coverage script to fail for developers using other operating systems like Linux or Windows. To ensure cross-platform compatibility, I recommend using a package like open-cli.
You would first add it to your dev dependencies:
yarn add -D open-cli
Then you can update the script.
"test:coverage": "vitest run --coverage && open-cli coverage/index.html"
| // reuseExistingServer: !process.env.CI, | ||
| // }, | ||
| webServer: { | ||
| command: "cd .. && yarn app build && yarn app start", |
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.
The webServer command includes yarn app build, which will run a production build every time the test server needs to be started. This can be time-consuming during local development.
Consider removing the build step from this command. Developers can run the build once manually when needed. The reuseExistingServer: !process.env.CI option will then work more effectively, as it will just start the server if it's not already running, without a full rebuild.
This will significantly speed up the local E2E testing workflow.
| command: "cd .. && yarn app build && yarn app start", | |
| command: "cd .. && yarn app start", |
📚 Storybook이 Chromatic에 배포되었습니다!
|
✅ Linked Issue
🔍 What I did