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

test(picker): Add UI test cases for timePicker and datePicker #77

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

Youyou-smiles
Copy link
Contributor

@Youyou-smiles Youyou-smiles commented Oct 10, 2024

PR

PR Checklist

Please check if your PR fulfills the following requirements:

  • The commit message follows our Commit Message Guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Summary by CodeRabbit

  • New Features

    • Introduced automated UI tests for the date-picker component, covering functionalities such as basic usage, date range selection, and custom week numbering.
    • Added automated UI tests for the time-picker component, focusing on default state, fixed time range, and disabled state interactions.
  • Bug Fixes

    • Enhanced error handling in test cases to ensure robustness against page errors.

Copy link

coderabbitai bot commented Oct 10, 2024

Walkthrough

This 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

File Path Change Summary
tests/date-picker/xdesign.spec.ts Introduced automated UI tests for the date-picker component, covering basic usage, date range selection, disabled states, input clearing, and custom week numbering. Each test captures screenshots and includes error handling.
tests/time-picker/xdesign.spec.ts Introduced automated UI tests for the time-picker component, focusing on default UI, fixed time range, selection of time ranges, and disabled states. Tests capture screenshots and include error handling.

Possibly related PRs

Suggested labels

e2e

Suggested reviewers

  • zzcr

Poem

In the garden of code, we hop and play,
Testing date and time in a joyful way.
With clicks and screenshots, we dance with glee,
Ensuring our components work flawlessly!
So here’s to the tests, both bright and bold,
A rabbit’s delight in the stories they hold! 🐇✨


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions github-actions bot added the e2e playwright label Oct 10, 2024
Copy link

@coderabbitai coderabbitai bot left a 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 improvement

The 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:

  1. Use data-testid attributes instead of text content for more robust selectors.
  2. Consider parameterizing the test to reduce duplication for different states.
  3. 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 range

This 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:

  1. Use data-testid attributes for more robust selectors.
  2. Add an assertion to verify that the time picker popup is visible before taking the screenshot.
  3. 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 enhancement

This 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:

  1. Use data-testid attributes for more robust selectors.
  2. Actually select a start and end time to test the full functionality.
  3. Verify that the selected time range is correctly displayed in the input field.
  4. 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 assertions

This 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:

  1. Use data-testid attributes for more robust selectors.
  2. Add an assertion to verify that the input is actually disabled.
  3. Test clicking the disabled input to ensure it doesn't open the time picker.
  4. 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 optimization

The 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:

  1. Consider using test.beforeEach() to handle common setup operations like navigation and error handling.
  2. Implement test.afterEach() for any necessary teardown operations.
  3. Create helper functions for common operations like capturing screenshots to reduce code duplication.
  4. 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:

  1. Use more descriptive variable names. For example:

    • input1 could be dateRangeInput
    • input2 could be dateTimeRangeInput
    • input3 could be monthRangeInput
    • input4 could be yearRangeInput
  2. 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 code
  3. Consider 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 file

This 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

📥 Commits

Files that changed from the base of the PR and between 9e4f2c5 and 66c223c.

⛔ 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

Comment on lines +4 to +50
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()

})
Copy link

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:

  1. Improve test readability and maintainability.
  2. Make it easier to identify which specific functionality fails if an error occurs.
  3. 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.

Comment on lines +102 to +111
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')
})
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

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:

  1. Set a value in the input field before testing.
  2. Trigger the clear action (e.g., by clicking the clear button).
  3. 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
e2e playwright
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant