Fixed the flaky test in user skill management cypress test #9148
Conversation
WalkthroughThe changes involve modifications to the Cypress end-to-end test suite for managing users and the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
cypress/pageobject/Hcx/HcxClaims.tsOops! Something went wrong! :( ESLint: 8.57.1 Error: Failed to load parser '@typescript-eslint/parser' declared in '.eslintrc.json': Cannot find module '@typescript-eslint/parser'
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 (
|
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Deploying care-fe with
|
| Latest commit: |
b8841fb
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://51f73ca1.care-fe.pages.dev |
| Branch Preview URL: | https://flaky-userskill-manage.care-fe.pages.dev |
CARE
|
||||||||||||||||||||||||||||
| Project |
CARE
|
| Branch Review |
flaky-userskill-manage
|
| Run status |
|
| Run duration | 04m 49s |
| Commit |
|
| Committer | Mohammed Nihal |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
126
|
| View all changes introduced in this branch ↗︎ | |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
cypress/e2e/users_spec/UsersManage.cy.ts (3)
Line range hint
42-63: Consider replacing arbitrary waits with notification checksThis test contains two instances of
cy.wait(5000)which can make the test flaky. Consider applying the same notification-based approach that was implemented in the "linking skills for a doctor users" test.Replace the wait commands with notification checks:
manageUserPage.clickAddSkillButton(); manageUserPage.clickCloseSlideOver(); - cy.wait(5000); + cy.verifyNotification("Skill added successfully"); + cy.closeNotification(); manageUserPage.clicklinkedskillbutton(); manageUserPage.assertSkillInAddedUserSkills(linkedskill); manageUserPage.clickCloseSlideOver(); - cy.wait(5000);
Line range hint
108-164: Consider breaking down the complex test into smaller focused testsWhile the test is well-structured, it's handling multiple scenarios in a single test case which can make maintenance and debugging more difficult. Consider splitting it into separate test cases:
- Verify no home facility scenario
- Test linking new facility
- Test home facility assignment
- Test facility unlinking
- Test doctor connect reflections
This would improve:
- Test maintainability
- Debugging experience
- Test failure isolation
Remaining arbitrary wait commands need to be replaced with proper assertions
The verification reveals multiple instances of arbitrary wait commands across the test suite that could lead to flaky tests:
- Several files use arbitrary wait times (2000-5000ms):
cypress/e2e/users_spec/UsersManage.cy.ts: Two instances ofcy.wait(5000)cypress/e2e/patient_spec/PatientRegistration.cy.ts: Multiple waits between 2000-5000mscypress/e2e/patient_spec/PatientConsultationDischarge.cy.ts: Multiplecy.wait(2000)- And more...
While some files correctly use route aliases for waiting (e.g.,
@loadFacilities,@shifting_download), there are many arbitrary waits that should be replaced with proper assertions or route waiting to make tests more reliable.🔗 Analysis chain
Line range hint
1-164: Verify all wait commands in the test suiteWhile the PR addresses some wait commands, let's ensure we haven't missed any other potential flaky wait patterns in the test suite.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for remaining cy.wait commands in the test files echo "Searching for remaining cy.wait commands in test files..." rg "cy\.wait\(" cypress/e2e/Length of output: 2548
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
cypress/e2e/users_spec/UsersManage.cy.ts(1 hunks)
🔇 Additional comments (1)
cypress/e2e/users_spec/UsersManage.cy.ts (1)
Line range hint 89-106: LGTM! Well-structured test with proper assertions
This test follows good practices by:
- Verifying error messages for invalid inputs
- Using proper assertions for validation
- Not relying on arbitrary wait times
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
cypress/pageobject/Hcx/HcxClaims.ts (1)
3-5: Consider extracting timeout to a configuration constant.While the 10-second timeout is reasonable, consider extracting it to a shared configuration constant for better maintainability. This would allow easier adjustment of timeouts across all tests.
+// At the top of the file +const DEFAULT_TIMEOUT = 10000; selectEligiblePolicy(policy: string) { - cy.get("#select-insurance-policy", { timeout: 10000 }) + cy.get("#select-insurance-policy", { timeout: DEFAULT_TIMEOUT }) .should("be.visible") .and("not.be.disabled");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
cypress/pageobject/Hcx/HcxClaims.ts(1 hunks)
🔇 Additional comments (1)
cypress/pageobject/Hcx/HcxClaims.ts (1)
3-5: LGTM! Good improvement in test reliability.
The added assertions properly handle element visibility and enabled state before interaction, which helps prevent flaky tests. The timeout setting and chained assertions follow Cypress best practices.
…k#9148) Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
Bug Fixes
New Features
Tests