-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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: removed toggleBar command and replaced with ts helper #33835
Conversation
WalkthroughThe recent changes across multiple Cypress test files involve replacing the Changes
Poem
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
/ci-test-limit |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/9289318516. |
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/9289318516. |
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: 5
Outside diff range and nitpick comments (27)
app/client/cypress/e2e/Regression/ClientSide/Widgets/Input/Inputv2_ShowStepArrows_spec.js (2)
Line range hint
29-29
: Avoid unnecessary template literals for static strings.- cy.testJsontext("showsteparrows", `{{false}}`); + cy.testJsontext("showsteparrows", "{{false}}"); - cy.testJsontext("showsteparrows", `{{true}}`); + cy.testJsontext("showsteparrows", "{{true}}");Also applies to: 35-35
Line range hint
9-39
: Consider refactoring this function expression to an arrow function for consistency and clarity.- function () { + () => {app/client/cypress/e2e/Regression/ClientSide/Widgets/CurrencyInput/CurrencyInput_ShowStepArrows_spec.js (2)
Line range hint
31-31
: Avoid unnecessary template literals for static strings.- cy.testJsontext("showsteparrows", `{{false}}`); + cy.testJsontext("showsteparrows", "{{false}}"); - cy.testJsontext("showsteparrows", `{{true}}`); + cy.testJsontext("showsteparrows", "{{true}}");Also applies to: 38-38
Line range hint
9-42
: Consider refactoring this function expression to an arrow function for consistency and clarity.- function () { + () => {app/client/cypress/e2e/Regression/ClientSide/Widgets/Form/FormWithSwitch_spec.js (1)
Line range hint
12-63
: Consider refactoring these function expressions to arrow functions for consistency and clarity.- function () { + () => {Also applies to: 36-62
app/client/cypress/e2e/Regression/ClientSide/Widgets/Datepicker/DatePicker_With_Switch_spec.js (1)
Line range hint
13-67
: Consider refactoring these function expressions to arrow functions for consistency and clarity.- function () { + () => {Also applies to: 31-52, 54-66
app/client/cypress/e2e/Regression/ClientSide/Widgets/Image/Image_spec.js (2)
Line range hint
54-54
: Consider using template literals for better readability and consistency.- cy.get(`${viewWidgetsPage.imageWidget} div[data-testid=styledImage]`).should("not.exist"); + cy.get(`${viewWidgetsPage.imageWidget} div[data-testid="styledImage"]`).should("not.exist");
Line range hint
14-88
: Convert all function expressions to arrow functions to align with modern JavaScript practices.- describe("Image Widget Functionality", function () { + describe("Image Widget Functionality", () => {app/client/cypress/e2e/Regression/ClientSide/Widgets/Checkbox/CheckBox_spec.js (1)
Line range hint
10-87
: Convert all function expressions to arrow functions to align with modern JavaScript practices.- describe("Checkbox Widget Functionality", function () { + describe("Checkbox Widget Functionality", () => {app/client/cypress/e2e/Regression/ClientSide/Widgets/Form/Form_With_CheckBox_spec.js (6)
Line range hint
35-35
: Consider using template literals for better readability and consistency.- cy.get(publish.checkboxWidget + " " + ".t--checkbox-widget-label").should( + cy.get(`${publish.checkboxWidget} .t--checkbox-widget-label`).should(
Line range hint
43-43
: Consider using template literals for better readability and consistency.- cy.get(publish.checkboxWidget + " " + ".bp3-align-right").should("exist"); + cy.get(`${publish.checkboxWidget} .bp3-align-right`).should("exist");
Line range hint
50-50
: Consider using template literals for better readability and consistency.- cy.get(publish.checkboxWidget + " " + ".t--checkbox-widget-label").should( + cy.get(`${publish.checkboxWidget} .t--checkbox-widget-label`).should(
Line range hint
60-60
: Consider using template literals for better readability and consistency.- cy.get(publish.checkboxWidget + " " + ".t--checkbox-widget-label").should( + cy.get(`${publish.checkboxWidget} .t--checkbox-widget-label`).should(
Line range hint
64-64
: Consider using template literals for better readability and consistency.- cy.get(publish.checkboxWidget + " " + ".t--checkbox-widget-label").should( + cy.get(`${publish.checkboxWidget} .t--checkbox-widget-label`).should(
Line range hint
9-121
: Convert all function expressions to arrow functions to align with modern JavaScript practices.- describe("Checkbox Widget Functionality", function () { + describe("Checkbox Widget Functionality", () => {app/client/cypress/e2e/Regression/ClientSide/Widgets/TreeSelect/Tree_Select_spec.ts (4)
Line range hint
8-146
: Convert all function expressions to arrow functions to align with modern JavaScript practices.- describe("Tree Select Widget", function () { + describe("Tree Select Widget", () => {
Line range hint
8-146
: Consider using arrow functions for all function expressions to maintain consistency with ES6 syntax.- it("4. should check that empty value is allowed in options", function () { + it("4. should check that empty value is allowed in options", () => {
Line range hint
8-146
: Ensure all function expressions are converted to arrow functions for consistency with modern JavaScript practices.- it("5. should check that more than empty value is not allowed in options", function () { + it("5. should check that more than empty value is not allowed in options", () => {
Line range hint
8-146
: Convert all function expressions to arrow functions to align with modern JavaScript practices.- describe("Tree Select Widget", function () { + describe("Tree Select Widget", () => {app/client/cypress/e2e/Regression/ClientSide/Widgets/ListV2/ListV2_PageNo_PageSize_spec.js (1)
Line range hint
110-110
: Consider using template literals for better readability and consistency.- cy.get(commonlocators.bodyTextStyle).first().should("have.text", "PageSize 4"); + cy.get(`${commonlocators.bodyTextStyle}`).first().should("have.text", "PageSize 4");app/client/cypress/e2e/Regression/ClientSide/Widgets/Multiselect/Multi_Select_Tree_spec.js (1)
Line range hint
10-142
: Convert function expression to an arrow function for consistency and modern JavaScript syntax.- function () { + () => {app/client/cypress/e2e/Regression/ClientSide/Widgets/Others/Progress_spec.js (1)
Line range hint
15-211
: Consider refactoring function expressions to arrow functions for improved readability and consistency with ES6 syntax.- it("1. Property: isIndeterminate, Toggle infinite loading", function () { + it("1. Property: isIndeterminate, Toggle infinite loading", () => {Apply similar changes to all other test cases.
app/client/cypress/support/commands.js (5)
Line range hint
16-16
: Consider using thenode:
protocol when importing Node.js built-in modules for clarity and consistency.- const path = require("path"); + const path = require("node:path");
Line range hint
112-112
: Simplify the computed expressions to enhance readability and performance.- const urlObject = new URL(url); + const urlObject = new URL(url.toString());Also applies to: 115-115, 772-772
Line range hint
208-208
: Use template literals instead of string concatenation for better readability and performance.- url: "api/v1/layouts/" + layoutId + "/pages/" + pageid + "?applicationId=" + appId, + url: `api/v1/layouts/${layoutId}/pages/${pageid}?applicationId=${appId}`,Also applies to: 220-220, 346-346, 352-352, 355-355, 376-376, 385-390, 413-413, 416-416, 431-431, 434-434, 576-576, 610-610
Line range hint
144-148
: Consider using optional chaining and for...of loops to simplify the code and enhance readability.- arr && arr.forEach((param) => { + for (const param of arr ?? []) {
Line range hint
367-367
: Declare variables separately to enhance code clarity and maintainability.- let pageid, layoutId, appId; + let pageid; + let layoutId; + let appId;
@@ -23,7 +23,7 @@ describe( | |||
it("1. Property: isIndeterminate, Toggle infinite loading", function () { | |||
cy.openPropertyPane("progresswidget"); | |||
// enable infinite loading | |||
cy.togglebar( | |||
agHelper.CheckUncheck( |
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.
Replace deprecated cy.togglebarDisable
with agHelper.CheckUncheck
for consistency.
- cy.togglebarDisable(
+ agHelper.CheckUncheck(
Committable suggestion was skipped due low confidence.
@@ -65,7 +65,7 @@ describe( | |||
deployMode.NavigateBacktoEditor(); | |||
//Image Widget Functionality To Check Visible Widget", function () { | |||
cy.openPropertyPane("imagewidget"); | |||
cy.togglebar(commonlocators.visibleCheckbox); | |||
agHelper.CheckUncheck(commonlocators.visibleCheckbox); |
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.
Replace function expression with an arrow function for consistency.
- it("3. Image Widget Functionality To Check/Uncheck Visible Widget", function () {
+ it("3. Image Widget Functionality To Check/Uncheck Visible Widget", () => {
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.
agHelper.CheckUncheck(commonlocators.visibleCheckbox); | |
it("3. Image Widget Functionality To Check/Uncheck Visible Widget", () => { |
@@ -31,7 +31,7 @@ describe( | |||
/** | |||
* @param{toggleButton Css} Assert to be checked | |||
*/ | |||
cy.togglebar(widgetsPage.defaultcheck); | |||
_.agHelper.CheckUncheck(widgetsPage.defaultcheck); |
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.
Replace function expression with an arrow function for consistency.
- it("Checkbox Widget Functionality", function () {
+ it("Checkbox Widget Functionality", () => {
Committable suggestion was skipped due low confidence.
@@ -45,7 +45,7 @@ | |||
}); | |||
it("Checkbox Functionality To Check Disabled Widget", function () { | |||
cy.openPropertyPane("checkboxwidget"); | |||
cy.togglebar(commonlocators.Disablejs + " " + "input"); | |||
_.agHelper.CheckUncheck(commonlocators.Disablejs + " " + "input"); |
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.
Consider using template literals for better readability and consistency.
- cy.get(publish.checkboxWidget + " " + "input").should("be.disabled");
+ cy.get(`${publish.checkboxWidget} input`).should("be.disabled");
Committable suggestion was skipped due low confidence.
_.agHelper.CheckUncheck( | ||
".t--property-control-serversidepagination input", | ||
); |
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.
Consider using template literals for better readability and consistency.
- cy.get(".t--property-control-serversidepagination input").should("have.text", "PageSize 2");
+ cy.get(`.t--property-control-serversidepagination input`).should("have.text", "PageSize 2");
Committable suggestion was skipped due low confidence.
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.
replace this .t--property-control-serversidepagination input with locator please
serverSidePaginationCheckbox in commonLocators.json
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.
@ApekshaBhosale, based on your request, here's the updated code snippet with the serverSidePaginationCheckbox
locator from commonlocators.json
:
// Toggle server-side pagination using the locator from commonLocators.json
_.agHelper.CheckUncheck(commonlocators.serverSidePaginationCheckbox);
This change replaces the direct CSS selector with a named locator, improving the maintainability and readability of the code. If there are any more adjustments or reviews needed, feel free to let me know!
Analysis chain
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if 'serverSidePaginationCheckbox' exists in commonlocators.json
cat app/client/cypress/locators/commonlocators.json | jq '.serverSidePaginationCheckbox'
Unrecoverable error: Failed to clone repository
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.
can we add this "t--property-control-allowclearingvalue input[type="checkbox"]" as locators as this has been used so many times in these specs?
_.agHelper.CheckUncheck( | ||
".t--property-control-serversidepagination input", | ||
); |
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.
replace this .t--property-control-serversidepagination input with locator please
serverSidePaginationCheckbox in commonLocators.json
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: 1
cy.togglebarDisable( | ||
".t--property-control-allowclearingvalue input[type='checkbox']", | ||
); | ||
cy.togglebarDisable(commonlocators.allowclearingValueInput); |
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.
Tip
Codebase Verification
The usage of cy.togglebarDisable
is still present in multiple test files. To maintain consistency with the new approach using _.agHelper.CheckUncheck
, consider replacing cy.togglebarDisable
in the following files:
app/client/cypress/e2e/Regression/ClientSide/Widgets/TreeSelect/Single_Select_Tree_spec.ts
app/client/cypress/e2e/Regression/ClientSide/Widgets/TreeSelect/Tree_Select_spec.ts
app/client/cypress/e2e/Regression/ClientSide/Widgets/CodeScanner/CodeScanner1_spec.ts
Analysis chain
Consider replacing cy.togglebarDisable
with a method consistent with _.agHelper.CheckUncheck
to maintain consistency across the test suite.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify if `cy.togglebarDisable` is replaced throughout the codebase.
# Test: Search for the function usage. Expect: No occurrences of the old function.
rg --type typescript $'cy.togglebarDisable'
Length of output: 77
Script:
#!/bin/bash
# Description: Verify if `cy.togglebarDisable` is replaced throughout the codebase.
# Test: Search for the function usage. Expect: No occurrences of the old function.
rg 'cy.togglebarDisable' --glob '*.ts'
Length of output: 955
/ci-test-limit |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/9320755723. |
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/9320755723. |
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.
let's move as many locators to locators file instead of using them directly @NandanAnantharamu
app/client/cypress/e2e/Regression/ClientSide/Widgets/JSONForm/JSONForm_ArrayField_spec.js
Outdated
Show resolved
Hide resolved
...client/cypress/e2e/Regression/ClientSide/Widgets/JSONForm/JSONForm_FieldProperties_1_spec.js
Outdated
Show resolved
Hide resolved
...client/cypress/e2e/Regression/ClientSide/Widgets/JSONForm/JSONForm_FieldProperties_2_spec.js
Outdated
Show resolved
Hide resolved
app/client/cypress/e2e/Regression/ClientSide/Widgets/Multiselect/MultiSelect2_spec.js
Show resolved
Hide resolved
/ci-test-limit |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/9365254766. |
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
Outside diff range and nitpick comments (1)
app/client/cypress/support/commands.js (1)
Line range hint
113-113
: Address static analysis suggestions for code simplification and modernization.
- Use literal keys instead of computed expressions where applicable.
- Convert function expressions to arrow functions to improve readability and reduce function scope complexity.
- Replace
forEach
loops withfor...of
loops for better performance and readability.- Implement optional chaining to simplify code dealing with potentially undefined properties or methods.
Example fixes:
- const [key, value] = param.split("="); + const [key, value] = param.split("="); // Use literal keys if applicable - arr.forEach((param) => { + for (const param of arr) { // Replace forEach with for...ofAlso applies to: 116-116, 145-149, 146-149, 766-766, 784-784, 790-790, 1097-1125, 1380-1389
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (13)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/JSONForm/JSONForm_ArrayField_spec.js (5 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/JSONForm/JSONForm_FieldProperties_1_spec.js (4 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/JSONForm/JSONForm_FieldProperties_2_spec.js (3 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/JSONForm/JSONForm_HiddenFields_1_spec.js (1 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/JSONForm/JSONForm_HiddenFields_2_spec.js (2 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/Multiselect/MultiSelect2_spec.js (5 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/Others/Progress_spec.js (5 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/Select/Select_spec.js (1 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_Widget_Add_button_spec.js (1 hunks)
- app/client/cypress/limited-tests.txt (1 hunks)
- app/client/cypress/locators/Widgets.json (1 hunks)
- app/client/cypress/locators/commonlocators.json (1 hunks)
- app/client/cypress/support/commands.js (2 hunks)
Files skipped from review as they are similar to previous changes (7)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/JSONForm/JSONForm_ArrayField_spec.js
- app/client/cypress/e2e/Regression/ClientSide/Widgets/JSONForm/JSONForm_FieldProperties_1_spec.js
- app/client/cypress/e2e/Regression/ClientSide/Widgets/JSONForm/JSONForm_FieldProperties_2_spec.js
- app/client/cypress/e2e/Regression/ClientSide/Widgets/JSONForm/JSONForm_HiddenFields_1_spec.js
- app/client/cypress/e2e/Regression/ClientSide/Widgets/JSONForm/JSONForm_HiddenFields_2_spec.js
- app/client/cypress/e2e/Regression/ClientSide/Widgets/Select/Select_spec.js
- app/client/cypress/limited-tests.txt
Additional context used
Biome
app/client/cypress/e2e/Regression/ClientSide/Widgets/Others/Progress_spec.js
[error] 23-35: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 36-45: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 46-55: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 56-63: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 66-70: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 71-86: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 87-96: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 97-106: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 107-114: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 115-128: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 130-137: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 139-150: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 152-177: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 179-190: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 15-191: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_Widget_Add_button_spec.js
[error] 23-56: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 58-87: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 89-124: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 126-294: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 18-305: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.app/client/cypress/e2e/Regression/ClientSide/Widgets/Multiselect/MultiSelect2_spec.js
[error] 141-171: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 173-201: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 250-284: Prefer for...of instead of forEach. (lint/complexity/noForEach)
forEach may lead to performance issues when working with large arrays. When combined with functions like filter or map, this causes multiple iterations over the same type.
[error] 249-285: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 287-299: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 301-316: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 17-317: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.app/client/cypress/support/commands.js
[error] 113-113: The computed expression can be simplified without the use of a string literal. (lint/complexity/useLiteralKeys)
Unsafe fix: Use a literal key instead.
[error] 116-116: The computed expression can be simplified without the use of a string literal. (lint/complexity/useLiteralKeys)
Unsafe fix: Use a literal key instead.
[error] 145-149: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
[error] 146-149: Prefer for...of instead of forEach. (lint/complexity/noForEach)
forEach may lead to performance issues when working with large arrays. When combined with functions like filter or map, this causes multiple iterations over the same type.
[error] 766-766: The computed expression can be simplified without the use of a string literal. (lint/complexity/useLiteralKeys)
Unsafe fix: Use a literal key instead.
[error] 784-784: The computed expression can be simplified without the use of a string literal. (lint/complexity/useLiteralKeys)
Unsafe fix: Use a literal key instead.
[error] 790-790: The computed expression can be simplified without the use of a string literal. (lint/complexity/useLiteralKeys)
Unsafe fix: Use a literal key instead.
[error] 1097-1125: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 1380-1389: Prefer for...of instead of forEach. (lint/complexity/noForEach)
forEach may lead to performance issues when working with large arrays. When combined with functions like filter or map, this causes multiple iterations over the same type.
Additional comments not posted (22)
app/client/cypress/e2e/Regression/ClientSide/Widgets/Others/Progress_spec.js (10)
26-26
: Replace deprecatedcy.togglebarDisable
withagHelper.CheckUncheck
for consistency.
32-32
: Replace deprecatedcy.togglebarDisable
withagHelper.CheckUncheck
for consistency.
48-48
: Replace deprecatedcy.togglebarDisable
withagHelper.CheckUncheck
for consistency.
52-52
: Replace deprecatedcy.togglebarDisable
withagHelper.CheckUncheck
for consistency.
74-74
: Replace deprecatedcy.togglebarDisable
withagHelper.CheckUncheck
for consistency.
80-80
: Replace deprecatedcy.togglebarDisable
withagHelper.CheckUncheck
for consistency.
99-99
: Replace deprecatedcy.togglebarDisable
withagHelper.CheckUncheck
for consistency.
103-103
: Replace deprecatedcy.togglebarDisable
withagHelper.CheckUncheck
for consistency.
117-117
: Replace deprecatedcy.togglebarDisable
withagHelper.CheckUncheck
for consistency.
123-123
: Replace deprecatedcy.togglebarDisable
withagHelper.CheckUncheck
for consistency.app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_Widget_Add_button_spec.js (1)
259-259
: Ensure the selector used for clicking is updated to reflect the new locator.app/client/cypress/e2e/Regression/ClientSide/Widgets/Multiselect/MultiSelect2_spec.js (5)
72-72
: EnsureagHelper.CheckUncheck
is used consistently for toggling checkbox properties.
106-106
: EnsureagHelper.CheckUncheck
is used consistently for toggling checkbox properties.
134-134
: Replace deprecatedcy.togglebarDisable
withagHelper.CheckUncheck
for consistency.
152-152
: EnsureagHelper.CheckUncheck
is used consistently for toggling checkbox properties.
310-310
: EnsureagHelper.CheckUncheck
is used consistently for toggling checkbox properties.app/client/cypress/locators/Widgets.json (1)
229-233
: New selectors added for checkboxes related to showing results, infinite loading, counterclockwise behavior, and server-side filtering input within the code scanner modal.app/client/cypress/locators/commonlocators.json (4)
237-237
: Check the consistency of the locatorallowclearingValueInput
with similar locators in the file.Verification successful
The locator
allowclearingValueInput
appears to be consistent with other locators in terms of naming conventions and structure. Here are some observations:
- The naming convention follows a pattern of describing the property control and the input type, e.g.,
allowclearingValueInput
,enableSearchLocCheckbox
,serverSidePaginationCheckbox
.- The structure of the locator string is consistent with other locators, using the
.t--property-control-
prefix followed by the specific control name and ending withinput[type='checkbox']
.Based on the provided output, the locator
allowclearingValueInput
is consistent with similar locators in the file.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the consistency of the locator `allowclearingValueInput` with similar locators. # Test: Search for similar locators. Expect: Consistent naming and usage. rg --type json $'input[type="checkbox"]' app/client/cypress/locators/Length of output: 12978
238-238
: Confirm that the locatorallowcountrycodechangeInput
is used correctly in the corresponding test scenarios.Verification successful
The locator
allowcountrycodechangeInput
is used in the test fileapp/client/cypress/e2e/Regression/ClientSide/Widgets/JSONForm/JSONForm_ArrayField_spec.js
. This confirms that the locator is indeed utilized in the test scenarios.
app/client/cypress/e2e/Regression/ClientSide/Widgets/JSONForm/JSONForm_ArrayField_spec.js
:agHelper.CheckUncheck(commonlocators.allowcountrycodechangeInput);
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the locator `allowcountrycodechangeInput` in test scenarios. # Test: Search for the locator usage. Expect: Correct usage in test scenarios. rg --type json $'allowcountrycodechangeInput' app/client/cypress/e2e/Length of output: 68
Script:
#!/bin/bash # Description: Verify the usage of the locator `allowcountrycodechangeInput` in the entire repository. # Test: Search for the locator usage. Expect: Correct usage in test scenarios. rg 'allowcountrycodechangeInput'Length of output: 346
239-240
: Validate the new locatorsallowsearchingInput
andallowsearchingInputTypeCheckbox
for proper integration and usage in tests.Verification successful
The new locators
allowsearchingInput
andallowsearchingInputTypeCheckbox
are properly integrated and used in the test files.
allowsearchingInput
is used inapp/client/cypress/e2e/Regression/ClientSide/Widgets/JSONForm/JSONForm_FieldProperties_2_spec.js
.allowsearchingInputTypeCheckbox
is used inapp/client/cypress/e2e/Regression/ClientSide/Widgets/Multiselect/MultiSelect2_spec.js
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the new locators `allowsearchingInput` and `allowsearchingInputTypeCheckbox` are integrated and used properly in tests. # Test: Search for the locators usage. Expect: Proper integration and usage in tests. rg --type json $'allowsearchingInput' app/client/cypress/e2e/Length of output: 60
Script:
#!/bin/bash # Description: Verify the new locators `allowsearchingInput` and `allowsearchingInputTypeCheckbox` are integrated and used properly in tests. # Test: Search for the locators usage across the entire codebase and in different file types. rg 'allowsearchingInput' rg 'allowsearchingInputTypeCheckbox'Length of output: 1285
236-236
: Ensure the locator forfilePickerMaxNoOfFiles
is correctly formatted and matches the intended element.Verification successful
The locator
filePickerMaxNoOfFiles
is correctly formatted and is used in the test fileapp/client/cypress/e2e/Regression/ClientSide/Widgets/Filepicker/FilePickerV2_spec.js
.
app/client/cypress/e2e/Regression/ClientSide/Widgets/Filepicker/FilePickerV2_spec.js
:cy.get(commonlocators.filePickerMaxNoOfFiles).type("2");
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the locator for `filePickerMaxNoOfFiles` is used in the test files. # Test: Search for the locator usage. Expect: At least one occurrence. rg --type json $'filePickerMaxNoOfFiles' app/client/cypress/e2e/Length of output: 63
Script:
#!/bin/bash # Description: Verify the locator for `filePickerMaxNoOfFiles` is used in the test files and main application code. # Test: Search for the locator usage in the entire Cypress directory. Expect: At least one occurrence. rg --type json $'filePickerMaxNoOfFiles' app/client/cypress/ # Test: Search for the locator usage in the main application code. Expect: At least one occurrence. rg --type json $'filePickerMaxNoOfFiles' app/client/src/Length of output: 248
Script:
#!/bin/bash # Description: Verify the locator for `filePickerMaxNoOfFiles` is used in JavaScript and TypeScript files in the Cypress directory and main application code. # Test: Search for the locator usage in JavaScript and TypeScript files in the Cypress directory. Expect: At least one occurrence. rg --type js,ts $'filePickerMaxNoOfFiles' app/client/cypress/ # Test: Search for the locator usage in JavaScript and TypeScript files in the main application code. Expect: At least one occurrence. rg --type js,ts $'filePickerMaxNoOfFiles' app/client/src/Length of output: 177
Script:
#!/bin/bash # Description: Verify the locator for `filePickerMaxNoOfFiles` is used in JavaScript and TypeScript files in the Cypress directory and main application code. # Test: Search for the locator usage in JavaScript files in the Cypress directory. Expect: At least one occurrence. rg --type js $'filePickerMaxNoOfFiles' app/client/cypress/ # Test: Search for the locator usage in TypeScript files in the Cypress directory. Expect: At least one occurrence. rg --type ts $'filePickerMaxNoOfFiles' app/client/cypress/ # Test: Search for the locator usage in JavaScript files in the main application code. Expect: At least one occurrence. rg --type js $'filePickerMaxNoOfFiles' app/client/src/ # Test: Search for the locator usage in TypeScript files in the main application code. Expect: At least one occurrence. rg --type ts $'filePickerMaxNoOfFiles' app/client/src/Length of output: 371
app/client/cypress/support/commands.js (1)
124-124
: Replace deprecatedcy.togglebarDisable
withagHelper.CheckUncheck
.- cy.togglebarDisable(datasource.useSelfSignedCert); + agHelper.CheckUncheck(datasource.useSelfSignedCert, false); // Assuming the second parameter handles the enable/disable stateLikely invalid or redundant comment.
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/9365254766. |
app/client/cypress/e2e/Regression/ClientSide/Widgets/JSONForm/JSONForm_ArrayField_spec.js
Outdated
Show resolved
Hide resolved
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: 1
Outside diff range and nitpick comments (3)
app/client/cypress/e2e/Regression/ClientSide/Widgets/Tab/Tab_spec.js (1)
Line range hint
11-179
: Convert function expressions to arrow functions for improved readability and reduced complexity.- describe("Tab widget test", { tags: ["@tag.Widget", "@tag.Tab"] }, function () { + describe("Tab widget test", { tags: ["@tag.Widget", "@tag.Tab"] }, () => {Repeat this pattern for all function expressions in this file.
app/client/cypress/support/commands.js (2)
Line range hint
113-113
: Simplify the computed expression by using a literal key.- const request = window.indexedDB.open("Appsmith", 2); + const request = window.indexedDB.open(Appsmith, 2); // Assuming 'Appsmith' is a constant or variable appropriately defined
Line range hint
145-149
: Change to an optional chain to simplify the code.- const arr = url.split("?")[1]?.split("&"); - const paramObj = {}; - arr && - arr.forEach((param) => { - const [key, value] = param.split("="); - paramObj[key] = value; - }); + const paramObj = Object.fromEntries(new URLSearchParams(url.split("?")[1]));
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/JSONForm/JSONForm_ArrayField_spec.js (5 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/Tab/Tab_spec.js (2 hunks)
- app/client/cypress/support/commands.js (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/JSONForm/JSONForm_ArrayField_spec.js
Additional context used
Biome
app/client/cypress/e2e/Regression/ClientSide/Widgets/Tab/Tab_spec.js
[error] 15-53: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 64-70: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 72-78: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 80-90: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 92-102: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 128-145: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 147-162: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 164-178: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 11-179: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.app/client/cypress/support/commands.js
[error] 113-113: The computed expression can be simplified without the use of a string literal. (lint/complexity/useLiteralKeys)
Unsafe fix: Use a literal key instead.
[error] 116-116: The computed expression can be simplified without the use of a string literal. (lint/complexity/useLiteralKeys)
Unsafe fix: Use a literal key instead.
[error] 145-149: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
[error] 766-766: The computed expression can be simplified without the use of a string literal. (lint/complexity/useLiteralKeys)
Unsafe fix: Use a literal key instead.
[error] 784-784: The computed expression can be simplified without the use of a string literal. (lint/complexity/useLiteralKeys)
Unsafe fix: Use a literal key instead.
[error] 790-790: The computed expression can be simplified without the use of a string literal. (lint/complexity/useLiteralKeys)
Unsafe fix: Use a literal key instead.
[error] 1087-1115: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
Additional comments not posted (4)
app/client/cypress/e2e/Regression/ClientSide/Widgets/Tab/Tab_spec.js (2)
47-47
: Replacecy.togglebar
withagHelper.CheckUncheck
to standardize toggle behavior.
74-74
: EnsureagHelper.CheckUncheck
is correctly replacingcy.togglebar
for consistent functionality across tests.app/client/cypress/support/commands.js (2)
Line range hint
1087-1115
: Convert this function expression to an arrow function for better readability and consistency.
[REFACTOR_SUGGESTion]- cy.intercept({ - method: "GET", - url: "/api/v1/product-alert/alert", - }, (req) => { - req.reply((res) => { - if (res) { - if (res.statusCode === 200) { - res.send({ - responseMeta: { - status: 200, - success: true, - }, - data: {}, - errorDisplay: "", - }); - } - } else { - // Do nothing or handle the case where the response object is not present - } - }); - }).as("productAlert"); + cy.intercept("/api/v1/product-alert/alert", (req) => { + req.reply((res) => { + if (res?.statusCode === 200) { + res.send({ + responseMeta: { + status: 200, + success: true, + }, + data: {}, + errorDisplay: "", + }); + } + }); + }).as("productAlert");
124-124
: EnsureagHelper.CheckUncheck
correctly handles all scenarios previously covered bytogglebar
.Verification successful
The previous script failed due to an incorrect file type specification. The correct file type should be JavaScript, not Python. Let's correct this and re-run the script to verify the usage of
agHelper.CheckUncheck
.
The search results indicate that
agHelper.CheckUncheck
is used consistently across multiple test files. This suggests that the replacement oftogglebar
withagHelper.CheckUncheck
has been implemented correctly and is being utilized throughout the test suite.
app/client/cypress/support/commands.js
app/client/cypress/e2e/Regression/ClientSide/Widgets/Switch/Switch_spec.js
app/client/cypress/e2e/Regression/ClientSide/Widgets/Multiselect/Multi_Select_Tree_spec.js
app/client/cypress/e2e/Regression/ClientSide/Widgets/Checkbox/CheckBox_spec.js
app/client/cypress/e2e/Regression/ClientSide/Widgets/Tab/Tab_spec.js
app/client/cypress/e2e/Regression/ClientSide/Widgets/CurrencyInput/CurrencyInput_ShowStepArrows_spec.js
app/client/cypress/e2e/Regression/ClientSide/Widgets/Select/Select_spec.js
app/client/cypress/e2e/Regression/ClientSide/Widgets/Others/Progress_spec.js
app/client/cypress/e2e/Regression/ClientSide/Widgets/Radio/Radio_spec.js
app/client/cypress/e2e/Regression/ClientSide/Widgets/JSONForm/JSONForm_ArrayField_spec.js
app/client/cypress/e2e/Regression/ClientSide/Widgets/JSONForm/JSONForm_FieldProperties_1_spec.js
app/client/cypress/e2e/Regression/ClientSide/Widgets/ListV2/ListV2_PageNo_PageSize_spec.js
app/client/cypress/e2e/Regression/ClientSide/Widgets/JSONForm/JSONForm_HiddenFields_2_spec.js
app/client/cypress/e2e/Regression/ClientSide/Widgets/JSONForm/JSONForm_HiddenFields_1_spec.js
app/client/cypress/e2e/Regression/ClientSide/Widgets/JSONForm/JSONForm_FormProperties_spec.js
app/client/cypress/e2e/Regression/ClientSide/Widgets/JSONForm/JSONForm_FieldProperties_2_spec.js
app/client/cypress/e2e/Regression/ClientSide/Widgets/JSONForm/JSONForm_FormBindings_spec.js
app/client/cypress/e2e/Regression/ClientSide/Widgets/ListV2/Listv2_Meta_Hydration_ServerSide_spec.js
app/client/cypress/e2e/Regression/ClientSide/Widgets/ListV2/Listv2_BasicClientSideData_spec.js
app/client/cypress/e2e/Regression/ClientSide/Widgets/ListV2/Childwigets/Listv2_Tabs_Widget_spec.js
app/client/cypress/e2e/Regression/ClientSide/Widgets/Input/Inputv2_ShowStepArrows_spec.js
app/client/cypress/e2e/Regression/ClientSide/Widgets/Image/Image_spec.js
app/client/cypress/e2e/Regression/ClientSide/Widgets/Input/Input_spec.js
app/client/cypress/e2e/Regression/ClientSide/Widgets/Dropdown/Dropdown_spec.js
app/client/cypress/e2e/Regression/ClientSide/Widgets/Datepicker/DatePicker_With_Switch_spec.js
app/client/cypress/e2e/Regression/ClientSide/Widgets/Form/Form_With_CheckBox_spec.js
app/client/cypress/e2e/Regression/ClientSide/Widgets/Form/FormWithSwitch_spec.js
app/client/cypress/e2e/Regression/ClientSide/Widgets/Form/FormWidget_spec.js
app/client/cypress/e2e/Regression/ClientSide/Templates/Fork_Template_Existing_app_spec.js
Given the extensive usage and consistency, it appears that the change has been thoroughly integrated.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that `agHelper.CheckUncheck` is used consistently and correctly throughout the test suite. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type python $'agHelper.CheckUncheck'Length of output: 71
Script:
#!/bin/bash # Description: Verify that `agHelper.CheckUncheck` is used consistently and correctly throughout the test suite. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type js 'agHelper.CheckUncheck'Length of output: 7972
@@ -121,7 +121,7 @@ Cypress.Commands.add("testSelfSignedCertificateSettingsInREST", (isOAuth2) => { | |||
cy.get(datasource.useCertInAuth).should("not.exist"); | |||
cy.get(datasource.certificateDetails).should("not.exist"); | |||
// cy.TargetDropdownAndSelectOption(datasource.useSelfSignedCert, "Yes"); | |||
cy.togglebar(datasource.useSelfSignedCert); | |||
agHelper.CheckUncheck(datasource.useSelfSignedCert); |
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.
Replace cy.togglebarDisable
with a correct function call.
- cy.togglebarDisable(datasource.useSelfSignedCert);
+ agHelper.CheckUncheck(datasource.useSelfSignedCert, false); // Assuming false to uncheck based on context
Committable suggestion was skipped due to low confidence.
removed cy.toggleBar() from Commands.js file and replaced with ts helper method agHelper.CheckUncheck();
Link EE for runs - https://github.com/appsmithorg/appsmith-ee/pull/4311
/ok-to-test tags="@tag.All"
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/9312018494
Commit: 04e2bdf
Cypress dashboard url: Click here!
Summary by CodeRabbit
Refactor
cy.togglebar
withagHelper.CheckUncheck
across multiple test files for better consistency and maintainability in handling checkbox interactions.Chores