fix: flaky mongo spec test #36220
fix: flaky mongo spec test #36220
Conversation
AmanAgarwal041
requested review from
ApekshaBhosale and
sagar-qa007
as code owners
WalkthroughThis pull request introduces enhancements to the Cypress test suite for validating CRUD operations within a MongoDB context. Key modifications include improved dropdown item selection, a refactored row validation logic using a new helper function, and an updated test specification list that shifts focus from client-side to server-side tests. Additionally, a new method has been added to the Changes
Possibly related 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 using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
/ci-test-limit-count run_count=20 |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/10787240712. |
![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, codebase verification and nitpick comments (1)
app/client/cypress/e2e/Regression/ServerSide/GenerateCRUD/Mongo_Spec.ts (1)
44-47: Excellent enhancements to the CRUD validation process!The new click action on the dropdown to select the image option strengthens the CRUD validation coverage. And the refactored table validation logic using the
findTheDataRowhelper function is a great improvement in terms of robustness and adaptability.Just a minor suggestion to improve code readability:
Consider using more descriptive variable names for
col1Text,col2Text, andcol3Text. For example,imageUrl,pokemonId, andtypeswould make the code more self-explanatory.Also applies to: 116-143
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (3)
- app/client/cypress/e2e/Regression/ServerSide/GenerateCRUD/Mongo_Spec.ts (2 hunks)
- app/client/cypress/limited-tests.txt (1 hunks)
- app/client/cypress/support/Pages/Table.ts (1 hunks)
Additional context used
Path-based instructions (3)
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/GenerateCRUD/Mongo_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/Pages/Table.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 (3)
app/client/cypress/limited-tests.txt (1)
2-3: Great work shifting the focus to server-side tests!The changes align perfectly with the PR objective of fixing the flaky MongoDB spec test. Removing the client-side test and adding the relevant server-side test is a step in the right direction.
app/client/cypress/e2e/Regression/ServerSide/GenerateCRUD/Mongo_Spec.ts (1)
147-171: Great job abstracting the row-finding logic!The
findTheDataRowfunction is a valuable addition to the test suite. It encapsulates the logic for determining the row index based on the text in the first column, making the code more maintainable and less prone to errors.app/client/cypress/support/Pages/Table.ts (1)
62-70: Excellent addition to the Table class!The new
_tableColumnDataWithTextmethod is a valuable enhancement to theTableclass. It allows for more specific targeting of table column data based on both column index and text content, improving the flexibility and usability of the table manipulation features. Well done!
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10787240712. |
github-actions
bot
added
the
cypress-flaky-fix
github-actions
bot
added
Stability Pod
| }); | ||
|
|
||
| //Validating loaded JSON form | ||
| cy.xpath(locators._buttonByText("Update")).then((selector) => { |
There was a problem hiding this comment.
@sagar-qa007 @AmanAgarwal041 Is using an xpath a best practice in Cypress? Or do we have no other alternatives that could have been used here?
I'm asking this because the package cypress-xpath has been deprecated a year ago. Additionally, Cypress recommends using more specific ids to select elements rather than xpath.
There was a problem hiding this comment.
@mohanarpit Yeah we should ideally remove that and go for the css selectors. It was already there, but we can definitely update this.
cc @sagar-qa007
There was a problem hiding this comment.
Yes agree and noted. @mohanarpit
@AmanAgarwal041 It has present multiple places, can we update on that specs too? It will be good optimisation.
cy.xpath(locators._buttonByText("Update")).then((selector)
There was a problem hiding this comment.
@AmanAgarwal041 Is there a CSS selector already available for this validation today? If yes, can we change it right now? If not, then we'll leave it for the time being.
There was a problem hiding this comment.
@mohanarpit I am not really sure we have something like that, because we have a repetitive block in around 8 files. @sagar-qa007 Can we add a task to update this block at every place ?
List of files :
- app/client/cypress/e2e/Regression/ServerSide/GenerateCRUD/Mongo_Spec.ts
- app/client/cypress/e2e/Regression/ServerSide/GenerateCRUD/MongoURI_Spec.ts
- app/client/cypress/e2e/Regression/ServerSide/GenerateCRUD/MySQL1_Spec.ts
- app/client/cypress/e2e/Regression/ServerSide/GenerateCRUD/MySQL2_Spec.ts
- app/client/cypress/e2e/Regression/ServerSide/GenerateCRUD/Postgres1_Spec.ts
- app/client/cypress/e2e/Regression/ServerSide/GenerateCRUD/Postgres2_Spec.ts
- app/client/cypress/e2e/Regression/ServerSide/QueryPane/Mongo1_spec.ts
- app/client/cypress/e2e/Sanity/Datasources/MsSQL_Basic_Spec.ts
Description
RCA:
The test started acting flaky because of the ted mongo db had new entries which changed the index value of the record that we were trying to verify.
Solution:
We get the index value of the record by finding the record entry in a column and the traversing back to parent and getting index of the row item. This returned index is then used to match the record instead of just passing
0which will still work if the new entries are made in the db.Fixes #35149
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"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10789425505
Commit: dc720a1
Cypress dashboard.
Tags:
@tag.DatasourceSpec:
Tue, 10 Sep 2024 09:51:00 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Bug Fixes
Chores