-
Notifications
You must be signed in to change notification settings - Fork 276
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
fix(date-picker): [date-picker] fix date-picker e2e test error #2749
Conversation
WalkthroughThe pull request modifies the date picker test specification file, specifically changing how elements are located and accessed during testing. Instead of using global page context to find textboxes and clear icons, the test now uses a more localized approach by first identifying a demo element with the ID 'clear' and then performing subsequent element selections within this context. The expectations and interactions have also been updated to align with the new structure. Changes
Suggested labels
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
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 (
|
WalkthroughThis pull request addresses an end-to-end test error in the date-picker component. The changes involve modifying the way elements are selected within the test, specifically by using a more specific locator for the demo section, which ensures that the correct elements are targeted during the test execution. Changes
|
@@ -4,10 +4,12 @@ test('[DatePicker] 测试清除输入', async ({ page }) => { | |||
page.on('pageerror', (exception) => expect(exception).toBeNull()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure that the page error handler is correctly set up to catch any exceptions that may occur during the test execution. This is crucial for identifying issues early in the test lifecycle.
[e2e-test-warn] The title of the Pull request should look like "fix(vue-renderless): [action-menu, alert] fix xxx bug". Please make sure you've read our contributing guide |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
examples/sites/demos/pc/app/date-picker/clear.spec.ts (3)
8-8
: Blank line acknowledged.
Maintaining spacing can aid readability, so feel free to retain or remove it as needed to align with your style guidelines.
9-11
: Consider a more explicit identification strategy.
Usingnth(0)
,nth(1)
, andnth(2)
is valid but can be fragile if new textboxes are added or the DOM changes. You might consider adding a test locator attribute (e.g.,data-test-id
) or using a more descriptive role/property to increase maintainability and clarity of these selectors.
12-12
: Locator is well-targeted, but adding a test locator could improve resilience.
Relying on CSS classes (e.g.,.baseClearicon
) is fine until a refactoring or style change occurs. In production-scale systems, consider adding a stable test locator attribute to ensure your tests remain reliable through UI updates.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
examples/sites/demos/pc/app/date-picker/clear.spec.ts
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: PR Unit Test
- GitHub Check: PR E2E Test (pnpm test:e2e3)
🔇 Additional comments (1)
examples/sites/demos/pc/app/date-picker/clear.spec.ts (1)
7-7
: Good approach to scope element selection within a specific demo.
By assigning thedemo
variable a locator of#clear
, you effectively confine the test scope to this particular demo instance, minimizing the risk of accidentally interacting with elements outside the intended area.
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