Fix Execution View Test #612
Conversation
Signed-off-by: James <[email protected]>
Signed-off-by: James <[email protected]>
packages/zapp/console/src/components/Executions/ExecutionDetails/ExecutionDetails.tsx
Outdated
Show resolved
Hide resolved
packages/zapp/console/src/components/Executions/ExecutionDetails/ExecutionNodeViews.tsx
Outdated
Show resolved
Hide resolved
packages/zapp/console/src/components/Executions/ExecutionDetails/Timeline/ExecutionTimeline.tsx
Outdated
Show resolved
Hide resolved
packages/zapp/console/src/components/Executions/Tables/NodeExecutionsTable.tsx
Outdated
Show resolved
Hide resolved
| const filterState = useNodeExecutionFiltersState(); | ||
|
|
||
| const showUnknownNodes = useMemo<Boolean>(() => { | ||
| if ( |
There was a problem hiding this comment.
nit can you add a comment for intent here? you're checking the "Active" filter right? Can you write a generic function that retrieves a particular filter from a list, so this doesn't regress in case we change the filters?
| if (nodeExecutions.length > 0) { | ||
| fetchData(nodeExecutions, queryClient); | ||
| } | ||
| fetchData(nodeExecutions, queryClient); |
There was a problem hiding this comment.
Why did we get rid of the length check?
| return <LoadingComponent />; | ||
| } | ||
| const renderTab = (tabType) => { | ||
| if (loading) { |
There was a problem hiding this comment.
nit: this is hard to read, can we rename loading to nodeExecutionsLoading or something?
| <WaitForQuery | ||
| errorComponent={DataError} | ||
| query={childGroupsQuery} | ||
| loadingComponent={LoadingComponent} |
There was a problem hiding this comment.
if the WaitForQuery component already takes in a loading component, why do we need the extra check on line 148?
packages/zapp/console/src/components/Executions/Tables/test/NodeExecutionsTable.test.tsx
Show resolved
Hide resolved
yes. I'm on it |
Signed-off-by: James <[email protected]>
packages/zapp/console/src/components/Executions/Tables/test/NodeExecutionsTable.test.tsx
Outdated
Show resolved
Hide resolved
packages/zapp/console/src/components/Executions/Tables/test/NodeExecutionsTable.test.tsx
Outdated
Show resolved
Hide resolved
Signed-off-by: James <[email protected]>
Signed-off-by: James <[email protected]>
Signed-off-by: James <[email protected]>
Codecov Report
@@ Coverage Diff @@
## devmain #612 +/- ##
===========================================
- Coverage 67.35% 66.23% -1.12%
===========================================
Files 398 441 +43
Lines 9296 10440 +1144
Branches 1590 1793 +203
===========================================
+ Hits 6261 6915 +654
- Misses 3035 3525 +490
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Signed-off-by: Olga Nad <[email protected]>
…to james/execution-view-test-2
Signed-off-by: Olga Nad <[email protected]>
Fixing the failing tests for the NodeExecutionsTable
Type
Are all requirements met?
Complete description
Except the NodeExecutionsTable tests, this PR fixes the filters on the table to apply for future nodes that are manually added.
Tracking Issue
fixes flyteorg/flyte#604