-
Notifications
You must be signed in to change notification settings - Fork 17
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
test(picker): Add UI test cases for timePicker and datePicker #77
base: dev
Are you sure you want to change the base?
Conversation
WalkthroughThis pull request introduces two new test files for automated UI testing of the date-picker and time-picker components using Playwright. The tests cover various functionalities, including basic usage, date range selection, disabled states, and input clearing for the date-picker, while the time-picker tests focus on default UI states, fixed time ranges, and disabled states. Each test suite includes error handling and captures screenshots to verify the UI's behavior during interactions. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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 (9)
tests/time-picker/xdesign.spec.ts (5)
4-28
: Comprehensive test case with room for improvementThe test case effectively covers multiple states of the time-picker UI, including default, hover, popup, and time option hover states. The use of screenshots for visual regression testing is commendable.
However, consider the following improvements:
- Use data-testid attributes instead of text content for more robust selectors.
- Consider parameterizing the test to reduce duplication for different states.
- Add assertions for element visibility or state before taking screenshots.
Here's an example of how you could refactor the selectors:
const demo = page.locator('[data-testid="basic-usage-demo"]'); const input = demo.locator('[data-testid="time-picker-input"]'); const timeOption = page.locator('[data-testid="time-option-39"]');And an example of adding visibility assertions:
await expect(input).toBeVisible(); await input.hover(); await expect(demo).toHaveScreenshot('basic-hover.png');
30-40
: Concise test case for fixed time rangeThis test case effectively checks the UI for fixed time range options. The focus on a specific feature is good for targeted testing.
Suggestions for improvement:
- Use data-testid attributes for more robust selectors.
- Add an assertion to verify that the time picker popup is visible before taking the screenshot.
- Consider testing the selection of a time within the fixed range.
Here's an example of how you could improve the test:
test('固定时间范围--UI截图', async ({ page }) => { page.on('pageerror', (exception) => expect(exception).toBeNull()); await page.goto('time-picker#picker-options'); const demo = page.locator('[data-testid="picker-options-demo"]'); const input = demo.locator('[data-testid="time-picker-input"]'); const popup = page.locator('[data-testid="time-picker-popup"]'); await input.click(); await expect(popup).toBeVisible(); await expect(page).toHaveScreenshot('picker-options.png'); // Test selecting a time const timeOption = popup.locator('[data-testid="time-option-10:00"]'); await timeOption.click(); await expect(input).toHaveValue('10:00:00'); });
42-55
: Time range selection test case needs enhancementThis test case covers the hover and click states of the time range picker, which is a good start. However, it doesn't fully test the functionality of selecting a time range.
Suggestions for improvement:
- Use data-testid attributes for more robust selectors.
- Actually select a start and end time to test the full functionality.
- Verify that the selected time range is correctly displayed in the input field.
- Consider testing edge cases, such as selecting an invalid range.
Here's an example of how you could enhance the test:
test('选择时间范围--UI截图', async ({ page }) => { page.on('pageerror', (exception) => expect(exception).toBeNull()); await page.goto('time-picker#is-range'); const demo = page.locator('[data-testid="is-range-demo"]'); const input = demo.locator('[data-testid="time-range-input"]'); const popup = page.locator('[data-testid="time-range-popup"]'); await input.hover(); await expect(demo).toHaveScreenshot('is-range-hover.png'); await input.click(); await expect(popup).toBeVisible(); await expect(page).toHaveScreenshot('is-range-click.png'); // Select start and end time const startTime = popup.locator('[data-testid="start-time-10:00"]'); const endTime = popup.locator('[data-testid="end-time-11:00"]'); await startTime.click(); await endTime.click(); await expect(input).toHaveValue('10:00:00 - 11:00:00'); // Test invalid range const invalidEndTime = popup.locator('[data-testid="end-time-09:00"]'); await invalidEndTime.click(); await expect(page.locator('[data-testid="error-message"]')).toBeVisible(); });
57-66
: Disabled state test case needs additional assertionsThis test case correctly focuses on the disabled state of the time-picker, which is crucial for accessibility. However, it could be more comprehensive.
Suggestions for improvement:
- Use data-testid attributes for more robust selectors.
- Add an assertion to verify that the input is actually disabled.
- Test clicking the disabled input to ensure it doesn't open the time picker.
- Consider testing the visual appearance of the disabled state (e.g., cursor style).
Here's an example of how you could enhance the test:
test('禁用状态--UI截图', async ({ page }) => { page.on('pageerror', (exception) => expect(exception).toBeNull()); await page.goto('time-picker#disabled'); const demo = page.locator('[data-testid="disabled-demo"]'); const input = demo.locator('[data-testid="disabled-time-picker-input"]'); // Verify the input is disabled await expect(input).toBeDisabled(); // Test hover state await input.hover(); await expect(demo).toHaveScreenshot('disabled-hover.png'); // Verify cursor style const cursorStyle = await input.evaluate((el) => window.getComputedStyle(el).cursor); expect(cursorStyle).toBe('not-allowed'); // Attempt to click and verify the picker doesn't open await input.click(); const popup = page.locator('[data-testid="time-picker-popup"]'); await expect(popup).not.toBeVisible(); });
1-67
: Well-structured test suite with room for optimizationThe test suite is logically organized and covers important aspects of the time-picker functionality. Each test case focuses on a specific feature or state, which is good for maintainability and clarity.
Suggestions for improvement:
- Consider using
test.beforeEach()
to handle common setup operations like navigation and error handling.- Implement
test.afterEach()
for any necessary teardown operations.- Create helper functions for common operations like capturing screenshots to reduce code duplication.
- Consider splitting the test suite into multiple files if it grows larger, grouping related tests together.
Here's an example of how you could refactor the test suite structure:
import { expect, test } from '@playwright/test'; test.describe('time-picker 组件xdesign规范', () => { test.beforeEach(async ({ page }) => { page.on('pageerror', (exception) => expect(exception).toBeNull()); }); const captureScreenshot = async (page, selector, name) => { await expect(page.locator(selector)).toBeInViewport(); await expect(page.locator(selector)).toHaveScreenshot(`${name}.png`); }; test('默认--UI截图', async ({ page }) => { await page.goto('time-picker#basic-usage'); // ... (rest of the test case) }); // ... (other test cases) });This structure will make the test suite more maintainable and easier to extend in the future.
tests/date-picker/xdesign.spec.ts (4)
52-82
: Enhance readability with improved variable naming and descriptive comments.To improve the clarity and maintainability of this test case, consider the following suggestions:
Use more descriptive variable names. For example:
input1
could bedateRangeInput
input2
could bedateTimeRangeInput
input3
could bemonthRangeInput
input4
could beyearRangeInput
Add comments to explain the purpose of each section. For example:
// Test date range selection const dateRangeInput = demo.getByPlaceholder('结束日期').first() await dateRangeInput.click() // ... rest of the code // Test date-time range selection const dateTimeRangeInput = demo.getByPlaceholder('结束日期').nth(1) await dateTimeRangeInput.click() // ... rest of the codeConsider adding assertions to verify the expected behavior after each interaction, not just capturing screenshots.
These changes will make the test case more self-explanatory and easier to maintain in the long run.
84-100
: Add assertions to verify the disabled state of inputs.While the test case captures screenshots of the disabled state, it would be more robust to include explicit assertions that verify the disabled state of the inputs. This ensures that the test not only visually checks the component but also programmatically confirms its state.
Consider adding assertions like:
await expect(input).toBeDisabled(); await expect(rangeInput).toBeDisabled();These assertions will help catch any regressions in the disabled state functionality, even if the visual appearance remains unchanged.
114-124
: Enhance test to verify the presence of week numbers.While the current test captures a screenshot of the date picker with custom week numbering, it doesn't explicitly verify the presence of these week numbers. To make the test more robust, consider adding assertions that check for the presence and correctness of the week numbers.
Here's an example of how you could enhance the test:
await input.click(); await expect(demoPage).toBeInViewport(); // Wait for the date picker to be visible const datePicker = page.locator('.tiny-date-picker__body'); await expect(datePicker).toBeVisible(); // Check for the presence of week numbers const weekNumbers = datePicker.locator('.tiny-date-table__week-number'); await expect(weekNumbers.first()).toBeVisible(); // Verify the content of the first week number (adjust as needed) await expect(weekNumbers.first()).toHaveText('1'); // Capture the screenshot await expect(demoPage).toHaveScreenshot('weeks.png');These additional checks will ensure that the week numbers are not only visually present but also correctly implemented in the date picker component.
1-3
: Add a file-level comment to explain the test suite's purpose and structure.To improve the overall readability and maintainability of this test file, consider adding a comprehensive comment at the beginning of the file. This comment should explain the purpose of the test suite, the component being tested, and provide an overview of the different test cases included.
For example:
/** * UI Tests for Date Picker Component * * This file contains Playwright tests for the Date Picker component, * following the xdesign specification. The test suite covers various * functionalities and states of the Date Picker, including: * * 1. Basic usage (default state) * 2. Date range selection * 3. Disabled state * 4. Input clearing functionality * 5. Custom week numbering * * Each test case navigates to a specific section of the component's * documentation, interacts with the component, and captures screenshots * to verify the visual state and behavior of the Date Picker. */ import { expect, test } from '@playwright/test' // ... rest of the fileThis comment will provide valuable context for developers who may work on or review these tests in the future.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (28)
tests/date-picker/xdesign.spec.ts-snapshots/basic-click1-chromium-win32.png
is excluded by!**/*.png
tests/date-picker/xdesign.spec.ts-snapshots/basic-click2-chromium-win32.png
is excluded by!**/*.png
tests/date-picker/xdesign.spec.ts-snapshots/basic-click3-chromium-win32.png
is excluded by!**/*.png
tests/date-picker/xdesign.spec.ts-snapshots/basic-click4-chromium-win32.png
is excluded by!**/*.png
tests/date-picker/xdesign.spec.ts-snapshots/basic-click5-chromium-win32.png
is excluded by!**/*.png
tests/date-picker/xdesign.spec.ts-snapshots/basic-click6-chromium-win32.png
is excluded by!**/*.png
tests/date-picker/xdesign.spec.ts-snapshots/basic-usage-chromium-win32.png
is excluded by!**/*.png
tests/date-picker/xdesign.spec.ts-snapshots/clear-hover-chromium-win32.png
is excluded by!**/*.png
tests/date-picker/xdesign.spec.ts-snapshots/date-range1-chromium-win32.png
is excluded by!**/*.png
tests/date-picker/xdesign.spec.ts-snapshots/date-range2-chromium-win32.png
is excluded by!**/*.png
tests/date-picker/xdesign.spec.ts-snapshots/date-range3-chromium-win32.png
is excluded by!**/*.png
tests/date-picker/xdesign.spec.ts-snapshots/date-range4-chromium-win32.png
is excluded by!**/*.png
tests/date-picker/xdesign.spec.ts-snapshots/disabled-hover-chromium-win32.png
is excluded by!**/*.png
tests/date-picker/xdesign.spec.ts-snapshots/disabled-range-chromium-win32.png
is excluded by!**/*.png
tests/date-picker/xdesign.spec.ts-snapshots/weeks-chromium-win32.png
is excluded by!**/*.png
tests/time-picker/xdesign.spec.ts-snapshots/basic-click-chromium-win32.png
is excluded by!**/*.png
tests/time-picker/xdesign.spec.ts-snapshots/basic-hover-chromium-win32.png
is excluded by!**/*.png
tests/time-picker/xdesign.spec.ts-snapshots/basic-usage-chromium-win32.png
is excluded by!**/*.png
tests/time-picker/xdesign.spec.ts-snapshots/disabled-hover-chromium-win32.png
is excluded by!**/*.png
tests/time-picker/xdesign.spec.ts-snapshots/is-range-chromium-win32.png
is excluded by!**/*.png
tests/time-picker/xdesign.spec.ts-snapshots/is-range-click-chromium-win32.png
is excluded by!**/*.png
tests/time-picker/xdesign.spec.ts-snapshots/is-range-hover-chromium-win32.png
is excluded by!**/*.png
tests/time-picker/xdesign.spec.ts-snapshots/picker-options-chromium-win32.png
is excluded by!**/*.png
tests/time-picker/xdesign.spec.ts-snapshots/size-default-chromium-win32.png
is excluded by!**/*.png
tests/time-picker/xdesign.spec.ts-snapshots/size-medium-chromium-win32.png
is excluded by!**/*.png
tests/time-picker/xdesign.spec.ts-snapshots/size-mini-chromium-win32.png
is excluded by!**/*.png
tests/time-picker/xdesign.spec.ts-snapshots/size-small-chromium-win32.png
is excluded by!**/*.png
tests/time-picker/xdesign.spec.ts-snapshots/time-hover-chromium-win32.png
is excluded by!**/*.png
📒 Files selected for processing (2)
- tests/date-picker/xdesign.spec.ts (1 hunks)
- tests/time-picker/xdesign.spec.ts (1 hunks)
🧰 Additional context used
test('默认--UI截图', async ({ page }) => { | ||
page.on('pageerror', (exception) => expect(exception).toBeNull()) | ||
await page.goto('date-picker#basic-usage') | ||
const demo = page.locator('#basic-usage .pc-demo') | ||
const demoPage = page.locator('#doc-layout div').filter({ hasText: '示例API基本用法支持选择日期、日期时间、周、月份、年份。' }).nth(2) | ||
await expect(demo).toBeInViewport() | ||
await expect(demo).toHaveScreenshot('basic-usage.png') | ||
|
||
const close = page.getByText('支持选择日期、日期时间、周、月份、年份。') | ||
const input1 = demo.getByPlaceholder('请选择日期').nth(1) | ||
const input2 = demo.getByPlaceholder('请选择日期').nth(3) | ||
const input3 = demo.getByRole('textbox', { name: '请选择周' }) | ||
const input4 = demo.getByRole('textbox', { name: '请选择月份' }) | ||
const input5 = demo.getByRole('textbox', { name: '请选择季度' }) | ||
const input6 = demo.getByRole('textbox', { name: '请选择年份' }) | ||
|
||
await input1.click() | ||
await expect(demoPage).toBeInViewport() | ||
await expect(demoPage).toHaveScreenshot('basic-click1.png') | ||
await close.click() | ||
|
||
await input2.click() | ||
await expect(demoPage).toBeInViewport() | ||
await expect(demoPage).toHaveScreenshot('basic-click2.png') | ||
await close.click() | ||
|
||
await input3.click() | ||
await expect(demoPage).toBeInViewport() | ||
await expect(demoPage).toHaveScreenshot('basic-click3.png') | ||
await close.click() | ||
|
||
await input4.click() | ||
await expect(demoPage).toBeInViewport() | ||
await expect(demoPage).toHaveScreenshot('basic-click4.png') | ||
await close.click() | ||
|
||
await input5.click() | ||
await expect(demoPage).toBeInViewport() | ||
await expect(demoPage).toHaveScreenshot('basic-click5.png') | ||
await close.click() | ||
|
||
await input6.click() | ||
await expect(demoPage).toBeInViewport() | ||
await expect(demoPage).toHaveScreenshot('basic-click6.png') | ||
await close.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.
🛠️ Refactor suggestion
Consider splitting the test case into smaller, more focused tests.
While this test case thoroughly covers various aspects of the date-picker's basic usage, it might be beneficial to split it into smaller, more focused tests. This approach would:
- Improve test readability and maintainability.
- Make it easier to identify which specific functionality fails if an error occurs.
- Allow for more granular testing and potentially faster test execution.
For example, you could create separate test cases for:
- Default state verification
- Date selection
- Date-time selection
- Week selection
- Month selection
- Quarter selection
- Year selection
This way, each test would focus on a specific functionality, making the test suite more robust and easier to maintain.
test('清除输入--UI截图', async ({ page }) => { | ||
page.on('pageerror', (exception) => expect(exception).toBeNull()) | ||
await page.goto('date-picker#clear') | ||
const demo = page.locator('#clear .pc-demo') | ||
|
||
const input = demo.getByRole('textbox').nth(1) | ||
await input.hover() | ||
await expect(demo).toBeInViewport() | ||
await expect(demo).toHaveScreenshot('clear-hover.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.
Expand test to verify input clearing functionality.
The current test case only checks the hover state of the input field but doesn't actually test the clearing functionality. To make this test more comprehensive, consider the following improvements:
- Set a value in the input field before testing.
- Trigger the clear action (e.g., by clicking the clear button).
- Verify that the input field is empty after clearing.
Here's an example of how you could expand the test:
const input = demo.getByRole('textbox').nth(1);
// Set a value
await input.fill('2023-01-01');
// Verify the value is set
await expect(input).toHaveValue('2023-01-01');
// Hover and capture screenshot
await input.hover();
await expect(demo).toHaveScreenshot('clear-hover.png');
// Clear the input
const clearButton = input.locator('..').getByRole('button');
await clearButton.click();
// Verify the input is cleared
await expect(input).toHaveValue('');
// Capture screenshot of cleared state
await expect(demo).toHaveScreenshot('clear-after.png');
This expanded test will provide better coverage of the clear input functionality.
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
Bug Fixes