Skip to content

Commit

Permalink
fix: effect dependency and tests
Browse files Browse the repository at this point in the history 
Signed-off-by: Olga Nad <[email protected]>
  • Loading branch information
@olga-union
olga-union committed Oct 18, 2022
1 parent 9ee47b3 commit 43fc397
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 65 deletions.
14 changes: 13 additions & 1 deletion packages/zapp/console/src/components/Executions/ExecutionDetails/ExecutionTabContent.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -81,6 +82,8 @@ export const ExecutionTabContent: React.FC<ExecutionTabContentProps> = ({
const [initialNodes, setInitialNodes] = useState<dNode[]>([]);
const [initialFilteredNodes, setInitialFilteredNodes] = useState<dNode[] | undefined>(undefined);
const [mergedDag, setMergedDag] = useState(null);
const [filters, setFilters] = useState<FilterOperation[]>(appliedFilters);
const [isFiltersChanged, setIsFiltersChanged] = useState<boolean>(false);

useEffect(() => {
const nodes: dNode[] = compiledWorkflowClosure
Expand All @@ -106,6 +109,15 @@ export const ExecutionTabContent: React.FC<ExecutionTabContentProps> = ({
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,
Expand All @@ -124,7 +136,7 @@ export const ExecutionTabContent: React.FC<ExecutionTabContentProps> = ({
setInitialFilteredNodes(filteredNodes);
}
}
}, [initialNodes, filteredNodeExecutions]);
}, [initialNodes, filteredNodeExecutions, isFiltersChanged]);

const [selectedNodes, setSelectedNodes] = useState<string[]>([]);

Expand Down
34 changes: 18 additions & 16 deletions .../zapp/console/src/components/Executions/ExecutionDetails/test/ExecutionNodeViews.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 }) => (
<div data-testid="node-execution-row">
<span id="node-execution-col-id">{execution?.id?.nodeId}</span>
{children}
<span id="node-execution-col-id">{nodeExecution?.id?.nodeId}</span>
</div>
)),
}));

jest.mock('components/Executions/ExecutionDetails/Timeline/ExecutionTimelineFooter', () => ({
ExecutionTimelineFooter: jest.fn(() => <div></div>),
}));

jest.mock('components/Executions/ExecutionDetails/Timeline/TimelineChart/index', () => ({
TimelineChart: jest.fn(() => <div></div>),
}));

jest.mock('components/Executions/ExecutionDetails/Timeline/NodeExecutionName', () => ({
NodeExecutionName: jest.fn(({ name }) => <div>{name}</div>),
}));

// ExecutionNodeViews uses query params for NE list, so we must match them
// for the list to be returned properly
const baseQueryParams = {
Expand Down Expand Up @@ -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);
Expand All @@ -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();
Expand Down
97 changes: 49 additions & 48 deletions packages/zapp/console/src/components/Executions/Tables/test/NodeExecutionsTable.test.tsx
View file
Original file line number Diff line number Diff line change
@@ -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<any>;
mockUseNodeExecutionFiltersState.mockReturnValue({ filters: [], appliedFilters: [] });

jest.mock('components/Executions/Tables/NodeExecutionRow', () => ({
NodeExecutionRow: jest.fn(({ nodeExecution }) => (
<div data-testid="node-execution-row">
Expand All @@ -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 = {};

Expand All @@ -89,39 +69,32 @@ const mockExecutionsById = (n: number, phases: NodeExecutionPhase[]) => {

describe('NodeExecutionsTableExecutions > Tables > NodeExecutionsTable', () => {
let queryClient: QueryClient;
let requestConfig: RequestConfig;
let fixture: ReturnType<typeof basicPythonWorkflow.generate>;
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(
<QueryClientProvider client={queryClient}>
<NodeExecutionsRequestConfigContext.Provider value={requestConfig}>
<NodeExecutionDetailsContextProvider workflowId={mockWorkflowId}>
<NodeExecutionsByIdContext.Provider value={nodeExecutionsById}>
<NodeExecutionsTable
initialNodes={initialNodes}
filteredNodeExecutions={filteredNodeExecutions}
/>
</NodeExecutionsByIdContext.Provider>
</NodeExecutionDetailsContextProvider>
</NodeExecutionsRequestConfigContext.Provider>
<NodeExecutionDetailsContextProvider workflowId={mockWorkflowId}>
<NodeExecutionsByIdContext.Provider value={nodeExecutionsById}>
<NodeExecutionsTable initialNodes={initialNodes} filteredNodes={filteredNodes} />
</NodeExecutionsByIdContext.Provider>
</NodeExecutionDetailsContextProvider>
</QueryClientProvider>,
);

it('renders empty content when there are no nodes', async () => {
const { queryByText, queryByTestId } = renderTable({
initialNodes: [],
nodeExecutionsById: {},
filteredNodeExecutions: [],
filteredNodes: [],
});

await waitFor(() => queryByText(noExecutionsFoundString));
Expand All @@ -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'));
Expand All @@ -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'));
Expand All @@ -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());
}
});
});

0 comments on commit 43fc397

Please sign in to comment.