UIREQ-937 create JEST/RTL test cases for RequestsFilters.js #1044

ssandupatla · 2023-05-17T11:25:20Z

UIREQ-937 create JEST/RTL test cases for RequestsFilters.js
URL: https://issues.folio.org/browse/UIREQ-937

github-actions · 2023-05-17T11:28:54Z

Jest Unit Test Statistics

    1 files ±0   42 suites +1 1m 40s ⏱️ +25s
441 tests +9 441 ✔️ +9 0 💤 ±0 0 ❌ ±0
442 runs +9 442 ✔️ +9 0 💤 ±0 0 ❌ ±0

Results for commit 3f25922. ± Comparison against base commit 9c88a35.

♻️ This comment has been updated with latest results.

github-actions · 2023-05-17T11:29:17Z

BigTest Unit Test Statistics

132 tests ±0 132 ✔️ ±0 27s ⏱️ +8s
    1 suites ±0     0 💤 ±0
    1 files ±0     0 ❌ ±0

Results for commit 3f25922. ± Comparison against base commit 9c88a35.

♻️ This comment has been updated with latest results.

artem-blazhko

Could you add changes to changelog file?
Could you check that RequestLevelFilter is rendered/not rendered?
Could you check that all necessary MultiSelectionFilter rendered?

artem-blazhko · 2023-05-24T09:09:24Z

src/components/RequestsFilters/RequestsFilters.test.js

@@ -0,0 +1,74 @@
+import React from 'react';


It is not necessary to import.

artem-blazhko · 2023-05-24T09:10:00Z

src/components/RequestsFilters/RequestsFilters.test.js

+jest.mock('@folio/stripes/components', () => ({
+ AccordionSet: jest.fn(({ children }) => <div>{children}</div>),
+ Accordion: jest.fn(({ displayClearButton, name, children, onClearFilter }) => (
+ <div>
+ <div>{displayClearButton ? <button type="button" onClick={() => onClearFilter()} data-testid={`clear-${name}`}>Clear</button> : null}</div>
+ <div data-testid={`accordion-${name}`}>{children}</div>
+ </div>
+ )),
+ FilterAccordionHeader: jest.fn(() => <div>FilterAccordionHeader</div>),
+}));
+
+jest.mock('@folio/stripes/smart-components', () => ({
+ CheckboxFilter: jest.fn(() => <div>CheckboxFilter</div>),
+ MultiSelectionFilter: jest.fn(() => <div>MultiSelectionFilter</div>),
+}));


We have separate files with mocks - test/jest/__mock__.
You can add your mocks there if those mocks do not exist yet.

artem-blazhko · 2023-05-24T09:12:26Z

src/components/RequestsFilters/RequestsFilters.test.js

+ const { getByTestId } = render(<RequestsFilters {...props} />);
+ const requestTypeButton = getByTestId('clear-requestType');
+ expect(requestTypeButton).toBeInTheDocument();
+ userEvent.click(requestTypeButton);


What is the reason to trigger click event here?
Is it necessary only for increasing code coverage?

artem-blazhko · 2023-05-24T09:12:45Z

src/components/RequestsFilters/RequestsFilters.test.js

+ const { getByTestId } = render(<RequestsFilters {...props} />);
+ const requestStatusButton = getByTestId('clear-requestStatus');
+ expect(requestStatusButton).toBeInTheDocument();
+ userEvent.click(requestStatusButton);


What is the reason to trigger click event here?
Is it necessary only for increasing code coverage?

artem-blazhko · 2023-05-24T09:12:52Z

src/components/RequestsFilters/RequestsFilters.test.js

+ const { getByTestId } = render(<RequestsFilters {...props} />);
+ const tagsButton = getByTestId('clear-tags');
+ expect(tagsButton).toBeInTheDocument();
+ userEvent.click(tagsButton);


What is the reason to trigger click event here?
Is it necessary only for increasing code coverage?

artem-blazhko · 2023-05-24T09:15:45Z

src/components/RequestsFilters/RequestsFilters.test.js

+ it('renders the request type filter', () => {
+ const { getByTestId } = render(<RequestsFilters {...props} />);
+ const requestTypeButton = getByTestId('clear-requestType');
+ expect(requestTypeButton).toBeInTheDocument();
+ userEvent.click(requestTypeButton);
+ });
+ it('renders the request status filter', () => {
+ const { getByTestId } = render(<RequestsFilters {...props} />);
+ const requestStatusButton = getByTestId('clear-requestStatus');
+ expect(requestStatusButton).toBeInTheDocument();
+ userEvent.click(requestStatusButton);
+ });
+ it('renders the tags filter', () => {
+ const { getByTestId } = render(<RequestsFilters {...props} />);
+ const tagsButton = getByTestId('clear-tags');
+ expect(tagsButton).toBeInTheDocument();
+ userEvent.click(tagsButton);
+ });


I see that the first three tests have the same render approach.
Could you add beforeEach statement and call render(<RequestsFilters {...props} />) inside?

artem-blazhko · 2023-05-24T09:21:54Z

src/components/RequestsFilters/RequestsFilters.test.js

+ expect(tagsButton).toBeInTheDocument();
+ userEvent.click(tagsButton);
+ });
+ it('render the accordion pickup service points', () => {


Could you mock PickupServicePointFilter and check that it is rendered. There is no sence to check rendering of inner components of PickupServicePointFilter. PickupServicePointFilter has or will have in future its own tests.

sonarcloud · 2023-06-02T06:57:00Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

artem-blazhko

Could you check the rendering of CheckboxFilter?
From the code perspective, I see that CheckboxFilter rendered twice. Would be great to check each render.

artem-blazhko · 2023-06-06T13:32:07Z

src/components/RequestsFilters/RequestsFilters.test.js

+ it('should click the onClear filters', () => {
+ userEvent.click(screen.getByTestId('clear-requestType'));
+ userEvent.click(screen.getByTestId('clear-requestStatus'));
+ userEvent.click(screen.getByTestId('clear-tags'));
+ expect(props.onClear).toHaveBeenCalled();
+ });


Could you create 3 separate tests (1 for each "click" action) and check that onClear was called with correct argument?

artem-blazhko · 2023-06-06T13:35:23Z

src/components/RequestsFilters/RequestsFilters.test.js

+ it('should render the PickupServicePointFilter', () => {
+ expect(screen.getByText(/PickupServicePointFilter/i)).toBeInTheDocument();
+ });
+ describe('MultiSelectionFilter and RequestLevelFilter', () => {


I think this describe is redundant.
Or at least if you want to have describe it makes sense to have one describe for each filter.

artem-blazhko · 2023-06-06T13:36:29Z

src/components/RequestsFilters/RequestsFilters.test.js

+ );
+ expect(getAllByText('MultiSelectionFilter')).toBeTruthy();
+ });
+ it('should render RequestLevelFilter', () => {


Could you add a case when titleLevelRequestsFeatureEnabled is false and check that RequestLevelFilter is not rendered?

artem-blazhko · 2023-06-06T13:36:49Z

test/jest/__mock__/stripesComponents.mock.js

 <div>
 {label}
 {children}
+ {/* <div>{<button type="button" onClick={() => onClearFilter()} data-testid={`clear-${name}`}>Clear</button> : null}</div> */}


do we need that comment?

artem-blazhko · 2023-06-06T13:38:16Z

test/jest/__mock__/stripesSmartComponents.mock.js

@@ -8,4 +8,6 @@ jest.mock('@folio/stripes/smart-components', () => ({
 SearchAndSort: jest.fn(() => null),
 ViewMetaData: jest.fn(() => null),
 withTags: jest.fn((WrappedComponent) => (props) => <WrappedComponent {...props} />),
+ // CheckboxFilter: jest.fn(() => <div>CheckboxFilter</div>),


Do we need that comment?

artem-blazhko · 2023-06-06T13:43:19Z

src/components/RequestsFilters/RequestsFilters.test.js

+ beforeEach(() => {
+ render(<RequestsFilters {...props} />);
+ });


Line 51 and 65 have their own renders. But here on the line 36 you will have one more render. I think it is not the best approach.

I would suggest wrapping your first two tests in describe and executing beforeEach inside describe.

artem-blazhko · 2023-06-06T13:43:47Z

test/jest/__mock__/stripesComponents.mock.js

+ <div>
+ <button
+ type="button"
+ onClick={() => onClearFilter()}


Suggested change

onClick={() => onClearFilter()}

onClick={onClearFilter}

Dmitriy-Litvinenko · 2023-06-08T14:33:11Z

test/jest/__mock__/stripesComponents.mock.js

@@ -2,14 +2,30 @@ import React from 'react';



Could you please revert your changes that don't need for current pull request (mock for AccordionSet, ConfirmationModal, ErrorModal, Select)?
CC: @artem-blazhko

Dmitriy-Litvinenko

Could you please fix feedbacks?

Dmitriy-Litvinenko · 2023-06-26T09:17:41Z

Could you please fix feedbacks?

artem-blazhko · 2023-07-03T08:49:33Z

test/jest/__mock__/stripesComponents.mock.js

+ Accordion: jest.fn(({ children, label, name, onClearFilter }) => (
 <div>
 {label}
 {children}
+ <div>
+ <button
+ type="button"
+ onClick={onClearFilter}
+ data-testid={`clear-${name}`}
+ >Clear
+ </button>
+ </div>
+ <div data-testid={`accordion-${name}`} />
 </div>


Suggested change

Accordion: jest.fn(({ children, label, name, onClearFilter }) => (

<div>

{label}

{children}

<div>

<button

type="button"

onClick={onClearFilter}

data-testid={`clear-${name}`}

>Clear

</button>

</div>

<div data-testid={`accordion-${name}`} />

</div>

Accordion: jest.fn(({ children, label, name, onClearFilter }) => (

<div data-testid={`accordion-${name}`} >

{label}

{children}

<div>

<button

type="button"

onClick={onClearFilter}

data-testid={`clear-${name}`}

>Clear

</button>

</div>

</div>

artem-blazhko · 2023-07-03T08:52:39Z

test/jest/__mock__/stripesComponents.mock.js

- {...props}
- />
- )),
+ Checkbox: jest.fn(() => <div>Checkbox</div>),


I do not see the usage of that component. Do we really need to mock it?

artem-blazhko · 2023-07-03T09:04:34Z

test/jest/__mock__/stripesComponents.mock.js

 Col: jest.fn(({ children }) => (
 <div data-test-col>
 {children}
 </div>
 )),
- ConfirmationModal: jest.fn(() => <div>ConfirmationModal</div>),
+ ConfirmationModal: jest.fn(({ heading, confirmLabel, cancelLabel, onConfirm, onCancel }) => (


I do not see the usage of that component. Do we really need to change the mock?

artem-blazhko · 2023-07-03T09:04:53Z

test/jest/__mock__/stripesComponents.mock.js

 Datepicker: jest.fn(() => <div>Datepicker</div>),
- ErrorModal: jest.fn(() => <div>ErrorModal</div>),
+ ErrorModal: jest.fn(({ label, content, buttonLabel, onClose }) => (


I do not see the usage of that component. Do we really need to change the mock?

artem-blazhko · 2023-07-03T09:05:18Z

test/jest/__mock__/stripesComponents.mock.js

@@ -75,7 +99,6 @@ jest.mock('@folio/stripes-components', () => ({
 )),
 MultiColumnList: jest.fn(({ children }) => (
 <div>
- <div>MultiColumnList</div>


I do not see the usage of that component. Do we really need to change the mock?

artem-blazhko · 2023-07-03T09:06:18Z

test/jest/__mock__/stripesComponents.mock.js

- TextField: jest.fn(({
- label,
- onChange,
- validate = jest.fn(),
- ...rest
- }) => {


Could you clarify why do we need to change that mock?

artem-blazhko · 2023-07-03T09:06:42Z

test/jest/__mock__/stripesComponents.mock.js

- onChange(e);
- };
-
+ Select: ({ name, label, onChange, required, children, value }) => {


Could you clarify why we need to change that mock?

artem-blazhko · 2023-07-03T09:06:47Z

test/jest/__mock__/stripesComponents.mock.js

 </div>
 );
- }),
+ },
+ TextArea: jest.fn(() => <div>TextArea</div>),


Could you clarify why we need to change that mock?

artem-blazhko · 2023-07-03T09:06:55Z

test/jest/__mock__/stripesComponents.mock.js

- }),
+ },
+ TextArea: jest.fn(() => <div>TextArea</div>),
+ TextField: jest.fn(() => <div>TextField</div>),


Could you clarify why we need to change that mock?

artem-blazhko · 2023-07-03T09:10:07Z

src/components/RequestsFilters/RequestsFilters.test.js

+ expect(screen.getByText(/PickupServicePointFilter/i)).toBeInTheDocument();
+ });
+ it('should render the MultiSelectionFilter', () => {
+ screen.debug();


Do we really need it here?

Included in RequestFilters.test.js

Updated

artem-blazhko · 2023-07-11T13:13:50Z

CHANGELOG.md

@@ -37,6 +37,7 @@
 * Cover ItemInformation by jest/RTL tests. Refs UIREQ-949.
 * Cover InstanceInformation by jest/RTL tests. Refs UIREQ-950.
 * Fix inconsistency in RTL/Jest tests. Refs UIREQ-979.
+* Create Jest/RTL test for RequestsFilters.js Refs UIREQ-937.


Could you move it to the bottom of the list?

Updated in Changelog.md

artem-blazhko · 2023-07-11T13:15:39Z

src/components/RequestsFilters/RequestsFilters.test.js

+Accordion.mockImplementation(jest.fn(({ children, label, name, onClearFilter }) => (
+ <div>
+ {label}
+ {children}
+ <div>
+ <button
+ type="button"
+ onClick={onClearFilter}
+ data-testid={`clear-${name}`}
+ >
+ Clear
+ </button>
+ </div>
+ <div data-testid={`accordion-${name}`} />
+ </div>
+)));
+
+AccordionSet.mockImplementation(jest.fn(({ children, onToggle }) => (
+ <div>
+ <button type="button" onClick={() => onToggle({ id: 'fulfillment-in-progress' })}>
+ Toggle in Progress
+ </button>
+ <button type="button" onClick={() => onToggle({ id: 'not-yet-filled' })}>
+ Toggle Not Yet Filled
+ </button>
+ {children}
+ </div>
+)));


Could you add your changes to test/__mock__stripesComponents.mock.js file?
That file store all popular mock from components library.

@artem-blazhko - We have added the mock functions to the test file because other test cases in this module will fail if added test/__mock__stripesComponents.mock.js

sonarcloud · 2023-07-18T06:06:53Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

Dmitriy-Litvinenko · 2023-08-31T11:33:47Z

Thank you for yours pull request. We will use it for increase code coverage.

ssandupatla requested review from VSnehalatha, KetineniM, cchitneedi and zburke May 18, 2023 09:01

artem-blazhko requested changes May 24, 2023

View reviewed changes

artem-blazhko requested a review from Dmitriy-Litvinenko May 24, 2023 09:24

ssandupatla requested a review from artem-blazhko June 2, 2023 13:15

artem-blazhko requested changes Jun 6, 2023

View reviewed changes

Dmitriy-Litvinenko requested changes Jun 8, 2023

View reviewed changes

Dmitriy-Litvinenko self-requested a review June 8, 2023 14:33

Dmitriy-Litvinenko reviewed Jun 12, 2023

View reviewed changes

UIREQ-937 - Create Jest/RTL test for RequestsFilters.js

bd398ed

ssandupatla force-pushed the UIREQ-937 branch from 951f05f to bd398ed Compare June 23, 2023 11:45

ssandupatla requested review from Dmitriy-Litvinenko and artem-blazhko June 23, 2023 11:47

artem-blazhko requested changes Jul 3, 2023

View reviewed changes

UIREQ-937 create JEST/RTL test cases for RequestsFilters.js

b6409e8

ssandupatla force-pushed the UIREQ-937 branch from 436f191 to b6409e8 Compare July 4, 2023 09:12

ssandupatla and others added 4 commits July 6, 2023 11:56

Delete stripesComponents.mock.js

b874553

Included in RequestFilters.test.js

Update RequestsFilters.test.js

c002594

Updated

Added stripesComponent mock file

c76cde5

Merge branch 'master' into UIREQ-937

b839b44

artem-blazhko reviewed Jul 11, 2023

View reviewed changes

Update CHANGELOG.md

c6a68fa

ssandupatla requested a review from artem-blazhko July 13, 2023 13:51

Merge branch 'master' into UIREQ-937

3f25922

Dmitriy-Litvinenko closed this Aug 31, 2023

UIREQ-937 create JEST/RTL test cases for RequestsFilters.js #1044

UIREQ-937 create JEST/RTL test cases for RequestsFilters.js #1044

Conversation

ssandupatla commented May 17, 2023

github-actions bot commented May 17, 2023 • edited Loading

Jest Unit Test Statistics

github-actions bot commented May 17, 2023 • edited Loading

BigTest Unit Test Statistics

artem-blazhko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

artem-blazhko May 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Jun 2, 2023

artem-blazhko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Dmitriy-Litvinenko left a comment

Choose a reason for hiding this comment

Dmitriy-Litvinenko commented Jun 26, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ssandupatla Jul 12, 2023 • edited Loading

Choose a reason for hiding this comment

sonarcloud bot commented Jul 18, 2023

Dmitriy-Litvinenko commented Aug 31, 2023

github-actions bot commented May 17, 2023 •

edited

Loading

github-actions bot commented May 17, 2023 •

edited

Loading

artem-blazhko May 24, 2023 •

edited

Loading

ssandupatla Jul 12, 2023 •

edited

Loading