fix: API multipart spec flakiness #35927
Conversation
AmanAgarwal041
requested review from
ApekshaBhosale and
sagar-qa007
as code owners
github-actions
bot
added
cypress-flaky-fix
|
Warning Rate limit exceeded@AmanAgarwal041 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 8 minutes and 49 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe recent changes focus on enhancing API testing functionality by transitioning from the Changes
Assessment against linked issues
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 Documentation and Community
|
|
/ci-test-limit |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/10592758486. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (2)
app/client/cypress/support/Pages/ApiPage.ts (2)
Line range hint
69-101: Refactor to avoid direct XPath usage and ensure best practices.The method
CreateApiuses direct XPath selectors and potentially flaky commands likecy.wait. Consider refactoring to use data attributes for selectors and avoid usingcy.waitas it can lead to non-deterministic behavior. Here's a suggested refactor:- this.agHelper.GetNClick(this._blankAPI, 0, true); + this.agHelper.GetNClick(this.locator.blankAPI, 0, true); // Assume `locator.blankAPI` is defined with a data attributeThis change promotes the use of data attributes for selectors, which is more maintainable and less prone to errors. Additionally, review the use of
cy.waitand replace it with more deterministic wait mechanisms, such as waiting for specific network calls or UI states.
Line range hint
155-164: Replace non-deterministic sleep with more reliable wait mechanisms.The use of
agHelper.Sleep()in theEnterURLmethod can lead to non-deterministic test behavior. Consider replacing it with more reliable wait mechanisms, such as waiting for specific network responses or UI changes:- this.agHelper.Sleep(); //Is needed for the entered url value to be registered, else failing locally & CI + cy.wait('@networkCall'); // Replace with an appropriate alias for a network callThis change ensures that the test waits for a specific condition to be met, which is more reliable and maintainable than a fixed-duration sleep.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (4)
- app/client/cypress/e2e/Regression/ServerSide/ApiTests/API_MultiPart_Spec.ts (6 hunks)
- app/client/cypress/limited-tests.txt (1 hunks)
- app/client/cypress/support/Objects/DataManager.ts (2 hunks)
- app/client/cypress/support/Pages/ApiPage.ts (1 hunks)
Additional context used
Path-based instructions (4)
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/ServerSide/ApiTests/API_MultiPart_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.
app/client/cypress/support/Objects/DataManager.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.
app/client/cypress/support/Pages/ApiPage.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
Files selected for processing (4)
- app/client/cypress/e2e/Regression/ServerSide/ApiTests/API_MultiPart_Spec.ts (6 hunks)
- app/client/cypress/limited-tests.txt (1 hunks)
- app/client/cypress/support/Objects/DataManager.ts (2 hunks)
- app/client/cypress/support/Pages/ApiPage.ts (1 hunks)
Additional context used
Path-based instructions (4)
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/ServerSide/ApiTests/API_MultiPart_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.
app/client/cypress/support/Objects/DataManager.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.
app/client/cypress/support/Pages/ApiPage.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.
| @@ -303,7 +303,7 @@ export class ApiPage { | |||
| | "RAW", | |||
| ) { | |||
| this.agHelper.GetNClick(this._bodyTypeSelect); | |||
| cy.xpath(this._bodyTypeToSelect(subTabName)).should("be.visible").click(); | |||
| this.agHelper.GetNClick(this._bodyTypeToSelect(subTabName)); | |||
There was a problem hiding this comment.
Consider restoring visibility checks for robustness.
Dear student, it appears you've removed the visibility check before clicking on elements. While this simplifies the code, it might lead to issues where clicks fail because elements are not yet visible. In the world of testing, especially with asynchronous UI operations, it's prudent to ensure elements are visible and interactable before performing actions on them. This helps in making the tests more reliable and less flaky.
Consider the following adjustment to maintain the robustness of your test interactions:
- this.agHelper.GetNClick(this._bodyTypeToSelect(subTabName));
+ cy.xpath(this._bodyTypeToSelect(subTabName)).should('be.visible').click();This change ensures that the element is visible before we attempt to click on it, thus reducing the likelihood of encountering errors during test execution.
Committable suggestion was skipped due to low confidence.
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10592758486. |
|
/ci-test-limit runId=10592758486 |
5 similar comments
|
/ci-test-limit runId=10592758486 |
|
/ci-test-limit runId=10592758486 |
|
/ci-test-limit runId=10592758486 |
|
/ci-test-limit runId=10592758486 |
|
/ci-test-limit runId=10592758486 |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/10593209241. |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/10593209806. |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/10593209957. |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/10593208901. |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/10593211528. |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/10593207908. |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/10593211614. |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/10593212100. |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10593205143. |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10593207114. |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10593207132. |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10593209806. |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10593206548. |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10593206265. |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10593211614. |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10593208901. |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10593209957. |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10593206812. |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10593211528. |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10593208826. |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10593207908. |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10593212100. |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10593209241. |
Description
app/client/cypress/e2e/Regression/ServerSide/ApiTests/API_MultiPart_Spec.tsspec file.cloudinaryfor multipart upload testcloudinaryapi was expecting error if the array type was selected for multipart form data key instead of file type.cloudinaryapi to ted apiNonetype selection which goes out of the view port after the multipart form data is selected in the dropdown.Fixes #35137
or
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.Datasource, @tag.Binding, @tag.Visual"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10594951475
Commit: 65218a2
Cypress dashboard.
Tags:
@tag.Datasource, @tag.Binding, @tag.VisualSpec:
Wed, 28 Aug 2024 11:58:03 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Refactor