-
Notifications
You must be signed in to change notification settings - Fork 18
test(user-link): add user-link e2e UI test #66
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
WalkthroughThis pull request introduces two new Playwright test suites for UI components: one for the locales component and another for the user-link component. Each suite includes tests that validate the rendering and functionality of their respective components by navigating to specific pages, capturing screenshots, and asserting visibility of elements during user interactions. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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 (
|
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: 2
🧹 Outside diff range and nitpick comments (7)
tests/locales/xedsign.spec.ts (3)
1-3
: Consider translating the test suite description to English.The test suite description is currently in Chinese. For consistency and to improve readability for non-Chinese speaking developers, consider translating it to English.
Suggested change:
-test.describe('locales组件xdesign规范', () => { +test.describe('Locales component xdesign specification', () => {
4-10
: Approve test setup and suggest translating the test case title.The test setup, including error handling, navigation, and initial assertions, is well-structured. However, for consistency, consider translating the test case title to English.
Suggested change:
- test('默认--UI截图', async ({ page }) => { + test('Default--UI screenshot', async ({ page }) => {
12-16
: Approve hover interaction test with suggestions for improvement.The hover interaction test is well-structured. However, consider the following improvements:
- Translate the Chinese comment to English for consistency.
- Use a more robust selector for the 'zhCN' link to avoid potential issues if the page structure changes.
Suggested changes:
- // hover时截图 + // Take screenshot on hover - const link = demo.getByText('zhCN').nth(1) + const link = demo.getByRole('link', { name: 'zhCN' }).nth(1)Also, consider adding a data-testid attribute to the link for even more robust selection:
<a href="#" data-testid="zh-cn-link">zhCN</a>Then you can select it like this:
const link = demo.getByTestId('zh-cn-link')tests/user-link/xedsign.spec.ts (4)
1-3
: Consider translating the test suite description to English.The test suite description is currently in Chinese. For consistency and to improve readability for non-Chinese speaking developers, consider translating it to English.
Suggested translation:
test.describe('user-link component xdesign specification', () => {
4-10
: Approve test setup and initial assertions, but consider translating the test case title.The test setup, error handling, navigation, and initial assertions are well-structured and appropriate for UI testing. However, for consistency, consider translating the test case title to English.
Suggested translation:
test('Default--UI screenshot', async ({ page }) => {
18-22
: Approve expand button test, but suggest using a more robust locator.The expand button test logic is correct and consistent with the previous interaction test. However, using text content for locating elements can be fragile, especially if the application supports multiple languages.
Consider using a more robust locator that doesn't depend on the button text. For example:
const expand = page.locator('[data-testid="expand-button"]')This assumes you can add a
data-testid
attribute to the expand button in the component code.
1-23
: Overall, well-structured e2e test with room for minor improvements.This test file provides a comprehensive e2e test for the user-link component, covering important UI interactions and visual regression testing. The test structure is clear and the assertions are appropriate.
Areas for improvement:
- Ensure language consistency by translating Chinese text to English throughout the file.
- Update comments to accurately reflect test actions (e.g., click vs. hover).
- Consider using more robust element locators that are less dependent on text content.
These changes will enhance the maintainability and clarity of the test suite.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
🔇 Files ignored due to path filters (5)
tests/locales/xedsign.spec.ts-snapshots/custom-chromium-win32.png
is excluded by!**/*.png
tests/locales/xedsign.spec.ts-snapshots/custom-service-chromium-win32.png
is excluded by!**/*.png
tests/user-link/xedsign.spec.ts-snapshots/custom-chromium-win32.png
is excluded by!**/*.png
tests/user-link/xedsign.spec.ts-snapshots/custom-service-chromium-win32.png
is excluded by!**/*.png
tests/user-link/xedsign.spec.ts-snapshots/expand-chromium-win32.png
is excluded by!**/*.png
📒 Files selected for processing (2)
- tests/locales/xedsign.spec.ts (1 hunks)
- tests/user-link/xedsign.spec.ts (1 hunks)
import { expect, test } from '@playwright/test' | ||
|
||
test.describe('locales组件xdesign规范', () => { | ||
test('默认--UI截图', async ({ page }) => { | ||
page.on('pageerror', (exception) => expect(exception).toBeNull()) | ||
await page.goto('locales#custom-service') | ||
const demo = page.locator('#custom-service .pc-demo') | ||
const demoPage = page.locator('#doc-layout div').filter({ hasText: '示例API自定义服务get-locale' }).nth(2) | ||
await expect(demo).toBeInViewport() | ||
await expect(demo).toHaveScreenshot('custom-service.png') | ||
|
||
// hover时截图 | ||
const link = demo.getByText('zhCN').nth(1) | ||
await link.hover() | ||
await expect(demoPage).toBeInViewport() | ||
await expect(demoPage).toHaveScreenshot('custom.png') | ||
}) | ||
}) |
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.
🛠️ Refactor suggestion
Suggest additional test coverage and improved error handling.
The current test provides good coverage for the UI appearance of the custom service component. However, consider the following improvements:
- Add functional tests to verify the behavior of the component, not just its appearance.
- Implement error handling or retry logic for potentially flaky UI interactions.
- Consider testing different locales to ensure the component behaves correctly in various language settings.
Example of adding a functional test:
test('Locale switching functionality', async ({ page }) => {
await page.goto('locales#custom-service');
const demo = page.locator('#custom-service .pc-demo');
// Click on 'zhCN' link
await demo.getByRole('link', { name: 'zhCN' }).click();
// Assert that the locale has changed
await expect(demo.locator('.current-locale')).toHaveText('zh-CN');
// Click on 'enUS' link
await demo.getByRole('link', { name: 'enUS' }).click();
// Assert that the locale has changed
await expect(demo.locator('.current-locale')).toHaveText('en-US');
});
Example of adding retry logic:
const retry = async (action, maxAttempts = 3) => {
for (let attempt = 1; attempt <= maxAttempts; attempt++) {
try {
await action();
return;
} catch (error) {
if (attempt === maxAttempts) throw error;
await page.waitForTimeout(1000); // Wait before retrying
}
}
};
// Use it in your test
await retry(async () => {
await link.hover();
await expect(demoPage).toBeInViewport();
});
// hover时截图 | ||
const link = demo.getByText('test3') | ||
await link.click() | ||
await expect(demoPage).toBeInViewport() | ||
await expect(demoPage).toHaveScreenshot('custom.png') |
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.
Correct the comment to match the actual test behavior.
The comment suggests that this section is for capturing a screenshot on hover, but the test actually performs a click action. Update the comment to accurately reflect the test behavior.
Suggested change:
// Click interaction screenshot
Also, consider adding a hover interaction test if that was the original intention.
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
locales
anduser-link
components to validate rendering and functionality.Tests