From 43fc397827bf935c8e03a13327c2f9fc97e5f53b Mon Sep 17 00:00:00 2001 From: Olga Nad Date: Tue, 18 Oct 2022 16:12:41 -0500 Subject: [PATCH] fix: effect dependency and tests Signed-off-by: Olga Nad --- .../ExecutionDetails/ExecutionTabContent.tsx | 14 ++- .../test/ExecutionNodeViews.test.tsx | 34 ++++--- .../Tables/test/NodeExecutionsTable.test.tsx | 97 ++++++++++--------- 3 files changed, 80 insertions(+), 65 deletions(-) diff --git a/packages/zapp/console/src/components/Executions/ExecutionDetails/ExecutionTabContent.tsx b/packages/zapp/console/src/components/Executions/ExecutionDetails/ExecutionTabContent.tsx index d48b24b5c..db69cd778 100644 --- a/packages/zapp/console/src/components/Executions/ExecutionDetails/ExecutionTabContent.tsx +++ b/packages/zapp/console/src/components/Executions/ExecutionDetails/ExecutionTabContent.tsx @@ -16,6 +16,7 @@ import { FilterOperationName, FilterOperationValueList, } from 'models/AdminEntity/types'; +import { isEqual } from 'lodash'; import { useNodeExecutionContext } from '../contextProvider/NodeExecutionDetails'; import { NodeExecutionsByIdContext } from '../contexts'; import { NodeExecutionsTable } from '../Tables/NodeExecutionsTable'; @@ -81,6 +82,8 @@ export const ExecutionTabContent: React.FC = ({ const [initialNodes, setInitialNodes] = useState([]); const [initialFilteredNodes, setInitialFilteredNodes] = useState(undefined); const [mergedDag, setMergedDag] = useState(null); + const [filters, setFilters] = useState(appliedFilters); + const [isFiltersChanged, setIsFiltersChanged] = useState(false); useEffect(() => { const nodes: dNode[] = compiledWorkflowClosure @@ -106,6 +109,15 @@ export const ExecutionTabContent: React.FC = ({ setInitialNodes(plainNodes); }, [compiledWorkflowClosure, dynamicWorkflows]); + useEffect(() => { + if (!isEqual(filters, appliedFilters)) { + setFilters(appliedFilters); + setIsFiltersChanged(true); + } else { + setIsFiltersChanged(false); + } + }, [appliedFilters]); + useEffect(() => { if (appliedFilters.length > 0) { // if filter was apllied, but filteredNodeExecutions is empty, we only appliied Phase filter, @@ -124,7 +136,7 @@ export const ExecutionTabContent: React.FC = ({ setInitialFilteredNodes(filteredNodes); } } - }, [initialNodes, filteredNodeExecutions]); + }, [initialNodes, filteredNodeExecutions, isFiltersChanged]); const [selectedNodes, setSelectedNodes] = useState([]); diff --git a/packages/zapp/console/src/components/Executions/ExecutionDetails/test/ExecutionNodeViews.test.tsx b/packages/zapp/console/src/components/Executions/ExecutionDetails/test/ExecutionNodeViews.test.tsx index d8a5a683f..76c24e3d9 100644 --- a/packages/zapp/console/src/components/Executions/ExecutionDetails/test/ExecutionNodeViews.test.tsx +++ b/packages/zapp/console/src/components/Executions/ExecutionDetails/test/ExecutionNodeViews.test.tsx @@ -11,24 +11,26 @@ import { createTestQueryClient } from 'test/utils'; import { tabs } from '../constants'; import { ExecutionNodeViews } from '../ExecutionNodeViews'; -jest.mock('chart.js', () => ({ - Chart: { register: () => null }, - Tooltip: { positioners: { cursor: () => null } }, - registerables: [], -})); - -jest.mock('chartjs-plugin-datalabels', () => ({ - ChartDataLabels: null, -})); - jest.mock('components/Executions/Tables/NodeExecutionRow', () => ({ - NodeExecutionRow: jest.fn(({ children, execution }) => ( + NodeExecutionRow: jest.fn(({ nodeExecution }) => (
- {execution?.id?.nodeId} - {children} + {nodeExecution?.id?.nodeId}
)), })); + +jest.mock('components/Executions/ExecutionDetails/Timeline/ExecutionTimelineFooter', () => ({ + ExecutionTimelineFooter: jest.fn(() =>
), +})); + +jest.mock('components/Executions/ExecutionDetails/Timeline/TimelineChart/index', () => ({ + TimelineChart: jest.fn(() =>
), +})); + +jest.mock('components/Executions/ExecutionDetails/Timeline/NodeExecutionName', () => ({ + NodeExecutionName: jest.fn(({ name }) =>
{name}
), +})); + // ExecutionNodeViews uses query params for NE list, so we must match them // for the list to be returned properly const baseQueryParams = { @@ -77,7 +79,7 @@ describe('ExecutionNodeViews', () => { await waitFor(() => getByText(tabs.nodes.label)); const nodesTab = getByText(tabs.nodes.label); - const graphTab = getByText(tabs.graph.label); + const timelineTab = getByText(tabs.timeline.label); // Ensure we are on Nodes tab fireEvent.click(nodesTab); @@ -96,11 +98,11 @@ describe('ExecutionNodeViews', () => { await waitFor(() => queryByText(failedNodeName)); expect(queryByText(succeededNodeName)).not.toBeInTheDocument(); - expect(getByText(failedNodeName)).toBeInTheDocument(); + expect(queryByText(failedNodeName)).toBeInTheDocument(); // Switch to the Graph tab fireEvent.click(statusButton); - fireEvent.click(graphTab); + fireEvent.click(timelineTab); await waitFor(() => queryByText(succeededNodeName)); expect(queryByText(succeededNodeName)).toBeInTheDocument(); diff --git a/packages/zapp/console/src/components/Executions/Tables/test/NodeExecutionsTable.test.tsx b/packages/zapp/console/src/components/Executions/Tables/test/NodeExecutionsTable.test.tsx index 4b7f06a32..e9600a9c3 100644 --- a/packages/zapp/console/src/components/Executions/Tables/test/NodeExecutionsTable.test.tsx +++ b/packages/zapp/console/src/components/Executions/Tables/test/NodeExecutionsTable.test.tsx @@ -1,27 +1,27 @@ import { render, waitFor } from '@testing-library/react'; import { NodeExecutionDetailsContextProvider } from 'components/Executions/contextProvider/NodeExecutionDetails'; -import { - NodeExecutionsByIdContext, - NodeExecutionsRequestConfigContext, -} from 'components/Executions/contexts'; +import { NodeExecutionsByIdContext } from 'components/Executions/contexts'; import { basicPythonWorkflow } from 'mocks/data/fixtures/basicPythonWorkflow'; import { noExecutionsFoundString } from 'common/constants'; import { mockWorkflowId } from 'mocks/data/fixtures/types'; import { insertFixture } from 'mocks/data/insertFixture'; import { mockServer } from 'mocks/server'; -import { RequestConfig } from 'models/AdminEntity/types'; import { NodeExecutionPhase } from 'models/Execution/enums'; import * as React from 'react'; import { dateToTimestamp } from 'common/utils'; import { QueryClient, QueryClientProvider } from 'react-query'; import { createTestQueryClient } from 'test/utils'; -import { NodeExecution } from 'models/Execution/types'; import { dNode } from 'models/Graph/types'; +import { useNodeExecutionFiltersState } from 'components/Executions/filters/useExecutionFiltersState'; import { NodeExecutionsTable } from '../NodeExecutionsTable'; jest.mock('components/Workflow/workflowQueries'); const { fetchWorkflow } = require('components/Workflow/workflowQueries'); +jest.mock('components/Executions/filters/useExecutionFiltersState'); +const mockUseNodeExecutionFiltersState = useNodeExecutionFiltersState as jest.Mock; +mockUseNodeExecutionFiltersState.mockReturnValue({ filters: [], appliedFilters: [] }); + jest.mock('components/Executions/Tables/NodeExecutionRow', () => ({ NodeExecutionRow: jest.fn(({ nodeExecution }) => (
@@ -46,26 +46,6 @@ const mockNodes = (n: number): dNode[] => { return nodes; }; -const mockNodeExecutions = (n: number, phases: NodeExecutionPhase[]): NodeExecution[] => { - const nodeExecutions: NodeExecution[] = []; - for (let i = 1; i <= n; i++) { - nodeExecutions.push({ - closure: { - createdAt: dateToTimestamp(new Date()), - outputUri: '', - phase: phases[i - 1], - }, - id: { - executionId: { domain: 'domain', name: 'name', project: 'project' }, - nodeId: `node${i}`, - }, - inputUri: '', - scopedId: `n${i}`, - }); - } - return nodeExecutions; -}; - const mockExecutionsById = (n: number, phases: NodeExecutionPhase[]) => { const nodeExecutionsById = {}; @@ -89,31 +69,24 @@ const mockExecutionsById = (n: number, phases: NodeExecutionPhase[]) => { describe('NodeExecutionsTableExecutions > Tables > NodeExecutionsTable', () => { let queryClient: QueryClient; - let requestConfig: RequestConfig; let fixture: ReturnType; const initialNodes = mockNodes(2); beforeEach(() => { - requestConfig = {}; queryClient = createTestQueryClient(); fixture = basicPythonWorkflow.generate(); insertFixture(mockServer, fixture); fetchWorkflow.mockImplementation(() => Promise.resolve(fixture.workflows.top)); }); - const renderTable = ({ nodeExecutionsById, initialNodes, filteredNodeExecutions }) => + const renderTable = ({ nodeExecutionsById, initialNodes, filteredNodes }) => render( - - - - - - - + + + + + , ); @@ -121,7 +94,7 @@ describe('NodeExecutionsTableExecutions > Tables > NodeExecutionsTable', () => { const { queryByText, queryByTestId } = renderTable({ initialNodes: [], nodeExecutionsById: {}, - filteredNodeExecutions: [], + filteredNodes: [], }); await waitFor(() => queryByText(noExecutionsFoundString)); @@ -130,15 +103,14 @@ describe('NodeExecutionsTableExecutions > Tables > NodeExecutionsTable', () => { expect(queryByTestId('node-execution-row')).not.toBeInTheDocument(); }); - it('renders NodeExecutionRows with proper nodeExecutions', async () => { + it('renders NodeExecutionRows with initialNodes when no filteredNodes were provided', async () => { const phases = [NodeExecutionPhase.FAILED, NodeExecutionPhase.SUCCEEDED]; const nodeExecutionsById = mockExecutionsById(2, phases); - const filteredNodeExecutions = mockNodeExecutions(2, phases); const { queryAllByTestId } = renderTable({ initialNodes, nodeExecutionsById, - filteredNodeExecutions, + filteredNodes: undefined, }); await waitFor(() => queryAllByTestId('node-execution-row')); @@ -154,15 +126,15 @@ describe('NodeExecutionsTableExecutions > Tables > NodeExecutionsTable', () => { } }); - it('renders future nodes with UNDEFINED phase', async () => { - const phases = [NodeExecutionPhase.SUCCEEDED, NodeExecutionPhase.UNDEFINED]; - const nodeExecutionsById = mockExecutionsById(1, phases); - const filteredNodeExecutions = mockNodeExecutions(1, phases); + it('renders NodeExecutionRows with initialNodes even when filterNodes were provided, if appliedFilters is empty', async () => { + const phases = [NodeExecutionPhase.FAILED, NodeExecutionPhase.SUCCEEDED]; + const nodeExecutionsById = mockExecutionsById(2, phases); + const filteredNodes = mockNodes(1); const { queryAllByTestId } = renderTable({ initialNodes, nodeExecutionsById, - filteredNodeExecutions, + filteredNodes, }); await waitFor(() => queryAllByTestId('node-execution-row')); @@ -177,4 +149,33 @@ describe('NodeExecutionsTableExecutions > Tables > NodeExecutionsTable', () => { expect(renderedPhases[i]).toHaveTextContent(phases[i].toString()); } }); + + it('renders NodeExecutionRows with filterNodes if appliedFilters is not empty', async () => { + mockUseNodeExecutionFiltersState.mockReturnValueOnce({ + filters: [], + appliedFilters: [{ key: 'phase', operation: 'value_in', value: ['FAILED', 'SUCCEEDED'] }], + }); + + const phases = [NodeExecutionPhase.FAILED, NodeExecutionPhase.SUCCEEDED]; + const nodeExecutionsById = mockExecutionsById(2, phases); + const filteredNodes = mockNodes(1); + + const { queryAllByTestId } = renderTable({ + initialNodes, + nodeExecutionsById, + filteredNodes, + }); + + await waitFor(() => queryAllByTestId('node-execution-row')); + + expect(queryAllByTestId('node-execution-row')).toHaveLength(filteredNodes.length); + const ids = queryAllByTestId('node-execution-col-id'); + expect(ids).toHaveLength(filteredNodes.length); + const renderedPhases = queryAllByTestId('node-execution-col-phase'); + expect(renderedPhases).toHaveLength(filteredNodes.length); + for (const i in filteredNodes) { + expect(ids[i]).toHaveTextContent(filteredNodes[i].id); + expect(renderedPhases[i]).toHaveTextContent(phases[i].toString()); + } + }); });