-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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.
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
[Donot merge]: testing cypress flakiness #35874
Conversation
WalkthroughThe changes involve significant updates to the Cypress testing framework, transitioning from client-side tests to server-side API and CRUD operations. The Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Poem
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
|
/ci-test-limit |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/10555138082. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
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.
Additional comments not posted (4)
app/client/cypress/limited-tests.txt (4)
2-2
: LGTM!The test specification for API multipart handling is correctly added.
The code changes are approved.
3-3
: LGTM!The test specification for MongoDB CRUD operations is correctly added.
The code changes are approved.
4-4
: LGTM!The test specification for MongoDB URI CRUD operations is correctly added.
The code changes are approved.
5-5
: LGTM!The test specification for MySQL CRUD operations is correctly added.
The code changes are approved.
/ci-test-limit |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/10556501671. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- app/client/cypress/support/Pages/ApiPage.ts (1 hunks)
Additional context used
Path-based instructions (1)
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reintroduce visibility assertion or ensure element is interactable
The visibility assertion has been removed before clicking the element. This might lead to runtime errors if the element is not visible at the time of the click. Consider reintroducing the visibility assertion or ensuring the element is interactable before clicking it.
- this.agHelper.GetNClick(this._bodyTypeToSelect(subTabName));
+ cy.xpath(this._bodyTypeToSelect(subTabName)).should("be.visible");
+ this.agHelper.GetNClick(this._bodyTypeToSelect(subTabName));
Committable suggestion was skipped due to low confidence.
/ci-test-limit |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/10575233725. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- app/client/cypress/e2e/Regression/ServerSide/GenerateCRUD/MySQL2_Spec.ts (1 hunks)
Additional context used
Path-based instructions (1)
app/client/cypress/e2e/Regression/ServerSide/GenerateCRUD/MySQL2_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.
Additional comments not posted (4)
app/client/cypress/e2e/Regression/ServerSide/GenerateCRUD/MySQL2_Spec.ts (4)
132-132
: Verify the correctness of the new row index.Ensure that the new row index
12
aligns with the intended data structure and that the correct data row is being selected.
133-133
: Verify the correctness of the new row index.Ensure that the new row index
12
aligns with the intended data structure and that the correct data row is being asserted.
134-134
: Verify the correctness of the new row index.Ensure that the new row index
12
aligns with the intended data structure and that the correct data row is being used to generate store secret information.
138-138
: Verify the correctness of the new row index.Ensure that the new row index
12
aligns with the intended data structure and that the correct data row is being updated and verified.
/ci-test-limit |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/10576395138. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- app/client/cypress/e2e/Regression/ServerSide/GenerateCRUD/MySQL2_Spec.ts (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- app/client/cypress/e2e/Regression/ServerSide/GenerateCRUD/MySQL2_Spec.ts
/ci-test-limit |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/10587267939. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (2)
- app/client/cypress/e2e/Regression/ServerSide/ApiTests/API_MultiPart_Spec.ts (6 hunks)
- app/client/cypress/support/Objects/DataManager.ts (2 hunks)
Additional context used
Path-based instructions (2)
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.
Additional comments not posted (8)
app/client/cypress/e2e/Regression/ServerSide/ApiTests/API_MultiPart_Spec.ts (6)
5-5
: Good job adding the import!The
dataManager
import is necessary for accessing themultipartAPI
property.
132-133
: Great use of dynamic URL!Using a dynamic URL from
dataManager
enhances flexibility and allows for different environments or configurations.
149-149
: Good update on the success message!Updating the success message to reflect the new API name ensures consistency in feedback during tests.
167-167
: Nice update on the property field value!Updating the property field value for the image upload to reference
MultipartAPI.data.url
aligns the test with the new API structure.
171-171
: Good consistency in naming!Ensuring that the API entity selection references
MultipartAPI
maintains consistency with the new naming convention.
219-219
: Good update on the error message!Updating the error message to reflect the new API name ensures consistency in feedback during tests.
app/client/cypress/support/Objects/DataManager.ts (2)
83-84
: Great addition of multipartAPI property!Adding the
multipartAPI
property to theProduction
environment enhances the functionality of theDataManager
class by providing an endpoint specifically for multipart API interactions.
184-185
: Great addition of multipartAPI property!Adding the
multipartAPI
property to theStaging
environment enhances the functionality of theDataManager
class by providing an endpoint specifically for multipart API interactions.
/ci-test-limit |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/10593192904. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (2)
- app/client/cypress/e2e/Regression/ServerSide/GenerateCRUD/MySQL2_Spec.ts (2 hunks)
- app/client/cypress/limited-tests.txt (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- app/client/cypress/e2e/Regression/ServerSide/GenerateCRUD/MySQL2_Spec.ts
- app/client/cypress/limited-tests.txt
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10593192904. |
/ci-test-limit runId=10593192904 |
/ci-test-limit-count run_count=5 |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/10786776558. |
/ci-test-limit-count run_count=5 runId=10786510519 |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/10786791487. |
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10786791487.
|
/ci-test-limit |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/10790251410. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
app/client/cypress/e2e/Regression/ServerSide/GenerateCRUD/MongoURI_Spec.ts (1)
144-155
: Great job refactoring the network status assertions! This change improves code clarity and aligns with Cypress best practices.A few notes:
- Consolidating the assertions into a single
cy.wait
block that handles multiple interceptions enhances readability and efficiency. Well done!- Extracting the response statuses and asserting them using
expect
is a cleaner approach compared to the previousassertHelper
method.- Consider removing the commented-out
assertHelper.AssertNetworkStatus
lines to keep the code clean, unless you plan to use them later for reference.To remove the unused lines of code, apply this diff:
- // assertHelper.AssertNetworkStatus("@postExecute", 200); - // assertHelper.AssertNetworkStatus("@postExecute", 200);
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (2)
- app/client/cypress/e2e/Regression/ServerSide/GenerateCRUD/MongoURI_Spec.ts (1 hunks)
- app/client/cypress/limited-tests.txt (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- app/client/cypress/limited-tests.txt
Additional context used
Path-based instructions (1)
app/client/cypress/e2e/Regression/ServerSide/GenerateCRUD/MongoURI_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.
/ci-test-limit |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/10791048104. |
/ci-test-limit runId=10791048104 |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/10792637833. |
/ci-test-limit runId=10791048104 |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/10805752868. |
/ci-test-limit |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/10806018715. |
/ci-test-limit runId=10806018715 |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/10808322875. |
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10808322875. |
All the flakiness prs are merged. Hence closing this one. |
Description
This PR fixes the
cypress/e2e/Regression/ServerSide/GenerateCRUD/MySQL2_Spec.ts
spec flakiness file.Fixes #35150
or
Fixes
Issue URL
Warning
If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.
Automation
/ok-to-test tags=""
🔍 Cypress test results
Warning
Invalid tags. Please use
/ok-to-test tags="@tag.All"
or/test all
in the PR body to run all tests.Tags documentation.
List of valid tags.
Wed, 11 Sep 2024 08:52:39 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
Summary by CodeRabbit