Cypress tests replace cy.wait with cy.intercept or dynamic waits #8963
Conversation
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
Have you committed all the changes? 🤔 I'm not seeing any added cy.intercept calls in the changeset. |
|
@Jacobjeevan I reviewed all the Cypress test cases, and it looks like the necessary cy.intercept() calls were already in place. I went ahead and removed the cy.wait() calls where they were unnecessary and added two dynamic waits in specific files to ensure smoother test execution. If I misunderstood anything or missed any cy.intercept() implementations, please let me know. |
Jacobjeevan
added
needs testing
needs review
and removed
question
| @@ -43,7 +45,6 @@ describe("Inventory Management Section", () => { | |||
| // verify the last entry deletion | |||
| facilityPage.verifyStockInRow("#row-0", "Added Stock"); | |||
| facilityPage.verifyStockInRow("#row-1", "Used Stock"); | |||
| cy.wait(3000); | |||
There was a problem hiding this comment.
the intercept and API Verification is missing here
| @@ -59,7 +60,6 @@ describe("Inventory Management Section", () => { | |||
| facilityPage.verifyBadgeWithText(".badge-danger", "Low Stock"); | |||
| // modify with manual minimum badge | |||
| facilityPage.clickAddMinimumQuanitity(); | |||
| cy.wait(3000); | |||
There was a problem hiding this comment.
the intercept and API Verification is missing here
.env
Outdated
| @@ -8,6 +8,7 @@ REACT_PUBLIC_URL=https://care.ohc.network | |||
|
|
|||
| # Care API URL without the /api prefix | |||
| REACT_CARE_API_URL=https://careapi.ohc.network | |||
| # REACT_CARE_API_URL=http://localhost:9000 | |||
There was a problem hiding this comment.
- According to the issue, you should replace cy.wait with cy.intercept() to verify that the relevant APIs are being fetched. However, in your PR, you only removed cy.wait without adding any cy.intercept to the code. Please make the necessary changes.
- Make sure that post your changes, cypress action also passes fully
nihal467
added
test failed
changes required
and removed
needs testing
needs review
labels
|
@Rishith25 The cy.wait command was initially added in the test to prevent flakiness. The issue was created to replace hardcorded wait with cy.intercept, ensuring that tests dynamically wait for the necessary API to load and verify that the page is fully loaded before executing the next step in the test. |
|
👋 Hi, @Rishith25, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
WalkthroughThe pull request includes modifications to multiple Cypress test files, primarily focusing on the removal of hardcoded wait commands ( Changes
Assessment against linked issues
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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
CodeRabbit Configuration File (
|
![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 (7)
cypress/pageobject/Patient/PatientPrescription.ts (1)
64-64: LGTM! Consider usingverifyAndClickElementfor consistency.The change from hardcoded wait to visibility check is a good improvement that aligns with Cypress best practices and reduces test flakiness. For consistency with other methods in this class (e.g.,
clickAddPrescription), consider using the helper methodverifyAndClickElement.- cy.get("#discontinuedReason").should("be.visible").type(reason); + cy.verifyAndClickElement("#discontinuedReason", reason);cypress/pageobject/Patient/PatientFileupload.ts (1)
23-24: Great improvement using dynamic waits!The replacement of hardcoded waits with dynamic element state checks is a solid improvement that aligns with Cypress best practices. This change will make the tests more reliable by ensuring actions are only performed when elements are actually ready.
Consider adding custom timeout and error messages for better debugging:
- cy.get("#stop-recording").should("be.enabled").click(); - cy.get("#save-recording").should("be.enabled").click(); + cy.get("#stop-recording", { timeout: 10000 }) + .should("be.enabled", { message: "Stop recording button should be enabled" }) + .click(); + cy.get("#save-recording", { timeout: 10000 }) + .should("be.enabled", { message: "Save recording button should be enabled" }) + .click();cypress/e2e/facility_spec/FacilityCreation.cy.ts (5)
35-35: Consider using dynamic date generation instead of hardcoded date.Replace the hardcoded date with a dynamic date generation to prevent future maintenance issues and make tests more robust.
- const triageDate = "03122023"; + const triageDate = Cypress.dayjs().format('DDMMYYYY');
Line range hint
82-127: Add API interceptions for facility triage operations.The test performs multiple API operations without intercepting the requests. To improve reliability and align with the PR objectives of replacing hardcoded waits, consider adding
cy.intercept()for the following operations:
- Facility search API
- Triage form submission
- Triage data retrieval
- Triage edit operations
Example implementation:
// Before the test cy.intercept('GET', '/api/v1/facility/search*').as('facilitySearch'); cy.intercept('POST', '/api/v1/facility/*/triage').as('createTriage'); cy.intercept('GET', '/api/v1/facility/*/triage').as('getTriage'); cy.intercept('PUT', '/api/v1/facility/*/triage/*').as('updateTriage'); // After operations manageUserPage.typeFacilitySearch(facilityName2); cy.wait('@facilitySearch'); // After form submission manageUserPage.clickSubmit(); cy.wait('@createTriage'); // Before verifying table data cy.wait('@getTriage'); facilityPage.verifyTriageTableContains(initialTriageValue);
Line range hint
129-223: Improve test reliability with API interceptions and dynamic notification handling.The test performs facility creation with multiple API calls but lacks proper request interception. Also,
cy.closeNotification()might be timing-dependent.
- Add API interceptions:
// Before the test cy.intercept('POST', '/api/v1/facility').as('createFacility'); cy.intercept('POST', '/api/v1/facility/*/bed').as('addBedCapacity'); cy.intercept('POST', '/api/v1/facility/*/doctor').as('addDoctorCapacity'); cy.intercept('DELETE', '/api/v1/facility/*').as('deleteFacility'); // After form submissions facilityPage.submitForm(); cy.wait('@createFacility'); facilityPage.clickbedcapcityaddmore(); cy.wait('@addBedCapacity');
- Replace notification handling with a more reliable approach:
// Custom command in commands.ts Cypress.Commands.add('waitForNotification', (message: string) => { cy.get('.notification-message') .should('be.visible') .and('contain', message); cy.get('.notification-close') .should('be.visible') .click(); }); // In test cy.waitForNotification('Bed capacity added successfully');
Line range hint
225-266: Add API interceptions and improve URL verification.The test lacks proper request interception and the URL verification could be flaky.
// Before the test cy.intercept('POST', '/api/v1/facility').as('createFacility'); cy.intercept('POST', '/api/v1/facility/*/bed').as('addBedCapacity'); cy.intercept('POST', '/api/v1/facility/*/doctor').as('addDoctorCapacity'); cy.intercept('GET', '/api/v1/facility/search*').as('facilitySearch'); // After form submissions facilityPage.submitForm(); cy.wait('@createFacility').then((interception) => { const facilityId = interception.response.body.id; // Verify URL contains the new facility ID cy.url().should('include', `/facility/${facilityId}`); }); // After search manageUserPage.typeFacilitySearch(facilityName); cy.wait('@facilitySearch');
Line range hint
268-397: Add API interceptions for remaining facility operations.The remaining tests perform various facility operations without proper request interception. This could lead to flaky tests.
Add these interceptions:
// Facility operations cy.intercept('POST', '/api/v1/facility').as('createFacility'); cy.intercept('PUT', '/api/v1/facility/*').as('updateFacility'); cy.intercept('PUT', '/api/v1/facility/*/middleware').as('updateMiddleware'); // After facility update facilityPage.submitForm(); cy.wait('@updateFacility'); // After middleware update facilityPage.clickupdateMiddleWare(); cy.wait('@updateMiddleware');Consider creating a custom command for common facility operations to reduce code duplication:
// commands.ts Cypress.Commands.add('setupFacilityInterceptions', () => { cy.intercept('POST', '/api/v1/facility').as('createFacility'); cy.intercept('PUT', '/api/v1/facility/*').as('updateFacility'); cy.intercept('DELETE', '/api/v1/facility/*').as('deleteFacility'); // ... other common interceptions }); // In beforeEach beforeEach(() => { cy.viewport(1280, 720); cy.restoreLocalStorage(); cy.setupFacilityInterceptions(); cy.awaitUrl("/facility"); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (20)
.env(1 hunks)cypress/e2e/assets_spec/AssetHomepage.cy.ts(0 hunks)cypress/e2e/facility_spec/FacilityCreation.cy.ts(1 hunks)cypress/e2e/facility_spec/FacilityInventory.cy.ts(1 hunks)cypress/e2e/patient_spec/PatientConsultationCreation.cy.ts(0 hunks)cypress/e2e/patient_spec/PatientConsultationDischarge.cy.ts(0 hunks)cypress/e2e/patient_spec/PatientLogUpdate.cy.ts(0 hunks)cypress/e2e/patient_spec/PatientPrescription.cy.ts(0 hunks)cypress/e2e/patient_spec/PatientRegistration.cy.ts(0 hunks)cypress/e2e/users_spec/UsersCreation.cy.ts(0 hunks)cypress/e2e/users_spec/UsersManage.cy.ts(0 hunks)cypress/pageobject/Facility/FacilityCreation.ts(0 hunks)cypress/pageobject/Patient/PatientConsultation.ts(0 hunks)cypress/pageobject/Patient/PatientCreation.ts(0 hunks)cypress/pageobject/Patient/PatientDoctorNotes.ts(0 hunks)cypress/pageobject/Patient/PatientFileupload.ts(1 hunks)cypress/pageobject/Patient/PatientLogupdate.ts(0 hunks)cypress/pageobject/Patient/PatientPrescription.ts(1 hunks)cypress/pageobject/Patient/PatientTransfer.ts(0 hunks)cypress/pageobject/Users/ManageUserPage.ts(1 hunks)
💤 Files with no reviewable changes (14)
- cypress/e2e/assets_spec/AssetHomepage.cy.ts
- cypress/e2e/patient_spec/PatientConsultationCreation.cy.ts
- cypress/e2e/patient_spec/PatientConsultationDischarge.cy.ts
- cypress/e2e/patient_spec/PatientLogUpdate.cy.ts
- cypress/e2e/patient_spec/PatientPrescription.cy.ts
- cypress/e2e/patient_spec/PatientRegistration.cy.ts
- cypress/e2e/users_spec/UsersCreation.cy.ts
- cypress/e2e/users_spec/UsersManage.cy.ts
- cypress/pageobject/Facility/FacilityCreation.ts
- cypress/pageobject/Patient/PatientConsultation.ts
- cypress/pageobject/Patient/PatientCreation.ts
- cypress/pageobject/Patient/PatientDoctorNotes.ts
- cypress/pageobject/Patient/PatientLogupdate.ts
- cypress/pageobject/Patient/PatientTransfer.ts
✅ Files skipped from review due to trivial changes (1)
- .env
🔇 Additional comments (4)
cypress/pageobject/Patient/PatientFileupload.ts (1)
Line range hint 1-116: Excellent use of cy.intercept throughout the file!
The file demonstrates proper handling of API requests using cy.intercept and appropriate status code assertions. This is exactly the pattern we want to maintain for reliable tests.
cypress/e2e/facility_spec/FacilityInventory.cy.ts (1)
36-38:
Add cy.intercept for API calls to prevent test flakiness.
While the success message update is appropriate, the test is missing proper API request handling. Each inventory operation (create, modify, delete) should wait for its corresponding API response.
Example implementation:
// Before the test
cy.intercept('POST', '**/api/inventory/use_stock').as('updateStock')
cy.intercept('POST', '**/api/inventory/create').as('createInventory')
// In the test
facilityPage.clickAddInventory();
cy.wait('@createInventory')
facilityPage.verifySuccessNotification("Inventory created successfully");
facilityPage.clickAddInventory();
cy.wait('@updateStock')
facilityPage.verifySuccessNotification("Inventory use stock updated successfully");cypress/pageobject/Users/ManageUserPage.ts (2)
44-44: Good improvement in click reliability!
The addition of .should("be.visible") check before clicking is a good practice. It replaces the forced click with a dynamic wait that ensures the element is actually interactable, making the test more reliable.
Line range hint 1-190: Consider standardizing API interception patterns across methods
Some methods like navigateToProfile and clickAddSkillButton properly use cy.intercept to wait for API responses, while others don't. Consider standardizing this pattern across all methods that trigger API calls to ensure consistent test reliability.
For example, methods like clickLinkFacility, clickUnlinkFacilityButton, and clickUnlinkSkill likely trigger API calls and could benefit from similar interception patterns.
Let's verify which methods might need API interceptions:
|
👋 Hi, @Rishith25, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
|
@Rishith25 what is the status on this PR |
1 similar comment
|
@Rishith25 what is the status on this PR |
|
@nihal467 I am having exams I will try to work from Saturday and update the PR |
|
@Rishith25 what's the update |
|
I am working on it |
…ps://github.com/Rishith25/care_fe into cypress-tests-replace-cy.wait-with-cy.intercept
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
cypress/pageobject/Facility/FacilityCreation.ts (4)
Line range hint
289-291: Ensure consistent naming conventions in methodfillMiddleWareAddressThe method name
fillMiddleWareAddresshas inconsistent casing. Consider renaming it tofillMiddlewareAddressto adhere to camelCase naming conventions. The term 'middleware' is typically written as a single word with a lowercase 'w'.
Line range hint
293-298: Correct method nameclickupdateMiddleWareto follow camelCase conventionConsider renaming the method to
clickUpdateMiddlewareto adhere to camelCase naming conventions. This improves code readability and maintains consistency.
Line range hint
475-484: Replace static wait with dynamic wait inselectStateOnPincodemethodThe use of
wait(2000)introduces a fixed delay, which can cause flaky tests. Replace it with a dynamic wait that ensures the element is ready before proceeding.Apply this diff to remove the static wait:
selectStateOnPincode(stateName: string) { this.getStateElement() .scrollIntoView() - .wait(2000) .should("be.visible") .then(($element) => { const text = $element.text(); if (!text.includes(stateName)) { this.getStateElement().click(); cy.get("li[role=option]").contains(stateName).click(); } }); }
Line range hint
486-495: Eliminate fixed wait inselectDistrictOnPincodemethodUsing
wait(2000)introduces unnecessary delay and potential flakiness. Replace it with a dynamic wait to ensure the element is ready.Update the code as follows:
selectDistrictOnPincode(districtName: string) { this.getDistrictElement() .scrollIntoView() - .wait(2000) .should("be.visible") .then(($element) => { const text = $element.text(); if (!text.includes(districtName)) { this.getDistrictElement().click(); cy.get("li[role=option]").contains(districtName).click(); } }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (18)
cypress/e2e/assets_spec/AssetHomepage.cy.ts(0 hunks)cypress/e2e/facility_spec/FacilityCreation.cy.ts(1 hunks)cypress/e2e/facility_spec/FacilityHomepage.cy.ts(0 hunks)cypress/e2e/facility_spec/FacilityInventory.cy.ts(2 hunks)cypress/e2e/patient_spec/PatientConsultationCreation.cy.ts(0 hunks)cypress/e2e/patient_spec/PatientConsultationDischarge.cy.ts(0 hunks)cypress/e2e/patient_spec/PatientLogUpdate.cy.ts(0 hunks)cypress/e2e/patient_spec/PatientPrescription.cy.ts(0 hunks)cypress/e2e/patient_spec/PatientRegistration.cy.ts(0 hunks)cypress/e2e/users_spec/UsersCreation.cy.ts(0 hunks)cypress/e2e/users_spec/UsersManage.cy.ts(0 hunks)cypress/pageobject/Facility/FacilityCreation.ts(2 hunks)cypress/pageobject/Patient/PatientConsultation.ts(0 hunks)cypress/pageobject/Patient/PatientCreation.ts(0 hunks)cypress/pageobject/Patient/PatientDoctorNotes.ts(1 hunks)cypress/pageobject/Patient/PatientFileupload.ts(2 hunks)cypress/pageobject/Patient/PatientLogupdate.ts(0 hunks)cypress/pageobject/Users/ManageUserPage.ts(1 hunks)
💤 Files with no reviewable changes (12)
- cypress/e2e/assets_spec/AssetHomepage.cy.ts
- cypress/e2e/facility_spec/FacilityHomepage.cy.ts
- cypress/e2e/patient_spec/PatientConsultationCreation.cy.ts
- cypress/e2e/patient_spec/PatientConsultationDischarge.cy.ts
- cypress/e2e/patient_spec/PatientLogUpdate.cy.ts
- cypress/e2e/patient_spec/PatientPrescription.cy.ts
- cypress/e2e/patient_spec/PatientRegistration.cy.ts
- cypress/e2e/users_spec/UsersCreation.cy.ts
- cypress/e2e/users_spec/UsersManage.cy.ts
- cypress/pageobject/Patient/PatientConsultation.ts
- cypress/pageobject/Patient/PatientCreation.ts
- cypress/pageobject/Patient/PatientLogupdate.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- cypress/e2e/facility_spec/FacilityCreation.cy.ts
- cypress/e2e/facility_spec/FacilityInventory.cy.ts
- cypress/pageobject/Patient/PatientDoctorNotes.ts
- cypress/pageobject/Patient/PatientFileupload.ts
- cypress/pageobject/Users/ManageUserPage.ts
🔇 Additional comments (1)
cypress/pageobject/Facility/FacilityCreation.ts (1)
287-287: LGTM!
The added line correctly closes the map after ensuring the close button is visible, improving test reliability.
| cy.get("div#id").click(); | ||
| cy.get("div#id ul li").contains(name).click(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid using generic IDs like id in selectors
Using div#id as a selector might lead to unintended behavior if multiple elements share the generic ID id. Consider using a more specific and unique ID to target the desired element.
Proposed Changes
Fixes #8925
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Documentation