-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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: updated tests with aghelper table methods #33737
Conversation
/ci-test-limit |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/9249911713. |
WalkthroughWalkthroughThe changes primarily involve replacing instances of the Changes
Sequence Diagram(s) (Beta)sequenceDiagram
participant Cypress Test as Cypress Test
participant Table Object as table
participant Backend as Backend
Cypress Test->>Backend: cy.wait("@updateLayout")
Backend-->>Cypress Test: Layout updated
Cypress Test->>Table Object: table.SelectTableRow(0, 0, true, "v2")
Table Object-->>Cypress Test: Row 0 Selected
sequenceDiagram
participant Cypress Test as Cypress Test
participant Table Object as table
Cypress Test->>Table Object: table.SelectTableRow(1, 0, true, "v2")
Table Object-->>Cypress Test: Row 1 Selected
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (5)
Additional context usedLearnings (2)
Biome
Additional comments not posted (7)
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 (
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/9249911713. |
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: 9
Outside diff range and nitpick comments (8)
app/client/cypress/limited-tests.txt (1)
Line range hint
34-34
: Correct the grammatical error for clarity.- #ci-test-limit uses this file to run minimum of specs. Do not run entire suite with this command. + #ci-test-limit uses this file to run a minimum number of specs. Do not run the entire suite with this command.app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/non_ascii_column_name_spec.js (1)
Line range hint
26-33
: Consider using afor...of
loop for better readability and performance.- Object.keys(data[0]).forEach((column) => { - cy.get(`.t--widget-tablewidgetv2 .thead .th[data-header="${column}"]`).should("exist"); - }); + for (const column of Object.keys(data[0])) { + cy.get(`.t--widget-tablewidgetv2 .thead .th[data-header="${column}"]`).should("exist"); + }app/client/cypress/e2e/Regression/ClientSide/Binding/Input_NavigateTo_validation_spec.js (2)
Line range hint
36-43
: Consider converting function expressions to arrow functions for consistency and clarity.- function () { + () => {Also applies to: 45-55, 57-79, 23-80
Line range hint
64-64
: Use template literals for string concatenation.- cy.log("the value is" + tabValue); + cy.log(`the value is ${tabValue}`);app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_Button_Icon_validation_spec.js (1)
Line range hint
15-29
: Consider converting function expressions to arrow functions for consistency and clarity.- function () { + () => {Also applies to: 31-55, 57-75, 77-93, 10-94
app/client/cypress/support/commands.js (3)
Line range hint
16-16
: Use thenode:
protocol when importing Node.js built-in modules.- const path = require("path"); + const path = require("node:path");
Line range hint
145-149
: Simplify the handling of URL query parameters using optional chaining andfor...of
.- arr && - arr.forEach((param) => { - const [key, value] = param.split("="); - paramObj[key] = value; - }); + for (const param of arr ?? []) { + const [key, value] = param.split("="); + paramObj[key] = value; + }
Line range hint
209-209
: Replace string concatenation with template literals for better readability and performance.- urlObject.pathname = urlObject.pathname + "/edit"; + urlObject.pathname = `${urlObject.pathname}/edit`; - "docker run -d --name " + containerName + ' -p 8081:80 -p 9002:9002 -v "' + path + '/stacks:/appsmith-stacks" ' + version; + `docker run -d --name ${containerName} -p 8081:80 -p 9002:9002 -v "${path}/stacks:/appsmith-stacks" ${version}`;Also applies to: 221-221, 347-347, 353-353, 356-356, 377-377, 386-391, 414-414, 417-417, 432-432, 435-435, 581-581, 615-615
cy.wait("@updateLayout") | ||
_.table.SelectTableRow(0, 0, true, "v2"); |
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.
Refactor to use arrow functions for consistency and modern JavaScript practices.
- it("1. Input widget test with default value from table widget", function () {
+ it("1. Input widget test with default value from table widget", () => {
Committable suggestion was skipped due low confidence.
@@ -43,7 +44,7 @@ describe( | |||
cy.readTabledataPublish("1", "0").then((tabDataP) => { | |||
const tabValueP = tabDataP; | |||
cy.log(tabValueP); | |||
cy.isSelectRow(1); | |||
table.SelectTableRow(1); |
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.
Refactor to use arrow functions for consistency and modern JavaScript practices.
- it("2. Validate NavigateTo Page functionality ", function () {
+ it("2. Validate NavigateTo Page functionality ", () => {
Committable suggestion was skipped due low confidence.
@@ -55,7 +56,7 @@ describe( | |||
cy.wait(2000); | |||
deployMode.DeployApp(); | |||
cy.get(widgetsPage.chartWidget).should("not.exist"); | |||
cy.isSelectRow(1); | |||
table.SelectTableRow(1); |
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.
Refactor to use arrow functions for consistency and modern JavaScript practices.
- it("1. Create MyPage and valdiate if its successfully created", function () {
+ it("1. Create MyPage and valdiate if its successfully created", () => {
Committable suggestion was skipped due low confidence.
@@ -35,7 +36,7 @@ describe( | |||
}); | |||
|
|||
it("2. validation of default data displayed in all widgets based on row selected", function () { | |||
cy.isSelectRow(1); | |||
table.SelectTableRow(1); |
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.
Refactor to use arrow functions for consistency and modern JavaScript practices.
- it("2. validation of default data displayed in all widgets based on row selected", function () {
+ it("2. validation of default data displayed in all widgets based on row selected", () => {
Committable suggestion was skipped due low confidence.
_.table.SelectTableRow(1); | ||
//should be 2 if we decide to show trigger errors in the debugger. | ||
_.debuggerHelper.AssertErrorCount(1); | ||
// Fix code | ||
cy.testJsontext("onrowselected", "{{console.log('test')}}"); | ||
cy.isSelectRow(1); | ||
_.table.SelectTableRow(1); |
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.
Updated method usage aligns with new testing standards. Consider converting function expressions to arrow functions for consistency and modern JavaScript practices.
- before(() => {
+ before(() => {
- it("1. Trigger errors need to be shown in the errors tab", function () {
+ it("1. Trigger errors need to be shown in the errors tab", () => {
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.
_.table.SelectTableRow(1); | |
//should be 2 if we decide to show trigger errors in the debugger. | |
_.debuggerHelper.AssertErrorCount(1); | |
// Fix code | |
cy.testJsontext("onrowselected", "{{console.log('test')}}"); | |
cy.isSelectRow(1); | |
_.table.SelectTableRow(1); | |
_.table.SelectTableRow(1); | |
//should be 2 if we decide to show trigger errors in the debugger. | |
_.debuggerHelper.AssertErrorCount(1); | |
// Fix code | |
cy.testJsontext("onrowselected", "{{console.log('test')}}"); | |
_.table.SelectTableRow(1); | |
before(() => { | |
// setup code | |
}); | |
it("1. Trigger errors need to be shown in the errors tab", () => { | |
// test code | |
}); |
@@ -42,7 +43,7 @@ describe( | |||
const tableRowTxt = text; | |||
dataSources.AddSuggestedWidget(Widgets.Table); | |||
EditorNavigation.SelectEntityByName("Table1", EntityType.Widget); | |||
cy.isSelectRow(1); | |||
table.SelectTableRow(1, 0, true, "v2"); | |||
cy.readTableV2dataPublish("1", "0").then((tabData) => { | |||
const tabValue = tabData; | |||
cy.log("the value is" + tabValue); |
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 string concatenation and converting function expressions to arrow functions.
- cy.log("the value is" + tabValue);
+ cy.log(`the value is ${tabValue}`);
- function () {
+ () => {
Also applies to: 68-68, 71-71, 20-90
Committable suggestion was skipped due low confidence.
@@ -61,7 +62,7 @@ describe( | |||
cy.wait(2000); | |||
deployMode.DeployApp(); | |||
cy.get(widgetsPage.chartWidget).should("not.exist"); | |||
cy.isSelectRow(1); | |||
table.SelectTableRow(1, 0, true, "v2"); |
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.
Refactor to use arrow functions for consistency and modern JavaScript practices.
- it("2. Validate NavigateTo Page functionality ", function () {
+ it("2. Validate NavigateTo Page functionality ", () => {
Committable suggestion was skipped due low confidence.
@@ -27,7 +27,7 @@ describe( | |||
cy.runQuery(); | |||
dataSources.AddSuggestedWidget(Widgets.Table); | |||
EditorNavigation.SelectEntityByName("Table1", EntityType.Widget); | |||
cy.isSelectRow(1); | |||
table.SelectTableRow(1, 0, true, "v2"); |
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.
Updated method usage is appropriate. Consider using template literals for better readability and performance in string operations.
- cy.log("the value is " + tabData);
+ cy.log(`the value is ${tabData}`);
Committable suggestion was skipped due low confidence.
// cy.wait("@updateLayout").isSelectRow(0); | ||
cy.wait("@updateLayout"); | ||
_.table.SelectTableRow(0); |
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.
Updated method usage is appropriate. Consider using template literals for better readability and performance in string operations.
- cy.log("the value is" + tabValue);
+ cy.log(`the value is ${tabValue}`);
Committable suggestion was skipped due low confidence.
app/client/cypress/e2e/Regression/ClientSide/Binding/TextTableV2_spec.js
Outdated
Show resolved
Hide resolved
app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV1/Table_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: 0
Outside diff range and nitpick comments (3)
app/client/cypress/support/commands.js (3)
Line range hint
16-16
: Use thenode:
protocol when importing Node.js built-in modules.- const path = require("path"); + const path = require("node:path");This change ensures compatibility with newer Node.js versions and follows best practices.
Line range hint
145-149
: Simplify the URL parameter extraction using optional chaining.- 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] || ""));This refactor simplifies the code and improves readability.
Line range hint
209-209
: Use template literals for string concatenation.- urlObject.pathname = urlObject.pathname + "/edit"; + urlObject.pathname = `${urlObject.pathname}/edit`; - "api/v1/layouts/" + layoutId + "/pages/" + pageid + "?applicationId=" + appId + `api/v1/layouts/${layoutId}/pages/${pageid}?applicationId=${appId}` - "docker stop " + containerName + `docker stop ${containerName}` - "docker inspect -f '{{ .Mounts }}' " + containerName + "|awk '{print $2}'" + `docker inspect -f '{{ .Mounts }}' ${containerName}|awk '{print $2}'`Using template literals improves readability and is a modern JavaScript practice.
Also applies to: 221-221, 347-347, 353-353, 356-356, 377-377, 386-391, 414-414, 417-417, 432-432, 435-435, 608-608
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 (5)
app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2_DefaultSearch_Input_spec.js (3)
Line range hint
39-39
: Use template literals for string concatenation for better readability and performance.- cy.log("the value is" + tabValue); + cy.log(`the value is ${tabValue}`);
Line range hint
17-45
: Consider refactoring this function expression to an arrow function for consistency and modern JavaScript practices.- it("1. Input widget test with default value from table widget", function () { + it("1. Input widget test with default value from table widget", () => {
Line range hint
12-46
: Refactor the outer function expression to an arrow function to maintain consistency and leverage ES6 features.- describe("Binding the Table and input Widget", { tags: ["@tag.Binding"] }, function () { + describe("Binding the Table and input Widget", { tags: ["@tag.Binding"] }, () => {app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2Widgets_NavigateTo_Validation_spec.js (2)
Line range hint
34-59
: Refactor this function expression to an arrow function to maintain consistency and leverage ES6 features.- it("1. Create MyPage and validate if its successfully created", function () { + it("1. Create MyPage and validate if its successfully created", () => {
Line range hint
61-67
: Refactor this function expression to an arrow function for consistency and modern JavaScript practices.- it("2. Validate NavigateTo Page functionality ", function () { + it("2. Validate NavigateTo Page functionality ", () => {
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 (4)
app/client/cypress/e2e/Regression/ServerSide/QueryPane/AddWidget_spec.js (1)
Line range hint
12-36
: Consider converting this function expression to an arrow function to maintain consistency and leverage lexical scoping.- function () { + () => {app/client/cypress/e2e/Regression/ServerSide/QueryPane/AddWidgetTableAndBind_spec.js (1)
Line range hint
20-90
: Consider converting this function expression to an arrow function to maintain consistency and leverage lexical scoping.- function () { + () => {app/client/cypress/e2e/Regression/ClientSide/Binding/JSObject_Postgress_Table_spec.js (1)
Line range hint
14-102
: Consider converting this function expression to an arrow function to maintain consistency and leverage lexical scoping.- function () { + () => {app/client/cypress/e2e/Regression/ClientSide/Widgets/Migration_Spec.js (1)
Line range hint
16-315
: Consider converting this function expression to an arrow function to maintain consistency and leverage lexical scoping.- function () { + () => {
updated isSelectedRow() with table_SelectedRow()
/ok-to-test tags="@tag.All"
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/9312024901
Commit: c308cbb
Cypress dashboard url: Click here!