Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Add UT for Detector List page #279

Merged
merged 1 commit into from
Aug 14, 2020

Conversation

ohltyler
Copy link
Contributor

@ohltyler ohltyler commented Aug 12, 2020

Issue #, if available: #191

Description of changes:

This PR adds several unit tests throughout the DetectorsList/ directory to increase code coverage and handle more test cases. Below are the before/after of the code coverage.

Before:

-----------------------------------------------------------------|----------|----------|----------|----------|-------------------|
File                                                             |  % Stmts | % Branch |  % Funcs |  % Lines | Uncovered Line #s |
-----------------------------------------------------------------|----------|----------|----------|----------|-------------------|
 public/pages/DetectorsList/components/EmptyMessage              |      100 |      100 |      100 |      100 |                   |
  EmptyMessage.tsx                                               |      100 |      100 |      100 |      100 |                   |
 public/pages/DetectorsList/components/ListActions               |       80 |      100 |    66.67 |       80 |                   |
  ListActions.tsx                                                |       80 |      100 |    66.67 |       80 |                56 |
 public/pages/DetectorsList/components/ListFilters               |      100 |      100 |      100 |      100 |                   |
  ListFilters.tsx                                                |      100 |      100 |      100 |      100 |                   |
 public/pages/DetectorsList/containers/ConfirmActionModals       |    89.74 |    88.46 |    91.67 |    89.74 |                   |
  ConfirmDeleteDetectorsModal.tsx                                |    81.82 |    85.29 |    85.71 |    81.82 |   168,170,171,175 |
  ConfirmStartDetectorsModal.tsx                                 |      100 |      100 |      100 |      100 |                   |
  ConfirmStopDetectorsModal.tsx                                  |      100 |    91.67 |      100 |      100 |                56 |
 public/pages/DetectorsList/containers/ConfirmActionModals/utils |      100 |    91.67 |    78.57 |      100 |                   |
  helpers.tsx                                                    |      100 |    91.67 |    78.57 |      100 |               117 |
 public/pages/DetectorsList/containers/List                      |    54.04 |    38.78 |    40.74 |    55.19 |                   |
  List.tsx                                                       |    54.04 |    38.78 |    40.74 |    55.19 |... 54,565,577,590 |
 public/pages/DetectorsList/utils                                |    72.97 |    66.67 |    66.67 |    72.22 |                   |
  constants.ts                                                   |      100 |      100 |      100 |      100 |                   |
  helpers.ts                                                     |    56.52 |    60.87 |       50 |    54.55 |... 99,100,101,104 |
  tableUtils.tsx                                                 |      100 |      100 |      100 |      100

After:

-----------------------------------------------------------------|----------|----------|----------|----------|-------------------|
File                                                             |  % Stmts | % Branch |  % Funcs |  % Lines | Uncovered Line #s |
-----------------------------------------------------------------|----------|----------|----------|----------|-------------------|
 public/pages/DetectorsList/components/EmptyMessage              |      100 |      100 |      100 |      100 |                   |
  EmptyMessage.tsx                                               |      100 |      100 |      100 |      100 |                   |
 public/pages/DetectorsList/components/ListActions               |       80 |      100 |    66.67 |       80 |                   |
  ListActions.tsx                                                |       80 |      100 |    66.67 |       80 |                56 |
 public/pages/DetectorsList/components/ListFilters               |      100 |      100 |      100 |      100 |                   |
  ListFilters.tsx                                                |      100 |      100 |      100 |      100 |                   |
 public/pages/DetectorsList/containers/ConfirmActionModals       |    94.87 |    92.31 |    91.67 |    94.87 |                   |
  ConfirmDeleteDetectorsModal.tsx                                |    90.91 |    91.18 |    85.71 |    90.91 |           170,171 |
  ConfirmStartDetectorsModal.tsx                                 |      100 |      100 |      100 |      100 |                   |
  ConfirmStopDetectorsModal.tsx                                  |      100 |    91.67 |      100 |      100 |                56 |
 public/pages/DetectorsList/containers/ConfirmActionModals/utils |      100 |    91.67 |    78.57 |      100 |                   |
  helpers.tsx                                                    |      100 |    91.67 |    78.57 |      100 |               117 |
 public/pages/DetectorsList/containers/List                      |    63.35 |    53.06 |     46.3 |    64.94 |                   |
  List.tsx                                                       |    63.35 |    53.06 |     46.3 |    64.94 |... 27,536,543,590 |
 public/pages/DetectorsList/utils                                |      100 |      100 |      100 |      100 |                   |
  constants.ts                                                   |      100 |      100 |      100 |      100 |                   |
  helpers.ts                                                     |      100 |      100 |      100 |      100 |                   |
  tableUtils.tsx                                                 |      100 |      100 |      100 |      100 |                   |

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@ohltyler ohltyler added the test fix/enhance testing label Aug 12, 2020
Copy link
Contributor

@yizheliu-amazon yizheliu-amazon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool. Thanks for the change. Btw, is it possible to increase coverage for public/pages/DetectorsList/containers/List

@ohltyler
Copy link
Contributor Author

Cool. Thanks for the change. Btw, is it possible to increase coverage for public/pages/DetectorsList/containers/List

Yeah, so I spent way too long trying to cover more test cases there, but ran into many issues with trying to keep a mock store of detectors and indices in addition to mocking http responses. Basically, the mock store cannot be dynamically changed within a test case, and the way the page currently renders is retrieve the store -> change a filter -> fetch new results from ES -> update store -> re-render page after store is updated. This is impossible to achieve since the store can't be updated, and the page gets stuck in a loading state.

As far as unit tests go, testing the page as a whole involves too many moving pieces I'd say. The parts of code that are not being covered here are being covered in the integration tests (filtering by detector state, etc.).

@yizheliu-amazon
Copy link
Contributor

Cool. Thanks for the change. Btw, is it possible to increase coverage for public/pages/DetectorsList/containers/List

Yeah, so I spent way too long trying to cover more test cases there, but ran into many issues with trying to keep a mock store of detectors and indices in addition to mocking http responses. Basically, the mock store cannot be dynamically changed within a test case, and the way the page currently renders is retrieve the store -> change a filter -> fetch new results from ES -> update store -> re-render page after store is updated. This is impossible to achieve since the store can't be updated, and the page gets stuck in a loading state.

As far as unit tests go, testing the page as a whole involves too many moving pieces I'd say. The parts of code that are not being covered here are being covered in the integration tests (filtering by detector state, etc.).

Covering in IT sounds good to me.

@ohltyler ohltyler merged commit 9e46462 into opendistro-for-elasticsearch:master Aug 14, 2020
@ohltyler ohltyler deleted the addUT branch August 14, 2020 18:53
yizheliu-amazon pushed a commit that referenced this pull request Aug 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
test fix/enhance testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants