Skip to content
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.

Sign up for GitHub

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: adding tests for JsObjects regression #38103

Closed
wants to merge 6 commits into from
Closed

test: adding tests for JsObjects regression #38103

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
291e1d0
test: adding tests for JsObjects regression
Dec 11, 2024
4006635
Merge branch 'release' into test/jsobjectsRegTests
Dec 12, 2024
22c8b62
updated locator changes
Dec 12, 2024
4000628
updated locators
Dec 12, 2024
bef8061
updated locators
Dec 12, 2024
1cf2727
updated the test to handle announcement modal
Dec 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 87 additions & 0 deletions app/client/cypress/e2e/Regression/ClientSide/JSObject/JSObject_Tests_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
import HomePage from "../../../../locators/HomePage";
import {
agHelper,
entityExplorer,
jsEditor,
propPane,
draggableWidgets,
locators,
} from "../../../../support/ee/ObjectsCore_EE";
import EditorNavigation, {
PageLeftPane,
PagePaneSegment,
} from "../../../../support/Pages/EditorNavigation";
import PageList from "../../../../support/Pages/PageList";

describe("Validate JSObj", {}, () => {
before(() => {});

Comment on lines +16 to +18
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Remove empty before hook

The empty before hook serves no purpose and should be removed as per best practices.

-describe("Validate JSObj", {}, () => {
-  before(() => {});
+describe("Validate JSObj", {}, () => {
📝 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.

Suggested change
describe("Validate JSObj", {}, () => {
before(() => {});
describe("Validate JSObj", {}, () => {

it("1. Verify adding JSObject and more actions options", () => {
jsEditor.CreateJSObject(
`setInterval(() => {
showAlert("Hi", "error")
}, 2500, "Int")`,
{
paste: true,
completeReplace: false,
toRun: true,
shouldCreateNewJSObj: true,
},
);
jsEditor.EnableDisableAsyncFuncSettings("myFun1");

// Add new JSObject
PageList.AddNewPage("New blank page");
PageLeftPane.switchSegment(PagePaneSegment.JS);
agHelper.GetNClick(locators._createNew);
agHelper.GetNClick(jsEditor._addJSObj);
agHelper.AssertContains("JSObject2", "exist", ".t--entity-name");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Use data-testid attributes instead of class selectors

Using class selectors (.t--entity-name) is against the provided coding guidelines. Use data-testid attributes instead.

-agHelper.AssertContains("JSObject2", "exist", ".t--entity-name");
+agHelper.AssertContains("JSObject2", "exist", "[data-testid='entity-name']");
-agHelper.AssertContains("JSObject3", "exist", ".t--entity-name");
+agHelper.AssertContains("JSObject3", "exist", "[data-testid='entity-name']");

Also applies to: 43-43

agHelper.GetNClick(EditorNavigation.locators.MinimizeBtn);
EditorNavigation.CloseAnnouncementModal();
agHelper.GetNClick(jsEditor._addJSObj);
agHelper.GetNClick(EditorNavigation.locators.MaximizeBtn);
agHelper.AssertContains("JSObject3", "exist", ".t--entity-name");

// Verify menu item
agHelper.GetNClick(jsEditor._jsPageActions, 0, true);
agHelper.AssertContains("Rename", "exist", HomePage.portalMenuItem);
agHelper.AssertContains("Show bindings", "exist", HomePage.portalMenuItem);
agHelper.AssertContains("Copy to page", "exist", HomePage.portalMenuItem);
agHelper.AssertContains("Move to page", "exist", HomePage.portalMenuItem);
agHelper.AssertContains("Delete", "exist", HomePage.portalMenuItem);

agHelper.GetNClick(jsEditor._moreActions, 0, true);
agHelper.AssertContains("Rename", "exist", HomePage.portalMenuItem);
agHelper.AssertContains("Copy to page", "exist", HomePage.portalMenuItem);
agHelper.AssertContains("Move to page", "exist", HomePage.portalMenuItem);
agHelper.AssertContains("Prettify code", "exist", HomePage.portalMenuItem);
agHelper.AssertContains("Delete", "exist", HomePage.portalMenuItem);
});

it("2. Verify alert message on page load", () => {
// Verify alert message on page load
EditorNavigation.NavigateToPage("Page1", true);
agHelper.ValidateToastMessage("Hi");
});
Comment on lines +61 to +65
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve test reliability by removing implicit waits

The test relies on the alert message appearing after page navigation. Consider using Cypress's built-in retry-ability instead of implicit waits:

-EditorNavigation.NavigateToPage("Page1", true);
-agHelper.ValidateToastMessage("Hi");
+EditorNavigation.NavigateToPage("Page1", true);
+agHelper.WaitUntilToastMessageAppears("Hi");

Committable suggestion skipped: line range outside the PR's diff.


it("3. Verify moving JSObject to new page", () => {
// Verify Move to Page
agHelper.GetNClick(jsEditor._jsPageActions, 0, true);
agHelper.HoverElement(
`${HomePage.portalMenuItem}:contains("Move to page")`,
);
agHelper.GetNClick(`${HomePage.portalMenuItem}:contains("Page2")`);
Comment on lines +70 to +73
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Replace string selectors with data attributes

Using string concatenation and CSS contains selectors is against best practices. Use data-testid attributes instead.

-agHelper.HoverElement(
-  `${HomePage.portalMenuItem}:contains("Move to page")`,
-);
-agHelper.GetNClick(`${HomePage.portalMenuItem}:contains("Page2")`);
+agHelper.HoverElement("[data-testid='move-to-page-menu-item']");
+agHelper.GetNClick("[data-testid='page2-menu-item']");

-agHelper.GetNClick(`.t--entity-name:contains("JSObject11")`);
+agHelper.GetNClick("[data-testid='js-object-11']");

Also applies to: 76-76


// Verify 'Run on page load' on new page
agHelper.GetNClick(`.t--entity-name:contains("JSObject11")`);
jsEditor.VerifyAsyncFuncSettings("myFun1");
});

it("4. Verify JSObject binding", () => {
PageLeftPane.switchSegment(PagePaneSegment.UI);
entityExplorer.DragDropWidgetNVerify(draggableWidgets.BUTTON, 500, 100);
propPane.EnterJSContext("onClick", "{{JSObject11.myFun1();}}", true, false);
agHelper.GetNClick(locators._widgetInDeployed("buttonwidget"));
agHelper.ValidateToastMessage("Hi");
});
});
2 changes: 1 addition & 1 deletion app/client/cypress/limited-tests.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# To run only limited tests - give the spec names in below format:
cypress/e2e/Regression/ClientSide/Templates/Fork_Template_spec.js
cypress/e2e/Regression/ClientSide/JSObject/JSObject_Tests_spec.ts
# For running all specs - uncomment below:
#cypress/e2e/**/**/*

Expand Down
5 changes: 4 additions & 1 deletion app/client/cypress/support/Pages/JSEditor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export class JSEditor {
public ee = ObjectsRegistry.EntityExplorer;
public propPane = ObjectsRegistry.PropertyPane;
private assertHelper = ObjectsRegistry.AssertHelper;
public runButtonLocator = "[data-testid='t--run-js-action']";
public runButtonLocator = ".run-js-action";
public settingsTriggerLocator = "[data-testid='t--js-settings-trigger']";
public contextMenuTriggerLocator = "[data-testid='t--more-action-trigger']";
public runFunctionSelectLocator = "[data-testid='t--js-function-run']";
Expand Down Expand Up @@ -89,6 +89,9 @@ export class JSEditor {
"//div[@data-testid='t--query-run-confirmation-modal']//span[text()='" +
text +
"']";
_addJSObj = '[data-testid="t--ide-tabs-add-button"]';
_jsPageActions = ".entity-context-menu";
_moreActions = '[data-testid="t--more-action-trigger"]';
Comment on lines +92 to +94
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Use data-testid instead of class selector

The _jsPageActions property uses a class selector which goes against the coding guidelines.

Apply this diff:

   _addJSObj = '[data-testid="t--ide-tabs-add-button"]';
-  _jsPageActions = ".entity-context-menu";
+  _jsPageActions = '[data-testid="t--entity-context-menu"]';
   _moreActions = '[data-testid="t--more-action-trigger"]';
📝 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.

Suggested change
_addJSObj = '[data-testid="t--ide-tabs-add-button"]';
_jsPageActions = ".entity-context-menu";
_moreActions = '[data-testid="t--more-action-trigger"]';
_addJSObj = '[data-testid="t--ide-tabs-add-button"]';
_jsPageActions = '[data-testid="t--entity-context-menu"]';
_moreActions = '[data-testid="t--more-action-trigger"]';

//#endregion

//#region constants
Expand Down
Loading