test: fixed datepicker skipped tests set 2 #37126
Conversation
NandanAnantharamu
requested review from
ApekshaBhosale and
sagar-qa007
as code owners
WalkthroughThe pull request updates the Cypress test suite for the DatePicker and related widgets, activating previously skipped tests and refining date selection methods. Key changes include direct interactions with the UI for selecting today's date, modifications to the Changes
Possibly related PRs
Suggested labels
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
Documentation and Community
|
|
/ci-test-limit |
github-actions
bot
added
skip-changelog
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/11575885809. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (6)
app/client/cypress/e2e/Regression/ClientSide/Binding/DatePicker_Text_spec.js (1)
Line range hint 1-150: Address coding guideline violations.
The file has some issues that need attention:
- Remove usage of
cy.wait(line 35) - Remove empty
afterEachblock as per guidelines
app/client/cypress/e2e/Regression/ClientSide/Widgets/Datepicker/DatePicker2_spec.js (3)
Line range hint 1-16: Consider improving test setup for better maintainability.
- Standardize import statements to use ES6 imports consistently instead of mixing require and import.
- Consider moving the property pane opening to individual tests for better test isolation.
- const commonlocators = require("../../../../../locators/commonlocators.json");
- const formWidgetsPage = require("../../../../../locators/FormWidgets.json");
- const publishPage = require("../../../../../locators/publishWidgetspage.json");
- const widgetsPage = require("../../../../../locators/Widgets.json");
- const dayjs = require("dayjs");
+ import commonlocators from "../../../../../locators/commonlocators.json";
+ import formWidgetsPage from "../../../../../locators/FormWidgets.json";
+ import publishPage from "../../../../../locators/publishWidgetspage.json";
+ import widgetsPage from "../../../../../locators/Widgets.json";
+ import dayjs from "dayjs";
import * as _ from "../../../../../support/Objects/ObjectsCore";Line range hint 73-156: Document and track skipped tests.
Several critical test cases are commented out without proper tracking:
- Min/max date validation
- Default date validation
- Required field validation
These tests cover important functionality and should be properly tracked and fixed.
Would you like me to help:
- Create GitHub issues to track each skipped test?
- Investigate why these tests are failing with the new date input property?
Line range hint 234-236: Remove afterEach hook as per coding guidelines.
The coding guidelines specifically mention avoiding after and afterEach hooks. Consider moving the navigation logic to individual tests.
- afterEach(() => {
- _.deployMode.NavigateBacktoEditor();
- });Add the navigation at the end of each test that needs it:
_.deployMode.NavigateBacktoEditor();app/client/cypress/support/commands.js (1)
527-535: LGTM! Consider adding error handling for invalid version.
The implementation correctly handles both v1 and v2 versions of the DatePicker. However, consider adding error handling for invalid version values.
Apply this diff to add error handling:
Cypress.Commands.add("setDate", (date, dateFormat, ver = "v2") => {
if (ver == "v2") {
const expDate = dayjs().add(date, "days").format("dddd, MMMM DD");
cy.get(`.react-datepicker__day[aria-label^="Choose ${expDate}"]`).click();
} else if (ver == "v1") {
const expDate = dayjs().add(date, "days").format(dateFormat);
const sel = `.DayPicker-Day[aria-label=\"${expDate}\"]`;
cy.get(sel).click();
+ } else {
+ throw new Error(`Invalid DatePicker version: ${ver}. Expected "v1" or "v2".`);
}
});app/client/cypress/support/widgetCommands.js (1)
840-840: LGTM! Consider using data- attributes if possible.*
The selector change from react-datepicker to DayPicker classes aligns with the datepicker component updates. While using class-based selectors is not ideal, it's acceptable here since we're interacting with a third-party component.
If possible, consider working with the UI component team to add data-* attributes to the datepicker for more reliable test selectors.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (6)
- app/client/cypress/e2e/Regression/ClientSide/Binding/DatePicker_Text_spec.js (6 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/Datepicker/DatePicker2_spec.js (1 hunks)
- app/client/cypress/limited-tests.txt (1 hunks)
- app/client/cypress/locators/FormWidgets.json (1 hunks)
- app/client/cypress/support/commands.js (1 hunks)
- app/client/cypress/support/widgetCommands.js (1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
app/client/cypress/e2e/Regression/ClientSide/Binding/DatePicker_Text_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.
app/client/cypress/e2e/Regression/ClientSide/Widgets/Datepicker/DatePicker2_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.
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/support/commands.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.
app/client/cypress/support/widgetCommands.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 (8)
app/client/cypress/limited-tests.txt (2)
Line range hint 1-8: Verify removal of Fork_Template_spec.js was intentional.
Based on the AI summary, the Fork_Template_spec.js entry was removed. Since this changes which tests run in CI, please confirm this was intentional and won't impact test coverage.
#!/bin/bash
# Description: Check if Fork_Template_spec.js still exists and is being tested elsewhere
# Expected: File should either not exist or be covered in another test configuration
echo "Checking for Fork Template spec..."
fd "Fork_Template_spec.js" --type f2-3: Verify the test file paths exist in the codebase.
The paths look correct, but let's verify they exist and are properly formatted.
✅ Verification successful
✓ Test paths are valid and correctly specified
Both test files exist at the exact paths specified in limited-tests.txt:
- app/client/cypress/e2e/Regression/ClientSide/Widgets/Datepicker/DatePicker2_spec.js
- app/client/cypress/e2e/Regression/ClientSide/Binding/DatePicker_Text_spec.js
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the existence of the specified test files
# Expected: Both files should exist in the codebase
echo "Checking DatePicker test files..."
fd "DatePicker2_spec.js|DatePicker_Text_spec.js" --type f
Length of output: 114
Script:
#!/bin/bash
# Let's try a more specific search for the exact paths mentioned
echo "Checking exact paths..."
ls -l app/client/cypress/e2e/Regression/ClientSide/Widgets/Datepicker/DatePicker2_spec.js 2>/dev/null
ls -l app/client/cypress/e2e/Regression/ClientSide/Binding/DatePicker_Text_spec.js 2>/dev/null
# Let's also do a broader search to see if the files might be in slightly different locations
echo -e "\nSearching for similar filenames..."
fd "DatePicker.*spec\.js" --type f
Length of output: 1224
app/client/cypress/e2e/Regression/ClientSide/Binding/DatePicker_Text_spec.js (6)
17-17: LGTM! Test is now enabled.
Successfully unskipped the test case as per PR objectives.
28-29: LGTM! Proper use of locators.
Implementation correctly uses formWidgetsPage locators instead of plain strings.
45-45: LGTM! Proper version specification.
Correctly uses setDate with version parameter for compatibility.
52-52: LGTM! Test is now enabled.
Successfully unskipped the test case as per PR objectives.
76-78: Consider using proper wait strategy.
While the locators are correctly used, multiple sequential clicks might need proper wait assertions to ensure stability.
Consider replacing with assertions:
-cy.get(formWidgetsPage.defaultDate).click();
-cy.get(formWidgetsPage.dayPickerToday).click();
-cy.get(formWidgetsPage.defaultDate).click();
+cy.get(formWidgetsPage.defaultDate).should('be.visible').click();
+cy.get(formWidgetsPage.dayPickerToday).should('be.visible').click();
+cy.get(formWidgetsPage.defaultDate).should('be.visible').click();90-90: LGTM! Tests are now enabled.
Successfully unskipped both test cases as per PR objectives.
Also applies to: 125-125
| "dayPickerNextButton": ".DayPicker-NavButton--next", | ||
| "dayPickerToday": ".ads-v2-datepicker__calender-today" |
There was a problem hiding this comment.
Replace class-based selectors with data- attributes*
The locators are using CSS class selectors which could be brittle if class names change. According to our coding guidelines, we should use data-* attributes for more stable selectors.
Consider updating the selectors to use data attributes:
- "dayPickerNextButton": ".DayPicker-NavButton--next",
- "dayPickerToday": ".ads-v2-datepicker__calender-today"
+ "dayPickerNextButton": "[data-testid='datepicker-next-button']",
+ "dayPickerToday": "[data-testid='datepicker-today-button']"Please ensure the corresponding data-testid attributes are added to the DatePicker component.
📝 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.
| "dayPickerNextButton": ".DayPicker-NavButton--next", | |
| "dayPickerToday": ".ads-v2-datepicker__calender-today" | |
| "dayPickerNextButton": "[data-testid='datepicker-next-button']", | |
| "dayPickerToday": "[data-testid='datepicker-today-button']" |
| @@ -22,7 +22,7 @@ describe( | |||
| it("DatePicker-Date Name validation", function () { | |||
| // changing the date to today | |||
| cy.get(formWidgetsPage.defaultDate).click(); | |||
| cy.SetDateToToday(); | |||
| cy.get(formWidgetsPage.dayPickerToday).click(); | |||
There was a problem hiding this comment.
Add explicit element readiness check before click.
Add visibility check before clicking to ensure element is ready for interaction.
- cy.get(formWidgetsPage.dayPickerToday).click();
+ cy.get(formWidgetsPage.dayPickerToday)
+ .should('be.visible')
+ .click();📝 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.
| cy.get(formWidgetsPage.dayPickerToday).click(); | |
| cy.get(formWidgetsPage.dayPickerToday) | |
| .should('be.visible') | |
| .click(); |
| Cypress.Commands.add("setDate", (date, dateFormate, ver = "v2") => { | ||
| if (ver == "v2") { | ||
| const expDate = dayjs().add(date, "days").format("dddd, MMMM DD"); | ||
| cy.get(`.react-datepicker__day[aria-label^="Choose ${expDate}"]`).click(); | ||
| } else if (ver == "v1") { | ||
| const expDate = dayjs().add(date, "days").format(dateFormate); | ||
| const sel = `.DayPicker-Day[aria-label=\"${expDate}\"]`; | ||
| cy.get(sel).click(); | ||
| } |
There was a problem hiding this comment.
Fix typo in parameter name.
The parameter name dateFormate appears to be a typo and should be dateFormat.
Apply this diff to fix the typo:
-Cypress.Commands.add("setDate", (date, dateFormate, ver = "v2") => {
+Cypress.Commands.add("setDate", (date, dateFormat, ver = "v2") => {
if (ver == "v2") {
const expDate = dayjs().add(date, "days").format("dddd, MMMM DD");
cy.get(`.react-datepicker__day[aria-label^="Choose ${expDate}"]`).click();
} else if (ver == "v1") {
- const expDate = dayjs().add(date, "days").format(dateFormate);
+ const expDate = dayjs().add(date, "days").format(dateFormat);
const sel = `.DayPicker-Day[aria-label=\"${expDate}\"]`;
cy.get(sel).click();
}📝 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.
| Cypress.Commands.add("setDate", (date, dateFormate, ver = "v2") => { | |
| if (ver == "v2") { | |
| const expDate = dayjs().add(date, "days").format("dddd, MMMM DD"); | |
| cy.get(`.react-datepicker__day[aria-label^="Choose ${expDate}"]`).click(); | |
| } else if (ver == "v1") { | |
| const expDate = dayjs().add(date, "days").format(dateFormate); | |
| const sel = `.DayPicker-Day[aria-label=\"${expDate}\"]`; | |
| cy.get(sel).click(); | |
| } | |
| Cypress.Commands.add("setDate", (date, dateFormat, ver = "v2") => { | |
| if (ver == "v2") { | |
| const expDate = dayjs().add(date, "days").format("dddd, MMMM DD"); | |
| cy.get(`.react-datepicker__day[aria-label^="Choose ${expDate}"]`).click(); | |
| } else if (ver == "v1") { | |
| const expDate = dayjs().add(date, "days").format(dateFormat); | |
| const sel = `.DayPicker-Day[aria-label=\"${expDate}\"]`; | |
| cy.get(sel).click(); | |
| } |
|
/ci-test-limit |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/11594465422. |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (2)
app/client/cypress/e2e/Regression/ClientSide/Widgets/Datepicker/DatePicker_With_Switch_spec.js (1)
Line range hint 63-65: Remove empty afterEach block
According to the coding guidelines, avoid using after and afterEach in test cases. This empty block can be removed.
Remove the entire block:
- afterEach(() => {
- // put your clean up code if any
- });app/client/cypress/fixtures/uiBindnewDsl.json (1)
96-96: Review the min/max date range configuration.
The date range (1920-2121) is unusually wide for testing purposes. Consider limiting it to a more reasonable range to:
- Improve test focus
- Prevent potential performance issues when rendering the date picker calendar
- "minDate": "1920-12-31T18:30:00.000Z",
- "maxDate": "2121-12-31T18:29:00.000Z",
+ "minDate": "2000-01-01T00:00:00.000Z",
+ "maxDate": "2030-12-31T23:59:59.000Z",Also applies to: 143-143, 151-151, 198-198
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (5)
- app/client/cypress/e2e/Regression/ClientSide/Binding/DatePicker_Text_spec.js (6 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/Datepicker/DatePicker_With_Switch_spec.js (2 hunks)
- app/client/cypress/fixtures/uiBindnewDsl.json (1 hunks)
- app/client/cypress/limited-tests.txt (1 hunks)
- app/client/cypress/locators/Widgets.json (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- app/client/cypress/e2e/Regression/ClientSide/Binding/DatePicker_Text_spec.js
- app/client/cypress/limited-tests.txt
🧰 Additional context used
📓 Path-based instructions (3)
app/client/cypress/e2e/Regression/ClientSide/Widgets/Datepicker/DatePicker_With_Switch_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.
app/client/cypress/fixtures/uiBindnewDsl.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/locators/Widgets.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.
🔇 Additional comments (2)
app/client/cypress/fixtures/uiBindnewDsl.json (2)
96-97: Consider revising the date format and default date configuration.
The date format includes hours (HH:mm) but might not be necessary since:
- Time precision is set to "minute"
- Default date is set to a future date (2024-10-24)
Also applies to: 121-121, 151-152, 176-176
74-74: Verify the binding between Text5 and DatePicker1.
The Text widget is bound to DatePicker1's selectedDate. Since this is a test fixture, ensure that:
- The binding syntax is being tested
- The date format display is being validated
Also applies to: 91-145, 146-200
✅ Verification successful
The binding between Text5 and DatePicker1 is properly tested in multiple test files
The binding is well covered in test files:
app/client/cypress/e2e/Regression/ClientSide/Binding/DatePicker_Text_spec.jsapp/client/cypress/e2e/Regression/ClientSide/Widgets/Datepicker/DatePickerV2_spec.jsapp/client/cypress/e2e/Regression/ClientSide/Widgets/Datepicker/DatePicker1_spec.js
All these files verify the binding syntax {{DatePicker1.selectedDate}} and the date format is validated through the testJsontext test helper.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for similar bindings in test files
rg "DatePicker.*\.selectedDate" "app/client/cypress/**/*_spec.js"
Length of output: 139
Script:
#!/bin/bash
# Let's first check if the file exists and its content
cat "app/client/cypress/fixtures/uiBindnewDsl.json"
# Then search for DatePicker tests in the correct cypress directory
rg "DatePicker.*\.selectedDate" "app/client/cypress/"
# Also look for any test files related to DatePicker
fd ".*spec.*" "app/client/cypress/" --exec grep -l "DatePicker"
Length of output: 23697
| it("DatePicker-Date change and validate switch widget status", function () { | ||
| cy.get(widgetsPage.datepickerInput).click({ force: true }); | ||
| cy.SetDateToToday(); | ||
| cy.get(widgetsPage.switchWidgetActive).should("be.visible"); | ||
| cy.get(".t--toast-action span") | ||
| .last() | ||
| .invoke("text") | ||
| .then((text) => { | ||
| const toasttext = text; | ||
| cy.log(toasttext); | ||
| expect(text.trim()).to.equal(toasttext.trim()); | ||
| }); | ||
| _.agHelper.GetNClick(widgetsPage.todayText) | ||
| _.agHelper.AssertClassExists('.bp3-switch', 't--switch-widget-active') |
There was a problem hiding this comment.
Remove force click and update selectors
- Remove
force: trueand handle any visibility issues properly - Replace class selectors with data-* attributes for better test stability
Apply these changes:
- cy.get(widgetsPage.datepickerInput).click({ force: true });
+ cy.get(widgetsPage.datepickerInput).should('be.visible').click();
_.agHelper.GetNClick(widgetsPage.todayText)
- _.agHelper.AssertClassExists('.bp3-switch', 't--switch-widget-active')
+ _.agHelper.AssertClassExists('[data-testid="switch-widget"]', 't--switch-widget-active')Committable suggestion was skipped due to low confidence.
| it("Date Widget with Reset widget being switch widget", function () { | ||
| EditorNavigation.SelectEntityByName("DatePicker1", EntityType.Widget); | ||
|
|
||
| cy.get(formWidgetsPage.datePickerInput).click(); | ||
| _.agHelper.GetNClick(widgetsPage.todayText) | ||
| cy.get(formWidgetsPage.defaultDate).click(); | ||
| _.agHelper.GetNClick('.ads-v2-datepicker__calender-today'); |
There was a problem hiding this comment.
Refactor selectors and remove redundant interactions
- Replace the class selector
.ads-v2-datepicker__calender-todaywith a data-* attribute - Remove redundant clicks on defaultDate element
Apply this refactor:
cy.get(formWidgetsPage.datePickerInput).click();
_.agHelper.GetNClick(widgetsPage.todayText)
- cy.get(formWidgetsPage.defaultDate).click();
- _.agHelper.GetNClick('.ads-v2-datepicker__calender-today');
cy.get(formWidgetsPage.defaultDate).click();Committable suggestion was skipped due to low confidence.
| "todayText": "span:contains('Today')", | ||
| "dayPickerToday": ".DayPicker-Day--today" |
There was a problem hiding this comment.
Use data- attributes for date picker selectors*
The current selectors are brittle and could break with internationalization or component library updates. Consider using data-* attributes instead:
- "todayText": "span:contains('Today')",
- "dayPickerToday": ".DayPicker-Day--today"
+ "todayText": "[data-testid='datepicker-today']",
+ "dayPickerToday": "[data-testid='datepicker-current-day']"Please update the component to include these data-testid attributes for more reliable test selectors.
Committable suggestion was skipped due to low confidence.
|
/ci-test-limit |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/11639647217. |
|
/ci-test-limit |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/11640798643. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
app/client/cypress/support/widgetCommands.js (1)
Line range hint
840-844: Use data- attribute selector instead of text-based selector.*The current selector
button:contains('Today')is brittle and locale-dependent. Consider using a data-* attribute selector as per the coding guidelines.Apply this change:
-cy.get("button:contains('Today')").click({ +cy.get("[data-testid='datepicker-today-button']").click({ force: true, });Also, ensure the corresponding data-* attribute is added to the Today button in the component.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
app/client/cypress/e2e/Regression/ClientSide/Binding/DatePicker_Text_spec.js(6 hunks)app/client/cypress/support/widgetCommands.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/client/cypress/e2e/Regression/ClientSide/Binding/DatePicker_Text_spec.js
🧰 Additional context used
📓 Path-based instructions (1)
app/client/cypress/support/widgetCommands.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.
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/11640798643. |
|
/ci-test-limit-count run_count=25 |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/11641236171. |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/11641236171. |
NandanAnantharamu
added
ok-to-test
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
app/client/cypress/e2e/Regression/ClientSide/Widgets/Datepicker/DatePicker_With_Switch_spec.js (1)
Line range hint
63-65: Remove empty afterEach blockAccording to the guidelines, we should avoid using
afterandafterEachin test cases. Remove this empty block if cleanup is not required.-afterEach(() => { - // put your clean up code if any -});
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
app/client/cypress/e2e/Regression/ClientSide/Widgets/Datepicker/DatePicker2_spec.js(1 hunks)app/client/cypress/e2e/Regression/ClientSide/Widgets/Datepicker/DatePicker_With_Switch_spec.js(2 hunks)app/client/cypress/support/commands.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/Datepicker/DatePicker2_spec.js
- app/client/cypress/support/commands.js
🧰 Additional context used
📓 Path-based instructions (1)
app/client/cypress/e2e/Regression/ClientSide/Widgets/Datepicker/DatePicker_With_Switch_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.
| it("DatePicker-Date change and validate switch widget status", function () { | ||
| cy.get(widgetsPage.datepickerInput).click({ force: true }); | ||
| cy.SetDateToToday(); | ||
| cy.get(widgetsPage.switchWidgetActive).should("be.visible"); | ||
| cy.get(".t--toast-action span") | ||
| .last() | ||
| .invoke("text") | ||
| .then((text) => { | ||
| const toasttext = text; | ||
| cy.log(toasttext); | ||
| expect(text.trim()).to.equal(toasttext.trim()); | ||
| }); | ||
| _.agHelper.GetNClick(widgetsPage.todayText); | ||
| _.agHelper.AssertClassExists(".bp3-switch", "t--switch-widget-active"); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove force click and update selectors
- Remove
force: trueand handle visibility properly - Replace class selector with data-* attribute
Apply these changes:
-cy.get(widgetsPage.datepickerInput).click({ force: true });
+cy.get(widgetsPage.datepickerInput)
+ .should('be.visible')
+ .click();
_.agHelper.GetNClick(widgetsPage.todayText);
-_.agHelper.AssertClassExists(".bp3-switch", "t--switch-widget-active");
+_.agHelper.AssertClassExists("[data-testid='switch-widget']", "t--switch-widget-active");Committable suggestion skipped: line range outside the PR's diff.
| cy.get(formWidgetsPage.datePickerInput).click(); | ||
| _.agHelper.GetNClick(widgetsPage.todayText); | ||
| cy.get(formWidgetsPage.defaultDate).click(); | ||
| _.agHelper.GetNClick(".ads-v2-datepicker__calender-today"); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor selectors and optimize interactions
- Replace class-based selector with data-* attribute
- Remove redundant clicks on defaultDate
- Add proper assertions for date selection
Apply this refactor:
cy.get(formWidgetsPage.datePickerInput).click();
_.agHelper.GetNClick(widgetsPage.todayText);
-cy.get(formWidgetsPage.defaultDate).click();
-_.agHelper.GetNClick(".ads-v2-datepicker__calender-today");
-cy.get(formWidgetsPage.defaultDate).click();
+cy.get(formWidgetsPage.defaultDate)
+ .should('be.visible')
+ .click()
+ .should('have.value', dayjs().format('DD/MM/YYYY'));Committable suggestion skipped: line range outside the PR's diff.
Updated common method to suite both v1 and v2 Skipped test updated locator change handled EE PR: appsmithorg/appsmith-ee#5477 /ok-to-test tags="@tag.All" <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/11677761197> > Commit: 56ecbce > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=11677761197&attempt=2" target="_blank">Cypress dashboard</a>. > Tags: `@tag.All` > Spec: > <hr>Tue, 05 Nov 2024 06:59:10 UTC <!-- end of auto-generated comment: Cypress test results --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Enhanced date selection process in DatePicker widget with improved test interactions. - Introduced a new structured JSON layout for the user interface, including multiple interactive widgets. - **Bug Fixes** - Activated previously skipped tests to ensure comprehensive validation of DatePicker functionality. - **Documentation** - Updated locators for form widgets to improve selector accuracy and functionality. - **Chores** - Refined various Cypress commands related to widget interactions for better reliability and performance. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: “NandanAnantharamu” <“nandan@thinkify.io”>
Updated common method to suite both v1 and v2
Skipped test updated
locator change handled
EE PR: https://github.com/appsmithorg/appsmith-ee/pull/5477
/ok-to-test tags="@tag.All"
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/11677761197
Commit: 56ecbce
Cypress dashboard.
Tags:
@tag.AllSpec:
Tue, 05 Nov 2024 06:59:10 UTC
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores