-
Notifications
You must be signed in to change notification settings - Fork 501
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 | Create Partial Investigation Log Update #9034
Conversation
WalkthroughThe changes introduce an end-to-end test suite for patient investigations using Cypress, encapsulated in the new file Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
💤 Files with no reviewable 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 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Deploying care-fe with Cloudflare Pages
|
CARE Run #3863
Run Properties:
|
Project |
CARE
|
Branch Review |
investigation-test
|
Run status |
Passed #3863
|
Run duration | 04m 55s |
Commit |
fc3eccdd10: New Cypress Test | Create Investigation Log Update
|
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 ↗︎ |
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: 3
🧹 Outside diff range and nitpick comments (3)
cypress/pageobject/Patient/PatientInvestigation.ts (1)
20-22
: LGTM! The new method follows page object pattern.The implementation is clean and consistent with other methods in the class.
Consider adding JSDoc documentation for better maintainability:
+ /** + * Clicks the "Log Lab Results" button in the investigation tab + */ clickLogLabResults() { cy.verifyAndClickElement("#log-lab-results", "Log Lab Results"); }cypress/e2e/patient_spec/PatientInvestigation.cy.ts (1)
1-4
: Standardize import paths and add type definitions.The import statements mix relative (
../../pageobject/Login/LoginPage
) and absolute paths (pageobject/Patient/PatientCreation
). Consider standardizing to absolute paths for better maintainability.-import { PatientPage } from "pageobject/Patient/PatientCreation"; -import PatientInvestigation from "pageobject/Patient/PatientInvestigation"; -import LoginPage from "../../pageobject/Login/LoginPage"; +import { PatientPage } from "@/pageobject/Patient/PatientCreation"; +import { PatientInvestigation } from "@/pageobject/Patient/PatientInvestigation"; +import { LoginPage } from "@/pageobject/Login/LoginPage";src/components/Facility/ConsultationDetails/ConsultationInvestigationsTab.tsx (1)
20-20
: LGTM! Good addition for test automation.The addition of the
id
attribute enhances the component's testability and aligns well with the PR's objective of implementing Cypress tests for the patient investigation workflow.Consider using a constant for the ID to maintain consistency and prevent typos between the component and test files. Example:
// In a shared constants file export const TEST_IDS = { LOG_LAB_RESULTS_BUTTON: 'log-lab-results' } as const;Then use it like:
id={TEST_IDS.LOG_LAB_RESULTS_BUTTON}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
cypress/e2e/patient_spec/PatientInvestigation.cy.ts
(1 hunks)cypress/pageobject/Patient/PatientInvestigation.ts
(1 hunks)src/components/Facility/ConsultationDetails/ConsultationInvestigationsTab.tsx
(1 hunks)
🔇 Additional comments (2)
cypress/pageobject/Patient/PatientInvestigation.ts (1)
20-22
: Verify test coverage for the new method.
Let's ensure the new method is properly utilized in the test suite.
✅ Verification successful
Test integration verified successfully
The clickLogLabResults()
method is properly integrated and used in the test suite:
- Correctly used in
PatientInvestigation.cy.ts
as part of an investigation creation test flow - No duplicate implementations of the log-lab-results selector found
- The selector usage is consistent across the codebase
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the new method is used in the test suite and there are no duplicate implementations
# Test 1: Check if the test file exists and uses clickLogLabResults
echo "Checking for test usage of clickLogLabResults..."
rg -l "clickLogLabResults" "cypress/e2e/patient_spec/PatientInvestigation.cy.ts"
# Test 2: Verify no duplicate implementations of log lab results clicking exist
echo "Checking for potential duplicate implementations..."
rg -g "!PatientInvestigation.ts" "cy\..*log-lab-results"
Length of output: 409
Script:
#!/bin/bash
# Let's check the actual content of the test file to verify proper usage
echo "Checking the test file content..."
rg "clickLogLabResults" -A 5 -B 5 "cypress/e2e/patient_spec/PatientInvestigation.cy.ts"
# Also verify the selector usage in other files to ensure consistency
echo "Checking for any other log-lab-results selector usage..."
rg "#log-lab-results" -A 2 -B 2
Length of output: 1046
cypress/e2e/patient_spec/PatientInvestigation.cy.ts (1)
27-30
: Verify custom command implementation.
The test uses custom commands like clickAndMultiSelectOption
. Let's verify its implementation for robustness.
✅ Verification successful
Let me gather more information about the custom command implementation since the previous search didn't show the actual implementation.
Custom command implementation looks robust and well-typed
The clickAndMultiSelectOption
command is properly implemented with:
- TypeScript type definitions in
cypress/support/index.ts
- Clear implementation in
cypress/support/commands.ts
that:- Handles both single and array inputs
- Uses proper Cypress commands for element interaction
- Follows a clear flow: click → select options → close dropdown
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for custom command definitions
echo "Searching for clickAndMultiSelectOption command definition..."
rg -A 10 "Cypress.Commands.add\('clickAndMultiSelectOption'" cypress/support/
echo "Checking for proper typing of custom commands..."
rg -l "declare global" cypress/support/
Length of output: 392
Script:
#!/bin/bash
# Search with broader pattern to find the command implementation
echo "Searching for clickAndMultiSelectOption implementation..."
rg -A 10 "clickAndMultiSelectOption" cypress/support/
# Also check index.ts since it was found to have global declarations
echo "Checking index.ts content..."
cat cypress/support/index.ts
Length of output: 3352
it("Create a investigation for a patient and verify its reflection", () => { | ||
patientPage.visitPatient(patientName); | ||
patientInvestigation.clickInvestigationTab(); | ||
patientInvestigation.clickLogLabResults(); | ||
cy.clickAndMultiSelectOption("#investigations", [ | ||
"Haematology", | ||
"Urine Test", | ||
]); | ||
cy.submitButton("Save Investigation"); | ||
cy.verifyNotification("Please Enter at least one value"); | ||
}); |
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.
Enhance test coverage and add assertions.
The current test only covers the error path. Consider adding:
- Positive test scenarios (successful investigation creation)
- Assertions for UI state changes
- Verification of investigation data persistence
Example additional test:
it("should successfully create and save an investigation", () => {
patientPage.visitPatient(patientName);
patientInvestigation.clickInvestigationTab();
patientInvestigation.clickLogLabResults();
// Select investigations
cy.clickAndMultiSelectOption("#investigations", ["Haematology"]);
// Fill required values
cy.get('[data-testid="haemoglobin-input"]').type("14.5");
// Save and verify
cy.submitButton("Save Investigation");
cy.verifyNotification("Investigation saved successfully");
// Verify investigation appears in the list
cy.get('[data-testid="investigations-list"]')
.should("contain", "Haematology")
.and("contain", "14.5");
});
describe("Patient Investigation Creation from Patient consultation page", () => { | ||
const loginPage = new LoginPage(); | ||
const patientPage = new PatientPage(); | ||
const patientInvestigation = new PatientInvestigation(); | ||
const patientName = "Dummy Patient 12"; | ||
|
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.
🛠️ Refactor suggestion
Improve test data management and suite description.
The hardcoded patient name ("Dummy Patient 12"
) should be moved to a test data fixture for better maintainability. Also, consider making the test suite description more specific to the functionality being tested.
+import testData from '../../fixtures/patient.json';
+
-describe("Patient Investigation Creation from Patient consultation page", () => {
+describe("Patient Investigation Workflow - Consultation Page", () => {
const loginPage = new LoginPage();
const patientPage = new PatientPage();
const patientInvestigation = new PatientInvestigation();
- const patientName = "Dummy Patient 12";
+ const { patientName } = testData;
Committable suggestion skipped: line range outside the PR's diff.
afterEach(() => { | ||
cy.saveLocalStorage(); | ||
}); |
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.
🛠️ Refactor suggestion
Add proper test data cleanup.
The afterEach hook only handles localStorage. Consider adding cleanup steps to ensure complete test isolation:
- Delete created investigations
- Reset any modified patient data
afterEach(() => {
cy.saveLocalStorage();
+ // Clean up created investigations
+ cy.task('dbCleanup', {
+ collection: 'investigations',
+ filter: { patientName }
+ });
});
Committable suggestion skipped: line range outside the PR's diff.
@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
Summary by CodeRabbit
New Features
Bug Fixes
Documentation