test(drawer): add drawer ui-test #59
Conversation
WalkthroughA new test suite has been added for the "drawer" component in the file Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range and nitpick comments (3)
tests/drawer/xdesign.spec.ts (3)
3-3: Consider using English for test descriptions.The test suite description is currently in Chinese. For better international collaboration and maintainability, consider using English for all test descriptions and comments.
Otherwise, the test suite structure follows Playwright's recommended practices.
4-12: LGTM: Well-structured test case with room for minor improvement.The test case follows a good structure with setup, action, and verification steps. The use of an error handler and screenshot comparison are excellent practices.
For improved readability, consider extracting the locator strings into constants at the top of the file. For example:
const BASIC_USAGE_SELECTOR = '#basic-usage'; const DRAWER_BOX_SELECTOR = '.tiny-drawer__box';This would make the selectors more maintainable and reduce the risk of typos.
1-34: Good test coverage with room for improvement and expansion.Overall, this test suite provides a solid foundation for UI testing of the drawer component. The tests are well-structured, consistent, and cover three important scenarios. However, there are a few areas for improvement:
Reduce code duplication as suggested in previous comments.
Consider adding negative test cases or edge case scenarios, such as:
- Attempting to open the drawer when it's already open
- Closing the drawer and verifying it's not visible
- Testing with very long content in the drawer
- Verifying keyboard accessibility (e.g., can the drawer be closed with the Esc key?)
Add assertions for specific content or elements within the drawer, not just its visibility.
Consider testing responsive behavior if the drawer component has different layouts for different screen sizes.
If applicable, test any animations or transitions associated with opening/closing the drawer.
By addressing these points, you can create a more comprehensive and robust test suite for the drawer component.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (3)
tests/drawer/xdesign.spec.ts-snapshots/basic-usage-chromium-win32.pngis excluded by!**/*.pngtests/drawer/xdesign.spec.ts-snapshots/footer-slot-chromium-win32.pngis excluded by!**/*.pngtests/drawer/xdesign.spec.ts-snapshots/tips-props-chromium-win32.pngis excluded by!**/*.png
Files selected for processing (1)
- tests/drawer/xdesign.spec.ts (1 hunks)
Additional comments not posted (1)
tests/drawer/xdesign.spec.ts (1)
1-1: LGTM: Imports are correct and necessary.The import statement is appropriate for Playwright tests, importing only the necessary functions.
| test('帮助提示--UI截图', async ({ page }) => { | ||
| page.on('pageerror', (exception) => expect(exception).toBeNull()) | ||
| await page.goto('drawer#tips-props') | ||
| const demo = page.locator('#tips-props') | ||
| await demo.getByRole('button', { name: '展开抽屉' }).click() | ||
| const body = demo.locator('.tiny-drawer__box') | ||
| await expect(body).toBeInViewport() | ||
| await expect(body).toHaveScreenshot('tips-props.png') | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider reducing duplication in test cases.
This test case follows a similar structure to the first one, which is good for consistency. However, there's duplication in the error handling setup and verification steps across all test cases.
Consider extracting these common steps into helper functions to reduce duplication and improve maintainability. For example:
async function setupTest(page, url) {
page.on('pageerror', (exception) => expect(exception).toBeNull());
await page.goto(url);
}
async function verifyDrawerVisibility(demo) {
const body = demo.locator('.tiny-drawer__box');
await expect(body).toBeInViewport();
return body;
}Then, you can use these helpers in each test case:
test('帮助提示--UI截图', async ({ page }) => {
await setupTest(page, 'drawer#tips-props');
const demo = page.locator('#tips-props');
await demo.getByRole('button', { name: '展开抽屉' }).click();
const body = await verifyDrawerVisibility(demo);
await expect(body).toHaveScreenshot('tips-props.png');
});This approach would make the tests more DRY and easier to maintain.
| test('底部插槽--UI截图', async ({ page }) => { | ||
| page.on('pageerror', (exception) => expect(exception).toBeNull()) | ||
| await page.goto('drawer#footer-slot') | ||
| const demo = page.locator('#footer-slot') | ||
| await demo.getByRole('button', { name: '底部插槽示例' }).click() | ||
| const body = demo.locator('.tiny-drawer__box') | ||
| await expect(body).toBeInViewport() | ||
| await expect(body).toHaveScreenshot('footer-slot.png') | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor test cases to improve maintainability and reduce duplication.
This test case, like the previous two, follows a consistent structure, which is commendable. However, the duplication across all three test cases suggests an opportunity for a more comprehensive refactoring.
Consider implementing a parameterized test approach:
interface TestCase {
name: string;
url: string;
buttonText: string;
screenshotName: string;
}
const testCases: TestCase[] = [
{ name: '基本用法', url: 'drawer#basic-usage', buttonText: '抽屉组件', screenshotName: 'basic-usage.png' },
{ name: '帮助提示', url: 'drawer#tips-props', buttonText: '展开抽屉', screenshotName: 'tips-props.png' },
{ name: '底部插槽', url: 'drawer#footer-slot', buttonText: '底部插槽示例', screenshotName: 'footer-slot.png' },
];
testCases.forEach(({ name, url, buttonText, screenshotName }) => {
test(`${name}--UI截图`, async ({ page }) => {
await setupTest(page, url);
const demo = page.locator(`#${url.split('#')[1]}`);
await demo.getByRole('button', { name: buttonText }).click();
const body = await verifyDrawerVisibility(demo);
await expect(body).toHaveScreenshot(screenshotName);
});
});This approach would significantly reduce code duplication, make adding new test cases easier, and improve overall maintainability of the test suite.
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit