-
Notifications
You must be signed in to change notification settings - Fork 522
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
New Cypress Test for User Creation #9986
Conversation
Caution Review failedThe pull request is closed. WalkthroughThis pull request introduces comprehensive changes across the Cypress testing infrastructure, focusing on documentation, test patterns, and code structure. The modifications span multiple files, including documentation, test specifications, page objects, and utility functions. Key changes involve standardizing data attributes, refactoring test data generation, simplifying Cypress commands, and enhancing test suite organization. The overall goal appears to be improving test maintainability, readability, and consistency across the testing framework. Changes
Possibly related PRs
Suggested Labels
Suggested Reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
Finishing Touches
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 Cloudflare Pages
|
CARE Run #4329
Run Properties:
|
Project |
CARE
|
Branch Review |
add-newuser-cypress
|
Run status |
Passed #4329
|
Run duration | 02m 15s |
Commit |
cca8491261: New Cypress Test for User Creation
|
Committer | Mohammed Nihal |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
0
|
Pending |
0
|
Skipped |
0
|
Passing |
5
|
View all changes introduced in this branch ↗︎ |
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: 6
🔭 Outside diff range comments (1)
cypress/e2e/patient_spec/patient_creation.cy.ts (1)
Line range hint
41-64
: Add error case tests for patient creation.The test suite only covers the happy path. Consider adding tests for validation errors and API failures.
Would you like me to generate additional test cases for error scenarios?
🧹 Nitpick comments (9)
cypress/e2e/users_spec/user_creation.cy.ts (1)
25-39
: Consider extracting test data to a separate utility.The test data object could be moved to a utility function similar to how facility data is managed, promoting reusability and consistency across user creation tests.
// utils/userData.ts export interface UserTestData { firstName: string; lastName: string; username: string; password: string; confirmPassword: string; email: string; phoneNumber: string; dateOfBirth: string; userType: string; state: string; district: string; localBody: string; ward: string; } export function generateUserData(firstName: string, lastName: string): UserTestData { return { firstName, lastName, username: generateUsername(firstName), password: "Test@123", confirmPassword: "Test@123", email: `${generateUsername(firstName)}@test.com`, phoneNumber: generatePhoneNumber(), dateOfBirth: "1990-01-01", userType: "Doctor", state: "Kerala", district: "Ernakulam", localBody: "Aluva", ward: "4", }; }cypress/support/commands.ts (1)
144-153
: Consider handling async loading of options.While the implementation is cleaner, it might fail if options are loaded asynchronously. Consider adding a timeout or retry mechanism.
cy.get('[role="listbox"]') + .should('exist') .find('[role="option"]') + .should('not.be.empty') .contains(reference) .should("be.visible") .click();cypress/pageObject/facility/FacilityCreation.ts (1)
102-113
: Optimize typing delay and API wait strategy.The current implementation might be unnecessarily slow and potentially flaky:
- 500ms delay between each character is quite long
- Waiting for API only after the last character might miss intermediate responses
- delay: 500, + delay: 100, force: true, }); - // Wait for the last character's API call - if (index === facilityName.length - 1) { - cy.wait("@searchFacility").its("response.statusCode").should("eq", 200); - } + // Wait for each API call to complete + cy.wait("@searchFacility");cypress/pageObject/Users/UserCreation.ts (1)
152-157
: Enhance API response verification.The current implementation only checks the status code. Consider verifying the response body structure and data.
verifyUserCreationApiCall() { cy.wait("@createUser").then((interception) => { expect(interception.response?.statusCode).to.equal(200); + expect(interception.response?.body).to.have.property('id'); + expect(interception.response?.body).to.have.property('username'); + expect(interception.response?.body).to.have.property('email'); }); return this; }cypress/e2e/patient_spec/patient_creation.cy.ts (2)
45-45
: Move hardcoded facility name to test fixtures.The facility name "Arike" is hardcoded in multiple places. Consider moving it to a fixture file for better maintainability.
+// cypress/fixtures/facilities.json +{ + "default": "Arike", + "facilities": [ + { + "name": "Arike", + "type": "Primary" + } + ] +}Then update the test:
+ cy.fixture('facilities').then((facilities) => { - facilityCreation.selectFacility("Arike"); + facilityCreation.selectFacility(facilities.default); + });Also applies to: 70-70
Line range hint
67-76
: Enhance test description and add more assertions.The test description "search patient with phone number and verifies details" could be more specific about what is being verified.
- it("search patient with phone number and verifies details", () => { + it("should display correct patient details when searched by phone number", () => { cy.loginByApi("staff"); facilityCreation.selectFacility("Arike"); patientCreation .clickSearchPatients() .searchPatient(TEST_PHONE) - .verifySearchResults(PATIENT_DETAILS); + .verifySearchResults(PATIENT_DETAILS) + .verifyPatientCardElements() + .verifyNoOtherPatientsDisplayed(); });src/components/Users/CreateUserForm.tsx (1)
223-227
: Comprehensive test coverage with consistent data-cy attributes.The data-cy attributes have been systematically added to all form inputs and interactive elements, following a consistent naming pattern. This will greatly facilitate the creation of reliable Cypress tests.
However, consider adding data-cy attributes to error message containers to enable validation testing.
<FormMessage /> +<FormMessage data-cy={`${field.name}-error`} />
Also applies to: 241-245, 260-264, 280-284, 299-301, 317-322, 338-342, 358-364, 379-382, 401-401, 419-421, 425-429, 449-453, 472-475, 491-493, 510-513, 521-523
cypress/docs/patterns.md (2)
Line range hint
29-36
: Add more examples for test data generation.While the test data management section provides a good foundation, it would be beneficial to include more examples specific to user creation, given the PR's focus.
// Generator functions for dynamic data const phoneNumber = generatePhoneNumber(); + +// User data generation example +const userData = { + firstName: generateName(), + lastName: generateName(), + email: generateEmail(), + phoneNumber: generatePhoneNumber(), +};
Line range hint
39-55
: Enhance test structure example with user creation specifics.The test structure example could be more relevant to the current PR's context.
describe("Feature Name", () => { - const page = new PageObject(); - const facilityType = "Primary Health Centre"; - const testData = generateTestData(); + const userPage = new UserCreation(); + const userData = generateUserData(); beforeEach(() => { // Setup }); - it("should perform action", () => { - page.navigateToOrganization("Kerala"); - page.navigateToFacilitiesList(); + it("should create a new user", () => { + userPage.fillUserForm(userData); + userPage.submitForm(); + userPage.verifyUserCreated(userData); }); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
cypress/docs/patterns.md
(3 hunks)cypress/e2e/facility_spec/facility_creation.cy.ts
(1 hunks)cypress/e2e/patient_spec/patient_creation.cy.ts
(4 hunks)cypress/e2e/users_spec/user_creation.cy.ts
(1 hunks)cypress/fixtures/users.json
(1 hunks)cypress/pageObject/Patients/PatientCreation.ts
(0 hunks)cypress/pageObject/Users/UserCreation.ts
(1 hunks)cypress/pageObject/facility/FacilityCreation.ts
(2 hunks)cypress/support/commands.ts
(1 hunks)cypress/support/index.ts
(0 hunks)cypress/utils/commonUtils.ts
(2 hunks)src/components/Users/CreateUserForm.tsx
(17 hunks)src/pages/Organization/components/AddUserSheet.tsx
(1 hunks)src/pages/Organization/components/LinkUserSheet.tsx
(2 hunks)src/pages/Organization/components/OrganizationLayout.tsx
(1 hunks)
💤 Files with no reviewable changes (2)
- cypress/pageObject/Patients/PatientCreation.ts
- cypress/support/index.ts
✅ Files skipped from review due to trivial changes (3)
- cypress/fixtures/users.json
- src/pages/Organization/components/LinkUserSheet.tsx
- src/pages/Organization/components/OrganizationLayout.tsx
🧰 Additional context used
📓 Learnings (4)
cypress/pageObject/facility/FacilityCreation.ts (2)
Learnt from: Jacobjeevan
PR: ohcnetwork/care_fe#9145
File: cypress/e2e/facility_spec/FacilityCreation.cy.ts:177-180
Timestamp: 2024-11-18T10:44:30.303Z
Learning: In `cypress/e2e/facility_spec/FacilityCreation.cy.ts`, when testing bed and staff capacity individually for additional error verification, we prefer to use separate method calls to handle bed and staff capacity, rather than using `facilityPage.createNewFacility(testFacilityData)` which also handles bed management.
Learnt from: nihal467
PR: ohcnetwork/care_fe#9846
File: cypress/pageObject/facility/FacilityCreation.ts:14-47
Timestamp: 2025-01-09T05:37:17.121Z
Learning: In the care_fe project's Cypress tests, facility test data is managed through the facilityData.ts utility which provides pre-validated data through the FacilityTestData interface, making additional validation in the FacilityCreation page object unnecessary.
cypress/e2e/facility_spec/facility_creation.cy.ts (3)
Learnt from: Jacobjeevan
PR: ohcnetwork/care_fe#9145
File: cypress/e2e/facility_spec/FacilityCreation.cy.ts:177-180
Timestamp: 2024-11-18T10:44:30.303Z
Learning: In `cypress/e2e/facility_spec/FacilityCreation.cy.ts`, when testing bed and staff capacity individually for additional error verification, we prefer to use separate method calls to handle bed and staff capacity, rather than using `facilityPage.createNewFacility(testFacilityData)` which also handles bed management.
Learnt from: kihan2518B
PR: ohcnetwork/care_fe#8956
File: cypress/e2e/facility_spec/FacilityCreation.cy.ts:261-261
Timestamp: 2024-12-04T18:58:47.241Z
Learning: In `cypress/e2e/facility_spec/FacilityCreation.cy.ts`, the test case titled "Create a new facility with no bed and doctor capacity" includes steps to select bed types and specializations before validation. This is done intentionally to verify that error messages are shown properly when no capacity is specified.
Learnt from: nihal467
PR: ohcnetwork/care_fe#9846
File: cypress/pageObject/facility/FacilityCreation.ts:14-47
Timestamp: 2025-01-09T05:37:17.121Z
Learning: In the care_fe project's Cypress tests, facility test data is managed through the facilityData.ts utility which provides pre-validated data through the FacilityTestData interface, making additional validation in the FacilityCreation page object unnecessary.
cypress/e2e/patient_spec/patient_creation.cy.ts (2)
Learnt from: kihan2518B
PR: ohcnetwork/care_fe#8956
File: cypress/e2e/facility_spec/FacilityCreation.cy.ts:261-261
Timestamp: 2024-12-04T18:58:47.241Z
Learning: In `cypress/e2e/facility_spec/FacilityCreation.cy.ts`, the test case titled "Create a new facility with no bed and doctor capacity" includes steps to select bed types and specializations before validation. This is done intentionally to verify that error messages are shown properly when no capacity is specified.
Learnt from: Jacobjeevan
PR: ohcnetwork/care_fe#9145
File: cypress/e2e/facility_spec/FacilityCreation.cy.ts:177-180
Timestamp: 2024-11-18T10:44:30.303Z
Learning: In `cypress/e2e/facility_spec/FacilityCreation.cy.ts`, when testing bed and staff capacity individually for additional error verification, we prefer to use separate method calls to handle bed and staff capacity, rather than using `facilityPage.createNewFacility(testFacilityData)` which also handles bed management.
cypress/pageObject/Users/UserCreation.ts (1)
Learnt from: Jacobjeevan
PR: ohcnetwork/care_fe#9080
File: cypress/pageobject/Users/ManageUserPage.ts:67-97
Timestamp: 2024-11-14T08:09:58.453Z
Learning: In Cypress test files (`cypress/pageobject/Users/ManageUserPage.ts`), when methods are called to check input provided earlier in the test, adding TypeScript interfaces, input validation, and custom error messages is unnecessary.
🔇 Additional comments (6)
src/pages/Organization/components/AddUserSheet.tsx (1)
31-34
: LGTM! Well-structured test selectors added.The added
data-cy
attributes follow good testing practices with clear, descriptive names that align with the component's purpose.Also applies to: 36-39
cypress/e2e/facility_spec/facility_creation.cy.ts (1)
15-17
: LGTM! Improved test isolation.Moving test data generation into the test case scope enhances test isolation and aligns with the project's facility test data management practices.
cypress/utils/commonUtils.ts (1)
Line range hint
9-28
: LGTM! Good refactor of the name generation function.The rename from
generatePatientName
togenerateName
makes the function more generic and reusable across different test scenarios.cypress/pageObject/facility/FacilityCreation.ts (1)
11-13
: LGTM! Method enables fluent chaining.The implementation correctly returns this for method chaining.
src/components/Users/CreateUserForm.tsx (1)
199-201
: LGTM! Good use of data-cy attribute for user type selection.The data-cy attribute follows the kebab-case naming convention and clearly identifies the element's purpose.
cypress/docs/patterns.md (1)
6-13
: Excellent documentation of preferred command usage.The documentation clearly demonstrates the preferred approach using
verifyAndClickElement
and explicitly shows what patterns to avoid. This will help maintain consistency across tests.
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
🧹 Nitpick comments (1)
cypress/pageObject/facility/FacilityCreation.ts (1)
92-99
: Consider making phone number validation configurable.The phone number validation message is hardcoded for Indian phone numbers (+91). Consider making this configurable to support international phone numbers if the application needs to be used in other regions.
- message: "Phone number must start with +91 followed by 10 digits", + message: `Phone number must start with ${PHONE_PREFIX} followed by ${PHONE_DIGITS} digits`,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
cypress/pageObject/Users/UserCreation.ts
(1 hunks)cypress/pageObject/facility/FacilityCreation.ts
(2 hunks)cypress/support/commands.ts
(2 hunks)cypress/support/index.ts
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- cypress/pageObject/Users/UserCreation.ts
- cypress/support/index.ts
🧰 Additional context used
📓 Learnings (1)
cypress/pageObject/facility/FacilityCreation.ts (2)
Learnt from: Jacobjeevan
PR: ohcnetwork/care_fe#9145
File: cypress/e2e/facility_spec/FacilityCreation.cy.ts:177-180
Timestamp: 2024-11-18T10:44:30.303Z
Learning: In `cypress/e2e/facility_spec/FacilityCreation.cy.ts`, when testing bed and staff capacity individually for additional error verification, we prefer to use separate method calls to handle bed and staff capacity, rather than using `facilityPage.createNewFacility(testFacilityData)` which also handles bed management.
Learnt from: nihal467
PR: ohcnetwork/care_fe#9846
File: cypress/pageObject/facility/FacilityCreation.ts:14-47
Timestamp: 2025-01-09T05:37:17.121Z
Learning: In the care_fe project's Cypress tests, facility test data is managed through the facilityData.ts utility which provides pre-validated data through the FacilityTestData interface, making additional validation in the FacilityCreation page object unnecessary.
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: cypress-run (1)
- GitHub Check: Cloudflare Pages: care-fe
🔇 Additional comments (3)
cypress/pageObject/facility/FacilityCreation.ts (1)
8-13
: LGTM! Well-structured navigation methods.The navigation methods follow Cypress best practices by using data-cy attributes and maintain consistency with the builder pattern.
cypress/support/commands.ts (2)
144-153
: LGTM! Improved select option implementation.The changes improve the command by:
- Using ARIA roles for better test reliability
- Including proper visibility checks
- Simplifying the implementation
192-203
: LGTM! Well-structured error message handling.The changes improve error message verification by:
- Adding proper TypeScript interface
- Including both label and message verification
- Ensuring visibility through scrolling
@nihal467 Your efforts have helped advance digital healthcare and TeleICU systems. 🚀 Thank you for taking the time out to make CARE better. We hope you continue to innovate and contribute; your impact is immense! 🙌 |
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
Release Notes
Documentation
Testing Improvements
User Interface
data-testid
attributes withdata-cy
attributes for better testing compatibility.Security
Chores