ui: Tests on react-routing components #44078
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This change is |
koorosh
force-pushed
the
ui-routing-tests
branch
from
72c36a5 to
2d074ee
Compare
koorosh
force-pushed
the
ui-routing-tests
branch
from
2b1226a to
32a7ca7
Compare
koorosh
changed the title
With current changes, react is upgraded to latest version (16.12.0) and following libraries are upgraded as well: react-redux, redux, redux-saga. These upgrades of major versions has required to do some code adjustments to properly migrate to newest versions. - redux. Provide valid type arguments for Dispatch type. - redux-saga. Changed import and usage of 'delay' function. - react-redux. `connect` function failed to correctly resolve returned type for `mapDispatchToProps` argument. As result it is provided as a function instead of object argument. This hack allows implicitly resolve types. - `CachedDataReducer` class is extended with one more optional type argument to specify the list of allowed `actionNamespace` literals instead of `string`. It was necessary to have strictly defined CachedDataReducer interface. - `normalizeConnectedComponent` function is used to do simple mapping from react's ExoticComponent to valid react component. This resolves issue of providing ConnectedComponents (which are ExoticComponent's, not regular react components) to Route component which has strict type validation of component types. Release note: None
- tests command is disabled for now because current state doesn't contain all fixes to run tests without failure. - Decommissioned Node History page was added so code has to be adjusted. Release note: None
Release note: None
- Decoupled App from rendering app into DOM. - Defined tests structure and initial tests. Release note: None
- Karma produced warning messages related to compilation of .ts|.tsx files to .js due to incorrect webpack configuration. - Added macha reporter to get readable test reports. Release note: None
- Tests are grouped in suites per route to maintain related tests per route; - There is two kinds of tests which test correct rendering of component for specific test and another test which validates correct redirection for routes; - Tests can be refactored to be constructed from configuration to reduce repeated tests, but it will be actual after migrating to latest version of react-router lib. - Added additional exports for components which is wrapped by Connect HOC because connect doesn't return valid react component, as result `enzym` library cannot find wrapped component in mounted components tree. This can be changed back with latest version of `react-router` + `react-connected-router` library which don't have this issue. Release note: None
Release note: None
Change back subscription to alertDataSync out of App context so it doesn't break existing behaviour. Release note: None
Release note: None
Release note: None
Release note: None
Release note: None
Release note: None
- Wrap NodeCanvasContainer and statementsPage components with `bindActionCreators` - Add route for DecommissionedNodeHistory page Release note: None
Release note: None
…ranch Release note: None
Testing with Enzyme framework is straight-froward and easy, but there is once
gotcha: ConnectedComponents (wrapped with Connect function) are not recognized
by Enzyme as react components and it is not possible to use enzymeWrapper.find(Component)
helper function to check whether connected component is rendered in DOM or not.
To make it clear there is an example how Connected Component is recognized in dom:
```
<[object Object] location={{...}} params={{...}} route={{...}} ...
```
as [object] not a Component.
To overcome this problem "not connected" child components are exported in
addition to connected. It allows to write tests against these components.
Release note: None
It is not necessary to wrap action creator object with function when connecting it to component. Release note: None
- Remove unnecessary re-exporting from index.ts file - Simplify exporting of DecommissionedNodeList component Release note: None
Release note: None
koorosh
force-pushed
the
ui-routing-tests
branch
from
32a7ca7 to
043019e
Compare
dhartunian
approved these changes
Jan 30, 2020
|
bors r+ |
Build failed (retrying...) |
craig bot
pushed a commit
that referenced
this pull request
Jan 30, 2020
44078: ui: Tests on react-routing components r=dhartunian a=Koorosh Depends on PR: #43846 `yarn-vendor` PR has to be merged: cockroachdb/yarn-vendored#15 Current PR includes minimal changes to functionality (only those which are required to isolate or get access to individual components under tests): - `src/index.tsx` file has been refactored into two parts: rendering to DOM and independent `App` component; - <Helmet> component (`react-helmet` library) has been refactored to avoid known issue during tests execution (nfl/react-helmet#373) - old version of react-router and `connect` component in tandem do not work well because connected component aren't valid react components. As result validation for correct routing was done by re-exporting wrapped components (instead of connected). It allowed to simplify testing with `enzyme#find` function which requires valid react component as an argument; - Tests are grouped in suites per route to maintain related tests per route; - There is two kinds of tests which test correct rendering of component for specific test and another test which validates correct redirection for routes; - Tests can be refactored to be constructed from configuration to reduce repeated tests, but it will be actual after migrating to latest version of react-router lib. 44545: sqlbase: add table generator for edge case datums r=rohany a=rohany Part of work for #44322. This PR adds a testing utility to generate a table full of edge case datums to test a variety of things in the future. Release note: None Co-authored-by: Andrii Vorobiov <[email protected]> Co-authored-by: Rohan Yadav <[email protected]>
Build succeeded |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Depends on PR: #43846
yarn-vendorPR has to be merged: cockroachdb/yarn-vendored#15Current PR includes minimal changes to functionality (only those which are required to isolate or get
access to individual components under tests):
src/index.tsxfile has been refactored into two parts: rendering to DOM and independentAppcomponent;react-helmetlibrary) has been refactored to avoid known issue during tests execution (Uncaught RangeError: Maximum call stack size exceeded nfl/react-helmet#373)connectcomponent in tandem do not work well because connectedcomponent aren't valid react components. As result validation for correct routing was done by re-exporting wrapped components (instead of connected). It allowed to simplify testing with
enzyme#findfunction which requires valid react component as an argument;specific test and another test which validates correct redirection for routes;
tests, but it will be actual after migrating to latest version of react-router lib.