test: fix Embedd Spec #37263
test: fix Embedd Spec #37263
Conversation
NandanAnantharamu
requested review from
ApekshaBhosale and
sagar-qa007
as code owners
WalkthroughThe changes in this pull request involve updates to the Cypress end-to-end test suite for embed settings. Key modifications include replacing direct Cypress commands with helper functions for element interactions and updating the test specification path in the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/11705676417. |
|
/ci-test-limit-count run_count=25 |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/11705702957. |
![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
🧹 Outside diff range and nitpick comments (3)
app/client/cypress/e2e/Regression/ClientSide/EmbedSettings/EmbedSettings_spec.js (3)
Line range hint
77-78: Remove explicit sleep commandsReplace
_.agHelper.Sleep()with proper Cypress wait conditions usingcy.waitUntil()or appropriate assertions.- _.agHelper.Sleep(2000); + cy.waitUntil(() => { + return cy.window().then((win) => { + return win.document.readyState === 'complete'; + }); + });Also applies to: 144-145
Line range hint
82-142: Refactor test structureThe test combines multiple scenarios in a single test case. Consider splitting into separate test cases:
- "should limit embedding"
- "should allow embedding everywhere"
- "should disable embedding everywhere"
This improves test isolation and maintenance.
Line range hint
6-20: Improve iframe handling reliabilityReplace the current iframe handling with a more robust approach:
const getIframeBody = () => { return cy .get(".t--widget-iframewidget iframe") .should('be.visible') + .should('have.prop', 'complete', true) .its("0.contentDocument.body") .should("not.be.empty") .then(cy.wrap); };
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
app/client/cypress/e2e/Regression/ClientSide/EmbedSettings/EmbedSettings_spec.js(2 hunks)app/client/cypress/limited-tests.txt(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
app/client/cypress/e2e/Regression/ClientSide/EmbedSettings/EmbedSettings_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.
🔇 Additional comments (2)
app/client/cypress/limited-tests.txt (1)
2-2: LGTM! Verifying the test file path.
The updated test specification path aligns with the PR objectives to focus on embed settings tests.
Let's verify the existence of the specified test file:
✅ Verification successful
Test file path verified successfully
The specified test file exists at the correct location in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the new test file exists
# Expected: The file should exist in the specified path
fd "EmbedSettings_spec.js" app/client/cypress/e2e/Regression/ClientSide/EmbedSettings/
Length of output: 167
app/client/cypress/e2e/Regression/ClientSide/EmbedSettings/EmbedSettings_spec.js (1)
41-41: LGTM: Improved element interaction
Good change to use the helper function for clicking elements. This makes the code more maintainable and consistent.
Also applies to: 67-67
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/11705702957. |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/11705676417. |
|
/ci-test-limit |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/11737896921. |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/11737896921. |
|
/ci-test-limit-count run_count=25 |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/11738256344. |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/11738256344. |
updated the button click logic in the test
Warning
Tests have not run on the HEAD a8eb509 yet
Fri, 08 Nov 2024 07:26:06 UTC
Summary by CodeRabbit
Bug Fixes
New Features
Documentation