From 546b3b6d562021bbe2a60182b467367d91190c77 Mon Sep 17 00:00:00 2001 From: Carina Ursu Date: Fri, 14 Apr 2023 13:36:31 -0700 Subject: [PATCH] chore: more tests Signed-off-by: Carina Ursu --- .../ExecutionDetails/ExecutionTab.tsx | 2 +- .../test/ExecutionNodeViews.test.tsx | 140 +++++++++++++++--- .../NodeExecutionDetailsPanelContent.test.tsx | 13 +- .../ExecutionDetails/test/TaskNames.test.tsx | 7 + .../Executions/Tables/NodeExecutionsTable.tsx | 124 +++++++++------- .../Tables/test/NodeExecutionsTable.test.tsx | 11 +- .../test/PausedTasksComponent.test.tsx | 12 +- 7 files changed, 217 insertions(+), 92 deletions(-) diff --git a/packages/console/src/components/Executions/ExecutionDetails/ExecutionTab.tsx b/packages/console/src/components/Executions/ExecutionDetails/ExecutionTab.tsx index 50d235097..865220744 100644 --- a/packages/console/src/components/Executions/ExecutionDetails/ExecutionTab.tsx +++ b/packages/console/src/components/Executions/ExecutionDetails/ExecutionTab.tsx @@ -32,7 +32,7 @@ export const ExecutionTab: React.FC = ({ tabType }) => {
{tabType === tabs.nodes.id && ( - + )} {tabType === tabs.graph.id && } {tabType === tabs.timeline.id && } diff --git a/packages/console/src/components/Executions/ExecutionDetails/test/ExecutionNodeViews.test.tsx b/packages/console/src/components/Executions/ExecutionDetails/test/ExecutionNodeViews.test.tsx index eb2302db5..fbd47a964 100644 --- a/packages/console/src/components/Executions/ExecutionDetails/test/ExecutionNodeViews.test.tsx +++ b/packages/console/src/components/Executions/ExecutionDetails/test/ExecutionNodeViews.test.tsx @@ -1,21 +1,26 @@ -import { fireEvent, render, waitFor } from '@testing-library/react'; +import * as React from 'react'; +import { fireEvent, render, waitFor, screen } from '@testing-library/react'; import { filterLabels } from 'components/Executions/filters/constants'; import { nodeExecutionStatusFilters } from 'components/Executions/filters/statusFilters'; import { oneFailedTaskWorkflow } from 'mocks/data/fixtures/oneFailedTaskWorkflow'; import { insertFixture } from 'mocks/data/insertFixture'; import { mockServer } from 'mocks/server'; import { Execution } from 'models/Execution/types'; -import * as React from 'react'; import { QueryClient, QueryClientProvider } from 'react-query'; import { createTestQueryClient } from 'test/utils'; import { ExecutionContext } from 'components/Executions/contexts'; -import { tabs } from '../constants'; +import { listNodeExecutions, listTaskExecutions } from 'models/Execution/api'; +import { NodeExecutionPhase } from 'models'; +import { mockWorkflowId } from 'mocks/data/fixtures/types'; +import { NodeExecutionDetailsContext } from 'components/Executions/contextProvider/NodeExecutionDetails'; +import { transformerWorkflowToDag } from 'components/WorkflowGraph/transformerWorkflowToDag'; import { ExecutionNodeViews } from '../ExecutionNodeViews'; +import { tabs } from '../constants'; jest.mock('components/Executions/Tables/NodeExecutionRow', () => ({ - NodeExecutionRow: jest.fn(({ nodeExecution }) => ( + NodeExecutionRow: jest.fn(({ node }) => (
- {nodeExecution?.id?.nodeId} + {node?.execution?.id?.nodeId}
)), })); @@ -40,6 +45,14 @@ jest.mock( NodeExecutionName: jest.fn(({ name }) =>
{name}
), }), ); +jest.mock('models/Execution/api', () => ({ + listNodeExecutions: jest.fn(), + listTaskExecutions: jest.fn(), +})); + +jest.mock('components/WorkflowGraph/transformerWorkflowToDag', () => ({ + transformerWorkflowToDag: jest.fn(), +})); // ExecutionNodeViews uses query params for NE list, so we must match them // for the list to be returned properly @@ -53,26 +66,87 @@ describe('ExecutionNodeViews', () => { let queryClient: QueryClient; let execution: Execution; let fixture: ReturnType; - beforeEach(() => { fixture = oneFailedTaskWorkflow.generate(); execution = fixture.workflowExecutions.top.data; insertFixture(mockServer, fixture); const nodeExecutions = fixture.workflowExecutions.top.nodeExecutions; - - mockServer.insertNodeExecutionList( - execution.id, - Object.values(nodeExecutions).map(({ data }) => data), - baseQueryParams, - ); - mockServer.insertNodeExecutionList( - execution.id, - [nodeExecutions.failedNode.data], - { - ...baseQueryParams, - filters: 'value_in(phase,FAILED)', - }, + const nodeExecutionsArray = Object.values(nodeExecutions).map( + ({ data }) => data, ); + transformerWorkflowToDag.mockImplementation(_ => { + return { + dag: { + id: 'start-node', + scopedId: 'start-node', + value: { + id: 'start-node', + }, + type: 4, + name: 'start', + nodes: [ + { + id: 'start-node', + scopedId: 'start-node', + value: { + inputs: [], + upstreamNodeIds: [], + outputAliases: [], + id: 'start-node', + }, + type: 4, + name: 'start', + nodes: [], + edges: [], + }, + { + id: 'end-node', + scopedId: 'end-node', + value: { + inputs: [], + upstreamNodeIds: [], + outputAliases: [], + id: 'end-node', + }, + type: 5, + name: 'end', + nodes: [], + edges: [], + }, + ...nodeExecutionsArray.map(n => ({ + id: n.id.nodeId, + scopedId: n.scopedId, + execution: n, + // type: dTypes.gateNode, + name: n.id.nodeId, + type: 3, + nodes: [], + edges: [], + })), + ], + }, + staticExecutionIdsMap: {}, + }; + }); + listNodeExecutions.mockImplementation((_, filters) => { + let finalNodes = nodeExecutionsArray; + if (filters?.filter?.length) { + const phases = filters?.filter + ?.filter(f => f.key === 'phase')?.[0] + .value?.map(f => NodeExecutionPhase[f]); + finalNodes = finalNodes.filter(n => { + return phases.includes(n.closure.phase); + }); + } + return Promise.resolve({ + entities: Object.values(finalNodes), + }); + }); + listTaskExecutions.mockImplementation(() => { + return Promise.resolve({ + entities: [], + }); + }); queryClient = createTestQueryClient(); }); @@ -80,7 +154,15 @@ describe('ExecutionNodeViews', () => { render( - + + + , ); @@ -90,7 +172,7 @@ describe('ExecutionNodeViews', () => { const failedNodeName = nodeExecutions.failedNode.data.id.nodeId; const succeededNodeName = nodeExecutions.pythonNode.data.id.nodeId; - const { getByText, queryByText, getByLabelText } = renderViews(); + const { getByText, queryByText, queryAllByTestId } = renderViews(); await waitFor(() => getByText(tabs.nodes.label)); @@ -99,21 +181,31 @@ describe('ExecutionNodeViews', () => { // Ensure we are on Nodes tab await fireEvent.click(nodesTab); + await waitFor(() => { + const nodes = queryAllByTestId('node-execution-row'); + return nodes?.length === 2; + }); + await waitFor(() => queryByText(succeededNodeName)); const statusButton = await waitFor(() => getByText(filterLabels.status)); // Apply 'Failed' filter and wait for list to include only the failed item await fireEvent.click(statusButton); + const failedFilter = await waitFor(() => - getByLabelText(nodeExecutionStatusFilters.failed.label), + screen.getByLabelText(nodeExecutionStatusFilters.failed.label), ); // Wait for succeeded task to disappear and ensure failed task remains await fireEvent.click(failedFilter); - await waitFor(() => queryByText(failedNodeName)); + await waitFor(() => { + const nodes = queryAllByTestId('node-execution-row'); + return nodes?.length === 1; + }); expect(queryByText(succeededNodeName)).not.toBeInTheDocument(); + expect(queryByText(failedNodeName)).toBeInTheDocument(); // Switch to the Graph tab @@ -121,7 +213,9 @@ describe('ExecutionNodeViews', () => { await fireEvent.click(timelineTab); await waitFor(() => queryByText(succeededNodeName)); + // expect all initital nodes to be rendered expect(queryByText(succeededNodeName)).toBeInTheDocument(); + expect(queryByText(failedNodeName)).toBeInTheDocument(); // Switch back to Nodes Tab and verify filter still applied await fireEvent.click(nodesTab); diff --git a/packages/console/src/components/Executions/ExecutionDetails/test/NodeExecutionDetailsPanelContent.test.tsx b/packages/console/src/components/Executions/ExecutionDetails/test/NodeExecutionDetailsPanelContent.test.tsx index 57dd268f1..4d41a184b 100644 --- a/packages/console/src/components/Executions/ExecutionDetails/test/NodeExecutionDetailsPanelContent.test.tsx +++ b/packages/console/src/components/Executions/ExecutionDetails/test/NodeExecutionDetailsPanelContent.test.tsx @@ -12,11 +12,14 @@ import { MemoryRouter } from 'react-router'; import { createTestQueryClient } from 'test/utils'; import { NodeExecutionDetailsPanelContent } from '../NodeExecutionDetailsPanelContent'; -jest.mock('components/Executions/ExecutionDetails/ExecutionDetailsActions', () => ({ - ExecutionDetailsActions: jest.fn(() => ( -
- )) -})); +jest.mock( + 'components/Executions/ExecutionDetails/ExecutionDetailsActions', + () => ({ + ExecutionDetailsActions: jest.fn(() => ( +
+ )), + }), +); jest.mock('components/Workflow/workflowQueries'); const { fetchWorkflow } = require('components/Workflow/workflowQueries'); diff --git a/packages/console/src/components/Executions/ExecutionDetails/test/TaskNames.test.tsx b/packages/console/src/components/Executions/ExecutionDetails/test/TaskNames.test.tsx index 67ac4c53c..0bc2d0f16 100644 --- a/packages/console/src/components/Executions/ExecutionDetails/test/TaskNames.test.tsx +++ b/packages/console/src/components/Executions/ExecutionDetails/test/TaskNames.test.tsx @@ -76,7 +76,14 @@ describe('ExecutionDetails > Timeline > TaskNames', () => { {}, + setShouldUpdate: () => {}, + shouldUpdate: false, }} > diff --git a/packages/console/src/components/Executions/Tables/NodeExecutionsTable.tsx b/packages/console/src/components/Executions/Tables/NodeExecutionsTable.tsx index 7a7623c3b..7dcea752c 100644 --- a/packages/console/src/components/Executions/Tables/NodeExecutionsTable.tsx +++ b/packages/console/src/components/Executions/Tables/NodeExecutionsTable.tsx @@ -21,7 +21,7 @@ import { useNodeExecutionsById, } from '../contextProvider/NodeExecutionDetails'; import { NodeExecutionRow } from './NodeExecutionRow'; -import { ExecutionFiltersState, useNodeExecutionFiltersState } from '../filters/useExecutionFiltersState'; +import { ExecutionFiltersState } from '../filters/useExecutionFiltersState'; import { searchNode } from '../utils'; import { nodeExecutionPhaseConstants } from '../constants'; import { NodeExecutionDynamicProvider } from '../contextProvider/NodeExecutionDetails/NodeExecutionDynamicProvider'; @@ -36,9 +36,10 @@ const mergeOriginIntoNodes = (target: dNode[], origin: dNode[]) => { if (!target?.length) { return target; } + const originClone = cloneDeep(origin); const newTarget = cloneDeep(target); newTarget?.forEach(value => { - const originalNode = origin.find( + const originalNode = originClone.find( og => og.id === value.id && og.scopedId === value.scopedId, ); const newNodes = mergeOriginIntoNodes( @@ -81,12 +82,12 @@ const filterNodes = ( let initialClone = cloneDeep(initialNodes); - initialClone.forEach(n => { + for (const n of initialClone) { n.nodes = filterNodes(n.nodes, nodeExecutionsById, appliedFilters); - }); + } initialClone = initialClone.filter(node => { - const hasFilteredChildren = node.nodes?.length; + const hasFilteredChildren = !!node.nodes?.length; const shouldBeIncluded = executionMatchesPhaseFilter( nodeExecutionsById[node.scopedId], appliedFilters[0], @@ -103,7 +104,7 @@ const filterNodes = ( return initialClone; }; -const isPhaseFilter = (appliedFilters: FilterOperation[]) => { +const isPhaseFilter = (appliedFilters: FilterOperation[] = []) => { if (appliedFilters.length === 1 && appliedFilters[0].key === 'phase') { return true; } @@ -115,53 +116,53 @@ const isPhaseFilter = (appliedFilters: FilterOperation[]) => { * NodeExecutions are expandable and will potentially render a list of child * TaskExecutions */ -export const NodeExecutionsTable: React.FC<{filterState: ExecutionFiltersState}> = ({filterState}) => { +export const NodeExecutionsTable: React.FC<{ + filterState: ExecutionFiltersState; +}> = ({ filterState }) => { const commonStyles = useCommonStyles(); const tableStyles = useExecutionTableStyles(); + const columnStyles = useColumnStyles(); + const { execution } = useContext(ExecutionContext); const { appliedFilters } = filterState; + const [filteredNodeExecutions, setFilteredNodeExecutions] = + useState(); const { nodeExecutionsById, initialDNodes: initialNodes } = useNodeExecutionsById(); + const [filters, setFilters] = useState([]); + const [originalNodes, setOriginalNodes] = useState([]); + // query to get filtered data to narrow down Table outputs const { nodeExecutionsQuery: filteredNodeExecutionsQuery } = - useExecutionNodeViewsStatePoll(execution, filterState?.appliedFilters); + useExecutionNodeViewsStatePoll(execution, filters); + const { compiledWorkflowClosure } = useNodeExecutionContext(); const [showNodes, setShowNodes] = useState([]); + const [initialFilteredNodes, setInitialFilteredNodes] = useState< dNode[] | undefined >(undefined); - const [originalNodes, setOriginalNodes] = useState( - appliedFilters.length > 0 && initialFilteredNodes - ? initialFilteredNodes - : initialNodes, - ); - - const [filters, setFilters] = useState(appliedFilters); - - const [isFiltersChanged, setIsFiltersChanged] = useState(false); - - const { compiledWorkflowClosure } = useNodeExecutionContext(); - - const columnStyles = useColumnStyles(); - // Memoizing columns so they won't be re-generated unless the styles change - const compiledNodes = extractCompiledNodes(compiledWorkflowClosure); - const columns = useMemo( - () => generateColumns(columnStyles, compiledNodes), - [columnStyles, compiledNodes], - ); - - const [filteredNodeExecutions, setFilteredNodeExecutions] = - useState(); + useEffect(() => { + // keep original nodes as a record of the nodes' toggle status + setOriginalNodes(prev => { + const newOgNodes = merge(initialNodes, prev); + if (stringifyIsEqual(prev, newOgNodes)) { + return prev; + } + return newOgNodes; + }); + }, [initialNodes]); + // wait for changes to filtered node executions useEffect(() => { if (filteredNodeExecutionsQuery.isFetching) { return; } - const newFilteredNodeExecutions = isPhaseFilter(filterState.appliedFilters) + const newFilteredNodeExecutions = isPhaseFilter(filters) ? undefined : filteredNodeExecutionsQuery.data; @@ -175,20 +176,20 @@ export const NodeExecutionsTable: React.FC<{filterState: ExecutionFiltersState}> }, [filteredNodeExecutionsQuery]); useEffect(() => { - const plainNodes = convertToPlainNodes(originalNodes || []); - setOriginalNodes(ogn => { - const newNodes = - appliedFilters.length > 0 && initialFilteredNodes - ? mergeOriginIntoNodes(initialFilteredNodes, plainNodes) - : merge(initialNodes, ogn); - - if (!stringifyIsEqual(newNodes, ogn)) { - return newNodes; - } - - return ogn; - }); - + const newShownNodes = + filters.length > 0 && initialFilteredNodes + ? // if there are filtered nodes, merge original ones into them to preserve toggle status + mergeOriginIntoNodes( + cloneDeep(initialFilteredNodes), + cloneDeep(originalNodes), + ) + : // else, merge originalNodes into initialNodes to preserve toggle status + mergeOriginIntoNodes( + cloneDeep(initialNodes), + cloneDeep(originalNodes), + ); + + const plainNodes = convertToPlainNodes(newShownNodes || []); const updatedShownNodesMap = plainNodes.map(node => { const execution = nodeExecutionsById?.[node?.scopedId]; return { @@ -198,19 +199,32 @@ export const NodeExecutionsTable: React.FC<{filterState: ExecutionFiltersState}> }; }); setShowNodes(updatedShownNodesMap); - }, [initialNodes, initialFilteredNodes, originalNodes, nodeExecutionsById]); + }, [ + initialNodes, + initialFilteredNodes, + originalNodes, + nodeExecutionsById, + filters, + ]); useEffect(() => { - if (!isEqual(filters, appliedFilters)) { - setFilters(appliedFilters); - setIsFiltersChanged(true); - } else { - setIsFiltersChanged(false); - } + setFilters(prev => { + if (isEqual(prev, appliedFilters)) { + return prev; + } + return JSON.parse(JSON.stringify(appliedFilters)); + }); }, [appliedFilters]); + // Memoizing columns so they won't be re-generated unless the styles change + const compiledNodes = extractCompiledNodes(compiledWorkflowClosure); + const columns = useMemo( + () => generateColumns(columnStyles, compiledNodes), + [columnStyles, compiledNodes], + ); + useEffect(() => { - if (appliedFilters.length > 0) { + if (filters.length > 0) { // if filter was apllied, but filteredNodeExecutions is empty, we only appliied Phase filter, // and need to clear out items manually if (!filteredNodeExecutions) { @@ -218,7 +232,7 @@ export const NodeExecutionsTable: React.FC<{filterState: ExecutionFiltersState}> const filteredNodes = filterNodes( initialNodes, nodeExecutionsById, - appliedFilters, + filters, ); setInitialFilteredNodes(filteredNodes); @@ -231,7 +245,7 @@ export const NodeExecutionsTable: React.FC<{filterState: ExecutionFiltersState}> setInitialFilteredNodes(filteredNodes); } } - }, [initialNodes, filteredNodeExecutions, isFiltersChanged]); + }, [initialNodes, filteredNodeExecutions, filters]); const toggleNode = async (id: string, scopedId: string, level: number) => { searchNode(originalNodes, 0, id, scopedId, level); diff --git a/packages/console/src/components/Executions/Tables/test/NodeExecutionsTable.test.tsx b/packages/console/src/components/Executions/Tables/test/NodeExecutionsTable.test.tsx index 7110c89fc..5652401bd 100644 --- a/packages/console/src/components/Executions/Tables/test/NodeExecutionsTable.test.tsx +++ b/packages/console/src/components/Executions/Tables/test/NodeExecutionsTable.test.tsx @@ -197,7 +197,7 @@ describe('NodeExecutionsTableExecutions > Tables > NodeExecutionsTable', () => { }; listNodeExecutions.mockImplementation(() => { return Promise.resolve({ - entities: filteredNodeExecutions, + entities: [filteredNodeExecutions], }); }); @@ -262,20 +262,17 @@ describe('NodeExecutionsTableExecutions > Tables > NodeExecutionsTable', () => { ), ); + debug(container); + await waitFor(() => { const rows = queryAllByTestId('node-execution-row'); - return rows.length === filteredNodeExecutions.length; + expect(rows).toHaveLength(filteredNodeExecutions.length); }); - expect(queryAllByTestId('node-execution-row')).toHaveLength( - filteredNodeExecutions.length, - ); - const ids = queryAllByTestId('node-execution-col-id'); expect(ids).toHaveLength(filteredNodeExecutions.length); const renderedPhases = queryAllByTestId('node-execution-col-phase'); expect(renderedPhases).toHaveLength(filteredNodeExecutions.length); - debug(container); for (const i in filteredNodeExecutions) { expect(ids[i]).toHaveTextContent(filteredNodeExecutions[i].id?.nodeId); diff --git a/packages/console/src/components/flytegraph/ReactFlow/test/PausedTasksComponent.test.tsx b/packages/console/src/components/flytegraph/ReactFlow/test/PausedTasksComponent.test.tsx index c7a218b33..6514f8424 100644 --- a/packages/console/src/components/flytegraph/ReactFlow/test/PausedTasksComponent.test.tsx +++ b/packages/console/src/components/flytegraph/ReactFlow/test/PausedTasksComponent.test.tsx @@ -97,7 +97,17 @@ describe('flytegraph > ReactFlow > PausedTasksComponent', () => { }} > {} }} + value={{ + nodeExecutionsById, + dagData: { + dagError: null, + mergedDag: {}, + }, + initialDNodes: props.nodes, + setCurrentNodeExecutionsById: () => {}, + setShouldUpdate: () => {}, + shouldUpdate: false, + }} >