refactor: clean up mock fixtures and re-enable tests for executions #130
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
Codecov Report
@@ Coverage Diff @@
## execution-data-refactor #130 +/- ##
===========================================================
+ Coverage 64.14% 73.16% +9.01%
===========================================================
Files 406 409 +3
Lines 6845 7158 +313
Branches 1106 1119 +13
===========================================================
+ Hits 4391 5237 +846
+ Misses 2454 1921 -533
Continue to review full report at Codecov.
|
podehnal
reviewed
Jan 4, 2021
| every(executions, nodeExecutionIsTerminal) && | ||
| executionIsTerminal(execution); | ||
| !executionIsTerminal(execution) || | ||
| some(executions, ne => !nodeExecutionIsTerminal(ne)); |
There was a problem hiding this comment.
nit: any reason to use the JS native ARRAY.some?
There was a problem hiding this comment.
Only that I wasn't aware it existed :-)
podehnal
reviewed
Jan 4, 2021
src/test/setupTests.ts
Outdated
| onUnhandledRequest: 'error' | ||
| onUnhandledRequest: (req) => { | ||
| const message = `Unexpected request: ${obj(req)}`; | ||
| console.error(message); |
There was a problem hiding this comment.
is this console.error statement expected?
There was a problem hiding this comment.
Oh I guess not. The thrown error will end up in the test output. I'll remove it.
podehnal
approved these changes
Jan 4, 2021
schottra
added a commit
that referenced
this pull request
Jan 4, 2021
…130) * test: adding test data for node executions * test: mocks and refactoring to re-enable NodeExecutionDetails tests * chore: lint error * test: getting first test for NE table working again * test: mocks and a couple of tests for NE table * refactor: msw handlers to use a backing map and return 404s * test: more tests for NE table * test: adding fixture for dynamic external workflow * test: using mock fixture for sub workflow tests * test: move remaining mocks to fixtures and fix tests * test: re-enabling more execution tests * fix: removing global query handlers for caching entitiesq * test: re-enable ExecutionNodeViews tests * fix: typo in import path * fix: show DataError by default for failed queries * chore: documentation * chore: pr feedback
schottra
added a commit
that referenced
this pull request
Jan 5, 2021
…130) * test: adding test data for node executions * test: mocks and refactoring to re-enable NodeExecutionDetails tests * chore: lint error * test: getting first test for NE table working again * test: mocks and a couple of tests for NE table * refactor: msw handlers to use a backing map and return 404s * test: more tests for NE table * test: adding fixture for dynamic external workflow * test: using mock fixture for sub workflow tests * test: move remaining mocks to fixtures and fix tests * test: re-enabling more execution tests * fix: removing global query handlers for caching entitiesq * test: re-enable ExecutionNodeViews tests * fix: typo in import path * fix: show DataError by default for failed queries * chore: documentation * chore: pr feedback
schottra
added a commit
that referenced
this pull request
Jan 6, 2021
* refactor: integrate react-query for execution data fetching (#123) * refactor: using react-query to load top level Execution * refactor: upgrading react-query and fixing execution termination * refactor: handle 401s on queries and do auth flow * refactor: adding conditional refresh for execution status * refactor: cleanup broken files after context refactor * chore: docs * refactor: Remove ExecutionDataCache in favor of react-query (#126) * refactor: first step of using queries for NE table * refactor: removing data cache from first layer of NE table * refactor: removing remaining execution data cache usage * refactor: rename QueryKey type and remove bug workaround * refactor: fixing remaining consumers of NEs * test: adds setup for mock-service-worker (#127) * test: add msw and basic handlers for a few types * test: add mock data for a basic workflow execution * test: fixing/removing tests after adding msw * test: throw on unexpected requests to msw * fix: upgrade TS to fix error and cleanup resulting errors * Migrate from TSLint to ESLint (#128) * ci: move from tslint->eslint * fix: addressing eslint errors * fix: remove passing of unused variable * ci: remove unnecessary prettier config * refactor: clean up mock fixtures and re-enable tests for executions (#130) * test: adding test data for node executions * test: mocks and refactoring to re-enable NodeExecutionDetails tests * chore: lint error * test: getting first test for NE table working again * test: mocks and a couple of tests for NE table * refactor: msw handlers to use a backing map and return 404s * test: more tests for NE table * test: adding fixture for dynamic external workflow * test: using mock fixture for sub workflow tests * test: move remaining mocks to fixtures and fix tests * test: re-enabling more execution tests * fix: removing global query handlers for caching entitiesq * test: re-enable ExecutionNodeViews tests * fix: typo in import path * fix: show DataError by default for failed queries * chore: documentation * chore: pr feedback * test: fixing storybook rendering for NE table * refactor: queries and loading states for (Task)ExecutionDetails * chore: cleanup unused code * fix: adds mock support for launch plans * test: update LoadingSpinner tests * fix: handle error case for NE children * chore: remove todo * fix: typo
service-github-lyft-semantic-release
added a commit
that referenced
this pull request
Jan 6, 2021
# [0.18.0](http://github.com/lyft/flyteconsole/compare/v0.17.8...v0.18.0) (2021-01-06) ### Features * improved data fetching for execution details page ([#131](http://github.com/lyft/flyteconsole/issues/131)) ([928f094](http://github.com/lyft/flyteconsole/commit/928f094bee6f84a34325eb1ab88c1ddf88d45541)), closes [#123](http://github.com/lyft/flyteconsole/issues/123) [#126](http://github.com/lyft/flyteconsole/issues/126) [#127](http://github.com/lyft/flyteconsole/issues/127) [#128](http://github.com/lyft/flyteconsole/issues/128) [#130](http://github.com/lyft/flyteconsole/issues/130)
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.
This PR fills out the mock support for executions and implements the remaining unit tests that needed refactoring.
createAdminServermodule. This uses a simple map-based store with keys being derived based on the entity type, url, and query string parameters. This mimics the way react-query keys its fetch operations (and in fact uses the same mechanism for generating query keys). This makes the server able to return proper 404 responses for test cases where we actually want an object to be missing, and gives more flexibility in the way we implement our mock response handlers. It also allows for using a shared endpoint handler for each entity type, instead of registering middleware matching an exact full url for every single object we want to be able to return.Executionbased on a specificWorkflow, aTaskExecutionfor a specific task, etc. This makes it easier to generate consistent objects which correctly imitate the way data works in a production environment.insertFixturefunction which will insert records into our mock server for all of the various entities in a test fixture object.stableStringifyfor use in hashing queries in the mock server.fixturesfolder containing logic to generate all the entities necessary for various use cases we need to test. These objects can be inserted into the mockServer with theinsertFixturehelper function.WaitForQueryhelper component and just returnnullif there is no data present. This saves an extra component in the tree and avoids a remount when the data is refreshing.useExecutionNodeViewsStateExecutionNodeViews,NodeExecutionDetails,NodeExecutionsTablegetCacheKeyDeepPartialhelper to add better type support for merge operations on mock objects.