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.

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

feat: setup Playwright e2e test including vFolder create/delete test #2647

Merged
merged 1 commit into from
Aug 22, 2024

Conversation

yomybaby
Copy link
Member

@yomybaby yomybaby commented Aug 20, 2024

Note

This PR is newly written referring to #2115

TL;DR

Added end-to-end testing setup using Playwright

What changed?

  • Installed Playwright as a dev dependency
  • Added Playwright configuration file (playwright.config.ts)
  • Created initial end-to-end tests for login functionality and virtual folder operations
  • Updated .gitignore to exclude Playwright-related files and directories
  • Added a data-testid attribute to the user dropdown button for easier test selection

How to test?

  1. Install dependencies: pnpm install
  2. Run the application locally
  3. Execute the tests: npx playwright test
  4. View the test results in the console or generated HTML report

Why make this change?

Implementing end-to-end tests helps ensure the application's critical paths function correctly from a user's perspective. This addition will improve the overall quality and reliability of the project by catching potential issues early in the development process and providing confidence during future changes or refactoring.

Copy link

graphite-app bot commented Aug 20, 2024

Your org requires the Graphite merge queue for merging into main

Add the label “flow:merge-queue” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “flow:hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

@github-actions github-actions bot added the size:XL 500~ LoC label Aug 20, 2024
Copy link
Member Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @yomybaby and the rest of your teammates on Graphite Graphite

Copy link

github-actions bot commented Aug 20, 2024

Coverage report for ./react

St.
Category Percentage Covered / Total
🔴 Statements 3.7% 215/5807
🔴 Branches 4.05% 159/3926
🔴 Functions 2.24% 43/1920
🔴 Lines 3.59% 204/5688

Test suite run success

71 tests passing in 7 suites.

Report generated by 🧪jest coverage report action from 7f61052

@yomybaby yomybaby marked this pull request as ready for review August 21, 2024 01:37
@yomybaby yomybaby mentioned this pull request Aug 21, 2024
6 tasks
@yomybaby yomybaby force-pushed the feature/setup-e2e-playwright branch 2 times, most recently from e4799e3 to 36eb42b Compare August 21, 2024 02:33
Copy link
Member

@gahyuun gahyuun left a comment

Choose a reason for hiding this comment

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

Is the command to execute test “npx playwright test” instead of “pnpm exec playwright test”?

@@ -0,0 +1,47 @@
import { test, expect } from "@playwright/test";
Copy link
Member

Choose a reason for hiding this comment

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

Parsing error: ESLint was configured to run on <tsconfigRootDir>/e2e/agent.test.ts using
I get the above error in all test files
In the tsconfig.json file, it looks like the e2e folder should be included in the include

Copy link
Member Author

Choose a reason for hiding this comment

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

You can see this error only in VSCode, right? This doesn't affect the running e2e test. If you can resolve this issue, please make a PR.

e2e/test-util.ts Outdated
}

export async function loginAsAdmin(page: Page) {
await login(page, "[email protected]", "wJalrXUt", "http://127.0.0.1:8090");
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't it be better to separate the endpoint or Id into a variable?
It would be inconvenient when setting the endpoint as test server.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it's better to manage a web server endpoint and web UI endpoint as well. But for now, it's better to keep now , and handle it later when we set up multiple Playwright configurations.

await page.goto(url.toString());
}

export async function createVFolderAndVerify(page: Page, folderName: string) {
Copy link
Member

Choose a reason for hiding this comment

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

Why does this function exist in test-util.ts and not in VFolder.test.ts?

Copy link
Member Author

Choose a reason for hiding this comment

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

When we write some test code which include a create vfolder such as model serving, this function clould be helpful. It's like a login function.

Copy link
Member

Choose a reason for hiding this comment

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

vfolder test fails.

Copy link
Member Author

Choose a reason for hiding this comment

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

Could you let me know about the details of the failed test?

Copy link
Member

Choose a reason for hiding this comment

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

This error occurs. I think Playwright can't seem to find the add button. Is this error only happening to me?

"Error: locator.click: Test timeout of 30000ms exceeded.
Call log:
  - waiting for getByRole('button', { name: 'plus Add' })"

@yomybaby yomybaby force-pushed the feature/setup-e2e-playwright branch from 36eb42b to ea2fa94 Compare August 22, 2024 06:49
@yomybaby yomybaby requested a review from gahyuun August 22, 2024 06:49
@github-actions github-actions bot added size:L 100~500 LoC and removed size:XL 500~ LoC labels Aug 22, 2024
@yomybaby yomybaby force-pushed the feature/setup-e2e-playwright branch 3 times, most recently from fe6a669 to e81d867 Compare August 22, 2024 10:30
Copy link

graphite-app bot commented Aug 22, 2024

Merge activity

…2647)

> [!NOTE]
> This PR is newly written referring to #2115

### TL;DR

Added end-to-end testing setup using Playwright

### What changed?

- Installed Playwright as a dev dependency
- Added Playwright configuration file (playwright.config.ts)
- Created initial end-to-end tests for login functionality and virtual folder operations
- Updated .gitignore to exclude Playwright-related files and directories
- Added a data-testid attribute to the user dropdown button for easier test selection

### How to test?

1. Install dependencies: `pnpm install`
2. Run the application locally
3. Execute the tests: `npx playwright test`
4. View the test results in the console or generated HTML report

### Why make this change?

Implementing end-to-end tests helps ensure the application's critical paths function correctly from a user's perspective. This addition will improve the overall quality and reliability of the project by catching potential issues early in the development process and providing confidence during future changes or refactoring.
@yomybaby yomybaby force-pushed the feature/setup-e2e-playwright branch from e81d867 to 7f61052 Compare August 22, 2024 11:32
@graphite-app graphite-app bot merged commit 7f61052 into main Aug 22, 2024
7 checks passed
@graphite-app graphite-app bot deleted the feature/setup-e2e-playwright branch August 22, 2024 11:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:L 100~500 LoC
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants