fix: Fix datepicker issue with aghelper #36325
Conversation
WalkthroughThe pull request introduces several changes primarily focused on the Datepicker widget's test cases. It reorganizes and renumbers existing tests in the Changes
Possibly related PRs
Suggested labels
Poem
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
Documentation and Community
|
|
/ci-test-limit-count run_count=20 |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/10877689623. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (1)
app/client/cypress/e2e/Regression/ClientSide/Widgets/Datepicker/DatePickerV2_spec.js (1)
165-203: Excellent test case! Just one small suggestion.This test case does a great job of ensuring that the Datepicker doesn't change the display date unless the user selects a date. It covers various scenarios like changing the min date, changing the date format, and clicking on the Datepicker.
One small improvement: Instead of hardcoding the date string
"2020-02-01"on line 186, consider using a relative date (e.g.,cy.clock()andCypress.moment()) to make the test more robust to future date changes.Otherwise, this is a very thorough and well-structured test case. Good job!
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (3)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/Datepicker/DatePickerV2_spec.js (3 hunks)
- app/client/cypress/limited-tests.txt (1 hunks)
- app/client/cypress/locators/FormWidgets.json (1 hunks)
Additional context used
Path-based instructions (3)
app/client/cypress/limited-tests.txt (1)
Pattern
app/client/cypress/**/**.*: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
app/client/cypress/locators/FormWidgets.json (1)
Pattern
app/client/cypress/**/**.*: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
app/client/cypress/e2e/Regression/ClientSide/Widgets/Datepicker/DatePickerV2_spec.js (1)
Pattern
app/client/cypress/**/**.*: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
Additional comments not posted (8)
app/client/cypress/limited-tests.txt (1)
2-2: Good work on updating the limited tests file! 👍The change in the test file path from
cypress/e2e/Regression/ClientSide/Templates/Fork_Template_spec.jstocypress/e2e/Regression/ClientSide/Widgets/Datepicker/DatePickerV2_spec.jsis a clear indication of the shift in testing focus towards the DatePicker widget. This is a great way to prioritize and streamline the testing efforts.Remember, class, it's important to keep this file up-to-date as the testing priorities evolve. Whenever you add new critical tests or decide to focus on specific areas, make sure to update the paths here accordingly. This will ensure that the limited test suite remains relevant and effective in catching potential issues early on.
Also, don't forget that you can always run the entire test suite by uncommenting the line
#cypress/e2e/**/**/*. However, use this option judiciously, as running all specs might be time-consuming and resource-intensive, especially in a CI environment.Keep up the good work, and happy testing! 🚀
app/client/cypress/locators/FormWidgets.json (5)
77-77: Great work, class!The new locator for the button variant toggle looks perfect. The CSS selector is spot-on, and the name clearly describes its purpose. Keep up the excellent work!
78-78: Excellent addition, everyone!The new locator for the min date text area is a great addition. The CSS selector targets the CodeMirror textarea precisely, and the name clearly conveys its purpose. Fantastic job!
79-79: Wonderful work, class!The new locator for the min date input is spot-on. The CSS selector targets the input section precisely, and the name clearly describes its purpose. You're all doing an amazing job!
80-80: Fantastic addition, everyone!The new locator for the date picker input is perfect. The CSS selector targets the input element precisely, and the name clearly conveys its purpose. You're all doing a terrific job!
81-81: Excellent work, class!The new locator for the day picker next button is a great addition. The CSS selector targets the button precisely, and the name clearly describes its purpose. You're all doing an outstanding job!
app/client/cypress/e2e/Regression/ClientSide/Widgets/Datepicker/DatePickerV2_spec.js (2)
Line range hint
86-112: Great work on this test case!The test case thoroughly covers the Datepicker input value changes with different date formats. It also tests the out-of-range scenario, which is important for ensuring the Datepicker behaves correctly in edge cases.
Keep up the good work!
Line range hint
113-159: Excellent test case for theisDirtymeta property!This test case does a great job of verifying the behavior of the
isDirtymeta property. It checks the initial value, the value after interaction, and the value after changing the default date.This is a comprehensive test that ensures the
isDirtyproperty works as expected in different scenarios. Well done!
app/client/cypress/e2e/Regression/ClientSide/Widgets/Datepicker/DatePickerV2_spec.js
Show resolved
Hide resolved
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10877689623. |
## Description **Root Cause Analysis (RCA):** In the local environment, the minDate field was consistently cleared as expected, but this behavior was not replicated in the CI pipeline. Updating the existing flow to address this issue could introduce other problems. The minDate text area having js date function in the CI pipeline, leading to repeated test failures. Multiple function attempts to resolve this issue were unsuccessful. **Solution**: I tried using the `agHelper` utility and updated the test flow with adding a new test case. EE PR: https://github.com/appsmithorg/appsmith-ee/pull/5139 Fixes #`36329` ## Automation /ok-to-test tags="@tag.ImportExport" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/10878662752> > Commit: 0985fc5 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=10878662752&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.ImportExport` > Spec: > <hr>Mon, 16 Sep 2024 06:53:39 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Enhanced Datepicker widget functionality with new locators for improved user interaction. - **Bug Fixes** - Improved test cases for the Datepicker widget, ensuring accurate validation of display data and date formats. - **Documentation** - Updated test specifications to reflect a shift in focus towards the Datepicker widget. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Description
Root Cause Analysis (RCA):
In the local environment, the minDate field was consistently cleared as expected, but this behavior was not replicated in the CI pipeline. Updating the existing flow to address this issue could introduce other problems. The minDate text area having js date function in the CI pipeline, leading to repeated test failures. Multiple function attempts to resolve this issue were unsuccessful.
Solution:
I tried using the
agHelperutility and updated the test flow with adding a new test case.EE PR: https://github.com/appsmithorg/appsmith-ee/pull/5139
Fixes #
36329Automation
/ok-to-test tags="@tag.ImportExport"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10878662752
Commit: 0985fc5
Cypress dashboard.
Tags:
@tag.ImportExportSpec:
Mon, 16 Sep 2024 06:53:39 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Bug Fixes
Documentation