-
Notifications
You must be signed in to change notification settings - Fork 99
(tests) O3-4316: Add unit tests for obsGroup type questions #416
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
base: main
Are you sure you want to change the base?
Conversation
Hi @NethmiRodrigo , could you review this PR Thanks! |
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.
Thanks @rdwaynedehoedt! Left a couple of suggestions:
src/components/interactive-builder/modals/question/question.modal.test.tsx
Outdated
Show resolved
Hide resolved
}, | ||
}, | ||
], | ||
} as ObsGroupQuestion, |
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.
Is this as ObsGroupQuestion
required?
render( | ||
<FormFieldProvider initialFormField={initialFormField}> | ||
<QuestionModal | ||
closeModal={mockCloseModal} | ||
schema={mockSchema} | ||
pageIndex={0} | ||
sectionIndex={0} | ||
questionIndex={0} | ||
onSchemaChange={mockOnSchemaChange} | ||
resetIndices={mockResetIndices} | ||
formField={initialFormField} | ||
/> | ||
</FormFieldProvider>, | ||
); |
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.
Prefer to have the `renderComponent as a separate function like so -
Lines 116 to 121 in d80c228
function renderComponent() { | |
render( | |
<FormFieldProvider initialFormField={formField}> | |
<PatientIdentifierTypeQuestion /> | |
</FormFieldProvider>, | |
); |
); | ||
|
||
expect(screen.getByText(/Question type/i)).toBeInTheDocument(); | ||
expect(screen.getByText(/Rendering type/i)).toBeInTheDocument(); |
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.
You should also make sure that the button Add grouped question
is visible
jest.mock('react-i18next', () => ({ | ||
useTranslation: () => ({ | ||
t: (key, defaultValue) => defaultValue || key, | ||
}), | ||
})); |
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.
Is this required? We already have a mock for react-i18next
in https://github.com/openmrs/openmrs-esm-form-builder/blob/main/__mocks__/react-i18next.js
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.
We should also test for things like adding a grouped question renders the input fields for another question and that deleting an obsgroup question does actually delete it
…dal.test.tsx Co-authored-by: Nethmi Rodrigo <[email protected]>
Co-authored-by: Nethmi Rodrigo <[email protected]>
label: 'Vital Signs', | ||
type: 'obsGroup', | ||
questionOptions: { | ||
rendering: 'group' as RenderType, |
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.
do we need explicit casting here?
label: 'Blood Pressure', | ||
type: 'obs', | ||
questionOptions: { | ||
rendering: 'text' as RenderType, |
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.
rendering: 'text' as RenderType, | |
rendering: 'text' , |
label: 'Temperature', | ||
type: 'obs', | ||
questionOptions: { | ||
rendering: 'number' as RenderType, |
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.
rendering: 'number' as RenderType, | |
rendering: 'number', |
Requirements
Summary
This PR adds unit tests for obsGroup type questions in the form engine. The tests validate the proper rendering and functionality of obsGroup components, ensuring they correctly handle nested observations and maintain data integrity.
Screenshots
N/A - This PR focuses on unit tests without UI changes.
Related Issue
https://openmrs.atlassian.net/browse/O3-4316
Other
The added tests improve coverage for the obsGroup functionality which was previously untested.