Spike: eHoldings - Investigate areas that need refactoring for Jest + RTL testing
- Denys Bohdan
UIEH-980 - Getting issue details... STATUS
Results of Владислав Велицкий review of ui-eholdings code in scope of UIEH-980 - Getting issue details... STATUS
Overview
During replacement proposed to create new test branch where team would write tests on jest library. After completing bigtest test would be removed.
Module has a lot of components, utils, hooks, redux support files which needed to be tested. It needs to create complex stories. There are list of all components below which must be tested. Complexity means that 1 is easy to use component and 3 is hard and need more time to test it.
Tests for actions, epic, reducers would be replaced quickly because libraries had same flow. Just need to move, for example: to.be.equal() => .toEqual()
Components from other libraries should be written in __mock__ folder. There are some of them presented here.
Information about Usage Consolidation feature was not included to this PR because this feature isn't available on master branch and all tests for it could be made in a scope of one task after merging
Components which should be refactored
| Component | Description | Сomplexity | Test status | Preliminary component test estimation | Estimation grouped |
|---|---|---|---|---|---|
| AccessTypeDisplay | - | 1 | ✔️ | ||
| AccessTypeEditSection | - | 1 | - | 1 | |
| AccessTypeSelect | - | 1 | - | 1 | |
| CustomLabelEditSection | - | 1 | - | 1 | |
| CustomLabelShowSection | - | 1 | - | 1 | |
| AccordionListHeader | suggestions: - move it so separate folder - move constants to constants folder - check propTypes | 1 | - | 1 | |
| 3 | |||||
| DetailsViewSection | - | 1 | - | 1 | |
| ApiLimitExceededErrorScreen | - | 1 | - | 1 | |
| FailedBackendErrorScreen | - | 1 | - | 1 | |
| InvalidBackendErrorScreen | - | 1 | - | 1 | |
| NoBackendErrorScreen | - | 1 | - | 1 | |
| UserNotAssignedToKbErrorScreen | - | 1 | - | 1 | |
| ExternalLink | - | 1 | - | 1 | |
| HiddenLabel | - | 1 | - | 1 | |
| KeyValueColumns | - | 1 | - | 1 | |
| List | - | 1 | - | 1 | |
| NoResultMessage | - | 1 | - | 1 | |
| PackageContentTypeField | - | 1 | - | 1 | |
| PackageNameField | - | 1 | - | 1 | |
| 3 | |||||
| PackageEdit | - | 1 | - | 1 | |
| ProxySelectField | - | 1 | - | 1 | |
| QueryNotFound | - | 1 | - | 1 | |
| CustomUrlFields | There are two same components - in resource and in title pages. Can remove code duplication | 1 | - | 1 | |
| SearchBadge | - | 1 | - | 1 | |
| 3 | |||||
| SearchPackageListItem | - | 1 | - | 1 | |
| RootProxySelectField | - | 1 | - | 1 | |
| TagsLabel | - | 1 | - | 1 | |
| AddTitleToPackage | - | 1 | - | 1 | |
| 3 | |||||
| CustomUrlFields | - | 1 | - | 1 | |
| DescriptionField | - | 1 | - | 1 | |
| EditionField | - | 1 | - | 1 | |
| TitleNameField | - | 1 | - | 1 | |
| PackageSelectField | - | 1 | - | 1 | |
| 3 | |||||
| PeerReviewedField | - | 1 | - | 1 | |
| PublicationTypeField | - | 1 | - | 1 | |
| PublisherNameField | - | 1 | - | 1 | |
| TitleListItem | - | 1 | - | 1 | |
| Toast | - | 1 | - | 1 | |
| 3 | |||||
| Toaster | - | 1 | - | 1 | |
| ToggleSwitch | the component has not been used, it is necessary to check whether it can be removed | 1 | - | 1 | |
| TokenField | - | 1 | - | 1 | |
| ContributorsList | - | 1 | - | 1 | |
| IdentifiersList | - | 1 | - | 1 | |
| 3 | |||||
| InternalLink | - | 1 | - | 1 | |
| ProxyDisplay | - | 1 | - | 1 | |
| shouldFocus | - | 1 | - | 1 | |
| TokenDisplay | - | 1 | - | 1 | |
| EHoldings | move from index.js to separate file and import it to index.js, it could help to keep project structure (one component consist of js file with code, index.js and test file) | 2 | - | 1 | |
| 3 | |||||
AgreementsList | - | 2 | - | 2 | 2 |
| CoverageDateList | component has containsNonEmptyObjectsWithStringValues function which is better to refactor | 1 | - | 2 | 2 |
| NavigationModal | - | 1 | - | 2 | 2 |
| PackageCoverageFields | could be splitted to smallest ones | 2 | - | 2 | 2 |
| SelectionStatus | have a lot of functions which is better to remove to separate file | 2 | - | 2 | 2 |
| ProviderListItem | - | 2 | - | 2 | 2 |
| QueryList | - | 1 | - | 2 | 2 |
| CoverageStatementFields | - | 1 | - | 2 | |
| VisibilityField | - | 1 | - | 2 | |
| 3 | |||||
| SearchPaneset | - | 2 | - | 2 | 2 |
| SettingsForm | - | 2 | - | 2 | 2 |
| TagsAccrodion | - | 1 | - | 2 | 2 |
| ContributorField | - | 2 | - | 2 | 2 |
| IdentifiersFields | - | 1 | - | 2 | 2 |
| TitleCreate | already refactored, could be used as an example | 2 | - | 2 | 2 |
| TitleEdit | already refactored, could be used as an example | 2 | - | 2 | 2 |
| PackageSearchList | identical to TitleSearchList, ProviderSearchList | 1 | - | 2 | |
| ProviderSearchList | identical to PackageSearchList, TitleSearchList | 1 | - | 2 | |
| TitleSearchList | identical to PackageSearchList, ProviderSearchList | 1 | - | 2 | |
| 3 | |||||
| AgreementsAccordion | - | 2 | - | 3 | 3 |
| PackageCreate | - | 2 | - | 3 | 3 |
| ResourceCoverageFields | - | 2 | - | 3 | 3 |
| CustomEmbargoFields | could be splitted | 2 | - | 3 | 3 |
| SearchFilters | - | 2 | - | 3 | 3 |
| CustomLabelField | - | 2 | - | 3 | 3 |
| SettingsCustomLabels | - | 3 | - | 3 | 3 |
| SettingsRootProxy | - | 2 | - | 3 | 3 |
| Settings | - | 3 | - | 3 | 3 |
| Tags | - | 2 | - | 3 | 3 |
| PackageFilterModal | - | 2 | - | 3 | 3 |
| DetailsView | it is better to split the component into several smaller ones | 3 | - | 5 | 5 |
| ProviderEdit | - | 2 | - | 5 | 5 |
| ResourceEdit | it is better to split the component into several smaller ones, at least each accordion could make component more readable and easier to testing | 3 | - | 5 | 5 |
| ScrollView | - | 2 | - | 5 | 5 |
| SearchForm | it is better to split the component into several smaller ones, at least each accordion could make component more readable and easier to testing | 3 | - | 5 | 5 |
| SearchModal | - | 2 | - | 5 | 5 |
| SettingsAccessStatusTypes | - | 3 | - | 5 | 5 |
| SettingsKnowledgeBase | suggestions: - check constants | 3 | - | 5 | 5 |
| SettingsAssignedUsers | - | 3 | - | 5 | 5 |
| ManagedPackageEdit | it is better to split the component into several smaller ones, at least each accordion could make component more readable and easier to testing | 3 | - | 8? | 8 |
| CustomPackageEdit | it is better to split the component into several smaller ones, at least each accordion could make component more readable and easier to testing | 3 | - | 8? | 8 |
| ProviderShow | it is better to split the component into several smaller ones, at least each accordion could make component more readable and easier to testing | 3 | - | 8? | 8 |
| PackageShow | it is better to split the component into several smaller ones, at least each accordion could make component more readable and easier to testing | 3 | - | 8? | 8 |
| ResourceEditCustomTitle | it is better to split the component into several smaller ones, at least each accordion could make component more readable and easier to testing | 3 | - | 8? | 8 |
| ResourceEditManagedTitle | it is better to split the component into several smaller ones, at least each accordion could make component more readable and easier to testing | 3 | - | 8? | 8 |
| ResourceShow | it is better to split the component into several smaller ones, at least each accordion could make component more readable and easier to testing | 3 | - | 8? | 8 |
| TitleShow | it is better to split the component into several smaller ones, at least each accordion could make component more readable and easier to testing | 3 | - | 8 | 8 |
| InlineForm | the component has not been used, it is necessary to check whether it can be removed | 1 | - | 0 | |
| ResultPane | the component has not been used, it is necessary to check whether it can be removed | 1 | - | 0 | |
| SearchPane | the component has not been used, it is necessary to check whether it can be removed | 1 | - | 0 | |
| PackageSearchFilters | - | 1 | - | 0 | |
| ProviderSearchFilters | - | 1 | - | 0 | |
| TitleSearchFilters | identical to PackageSearchList, ProviderSearchList | 1 | - | 0 | |
| PaneHeaderButton | the component has not been used, it is necessary to check whether it can be removed | 1 | - | 0 | |
| Route | - | 1 | - | 0 | |
| SearchLabel | - | 1 | - | ||
| Total: | 227 | 200 |
Learning
"no-redeclare"eslint rule was temporary excluded, because two libraries overlap, which is why more errors are thrown out. This rule could be move out after full replacement.
eHoldings components dependency diagram
On the diagram above you can see that the topmost components, that are composed of smaller components are located on the left side of the image. And the smaller components are located on the right side.
I'd suggest to move from right to left as it will allow us to gradually test and/or refactor components that are simpler.