Fixed Flaky Test in User skill management #9092
Conversation
WalkthroughThe changes in the pull request focus on the Changes
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/e2e/users_spec/UsersManage.cy.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'
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 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
CodeRabbit Configuration File (
|
❌ Deploy Preview for care-ohc failed.
|
Deploying care-fe with
|
| Latest commit: |
15daee9
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://2c435d40.care-fe.pages.dev |
| Branch Preview URL: | https://flaky-userskill.care-fe.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
cypress/e2e/users_spec/UsersManage.cy.ts (1)
10-15: LGTM! Consider using an interface for test data.The variable renaming to camelCase improves readability. However, consider grouping these test data variables into an interface for better maintainability.
interface TestUser { username: string; firstName: string; lastName: string; } const userSkill: TestUser = { username: "Dev Doctor", firstName: "Dev", lastName: "Doctor" };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
cypress/e2e/users_spec/UsersManage.cy.ts(5 hunks)
🔇 Additional comments (1)
cypress/e2e/users_spec/UsersManage.cy.ts (1)
58-62: Consider adding uniqueness checks for user filtering.
While splitting the search into first and last name is more granular, it could make the test flaky if multiple users share the same name components. Consider:
- Adding assertions to verify the exact number of filtered results
- Adding more specific filters if needed (e.g., email, role)
Let's check if there are other tests that might be affected by similar filtering:
| userPage.typeInSearchInput(usernameToLinkFacilitydoc1); | ||
| userPage.checkUsernameText(usernameToLinkFacilitydoc1); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Reduce code duplication in search-verify operations.
The search and verify operations are repeated multiple times with similar patterns. Consider extracting this into a helper method:
const searchAndVerifyUser = (username: string) => {
userPage.clearSearchInput();
userPage.typeInSearchInput(username);
userPage.checkUsernameText(username);
};Then use it like:
searchAndVerifyUser(usernameToLinkFacilitydoc1);Also applies to: 115-116, 126-127, 131-132
| userPage.typeInFirstName(firstNameUserSkill); | ||
| userPage.typeInLastName(lastNameUserSkill); | ||
| userPage.applyFilter(); | ||
| userPage.checkUsernameText(usernameToLinkSkill); | ||
| manageUserPage.clicklinkedskillbutton(); | ||
| manageUserPage.selectSkillFromDropdown(linkedskill); | ||
| manageUserPage.clickAddSkillButton(); |
There was a problem hiding this comment.
Replace hard-coded wait with proper assertions.
The comment "temporary hack to fix the failure" suggests this is a source of test flakiness. Instead of using cy.wait(5000), consider:
- Using Cypress's built-in retry-ability with proper assertions
- Waiting for specific network requests to complete
- Checking for specific UI states
Example fix:
// Instead of cy.wait(5000)
cy.intercept('GET', '**/api/skills').as('getSkills');
manageUserPage.clickCloseSlideOver();
cy.wait('@getSkills');
cy.get('[data-testid="skills-list"]').should('exist');
CARE
|
||||||||||||||||||||||||||||
| Project |
CARE
|
| Branch Review |
flaky-userskill
|
| Run status |
|
| Run duration | 05m 00s |
| Commit |
|
| Committer | Mohammed Nihal |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
127
|
| View all changes introduced in this branch ↗︎ | |
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
Bug Fixes
Refactor