(test) O3-4553: Add unit tests for question component #419

MohamedAliMefteh · 2025-03-22T01:20:45Z

Requirements

This PR has a title that briefly describes the work done including the ticket number. If there is a ticket, make sure your PR title includes a conventional commit label. See existing PR titles for inspiration.
My work conforms to the OpenMRS 3.0 Styleguide and design documentation.
My work includes tests or is validated by existing tests.

Summary

This PR adds unit tests for the Question Component used within modals to ensure the correct rendering, behavior, and functionality. The tests cover a wide range of scenarios, including interactions with form fields, conditional rendering, and component-specific functionality such as converting labels to camel case, toggling question visibility, and updating question info.

However, some of the tests are currently failing and require adjustments. Additionally, a few test cases are incomplete and need to be finalized later.

Key Changes:

Rendering & Interactions: Tests ensure the Question component renders correctly and interacts with the form fields, such as updating the questionId, toggling visibility of additional question info, and updating question info text.
Question Info Visibility: Tests handle toggling the visibility of the "Additional Question Info" section when radio buttons (Yes/No) are clicked.
Label Conversion: Added tests for converting the question label to camel-case when the corresponding button is clicked.
Component Rendering: Tests ensure that child components like RenderTypeComponent and QuestionTypeComponent are rendered when their corresponding properties are set.

Known Issues:

Test Failures: There are a few tests that are currently not passing. The failing tests are marked with comments in the code, and they need to be reviewed and adjusted to handle specific behaviors or edge cases correctly.
- Duplicate Question ID Validation: The test for checking duplicate question IDs is commented out due to an issue in mocking the validation function (checkIfQuestionIdExists). It needs to be reworked to properly test the validation logic.
- Rendering Type with Markdown: The test to ensure that label and required inputs are hidden when the rendering type is set to "markdown" is commented out. This needs to be revisited to handle conditional rendering based on the rendering property.
Incomplete Tests: There are two tests marked with TODO comments that need to be completed later:
- Question Required Status: The test for toggling between required and optional statuses is incomplete and needs to be updated to verify that the prop value is correctly passed down and updated.
- Rendering Type Select Options: The test for checking the select options when rendering type is clicked is a work-in-progress and needs to be finalized to validate the available options for the user.

Screenshots

Related Issue

https://openmrs.atlassian.net/browse/O3-4553

new translation keys:isQuestionInfoProvided,questionInfo,questionInfoPlaceholder

enhanced some code to match conventions

This commit has the test file for the question component. - For now only the tests only include the feature being introduced in this PR. - Future tests will be in a different PR. - Few changes has been made to the `Question` component: * Radio buttons in the same group should have both the attribute `defaultChecked` * Replaced `defaultChecked` with `checked`

…rm/question/question.test.tsx Co-authored-by: Nethmi Rodrigo <[email protected]>

NethmiRodrigo

Thanks @MohamedAliMefteh, a couple of suggestions:

NethmiRodrigo · 2025-03-24T08:26:43Z

src/components/interactive-builder/modals/question/question-form/question/question.test.tsx

+jest.mock('../rendering-types/rendering-type.component', () => ({
+  __esModule: true,
+  default: jest.fn(() => <div data-testid="render-type-component" />),
+}));
+
+jest.mock('../question-types/question-type.component', () => ({
+  __esModule: true,
+  default: jest.fn(() => <div data-testid="question-type-component" />),
+}));
+
+jest.mock('../common/required-label/required-label.component', () => ({
+  __esModule: true,
+  default: jest.fn(({ isRequired, text }) => (
+    <div data-testid="required-label">
+      {text} {isRequired ? '(Required)' : ''}
+    </div>
+  )),
+}));


Why are these internal components mocked? We should try to avoid mocking internal components like these as much as possible.

NethmiRodrigo · 2025-03-24T08:27:32Z

src/components/interactive-builder/modals/question/question-form/question/question.test.tsx

+jest.mock('./question.scss', () => ({
+  questionIdLabel: 'mock-question-id-label',
+}));


Why is a stylesheet mocked?

NethmiRodrigo · 2025-03-24T08:28:49Z

src/components/interactive-builder/modals/question/question-form/question/question.test.tsx

+  });
+
+  it('should render without crashing', () => {
+    renderWithFormFieldProvider(<Question checkIfQuestionIdExists={jest.fn()} />);


Define checkIfQuestionIdExists as a mock function at the top so you can just pass it in instead of defining it as jest.fn every time.

src/components/interactive-builder/modals/question/question-form/question/question.test.tsx

NethmiRodrigo · 2025-03-24T08:30:09Z

src/components/interactive-builder/modals/question/question-form/question/question.test.tsx

+  // it('should validate duplicate question ids', () => {
+  //   const checkIfQuestionIdExists = jest.fn(() => true);
+  //   renderWithFormFieldProvider(<Question checkIfQuestionIdExists={checkIfQuestionIdExists} />, {
+  //     formField: { ...initialFormField, id: 'duplicateId' },
+  //   });
+
+  //   expect(checkIfQuestionIdExists).toHaveBeenCalledWith('duplicateId');
+  //   expect(screen.getByText(/This question ID already exists/i)).toBeInTheDocument();
+  // });


Don't include commented out test cases

NethmiRodrigo · 2025-03-24T08:31:01Z

src/components/interactive-builder/modals/question/question-form/question/question.test.tsx

+  // it('should not show label and required inputs when rendering type is markdown', () => {
+  //   renderWithFormFieldProvider(<Question checkIfQuestionIdExists={jest.fn()} />, {
+  //     formField: { ...initialFormField, questionOptions: { rendering: 'markdown' } },
+  //   });
+
+  //   expect(screen.queryByRole('textbox', { name: /Label/i })).not.toBeInTheDocument();
+  //   expect(screen.queryByText(/Is this question a required or optional field?/i)).not.toBeInTheDocument();
+  // });


NethmiRodrigo · 2025-03-24T08:31:36Z

src/components/interactive-builder/modals/question/question-form/question/question.test.tsx

+  it('should render RenderTypeComponent when rendering type is selected', () => {
+    renderWithFormFieldProvider(<Question checkIfQuestionIdExists={jest.fn()} />, {
+      formField: { ...initialFormField, questionOptions: { rendering: 'text' } },
+    });
+
+    expect(screen.getByTestId('render-type-component')).toBeInTheDocument();
+    expect(RenderTypeComponent).toHaveBeenCalled();
+  });
+
+  it('should render QuestionTypeComponent when question type is selected', () => {
+    renderWithFormFieldProvider(<Question checkIfQuestionIdExists={jest.fn()} />, {
+      formField: { ...initialFormField, type: 'obs' },
+    });
+
+    expect(screen.getByTestId('question-type-component')).toBeInTheDocument();
+    expect(QuestionTypeComponent).toHaveBeenCalled();
+  });


These components should be rendered even if they are not in the field.

NethmiRodrigo · 2025-03-24T08:31:46Z

src/components/interactive-builder/modals/question/question-form/question/question.test.tsx

+    const renderingTypeSelect = screen.getByLabelText(/Rendering type/i);
+    fireEvent.click(renderingTypeSelect);
+
+    // TODO: test the select options.


NethmiRodrigo · 2025-03-24T08:32:53Z

src/components/interactive-builder/modals/question/question-form/question/question.test.tsx

+    // TODO: check if it gets the updated prop
+
+    const optionalRadio = screen.getByLabelText(/Optional/i);
+    fireEvent.click(optionalRadio);


Prefer to use userEvents over fireEvent - https://testing-library.com/docs/user-event/intro#differences-from-fireevent

NethmiRodrigo · 2025-03-24T08:32:58Z

src/components/interactive-builder/modals/question/question-form/question/question.test.tsx

+    const optionalRadio = screen.getByLabelText(/Optional/i);
+    fireEvent.click(optionalRadio);
+
+    // TODO: check if it gets the updated prop


MohamedAliMefteh · 2025-03-25T15:10:15Z

Thanks @MohamedAliMefteh, a couple of suggestions:

When writing these tests, I didn't take in consideration other tests written in openMRS form-builder.
I will try to follow the same rules within other tests.
I will tackle these changes Once I finish my proposal, I hope that's not a problem!

…ith useEvents and fixed other tests

enhanced some code to match conventions

This commit has the test file for the question component. - For now only the tests only include the feature being introduced in this PR. - Future tests will be in a different PR. - Few changes has been made to the `Question` component: * Radio buttons in the same group should have both the attribute `defaultChecked` * Replaced `defaultChecked` with `checked`

…ith useEvents and fixed other tests

…ithub.com/MohamedAliMefteh/openmrs-esm-form-builder into feat/add-unit-test-for-question-component

…onent

NethmiRodrigo · 2025-04-18T06:07:53Z

...components/interactive-builder/modals/question/question-form/question/question.component.tsx

I don't think this file should have been changed. Maybe a bad rebase?

yup definitely, I think it should be fixed now.

NethmiRodrigo · 2025-04-18T06:08:43Z

src/components/interactive-builder/modals/question/question-form/question/question.test.tsx

-  useFormField: () => ({ formField, setFormField: mockSetFormField }),
-}));
+const renderWithFormFieldProvider = (
+  ui: React.ReactElement,


Suggested change

ui: React.ReactElement,

component: React.ReactElement,

NethmiRodrigo · 2025-04-18T06:08:54Z

src/components/interactive-builder/modals/question/question-form/question/question.test.tsx

+) => {
+  return render(
+    <FormFieldProvider initialFormField={formField} selectedConcept={selectedConcept}>
+      {ui}


Suggested change

{ui}

{component}

NethmiRodrigo · 2025-04-18T06:09:05Z

src/components/interactive-builder/modals/question/question-form/question/question.test.tsx

+jest.mock('react-i18next', () => ({
+  useTranslation: jest.fn(() => ({
+    t: (key, fallback) => fallback || key,
+  })),
+}));


Does this need to be mocked?

NethmiRodrigo · 2025-04-18T06:09:53Z

src/components/interactive-builder/modals/question/question-form/question/question.test.tsx

+  it('should render all required fields', () => {
+    renderWithFormFieldProvider(<Question checkIfQuestionIdExists={checkIfQuestionIdExists} />);
+
+    expect(screen.getByRole('textbox', { name: /Question ID/i })).toBeInTheDocument();


Since the /i is there, the text doesn't need to strictly be in the case we expect.

Suggested change

expect(screen.getByRole('textbox', { name: /Question ID/i })).toBeInTheDocument();

expect(screen.getByRole('textbox', { name: /question id/i })).toBeInTheDocument();

NethmiRodrigo · 2025-04-18T06:10:26Z

src/components/interactive-builder/modals/question/question-form/question/question.test.tsx

+
+    expect(screen.getByLabelText(/Question type/i)).toBeInTheDocument();
+
+    expect(screen.getByLabelText(/Rendering type/i)).toBeInTheDocument();


Suggested change

expect(screen.getByLabelText(/Rendering type/i)).toBeInTheDocument();

expect(screen.getByLabelText(/rendering type/i)).toBeInTheDocument();

NethmiRodrigo · 2025-04-18T06:11:15Z

src/components/interactive-builder/modals/question/question-form/question/question.test.tsx

+  beforeEach(() => {
+    jest.clearAllMocks();
+  });


This is not required since this is done by default in our jest.config

NethmiRodrigo · 2025-04-18T06:11:26Z

src/components/interactive-builder/modals/question/question-form/question/question.test.tsx

-    ).toBeInTheDocument();
-    expect(screen.getByRole('radio', { name: /Yes/i })).toBeInTheDocument();
-    expect(screen.getByRole('radio', { name: /No/i })).toBeInTheDocument();
+  const checkIfQuestionIdExists = jest.fn(() => false);


Shouldn't this be at the top of the file?

NethmiRodrigo · 2025-04-18T06:13:08Z

src/components/interactive-builder/modals/question/question-form/question/question.test.tsx

+    await user.clear(idInput);
+    await user.type(idInput, 'newTestId');
+
+    expect((idInput as HTMLInputElement)).toHaveValue('newTestId');


Does this need to be casted as an HTMLInputElement?

NethmiRodrigo · 2025-04-18T06:15:33Z

src/components/interactive-builder/modals/question/question-form/question/question.test.tsx

+  it('should show appropriate rendering types for the selected question type', async () => {
+    const user = userEvent.setup();
+    renderWithFormFieldProvider(<Question checkIfQuestionIdExists={checkIfQuestionIdExists} />, {
+      formField: { ...initialFormField, type: 'encounterDatetime' },
+    });
+
+    const renderingTypeSelect = screen.getByLabelText(/Rendering type/i);
+
+    expect(renderingTypeSelect).toBeInTheDocument();
+
+    expect(screen.getByLabelText(/Question type/i)).toHaveValue('encounterDatetime');
+  });


Since this test case is supposed to test if the appropriate rendering types are shown, we should do more than checking if the select rendering type dropdown is there, we should also test the select options available. In this case, the question type is encounterDatetime, so the rendering type options should only be date or datetime

I added that and I just added the test cases for other question options to check renderingtypes for each option.

…onent

NethmiRodrigo

Thanks @MohamedAliMefteh, very close now, a few final nitpicks:

NethmiRodrigo · 2025-04-21T09:44:03Z

src/components/interactive-builder/modals/question/question-form/question/question.test.tsx

+    await user.clear(questionInfoInput);
+    await user.type(questionInfoInput, 'New question info');
+
+    expect((questionInfoInput as HTMLInputElement).value).toBe('New question info');


Suggested change

expect((questionInfoInput as HTMLInputElement).value).toBe('New question info');

expect(questionInfoInput).toHaveValue('New question info');

NethmiRodrigo · 2025-04-21T09:47:32Z

src/components/interactive-builder/modals/question/question-form/question/question.test.tsx

+    expect((screen.getByLabelText(/additional question info/i) as HTMLInputElement).value).toBe(
+      'Initial question info',
+    );


Suggested change

expect((screen.getByLabelText(/additional question info/i) as HTMLInputElement).value).toBe(

'Initial question info',

);

expect(screen.getByLabelText(/additional question info/i)).toHaveValue('Initial question info');

NethmiRodrigo · 2025-04-21T09:48:35Z

src/components/interactive-builder/modals/question/question-form/question/question.test.tsx

+    });
+  });
+
+  it('should handle initial state with questionInfo properly', () => {


Suggested change

it('should handle initial state with questionInfo properly', () => {

it('should load question info from the form field object', () => {

…ithub.com/MohamedAliMefteh/openmrs-esm-form-builder into feat/add-unit-test-for-question-component

NethmiRodrigo

Thanks @MohamedAliMefteh!

MohamedAliMefteh · 2025-04-21T10:40:57Z

Thanks @MohamedAliMefteh!

Thank you for the help!

unknown and others added 5 commits March 20, 2025 01:34

feat:add support for question info

8fb0431

new translation keys:isQuestionInfoProvided,questionInfo,questionInfoPlaceholder

feat:Add support for question info

0c84c76

enhanced some code to match conventions

Update src/components/interactive-builder/modals/question/question-fo…

0c5c826

…rm/question/question.test.tsx Co-authored-by: Nethmi Rodrigo <[email protected]>

feat:Add unit test for question component

73f5a0d

MohamedAliMefteh changed the title ~~Feat/add unit test for question component~~ (feat) O3-4553 add unit test for question component Mar 22, 2025

MohamedAliMefteh closed this Mar 22, 2025

MohamedAliMefteh reopened this Mar 22, 2025

Commented the parts that has issues and need to be fixed

71d91dd

NethmiRodrigo changed the title ~~(feat) O3-4553 add unit test for question component~~ (test) O3-4553: Add unit tests for question component Mar 24, 2025

Merge branch 'main' into feat/add-unit-test-for-question-component

3f77f16

NethmiRodrigo requested changes Mar 24, 2025

View reviewed changes

Removed unnecessary mocks for internal components,changed fireEvent w…

0f4a600

…ith useEvents and fixed other tests

MohamedAliMefteh requested a review from NethmiRodrigo April 7, 2025 17:02

unknown and others added 13 commits April 7, 2025 21:42

feat:Add support for question info

72f6fad

enhanced some code to match conventions

feat:Add unit test for question component

06d04d1

Commented the parts that has issues and need to be fixed

7b6465e

Removed unnecessary mocks for internal components,changed fireEvent w…

86a81f0

…ith useEvents and fixed other tests

Merge branch 'feat/add-unit-test-for-question-component' of https://g…

e1e6039

…ithub.com/MohamedAliMefteh/openmrs-esm-form-builder into feat/add-unit-test-for-question-component

Merge branch 'openmrs:main' into feat/add-unit-test-for-question-comp…

49893d8

…onent

Merge branch 'openmrs:main' into feat/add-unit-test-for-question-comp…

ec3dfdf

…onent

Merge branch 'openmrs:main' into feat/add-unit-test-for-question-comp…

8069791

…onent

Merge branch 'openmrs:main' into feat/add-unit-test-for-question-comp…

bf4bc66

…onent

Merge branch 'openmrs:main' into feat/add-unit-test-for-question-comp…

1e77b39

…onent

Merge branch 'openmrs:main' into feat/add-unit-test-for-question-comp…

ebcc5f4

…onent

Merge branch 'main' into feat/add-unit-test-for-question-component

9e67da0

NethmiRodrigo requested changes Apr 18, 2025

View reviewed changes

Merge branch 'openmrs:main' into feat/add-unit-test-for-question-comp…

82c7eb8

…onent

MohamedAliMefteh and others added 2 commits April 21, 2025 08:36

Merge branch 'openmrs:main' into feat/add-unit-test-for-question-comp…

2e5a518

…onent

added tests for the rendering type dropdown

6e5bf55

MohamedAliMefteh requested a review from NethmiRodrigo April 21, 2025 09:34

Merge branch 'main' into feat/add-unit-test-for-question-component

03c6294

NethmiRodrigo requested changes Apr 21, 2025

View reviewed changes

MohamedAliMefteh added 2 commits April 21, 2025 10:55

few changes to the tests

345a0f8

Merge branch 'feat/add-unit-test-for-question-component' of https://g…

9bba2cb

…ithub.com/MohamedAliMefteh/openmrs-esm-form-builder into feat/add-unit-test-for-question-component

MohamedAliMefteh requested a review from NethmiRodrigo April 21, 2025 10:03

NethmiRodrigo enabled auto-merge (squash) April 21, 2025 10:37

NethmiRodrigo approved these changes Apr 21, 2025

View reviewed changes

NethmiRodrigo merged commit a61a07a into openmrs:main Apr 21, 2025
6 checks passed

MohamedAliMefteh deleted the feat/add-unit-test-for-question-component branch April 21, 2025 13:11

denniskigen mentioned this pull request Apr 30, 2025

(chore) Release v3.1.0 #494

Merged

3 tasks

	expect(screen.getByRole('textbox', { name: /Question ID/i })).toBeInTheDocument();
	expect(screen.getByRole('textbox', { name: /question id/i })).toBeInTheDocument();


		expect(screen.getByLabelText(/Question type/i)).toBeInTheDocument();

		expect(screen.getByLabelText(/Rendering type/i)).toBeInTheDocument();

	expect((questionInfoInput as HTMLInputElement).value).toBe('New question info');
	expect(questionInfoInput).toHaveValue('New question info');

	it('should handle initial state with questionInfo properly', () => {
	it('should load question info from the form field object', () => {

(test) O3-4553: Add unit tests for question component #419

(test) O3-4553: Add unit tests for question component #419

Uh oh!

Conversation

MohamedAliMefteh commented Mar 22, 2025 • edited by NethmiRodrigo Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Requirements

Summary

Key Changes:

Known Issues:

Screenshots

Related Issue

Uh oh!

NethmiRodrigo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MohamedAliMefteh commented Mar 25, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NethmiRodrigo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NethmiRodrigo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MohamedAliMefteh commented Apr 21, 2025

Uh oh!

Uh oh!

MohamedAliMefteh commented Mar 22, 2025 •

edited by NethmiRodrigo

Loading