fix: Updated fix for embedSettings spec #38195
Conversation
WalkthroughThe pull request introduces modifications to the Cypress end-to-end test for embed settings, primarily focusing on improving clipboard functionality testing. The changes involve replacing the clipboard text reading method with a stub that mocks the Changes
Sequence DiagramsequenceDiagram
participant Test as Cypress Test
participant App as Application
participant Clipboard as Clipboard Stub
Test->>App: Navigate to Embed Settings
Test->>App: Modify Embed Settings
Test->>Clipboard: Mock writeText Method
Test->>App: Verify Iframe Submission
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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
|
|
/ci-test-limit-count run_count=50 |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/12365551194. |
![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 comments (3)
app/client/cypress/e2e/Regression/ClientSide/EmbedSettings/EmbedSettings_spec.js (3)
Line range hint
19-24: Remove cy.wait and agHelper.Sleep callsThe use of explicit waits and sleep calls violates the coding guidelines. Replace them with proper Cypress wait strategies.
Consider using these alternatives:
- _.agHelper.Sleep(5000) + cy.get('.t--widget-iframewidget iframe').should('be.visible') - cy.wait(8000) + cy.get('.t--widget-iframewidget iframe').should('have.attr', 'src').and('not.be.empty') - _.agHelper.Sleep(2000) + cy.get('.t--admin-settings-APPSMITH_ALLOWED_FRAME_ANCESTORS').should('be.visible')Also applies to: 54-55, 82-82, 102-102, 122-122
Line range hint
28-32: Avoid hardcoded URLs in testsUsing hardcoded URLs makes tests brittle and environment-dependent.
Consider:
- Using environment variables
- Generating test data dynamically
- Using mock data
- "https://app.appsmith.com/applications/6752ba5904a5f464099437ec/pages/6752ba5904a5f464099437f3" + Cypress.env('TEST_IFRAME_URL')
Line range hint
63-142: Refactor test structureThe current implementation combines multiple test cases into a single "it" block. This makes it harder to debug failures and understand test results.
Consider splitting into separate test cases:
it("1. Should limit embedding", function () { // Test limiting embedding }); it("2. Should allow embedding everywhere", function () { // Test allowing embedding }); it("3. Should disable embedding everywhere", function () { // Test disabling embedding });
🧹 Nitpick comments (1)
app/client/cypress/e2e/Regression/ClientSide/EmbedSettings/EmbedSettings_spec.js (1)
Line range hint
4-4: Add more specific test tagsThe current tag
@tag.Settingsis too generic. Consider adding more specific tags for better test organization.Example:
- { tags: ["@tag.Settings"] } + { tags: ["@tag.Settings", "@tag.EmbedSettings", "@tag.Regression"] }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/client/cypress/e2e/Regression/ClientSide/EmbedSettings/EmbedSettings_spec.js(1 hunks)app/client/cypress/limited-tests.txt(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
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.
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.
🔇 Additional comments (2)
app/client/cypress/limited-tests.txt (1)
2-2: LGTM!
The test specification update aligns with the PR objective.
app/client/cypress/e2e/Regression/ClientSide/EmbedSettings/EmbedSettings_spec.js (1)
51-53: LGTM: Clipboard stub implementation
The clipboard stub implementation is correct and follows Cypress best practices.
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/12365551194. |
sagar-qa007
changed the title
Description
Updated new code for the coping url from the clipboard in cypress as older code has been deprecated.
Fixes #
Issue Numberor
Fixes
Issue URLWarning
If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.
Automation
/ok-to-test tags="@tag.Settings"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/12368303561
Commit: 049ddf4
Cypress dashboard.
Tags:
@tag.SettingsSpec:
Tue, 17 Dec 2024 07:49:08 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
Bug Fixes
New Features