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(select): Add UI test cases for select component #62

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions tests/select/allow-create.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { expect, test } from '@playwright/test'

test.describe('select 组件xdesign规范', () => {
test('面板搜索--UI截图', async ({ page }) => {
page.on('pageerror', (exception) => expect(exception).toBeNull())
await page.goto('select#allow-create')

const demo = page.locator('#allow-create .pc-demo')
const select = demo.locator('.tiny-select')
const wrap = page.locator('.docs-tabs-wrap')

await expect(demo).toBeInViewport()
await expect(demo).toHaveScreenshot('default.png')

await select.nth(2).click()
await expect(wrap).toHaveScreenshot('top-create.png')
})
Comment on lines +15 to +17
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance test coverage and clarity.

  1. Clarify why the third select option is chosen. Add a comment explaining the significance of this choice:
+// Clicking the third option to test the 'allow-create' functionality
await select.nth(2).click()
  1. Consider taking a screenshot of the select component itself after the interaction, in addition to or instead of the wrapper:
-await expect(wrap).toHaveScreenshot('top-create.png')
+await expect(select).toHaveScreenshot('select-allow-create-after-interaction.png')
  1. Add assertions to verify the component's behavior after interaction. For example:
// Verify that a new option can be created
await select.locator('input').fill('New Option')
await page.keyboard.press('Enter')
await expect(select.locator('.tiny-select__selected-text')).toHaveText('New Option')

These changes will improve the test's clarity and coverage of the 'allow-create' functionality.

Comment on lines +5 to +17
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance test coverage for 'allow-create' functionality.

While the current test provides a good foundation for visual regression testing, it doesn't fully cover the 'allow-create' functionality. Consider the following improvements:

  1. Add assertions to verify the component's behavior after interaction:

    await select.nth(2).click();
    await select.locator('input').fill('New Option');
    await page.keyboard.press('Enter');
    await expect(select.locator('.tiny-select__selected-text')).toHaveText('New Option');
  2. Test edge cases:

    • Creating an option that already exists
    • Empty input
    • Very long input
  3. Test keyboard navigation:

    • Use arrow keys to navigate options
    • Use Enter to select an option or create a new one
  4. Consider taking a screenshot of the select component itself after the interaction:

    await expect(select).toHaveScreenshot('select-allow-create-after-interaction.png');

These changes will provide more comprehensive coverage of the 'allow-create' functionality and improve the overall quality of the test.

})
Comment on lines +1 to +18
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Expand test coverage for 'allow-create' functionality.

While this test provides a good start for visual regression testing, it doesn't fully cover the 'allow-create' functionality implied by the file name. Consider adding the following test cases:

  1. Test creating a new option:

    • Enter a new value that doesn't exist in the options list.
    • Verify that the new option is created and selected.
  2. Test searching existing options:

    • Enter a partial match for an existing option.
    • Verify that matching options are displayed and can be selected.
  3. Test edge cases:

    • Try creating an option that already exists.
    • Test with empty input.
    • Test with very long input.
  4. Test keyboard navigation:

    • Use arrow keys to navigate options.
    • Use Enter to select an option or create a new one.

Example:

test('Create new option', async ({ page }) => {
  // ... setup code ...
  await select.click();
  await select.locator('input').fill('New Option');
  await page.keyboard.press('Enter');
  await expect(select.locator('.tiny-select__selected-text')).toHaveText('New Option');
});

test('Search existing options', async ({ page }) => {
  // ... setup code ...
  await select.click();
  await select.locator('input').fill('existing');
  await expect(select.locator('.tiny-select__dropdown-item')).toHaveCount(2);
});

// ... more test cases ...

These additional tests will provide more comprehensive coverage of the 'allow-create' functionality.

Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
14 changes: 14 additions & 0 deletions tests/select/automatic-dropdown.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { expect, test } from '@playwright/test'

test.describe('select 组件xdesign规范', () => {
test('手动聚焦失焦--UI截图', async ({ page }) => {
Comment on lines +3 to +4
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 improving test suite structure and naming conventions.

  1. The test suite description is in Chinese. For better international collaboration, consider using English descriptions or adding English translations alongside the Chinese text.

  2. The test suite contains only one test case. Consider adding more test cases to cover different scenarios for the select component, such as:

    • Opening and closing the dropdown
    • Selecting an option
    • Keyboard navigation
    • Disabled state behavior

Example of improved structure:

test.describe('Select Component UI Tests', () => {
  test('Manual focus and blur - UI screenshot', async ({ page }) => {
    // Existing test code
  });

  test('Open and close dropdown', async ({ page }) => {
    // New test case
  });

  // More test cases...
});

page.on('pageerror', (exception) => expect(exception).toBeNull())
await page.goto('select#automatic-dropdown')

Comment on lines +5 to +7
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Improve error handling and navigation.

  1. Enhance the error handling to catch all types of errors:
page.on('console', (msg) => {
  if (msg.type() === 'error') {
    throw new Error(`Console error: ${msg.text()}`);
  }
});
  1. The navigation URL seems incorrect. For page navigation, use a full URL:
await page.goto('http://your-test-server.com/path-to-page#automatic-dropdown')

Ensure the correct URL is used based on your test environment setup.

const demo = page.locator('#automatic-dropdown .pc-demo')
const select = demo.locator('.tiny-select')

await expect(demo).toBeInViewport()
await expect(demo).toHaveScreenshot('default.png')
Comment on lines +8 to +12
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance test case to match its description and improve assertions.

The test case title suggests manual focus and blur actions, but the current implementation doesn't perform these actions. Consider the following improvements:

  1. Implement focus and blur actions:
await select.focus();
await expect(select).toBeFocused();
await select.blur();
await expect(select).not.toBeFocused();
  1. Add more specific assertions for the select component's state:
await expect(select).toBeVisible();
await expect(select).not.toHaveClass(/(?:is-open|active)/);
  1. Take screenshots after each action to verify visual changes:
await expect(demo).toHaveScreenshot('default.png');
await select.focus();
await expect(demo).toHaveScreenshot('focused.png');
await select.blur();
await expect(demo).toHaveScreenshot('blurred.png');
  1. Consider adding a test for opening the dropdown:
await select.click();
await expect(select).toHaveClass(/(?:is-open|active)/);
await expect(demo).toHaveScreenshot('open-dropdown.png');

})
})
72 changes: 72 additions & 0 deletions tests/select/basic-usage.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import { expect, test } from '@playwright/test'

test.describe('select 组件xdesign规范', () => {
test('标签式--UI截图', async ({ page }) => {
page.on('pageerror', (exception) => expect(exception).toBeNull())
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance error handling

Consider modifying the error handling to provide more context:

page.on('pageerror', (exception) => {
  console.error(`Page error: ${exception.message}`);
  expect(exception).toBeNull();
});

This will make debugging easier by logging the error message before failing the test.

Also applies to: 43-43

await page.goto('select#basic-usage')

const demo = page.locator('#basic-usage .pc-demo')
const select = demo.locator('.tiny-select').nth(0)
const suffix = select.locator('.tiny-input__suffix-inner')
const wrap = page.locator('.docs-tabs-wrap')
const menu = page.locator('body > .tiny-select-dropdown').nth(0)

// 默认
await expect(demo).toBeInViewport()
await expect(demo).toHaveScreenshot('default.png')

// 悬浮高亮
await select.hover()
await page.waitForTimeout(100)
await expect(demo).toHaveScreenshot('hover.png')

// 点击下拉
await suffix.click()
await page.waitForTimeout(100)

// 悬浮item
await menu.locator('.tiny-option').nth(2).hover()
await page.waitForTimeout(100)
await expect(wrap).toHaveScreenshot('item-hover.png')

// 点击选中 item,收起弹窗
await menu.locator('.tiny-option').nth(2).click()
await expect(wrap).toHaveScreenshot('item-click.png')

// 再次点击下拉
await suffix.click()
await page.waitForTimeout(100)
await expect(wrap).toHaveScreenshot('item-checked.png')
})
Comment on lines +4 to +40
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve test stability and maintainability

  1. Replace fixed timeouts with Playwright's built-in waiting mechanisms:

    await page.waitForSelector('.tiny-select-dropdown', { state: 'visible' });
  2. Consider refactoring common steps into a reusable function:

    async function testSelectInteraction(page, selectIndex, optionIndex, screenshotPrefix) {
      // ... common test steps ...
    }
  3. Use data attributes for more resilient element selection:

    const select = demo.locator('[data-test-id="label-style-select"]');

These changes will make the tests more stable and easier to maintain.


test('配置式--UI截图', async ({ page }) => {
page.on('pageerror', (exception) => expect(exception).toBeNull())
await page.goto('select#basic-usage')

const demo = page.locator('#basic-usage .pc-demo')
const select = demo.locator('.tiny-select').nth(1)
const suffix = select.locator('.tiny-input__suffix-inner')
const wrap = page.locator('.docs-tabs-wrap')
const menu = page.locator('body > .tiny-select-dropdown').nth(0)

await expect(demo).toBeInViewport()
await expect(demo).toHaveScreenshot('default1.png')

await select.hover()
await page.waitForTimeout(100)
await expect(demo).toHaveScreenshot('hover1.png')

await suffix.click()
await page.waitForTimeout(100)
await menu.locator('.tiny-option').nth(3).hover()
await page.waitForTimeout(100)
await expect(wrap).toHaveScreenshot('item-hover1.png')

await menu.locator('.tiny-option').nth(3).click()
await expect(wrap).toHaveScreenshot('item-click1.png')

await suffix.click()
await page.waitForTimeout(100)
await expect(wrap).toHaveScreenshot('item-checked1.png')
})
Comment on lines +42 to +71
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve screenshot naming and reduce duplication

  1. Use more descriptive names for screenshots:

    -await expect(demo).toHaveScreenshot('default1.png')
    +await expect(demo).toHaveScreenshot('config-style-default.png')
  2. Implement the refactoring suggestion from the previous comment to reduce duplication between this test case and the first one.

These changes will make the tests more maintainable and the screenshots more meaningful.

Committable suggestion was skipped due to low confidence.

})
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
14 changes: 14 additions & 0 deletions tests/select/binding-obj.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { expect, test } from '@playwright/test'

test.describe('select 组件xdesign规范', () => {
test('绑定值为对象--UI截图', async ({ page }) => {
page.on('pageerror', (exception) => expect(exception).toBeNull())
await page.goto('select#binding-obj')

Comment on lines +4 to +7
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance test case readability and robustness.

  1. As suggested in a previous review, consider using English for the test case description to improve international collaboration.
  2. The current navigation method using a selector might be brittle. Consider using a more robust navigation method.
  3. The error handler setup is a good practice for catching unexpected errors.

Apply these changes:

-test('绑定值为对象--UI截图', async ({ page }) => {
+test('Binding object value - UI screenshot', async ({ page }) => {
   page.on('pageerror', (exception) => expect(exception).toBeNull())
-  await page.goto('select#binding-obj')
+  await page.goto('/path/to/select-component-page')
+  await page.waitForSelector('#binding-obj')

Replace '/path/to/select-component-page' with the actual path to the page containing the select component.

Committable suggestion was skipped due to low confidence.

const demo = page.locator('#binding-obj .pc-demo')
const select = demo.locator('.tiny-select')

Comment on lines +4 to +10
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance test case readability and robustness.

  1. Consider using English for the test case description to improve international collaboration.

  2. The current navigation method using a selector might be brittle. Consider using a more robust navigation method.

  3. Change the test case description to English:

-test('绑定值为对象--UI截图', async ({ page }) => {
+test('Binding object value - UI screenshot', async ({ page }) => {
  1. Use a more robust navigation method:
-await page.goto('select#binding-obj')
+await page.goto('/path/to/select-component-page')
+await page.waitForSelector('#binding-obj')

Replace '/path/to/select-component-page' with the actual path to the page containing the select component.

Committable suggestion was skipped due to low confidence.

await expect(demo).toBeInViewport()
await expect(demo).toHaveScreenshot('default.png')
})
Comment on lines +11 to +13
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance test coverage with interaction and functionality checks.

While the current test verifies the initial UI state, it doesn't test the select component's interaction or functionality. Consider adding more test cases to cover these aspects.

Add test cases for:

  1. Opening the select dropdown
  2. Selecting an option
  3. Verifying the selected value

Example:

await select.click();
await page.waitForSelector('.tiny-select-dropdown');
await page.click('.tiny-select-dropdown-item:first-child');
await expect(select).toHaveText('Selected Option');

Also, consider adding a test case for the specific object binding behavior mentioned in the test description.

Comment on lines +8 to +13
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance test coverage with interaction and functionality checks.

The current test verifies the initial UI state and uses screenshot comparison for visual regression testing, which are good practices. However, it doesn't test the select component's interaction or functionality.

Consider adding more test cases to cover these aspects:

  1. Opening the select dropdown
  2. Selecting an option
  3. Verifying the selected value

Example:

test('Binding object value - Interaction and functionality', async ({ page }) => {
  // ... (setup code)

  const select = demo.locator('.tiny-select');
  await select.click();
  await page.waitForSelector('.tiny-select-dropdown');
  await page.click('.tiny-select-dropdown-item:first-child');
  await expect(select).toHaveText('Selected Option');

  // Add assertions to verify the bound object value
});

Also, consider adding a test case for the specific object binding behavior mentioned in the test description.

})
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
16 changes: 16 additions & 0 deletions tests/select/cache-usage.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { expect, test } from '@playwright/test'

test.describe('select 组件xdesign规范', () => {
test('自动缓存--UI截图', async ({ page }) => {
page.on('pageerror', (exception) => expect(exception).toBeNull())
await page.goto('select#cache-usage')

Comment on lines +6 to +7
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Use a full URL for page navigation.

The current navigation uses a fragment identifier, which might not be sufficient for proper page navigation. Consider using a full URL to ensure consistent test execution across different environments.

Replace the current navigation with a full URL:

await page.goto('https://your-base-url.com/select#cache-usage')

Replace 'https://your-base-url.com' with the actual base URL of your application.

const demo = page.locator('#cache-usage .pc-demo')
const select = demo.locator('.tiny-select')
const wrap = page.locator('.docs-tabs-wrap')

await select.click()
Comment on lines +8 to +12
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add assertion to verify select component interaction.

The test clicks on the select component but doesn't verify if it opened successfully. Adding an assertion would make the test more robust.

Consider adding an assertion after the click, for example:

await select.click()
await expect(select).toHaveClass(/tiny-select--open/) // Adjust the class name as per your implementation

This ensures that the select component responds correctly to the click interaction.

await expect(wrap).toBeInViewport()
await expect(wrap).toHaveScreenshot('default.png')
})
Comment on lines +13 to +15
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance test coverage for caching behavior and clarify screenshot purpose.

While the test checks for visual consistency, it doesn't explicitly verify the caching behavior mentioned in the test title. Additionally, the purpose of the screenshot comparison isn't clear from the context.

  1. Add assertions to verify caching behavior. For example:
// Perform an action that should trigger caching
await select.click()
await select.locator('option').nth(1).click()

// Close and reopen the select
await page.click('body') // Click outside to close
await select.click()

// Verify that the previously selected option is still selected (cached)
await expect(select.locator('.tiny-select__selected-text')).toHaveText('Expected Cached Value')
  1. Add a comment explaining the purpose of the screenshot comparison:
// Verify the visual state of the select component after caching
await expect(wrap).toHaveScreenshot('default.png')

These changes will make the test more comprehensive and its purpose clearer.

})
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
14 changes: 14 additions & 0 deletions tests/select/clear-no-match-value.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { expect, test } from '@playwright/test'

test.describe('select 组件xdesign规范', () => {
test('自动清除不匹配的值--UI截图', async ({ page }) => {
page.on('pageerror', (exception) => expect(exception).toBeNull())
await page.goto('select#clear-no-match-value')

const demo = page.locator('#clear-no-match-value .pc-demo')
const select = demo.locator('.tiny-select')

await expect(demo).toBeInViewport()
await expect(demo).toHaveScreenshot('clear-no-match-value.png')
Comment on lines +8 to +12
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 adding more specific assertions before screenshot comparison.

While the screenshot comparison is valuable for UI testing, adding more specific assertions about the select component's behavior would enhance the test's robustness and provide clearer failure messages if issues arise.

Consider adding assertions to check:

  1. The initial state of the select component.
  2. The behavior when a non-matching value is entered.
  3. The final state after clearing the non-matching value.

Here's an example of how you might add these assertions:

const select = demo.locator('.tiny-select')
const input = select.locator('input')

// Check initial state
await expect(input).toHaveValue('')

// Enter a non-matching value
await input.fill('NonMatchingValue')
await input.press('Enter')

// Check that the value is cleared
await expect(input).toHaveValue('')

// Proceed with the screenshot comparison
await expect(demo).toHaveScreenshot('clear-no-match-value.png')

})
Comment on lines +4 to +13
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Implement specific assertions and interactions to test the functionality.

The current test only captures a screenshot without actually testing the "clear no match value" functionality. To improve the test's effectiveness:

  1. Interact with the select component by entering a non-matching value.
  2. Add specific assertions to verify the behavior.
  3. Capture the screenshot after the interaction to verify the final state.

Here's an example of how you could improve the test:

test('Automatically clear non-matching values - UI screenshot', async ({ page }) => {
  page.on('pageerror', (exception) => expect(exception).toBeNull())
  await page.goto('select#clear-no-match-value')

  const demo = page.locator('#clear-no-match-value .pc-demo')
  const select = demo.locator('.tiny-select')
  const input = select.locator('input')

  await expect(demo).toBeInViewport()

  // Check initial state
  await expect(input).toHaveValue('')

  // Enter a non-matching value
  await input.fill('NonMatchingValue')
  await input.press('Enter')

  // Check that the value is cleared
  await expect(input).toHaveValue('')

  // Capture the final state
  await expect(demo).toHaveScreenshot('clear-no-match-value.png')
})

This implementation addresses the previous review comments and provides a more robust test of the component's functionality.

})
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
17 changes: 17 additions & 0 deletions tests/select/clearable.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { expect, test } from '@playwright/test'

test.describe('select 组件xdesign规范', () => {
test('可清除--UI截图', async ({ page }) => {
page.on('pageerror', (exception) => expect(exception).toBeNull())
await page.goto('select#clearable')
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 using a more robust navigation method.

The current navigation method uses a URL fragment, which might be brittle if the URL structure changes. Consider using a more robust method, such as navigating to a base URL and then interacting with the page to reach the desired component.

Here's a suggested approach:

await page.goto('your-base-url');
await page.click('text=Select Component');
await page.click('text=Clearable');

This approach is more resilient to changes in URL structure and provides a clearer picture of the navigation flow.


const demo = page.locator('#clearable .pc-demo')
const select = demo.locator('.tiny-select')

await expect(demo).toBeInViewport()
await expect(demo).toHaveScreenshot('default.png')

await select.hover()
await expect(demo).toHaveScreenshot('hover.png')
})
Comment on lines +12 to +16
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add assertions for clearable functionality.

While the visual regression tests are good, the test doesn't explicitly verify the clearable functionality mentioned in the test title. Consider adding interactions and assertions to test the clearable feature.

Suggested additions:

// Select an option
await select.click();
await page.click('text=Option 1');

// Verify selected option
await expect(select).toContainText('Option 1');

// Clear the selection
await select.locator('.tiny-select__clear').click();

// Verify the selection is cleared
await expect(select).not.toContainText('Option 1');

// Capture screenshot of cleared state
await expect(demo).toHaveScreenshot('cleared.png');

These additions will ensure that the clearable functionality is properly tested.

})
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
110 changes: 110 additions & 0 deletions tests/select/collapse-tags.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
import { expect, test } from '@playwright/test'

test.describe('select 组件xdesign规范', () => {
Comment on lines +1 to +3
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 improving test structure and organization.

While the current structure is functional, consider the following improvements:

  1. Extract common setup code into a beforeEach() block or a custom fixture to reduce duplication across test cases.
  2. Create helper functions for common operations like "click and wait for menu" or "hover and capture screenshot".
  3. Use parameterized tests to reduce duplication between similar test cases.

These changes would improve maintainability and readability of the test suite.

test('多选折叠tag默认--UI截图', async ({ page }) => {
page.on('pageerror', (exception) => expect(exception).toBeNull())
await page.goto('select#collapse-tags')

const demo = page.locator('#collapse-tags .pc-demo')
const select = demo.locator('.tiny-select').nth(0)
const suffix = select.locator('.tiny-input__suffix-inner')
const wrap = page.locator('.docs-tabs-wrap')
const menu = page.locator('body > .tiny-select-dropdown').nth(0)

await expect(demo).toBeInViewport()
await expect(demo).toHaveScreenshot('default.png')

await select.hover()
await page.waitForTimeout(200)
await expect(wrap).toHaveScreenshot('hover.png')

await suffix.nth(0).click()
await page.waitForTimeout(200)

await menu.locator('.tiny-option').nth(2).hover()
await page.waitForTimeout(200)
await expect(wrap).toHaveScreenshot('item-hover.png')

await menu.locator('.tiny-option').nth(3).click()
await page.waitForTimeout(200)
await expect(wrap).toHaveScreenshot('item-click.png')

await suffix.nth(0).click()
await page.waitForTimeout(200)
await expect(wrap).toHaveScreenshot('item-checked.png')
})
Comment on lines +4 to +35
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance test reliability and readability.

Consider the following improvements:

  1. Replace page.waitForTimeout() with more stable waiting mechanisms, such as waitForSelector() or waitForLoadState(). This will make the tests more reliable across different execution environments.

  2. Add comments to explain the purpose of each step in the test. This will make it easier for other developers to understand and maintain the test.

  3. Use more descriptive names for screenshots, including the test case name and the specific state being captured. For example, default.png could be multi_select_default_state.png.

Here's an example of how you could refactor the beginning of the test:

test('多选折叠tag默认--UI截图', async ({ page }) => {
  page.on('pageerror', (exception) => expect(exception).toBeNull())
  await page.goto('select#collapse-tags')

  // Set up locators
  const demo = page.locator('#collapse-tags .pc-demo')
  const select = demo.locator('.tiny-select').nth(0)
  const suffix = select.locator('.tiny-input__suffix-inner')
  const wrap = page.locator('.docs-tabs-wrap')
  const menu = page.locator('body > .tiny-select-dropdown').nth(0)

  // Verify initial state
  await expect(demo).toBeInViewport()
  await expect(demo).toHaveScreenshot('multi_select_default_state.png')

  // Test hover state
  await select.hover()
  await select.waitFor({ state: 'stable' })
  await expect(wrap).toHaveScreenshot('multi_select_hover_state.png')

  // Continue with the rest of the test...
}

These changes will improve the reliability and readability of the test case.


test('hoverExpand 折叠tag--UI截图', async ({ page }) => {
page.on('pageerror', (exception) => expect(exception).toBeNull())
await page.goto('select#collapse-tags')

const demo = page.locator('#collapse-tags .pc-demo')
const select = demo.locator('.tiny-select').nth(1)
const suffix = select.locator('.tiny-input__suffix-inner')
const wrap = page.locator('.docs-tabs-wrap')
const menu = page.locator('body > .tiny-select-dropdown').nth(0)

await expect(demo).toBeInViewport()
await expect(demo).toHaveScreenshot('default1.png')

await select.hover()
await page.waitForTimeout(200)
await expect(wrap).toHaveScreenshot('hover1.png')

await suffix.nth(0).click()
await page.waitForTimeout(200)

await menu.locator('.tiny-option').nth(2).hover()
await page.waitForTimeout(200)
await expect(wrap).toHaveScreenshot('item-hover1.png')

await menu.locator('.tiny-option').nth(3).click()
await page.waitForTimeout(200)
await expect(wrap).toHaveScreenshot('item-click1.png')

await suffix.nth(0).click()
await page.waitForTimeout(200)
await expect(wrap).toHaveScreenshot('item-checked1.png')
})
Comment on lines +37 to +68
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Reduce code duplication and improve test structure.

This test case is very similar to the first one, which indicates an opportunity for refactoring:

  1. Extract common setup code into a beforeEach() block or a custom fixture.
  2. Create helper functions for common operations like "click and wait for menu" or "hover and capture screenshot".
  3. Use parameterized tests to reduce duplication between similar test cases.

Here's an example of how you could refactor this:

import { test as base, expect, Locator } from '@playwright/test';

interface SelectTestFixtures {
  setupSelect: (index: number) => Promise<{
    demo: Locator;
    select: Locator;
    suffix: Locator;
    wrap: Locator;
    menu: Locator;
  }>;
}

const test = base.extend<SelectTestFixtures>({
  setupSelect: async ({ page }, use) => {
    await use(async (index: number) => {
      await page.goto('select#collapse-tags');
      const demo = page.locator('#collapse-tags .pc-demo');
      const select = demo.locator('.tiny-select').nth(index);
      const suffix = select.locator('.tiny-input__suffix-inner');
      const wrap = page.locator('.docs-tabs-wrap');
      const menu = page.locator('body > .tiny-select-dropdown').nth(0);
      return { demo, select, suffix, wrap, menu };
    });
  },
});

test.describe('select 组件xdesign规范', () => {
  for (const [index, name] of [[0, 'default'], [1, 'hoverExpand']]) {
    test(`${name} 折叠tag--UI截图`, async ({ setupSelect }) => {
      const { demo, select, suffix, wrap, menu } = await setupSelect(index);

      // Test default state
      await expect(demo).toBeInViewport();
      await expect(demo).toHaveScreenshot(`${name}_default_state.png`);

      // Test hover state
      await select.hover();
      await select.waitFor({ state: 'stable' });
      await expect(wrap).toHaveScreenshot(`${name}_hover_state.png`);

      // Continue with other test steps...
    });
  }
});

This refactoring reduces duplication, improves readability, and makes it easier to add or modify test cases in the future.


test('clickExpand 折叠tag--UI截图', async ({ page }) => {
page.on('pageerror', (exception) => expect(exception).toBeNull())
await page.goto('select#collapse-tags')

const demo = page.locator('#collapse-tags .pc-demo')
const select = demo.locator('.tiny-select').nth(2)
const suffix = select.locator('.tiny-input__suffix-inner')
const wrap = page.locator('.docs-tabs-wrap')
const menu = page.locator('body > .tiny-select-dropdown').nth(0)
const collapseButton = select.locator('.tiny-select__tags-collapse')

// 默认显示2行
await expect(demo).toBeInViewport()
await expect(demo).toHaveScreenshot('default2.png')

// 点击展开
await collapseButton.click()
await page.waitForTimeout(200)
await expect(wrap).toHaveScreenshot('click-expand2.png')

// 点击收起
await select.locator('.tiny-select__collapse-text').click()
await page.waitForTimeout(200)
await expect(wrap).toHaveScreenshot('unexpand2.png')

await suffix.nth(0).click()
await page.waitForTimeout(200)

await menu.locator('.tiny-option').nth(2).hover()
await page.waitForTimeout(200)
await expect(wrap).toHaveScreenshot('item-hover2.png')

await menu.locator('.tiny-option').nth(3).click()
await page.waitForTimeout(200)
await expect(wrap).toHaveScreenshot('item-click2.png')

await suffix.nth(0).click()
await page.waitForTimeout(200)
await expect(wrap).toHaveScreenshot('item-checked2.png')
})
Comment on lines +70 to +109
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve test structure and reduce duplication for the clickExpand test.

This test case introduces new interactions specific to the clickExpand functionality. While it's good that it tests this unique feature, there are still opportunities for improvement:

  1. Utilize the common setup and interaction patterns suggested in the previous comments.
  2. Clearly comment the new interactions (expand and collapse) to explain their purpose.
  3. Consider creating a separate describe block for the clickExpand functionality to group related tests.

Here's an example of how you could refactor this test case:

test.describe('clickExpand 折叠tag', () => {
  let demo: Locator, select: Locator, suffix: Locator, wrap: Locator, menu: Locator, collapseButton: Locator;

  test.beforeEach(async ({ setupSelect }) => {
    ({ demo, select, suffix, wrap, menu } = await setupSelect(2));
    collapseButton = select.locator('.tiny-select__tags-collapse');
  });

  test('默认显示2行', async () => {
    await expect(demo).toBeInViewport();
    await expect(demo).toHaveScreenshot('clickExpand_default_state.png');
  });

  test('点击展开和收起', async () => {
    // Expand tags
    await collapseButton.click();
    await select.waitFor({ state: 'stable' });
    await expect(wrap).toHaveScreenshot('clickExpand_expanded_state.png');

    // Collapse tags
    await select.locator('.tiny-select__collapse-text').click();
    await select.waitFor({ state: 'stable' });
    await expect(wrap).toHaveScreenshot('clickExpand_collapsed_state.png');
  });

  test('选择操作', async () => {
    // Open dropdown
    await suffix.click();
    await menu.waitFor({ state: 'visible' });

    // Hover over option
    await menu.locator('.tiny-option').nth(2).hover();
    await expect(wrap).toHaveScreenshot('clickExpand_option_hover.png');

    // Select option
    await menu.locator('.tiny-option').nth(3).click();
    await select.waitFor({ state: 'stable' });
    await expect(wrap).toHaveScreenshot('clickExpand_option_selected.png');

    // Close dropdown
    await suffix.click();
    await menu.waitFor({ state: 'hidden' });
    await expect(wrap).toHaveScreenshot('clickExpand_final_state.png');
  });
});

This refactoring improves the structure of the test, makes it more readable, and reduces duplication while still testing all the necessary interactions.

})
Comment on lines +1 to +110
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Overall improvements for the test suite

The test suite provides good coverage for the select component with collapsing tags. However, there are several areas where it can be improved:

  1. Reduce duplication: Extract common setup and interaction patterns into helper functions or fixtures.

  2. Improve reliability: Replace page.waitForTimeout() with more stable waiting mechanisms like waitForSelector() or waitForLoadState().

  3. Enhance readability: Add comments to explain the purpose of each step in the tests and use more descriptive names for screenshots.

  4. Optimize structure: Consider using parameterized tests for similar scenarios and separate describe blocks for distinct functionalities.

  5. Consistent naming: Use a consistent naming convention for screenshot files across all tests.

By implementing these improvements, the test suite will become more maintainable, reliable, and easier to understand for other developers.

Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
14 changes: 14 additions & 0 deletions tests/select/copy-multi.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { expect, test } from '@playwright/test'

test.describe('select 组件xdesign规范', () => {
test('多选可复制--UI截图', async ({ page }) => {
Comment on lines +3 to +4
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance test suite structure and naming.

Consider the following improvements:

  1. Use English descriptions for better international collaboration.
  2. Add more test cases to cover different aspects of the multi-select functionality.

Example refactor:

test.describe('Select Component - XDesign Specification', () => {
  test('Multi-select with copy feature - UI screenshot', async ({ page }) => {
    // Existing test code
  });

  // Additional test cases
  test('Multi-select interaction - selecting multiple options', async ({ page }) => {
    // New test code
  });

  test('Copy functionality - copying selected options', async ({ page }) => {
    // New test code
  });
});

page.on('pageerror', (exception) => expect(exception).toBeNull())
await page.goto('select#copy-multi')

const demo = page.locator('#copy-multi .pc-demo')
const select = demo.locator('.tiny-select')

await expect(demo).toBeInViewport()
Comment on lines +8 to +11
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance test coverage with additional checks and interactions.

While checking if the demo is in the viewport is good, consider adding more specific checks and interactions:

  1. Verify that the select component is visible and enabled.
  2. Interact with the select component to test its functionality.
  3. Check for specific attributes or classes that indicate the component's state.

Add the following checks and interactions:

await expect(select).toBeVisible();
await expect(select).toBeEnabled();

// Open the select dropdown
await select.click();

// Verify that the dropdown is open
await expect(select).toHaveClass(/tiny-select--open/);

// Select multiple options (adjust selectors as needed)
await select.locator('.tiny-select-option').nth(0).click();
await select.locator('.tiny-select-option').nth(1).click();

// Verify selected options
const selectedOptions = await select.locator('.tiny-select-selection-item').count();
expect(selectedOptions).toBe(2);

// Test copy functionality (if applicable)
// This part depends on how the copy feature is implemented

await expect(demo).toHaveScreenshot('default.png')
})
Comment on lines +12 to +13
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve robustness of screenshot comparison.

While screenshot comparison is valuable for UI testing, it can be sensitive to minor visual differences. Consider the following improvements:

  1. Use a more specific selector for the screenshot to reduce noise from surrounding elements.
  2. Add a custom comparator function to allow for small pixel differences.
  3. Consider taking multiple screenshots for different states of the select component.

Enhance the screenshot comparison:

await expect(select).toHaveScreenshot('select-default.png', {
  maxDiffPixels: 100, // Allow for small differences
});

// Open the select dropdown
await select.click();

// Take a screenshot of the open state
await expect(select).toHaveScreenshot('select-open.png', {
  maxDiffPixels: 100,
});

// Select multiple options
await select.locator('.tiny-select-option').nth(0).click();
await select.locator('.tiny-select-option').nth(1).click();

// Take a screenshot of the selected state
await expect(select).toHaveScreenshot('select-selected.png', {
  maxDiffPixels: 100,
});

Comment on lines +8 to +13
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance test implementation for better coverage and reliability.

While the basic structure is good, the test can be improved to better match its description and provide more comprehensive coverage:

  1. Add interactions with the select component to test multi-select functionality.
  2. Implement a check for the copy feature.
  3. Enhance the screenshot comparison as suggested in the previous review.

Here's a suggested implementation:

const demo = page.locator('#copy-multi .pc-demo')
const select = demo.locator('.tiny-select')

await expect(demo).toBeInViewport()
await expect(select).toBeVisible()
await expect(select).toBeEnabled()

// Open the select dropdown
await select.click()
await expect(select).toHaveClass(/tiny-select--open/)

// Select multiple options
await select.locator('.tiny-select-option').nth(0).click()
await select.locator('.tiny-select-option').nth(1).click()

// Verify selected options
const selectedOptions = await select.locator('.tiny-select-selection-item').count()
expect(selectedOptions).toBe(2)

// Test copy functionality (adjust based on your implementation)
const copyButton = select.locator('.tiny-select-copy-button')
await copyButton.click()

// Verify copied content (this will depend on how you've implemented the copy feature)
const copiedText = await page.evaluate(() => navigator.clipboard.readText())
expect(copiedText).toContain('Selected Option 1, Selected Option 2')

// Take screenshots for different states
await expect(select).toHaveScreenshot('select-default.png', { maxDiffPixels: 100 })
await select.click() // Open dropdown
await expect(select).toHaveScreenshot('select-open.png', { maxDiffPixels: 100 })

This implementation provides more comprehensive testing of the multi-select and copy features, as well as improved screenshot comparisons.

})
14 changes: 14 additions & 0 deletions tests/select/copy-single.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { expect, test } from '@playwright/test'

test.describe('select 组件xdesign规范', () => {
test('单选可复制--UI截图', async ({ page }) => {
page.on('pageerror', (exception) => expect(exception).toBeNull())
await page.goto('select#copy-single')

const demo = page.locator('#copy-single .pc-demo')
const select = demo.locator('.tiny-select')

await expect(demo).toBeInViewport()
await expect(demo).toHaveScreenshot('default.png')
})
Comment on lines +8 to +13
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance test coverage with additional assertions and interactions.

While the current test checks for visibility and takes a screenshot, it doesn't interact with the select component or verify its functionality. Consider adding more assertions and interactions to thoroughly test the single-select with copy functionality.

Here's an example of how you could enhance the test:

test('Single select with copy functionality - UI screenshot', async ({ page }) => {
  page.on('pageerror', (exception) => expect(exception).toBeNull())
  await page.goto('http://your-base-url/select#copy-single')

  const demo = page.locator('#copy-single .pc-demo')
  const select = demo.locator('.tiny-select')

  await expect(demo).toBeInViewport()
  await expect(demo).toHaveScreenshot('default.png')

  // Open the select dropdown
  await select.click()
  await expect(select).toHaveClass(/tiny-select--open/)

  // Select an option
  const option = select.locator('.tiny-select-option').nth(1)
  await option.click()

  // Verify the selected value
  const selectedValue = await select.locator('.tiny-select-selection-text').textContent()
  expect(selectedValue).toBe('Expected Value')

  // Test copy functionality
  const copyButton = select.locator('.tiny-select-copy-button')
  await copyButton.click()

  // Verify copied content (this might require additional setup to access clipboard)
  // const clipboardContent = await page.evaluate(() => navigator.clipboard.readText())
  // expect(clipboardContent).toBe('Expected Copied Value')

  // Take a screenshot of the selected state
  await expect(demo).toHaveScreenshot('selected.png')
}

This enhanced test interacts with the select component, verifies the selection, and attempts to test the copy functionality. Note that accessing the clipboard might require additional setup in your Playwright configuration.

})
24 changes: 24 additions & 0 deletions tests/select/disabled.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { expect, test } from '@playwright/test'

test.describe('select 组件xdesign规范', () => {
test('禁用--UI截图', async ({ page }) => {
page.on('pageerror', (exception) => expect(exception).toBeNull())
await page.goto('select#disabled')

const demo = page.locator('#disabled .pc-demo')
const select = demo.locator('.tiny-select')
const wrap = page.locator('.docs-tabs-wrap')

Comment on lines +4 to +11
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve test robustness and internationalization.

  1. The test description is in Chinese. Consider translating it to English for better international collaboration.
  2. The use of nth selectors might be fragile if the order of elements changes. Consider using more specific selectors or data attributes for robustness.

Here's a suggested improvement:

-  test('禁用--UI截图', async ({ page }) => {
+  test('Disabled state - UI screenshots', async ({ page }) => {
   // ... (keep error handling)
   await page.goto('select#disabled')

   const demo = page.locator('#disabled .pc-demo')
-  const select = demo.locator('.tiny-select')
+  const select = demo.locator('[data-testid="disabled-select"]')
   const wrap = page.locator('.docs-tabs-wrap')

Then, update your HTML to include data-testid attributes:

<div class="tiny-select" data-testid="disabled-select">
  <!-- select content -->
</div>

await expect(demo).toBeInViewport()
await expect(wrap).toHaveScreenshot('default.png')

await select.nth(1).locator('.tiny-input__suffix-inner').click()
await expect(wrap).toHaveScreenshot('hover1.png')
Comment on lines +12 to +16
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add explicit assertion for disabled state.

While the test captures screenshots, it doesn't explicitly verify that the select component is disabled. Adding this assertion would improve the test's reliability.

Consider adding an assertion to verify the disabled state:

   await expect(demo).toBeInViewport()
   await expect(wrap).toHaveScreenshot('default.png')

+  // Verify that the select is disabled
+  await expect(select.nth(1)).toHaveAttribute('aria-disabled', 'true')

   await select.nth(1).locator('.tiny-input__suffix-inner').click()
   await expect(wrap).toHaveScreenshot('hover1.png')
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
await expect(demo).toBeInViewport()
await expect(wrap).toHaveScreenshot('default.png')
await select.nth(1).locator('.tiny-input__suffix-inner').click()
await expect(wrap).toHaveScreenshot('hover1.png')
await expect(demo).toBeInViewport()
await expect(wrap).toHaveScreenshot('default.png')
// Verify that the select is disabled
await expect(select.nth(1)).toHaveAttribute('aria-disabled', 'true')
await select.nth(1).locator('.tiny-input__suffix-inner').click()
await expect(wrap).toHaveScreenshot('hover1.png')


await select.nth(2).locator('.tiny-input__suffix-inner').click()
await expect(wrap).toHaveScreenshot('hover2.png')

await select.nth(3).locator('.tiny-input__suffix-inner').click()
await expect(wrap).toHaveScreenshot('hover3.png')
Comment on lines +15 to +22
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance test robustness and coverage.

While the current test captures screenshots for different states, there are opportunities to improve its robustness and coverage:

  1. The use of nth selectors might be fragile if the order of elements changes. Consider using more specific selectors or data attributes.
  2. There's no explicit assertion to verify that the select components are actually disabled.
  3. The test doesn't check for any visual changes between screenshots, relying solely on the toHaveScreenshot matcher.

Consider the following improvements:

  1. Use data attributes for more robust element selection:

    <div class="tiny-select" data-testid="disabled-select-1">

    Then in the test:

    await page.locator('[data-testid="disabled-select-1"] .tiny-input__suffix-inner').click()
  2. Add assertions to verify the disabled state:

    await expect(select.nth(1)).toHaveAttribute('disabled', '')
  3. Consider using visual comparison tools or custom logic to assert changes between screenshots.

  4. Add keyboard navigation tests to ensure accessibility:

    await page.keyboard.press('Tab')
    await expect(page.locator('[data-testid="disabled-select-1"]')).not.toBeFocused()

Comment on lines +18 to +22
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Refactor repeated interactions and enhance assertions.

The test repeats similar interactions for multiple select components. This could be refactored to reduce duplication. Additionally, there's no verification that clicking a disabled select doesn't open the dropdown.

Consider refactoring the interactions and adding a dropdown visibility check:

const selectCount = 3;
for (let i = 1; i <= selectCount; i++) {
  await select.nth(i).locator('.tiny-input__suffix-inner').click();
  await expect(wrap).toHaveScreenshot(`hover${i}.png`);
  
  // Verify that clicking doesn't open the dropdown
  await expect(page.locator('.tiny-select__dropdown')).not.toBeVisible();
}

})
})
Comment on lines +1 to +24
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Expand test coverage with additional scenarios.

While the current test provides a good foundation for visual regression testing, it could be more comprehensive. Consider adding the following test cases:

  1. Verify that disabled options within an enabled select cannot be selected.
  2. Test keyboard navigation to ensure disabled selects are skipped.
  3. Check that the disabled select doesn't trigger any events when clicked.
  4. Verify the correct ARIA attributes for accessibility.
  5. Test the behavior when programmatically trying to change the value of a disabled select.

Here's an example of an additional test case you could add:

test('Disabled select - Interaction behavior', async ({ page }) => {
  await page.goto('select#disabled');
  const disabledSelect = page.locator('[data-testid="disabled-select-1"]');
  
  // Verify that clicking doesn't open the dropdown
  await disabledSelect.click();
  await expect(page.locator('.tiny-select__dropdown')).not.toBeVisible();
  
  // Verify that the value doesn't change when trying to set it programmatically
  const initialValue = await disabledSelect.inputValue();
  await page.evaluate(() => {
    const select = document.querySelector('[data-testid="disabled-select-1"]');
    select.value = 'new value';
  });
  await expect(disabledSelect).toHaveValue(initialValue);
  
  // Check ARIA attributes
  await expect(disabledSelect).toHaveAttribute('aria-disabled', 'true');
  
  // Test keyboard navigation
  await page.keyboard.press('Tab');
  await expect(disabledSelect).not.toBeFocused();
});

These additional tests will provide a more comprehensive validation of the disabled select component's behavior.

22 changes: 22 additions & 0 deletions tests/select/filter-method.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { expect, test } from '@playwright/test'

test.describe('select 组件xdesign规范', () => {
test('可过滤--UI截图', async ({ page }) => {
page.on('pageerror', (exception) => expect(exception).toBeNull())
await page.goto('select#filter-method')

const demo = page.locator('#filter-method .pc-demo')
const select = demo.locator('.tiny-select')
const wrap = page.locator('.docs-tabs-wrap')
Comment on lines +4 to +10
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve robustness of page navigation

The current navigation method uses a URL fragment, which might be fragile if the URL structure changes. Consider using a more robust method for page navigation, such as environment variables or configuration files to store and manage URLs.

Example of a more robust navigation approach:

const { testUrlBase } = process.env;
await page.goto(`${testUrlBase}/select#filter-method`);

Also, consider translating the test case description to English or providing a bilingual description for better international collaboration.


await expect(demo).toBeInViewport()
await expect(demo).toHaveScreenshot('default.png')

await select.nth(0).click()
await expect(wrap).toHaveScreenshot('unfilter.png')
Comment on lines +12 to +16
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add assertions to verify select dropdown state

While capturing screenshots is useful for visual regression testing, it's also important to assert the state of the UI elements programmatically. Consider adding assertions to verify that the select dropdown is open after clicking.

Example of additional assertions:

await select.nth(0).click();
await expect(select.nth(0)).toHaveClass(/is-open/); // Assuming the open state is indicated by a class
await expect(select.nth(0).locator('.tiny-select__dropdown')).toBeVisible();
await expect(wrap).toHaveScreenshot('unfilter.png');


await select.nth(0).locator('.tiny-input__inner').fill('北京')
await select.nth(0).locator('.tiny-input__inner').press('Enter')
await expect(demo).toHaveScreenshot('filter.png')
})
Comment on lines +18 to +21
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance filter testing and parameterize filter value

The current test captures a screenshot after filtering but doesn't programmatically verify the filtering results. Additionally, the hardcoded filter value '北京' might make the test less flexible.

Consider the following improvements:

  1. Add assertions to verify the filtering results:
const filterValue = '北京';
await select.nth(0).locator('.tiny-input__inner').fill(filterValue);
await select.nth(0).locator('.tiny-input__inner').press('Enter');
await expect(select.nth(0).locator('.tiny-select__dropdown-item')).toContainText(filterValue);
await expect(select.nth(0).locator('.tiny-select__dropdown-item:not(:has-text("' + filterValue + '"))')).toHaveCount(0);
await expect(demo).toHaveScreenshot('filter.png');
  1. Consider parameterizing the filter value for more flexible testing:
const testCases = ['北京', 'Beijing', '上海', 'Shanghai'];
for (const filterValue of testCases) {
  test(`可过滤--UI截图 (Filter value: ${filterValue})`, async ({ page }) => {
    // ... (rest of the test code)
    await select.nth(0).locator('.tiny-input__inner').fill(filterValue);
    // ... (assertions)
  });
}

})
15 changes: 15 additions & 0 deletions tests/select/hide-drop.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { expect, test } from '@playwright/test'

test.describe('select 组件xdesign规范', () => {
test('隐藏下拉--UI截图', async ({ page }) => {
page.on('pageerror', (exception) => expect(exception).toBeNull())
await page.goto('select#hide-drop')

const demo = page.locator('#hide-drop .pc-demo')
const select = demo.locator('.tiny-select')
const wrap = page.locator('.docs-tabs-wrap')

await expect(demo).toBeInViewport()
await expect(demo).toHaveScreenshot('default.png')
Comment on lines +12 to +13
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance test coverage for "hide dropdown" functionality.

While the current assertions check for visibility and take a screenshot, they don't specifically test the "hide dropdown" functionality mentioned in the test title.

Consider adding interactions and assertions to verify the dropdown hiding behavior:

await select.click();
await expect(select.locator('.tiny-select-dropdown')).toBeVisible();
await page.keyboard.press('Escape');
await expect(select.locator('.tiny-select-dropdown')).toBeHidden();
await expect(demo).toHaveScreenshot('dropdown-hidden.png');

This addition will make the test more comprehensive and aligned with its title.

})
})
Comment on lines +3 to +15
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Expand test coverage with additional test cases.

The current test suite contains only one test case focused on UI screenshot comparison. To improve the robustness of your testing, consider adding more test cases that cover different scenarios and interactions with the select component.

Here are some suggestions for additional test cases:

  1. Test opening and closing the dropdown through various user interactions (click, keyboard navigation).
  2. Verify correct rendering of dropdown options.
  3. Test selection of options and updating of the select value.
  4. Check accessibility features like keyboard navigation and screen reader support.
  5. Test any specific behaviors or edge cases mentioned in the component's requirements.

Example:

test('Keyboard navigation works correctly', async ({ page }) => {
  // ... setup code ...
  await select.focus();
  await page.keyboard.press('ArrowDown');
  await expect(select.locator('.tiny-select-dropdown')).toBeVisible();
  // ... more assertions ...
});

These additional tests will provide a more comprehensive validation of the select component's functionality.

Comment on lines +12 to +15
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance test coverage for "hide dropdown" functionality.

While the current assertions check for visibility and take a screenshot, they don't specifically test the "hide dropdown" functionality mentioned in the test title. This comment builds upon and expands the previous review suggestion.

Consider adding the following enhancements:

  1. Test the dropdown visibility states:
await select.click();
await expect(select.locator('.tiny-select-dropdown')).toBeVisible();
await page.keyboard.press('Escape');
await expect(select.locator('.tiny-select-dropdown')).toBeHidden();
  1. Add more comprehensive test cases:
test('Select options and update value', async ({ page }) => {
  // ... setup code ...
  await select.click();
  await select.locator('.tiny-select-dropdown-item').nth(1).click();
  await expect(select).toHaveText('Selected Option');
});

test('Keyboard navigation works correctly', async ({ page }) => {
  // ... setup code ...
  await select.focus();
  await page.keyboard.press('ArrowDown');
  await expect(select.locator('.tiny-select-dropdown')).toBeVisible();
  // ... more assertions ...
});

These additions will make the test suite more robust and aligned with its intended purpose.

14 changes: 14 additions & 0 deletions tests/select/input-box-type.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { expect, test } from '@playwright/test'

test.describe('select 组件xdesign规范', () => {
test('输入框类型--UI截图', async ({ page }) => {
page.on('pageerror', (exception) => expect(exception).toBeNull())
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Refine the error handling approach.

While it's good to handle page errors, expecting all exceptions to be null might be too broad and could potentially hide actual issues that should be addressed.

Consider a more specific error handling approach:

  1. Log the error for debugging purposes.
  2. Fail the test only for specific, expected errors.

Here's an example of how you could refine the error handling:

page.on('pageerror', (exception) => {
  console.error('Page error:', exception);
  // Only fail for specific errors, e.g., related to the select component
  if (exception.message.includes('select')) {
    throw exception;
  }
});

This approach will give you more visibility into potential issues while allowing the test to continue for non-critical errors.

await page.goto('select#input-box-type')

const demo = page.locator('#input-box-type .pc-demo')
const select = demo.locator('.tiny-select')

await expect(demo).toBeInViewport()
await expect(demo).toHaveScreenshot('input-box-type.png')
})
Comment on lines +4 to +13
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Good start, but consider expanding test coverage.

The current test case for the input box type UI is well-implemented and covers the basic visual appearance. However, to ensure comprehensive coverage of the select component's functionality, consider adding more test cases as suggested in the previous review.

Here are some additional test cases to consider:

  1. Test the select component in its open state.
  2. Verify the behavior when selecting an option.
  3. Test with different numbers of options (e.g., no options, many options).
  4. Check the component's responsive behavior on different screen sizes.
  5. Test any custom styling or themes applied to the select component.

Example of an additional test case:

test('Select Component - Open State', async ({ page }) => {
  // ... (setup code similar to the existing test)

  await select.click(); // Open the select dropdown
  await expect(select).toHaveClass(/is-open/); // Assuming there's an 'is-open' class
  await expect(demo).toHaveScreenshot('input-box-type-open.png');
});

Adding these test cases will provide more comprehensive coverage of the select component's functionality and appearance.

})
Comment on lines +1 to +14
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 adding more test cases for comprehensive coverage.

While the current test case for the input box type UI is valuable, consider expanding the test suite to cover more aspects of the select component. This will ensure a more robust testing strategy.

Some suggestions for additional test cases:

  1. Test the select component in its open state.
  2. Verify the behavior when selecting an option.
  3. Test with different numbers of options (e.g., no options, many options).
  4. Check the component's responsive behavior on different screen sizes.
  5. Test any custom styling or themes applied to the select component.

Example of an additional test case:

test('Select Component - Open State', async ({ page }) => {
  // ... (setup code similar to the existing test)

  await select.click(); // Open the select dropdown
  await expect(select).toHaveClass(/is-open/); // Assuming there's an 'is-open' class
  await expect(demo).toHaveScreenshot('input-box-type-open.png');
});

Adding these test cases will provide more comprehensive coverage of the select component's functionality and appearance.

Loading