test: verifying replication of JS Object settings during copy action #38430
Conversation
shadabbuchh
requested review from
ApekshaBhosale and
sagar-qa007
as code owners
WalkthroughA new Cypress test specification has been introduced to validate JavaScript Object settings when copied between pages, specifically addressing bug #31475. The test ensures that the "Run on page load" setting remains enabled after copying a JavaScript object to a different page. Additionally, the path for limited tests has been updated to include this new test specification, indicating a shift in focus towards testing the functionality of copied JS objects. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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 using PR comments)
Other keywords and placeholders
Documentation and Community
|
github-actions
bot
added
skip-changelog
|
/ci-test-limit-count run_count=20 update_snapshot=true specs_to_run=cypress/e2e/Regression/ClientSide/JSObject/Copied_JSObject_Settings_Bug31475_spec.ts |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/12557740668. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/client/cypress/e2e/Regression/ClientSide/JSObject/Copied_JSObject_Settings_Bug31475_spec.ts (1)
31-34: Use data- attributes for locators instead of text-based selectors.*The code here uses
.contains("Copy to page")and.contains("Page2"). Although this works, our guidelines recommend using data-* attributes as selectors for locator stability and clarity.-agHelper.HoverElement(`${HomePage.portalMenuItem}:contains("Copy to page")`); -agHelper.GetNClick(`${HomePage.portalMenuItem}:contains("Page2")`); +// Example: Suppose we have data-cy for these menu items +agHelper.HoverElement('[data-cy="copy-to-page"]'); +agHelper.GetNClick('[data-cy="copy-to-page2"]');
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/client/cypress/e2e/Regression/ClientSide/JSObject/Copied_JSObject_Settings_Bug31475_spec.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
app/client/cypress/e2e/Regression/ClientSide/JSObject/Copied_JSObject_Settings_Bug31475_spec.ts (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.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/client/cypress/limited-tests.txt(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
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.
| @@ -1,5 +1,5 @@ | |||
| # To run only limited tests - give the spec names in below format: | |||
| cypress/e2e/Regression/ClientSide/VisualTests/JSEditorIndent_spec.js | |||
| cypress/e2e/Regression/ClientSide/JSObject/Copied_JSObject_Settings_Bug31475_spec.ts | |||
There was a problem hiding this comment.
💡 Codebase verification
Path in limited-tests.txt needs correction
The test file exists but at a different path: app/client/cypress/e2e/Regression/ClientSide/JSObject/Copied_JSObject_Settings_Bug31475_spec.ts
Please update the path in limited-tests.txt to include the full path from the repository root.
🔗 Analysis chain
LGTM! Verify the test file implementation.
The test path aligns with the PR objectives for testing JS Object settings during copy action.
Let's verify the test file exists and follows Cypress best practices:
Test file referenced in limited-tests.txt is missing
The test file cypress/e2e/Regression/ClientSide/JSObject/Copied_JSObject_Settings_Bug31475_spec.ts specified in limited-tests.txt does not exist in the codebase. Let's verify if it's located in a different directory:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify test file exists and check for common anti-patterns
# Check if the test file exists
FILE_PATH="cypress/e2e/Regression/ClientSide/JSObject/Copied_JSObject_Settings_Bug31475_spec.ts"
if ! fd "$FILE_PATH"; then
echo "Error: Test file not found"
exit 1
fi
# Check for anti-patterns in the test file
echo "Checking for Cypress anti-patterns..."
# Check for cy.wait usage
rg "cy\.wait" "$FILE_PATH"
# Check for cy.pause usage
rg "cy\.pause" "$FILE_PATH"
# Check for agHelper.sleep usage
rg "agHelper\.sleep" "$FILE_PATH"
# Check for it.only usage
rg "it\.only" "$FILE_PATH"
# Check for after/afterEach blocks
rg "after(Each)?\s*\(" "$FILE_PATH"
# Check for proper selector usage (avoid plain strings)
rg "cy\.(get|contains|find)\(['\"]" "$FILE_PATH"
Length of output: 1668
Script:
#!/bin/bash
# Search for the test file by name
fd "Copied_JSObject_Settings_Bug31475_spec.ts"
# Search for any file containing the bug number
fd -t f "31475"
# Search for similar test files in the JSObject directory
fd -t f . "app/client/cypress" -e ts -e js | rg "JSObject.*spec"
Length of output: 1007
|
/ci-test-limit-count run_count=20 update_snapshot=true specs_to_run=cypress/e2e/Regression/ClientSide/JSObject/Copied_JSObject_Settings_Bug31475_spec.ts |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/12558120288. |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/12558120288. |
|
This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected. |
Description
This PR introduces a test case designed to validate whether the "Run on page load" setting for a specific function in a JS Object is correctly retained when the object is copied to another page.
Relevant bug: #31475
Automation
/ok-to-test tags="@tag.JS"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/12558090952
Commit: 2c9eb15
Cypress dashboard.
Tags:
@tag.JSSpec:
Tue, 31 Dec 2024 10:36:11 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit