Skip to content

Commit

Permalink
Fix Execution View Test (#612)
Browse files Browse the repository at this point in the history
* fix: test for executions table

Signed-off-by: James <[email protected]>

* fix: execution node table with filter state

Signed-off-by: James <[email protected]>

* fix: reduce code duplication in tests, fix loading in table

Signed-off-by: James <[email protected]>

* fix: add tests for execution tab content

Signed-off-by: James <[email protected]>

* fix: remove filter unknown nodes

Signed-off-by: James <[email protected]>

* fix: added filteredNodeExecutions to make filter work

Signed-off-by: James <[email protected]>

* fix: tests feedback

Signed-off-by: Olga Nad <[email protected]>

* fix: execution node views

Signed-off-by: Olga Nad <[email protected]>

Signed-off-by: James <[email protected]>
Signed-off-by: Olga Nad <[email protected]>
Co-authored-by: Olga Nad <[email protected]>
  • Loading branch information
james-union and olga-union authored Oct 11, 2022
1 parent 8c97c55 commit 5ab40b1
Show file tree
Hide file tree
Showing 7 changed files with 362 additions and 497 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export const ExecutionNodeViews: React.FC<ExecutionNodeViewsProps> = ({ executio
const tabState = useTabState(tabs, defaultTab);
const queryClient = useQueryClient();
const requestConfig = useContext(NodeExecutionsRequestConfigContext);
const [loading, setLoading] = useState<boolean>(true);
const [nodeExecutionsLoading, setNodeExecutionsLoading] = useState<boolean>(true);

const {
closure: { abortMetadata, workflowId },
Expand All @@ -82,9 +82,9 @@ export const ExecutionNodeViews: React.FC<ExecutionNodeViewsProps> = ({ executio

useEffect(() => {
let isCurrent = true;
setLoading(true);

async function fetchData(baseNodeExecutions, queryClient) {
setNodeExecutionsLoading(true);
const newValue = await Promise.all(
baseNodeExecutions.map(async (baseNodeExecution) => {
const taskExecutions = await fetchTaskExecutionList(queryClient, baseNodeExecution.id);
Expand Down Expand Up @@ -115,12 +115,16 @@ export const ExecutionNodeViews: React.FC<ExecutionNodeViewsProps> = ({ executio

if (isCurrent) {
setNodeExecutionsWithResources(newValue);
setLoading(false);
setNodeExecutionsLoading(false);
}
}

if (nodeExecutions.length > 0) {
fetchData(nodeExecutions, queryClient);
} else {
if (isCurrent) {
setNodeExecutionsLoading(false);
}
}
return () => {
isCurrent = false;
Expand All @@ -146,19 +150,26 @@ export const ExecutionNodeViews: React.FC<ExecutionNodeViewsProps> = ({ executio
);
};

const renderTab = (tabType) => (
<WaitForQuery
errorComponent={DataError}
query={childGroupsQuery}
loadingComponent={LoadingComponent}
>
{() => <ExecutionTab tabType={tabType} abortMetadata={abortMetadata ?? undefined} />}
</WaitForQuery>
);

if (loading) {
return <LoadingComponent />;
}
const renderTab = (tabType) => {
if (nodeExecutionsLoading) {
return <LoadingComponent />;
}
return (
<WaitForQuery
errorComponent={DataError}
query={childGroupsQuery}
loadingComponent={LoadingComponent}
>
{() => (
<ExecutionTab
tabType={tabType}
abortMetadata={abortMetadata ?? undefined}
filteredNodeExecutions={nodeExecutionsQuery.data ?? []}
/>
)}
</WaitForQuery>
);
};

return (
<>
Expand All @@ -169,18 +180,20 @@ export const ExecutionNodeViews: React.FC<ExecutionNodeViewsProps> = ({ executio
</Tabs>
<NodeExecutionDetailsContextProvider workflowId={workflowId}>
<NodeExecutionsByIdContext.Provider value={nodeExecutionsById}>
{nodeExecutions.length > 0 ? (
<div className={styles.nodesContainer}>
{tabState.value === tabs.nodes.id && (
<div className={styles.filters}>
<ExecutionFilters {...filterState} />
</div>
)}
<WaitForQuery errorComponent={DataError} query={nodeExecutionsQuery}>
{() => renderTab(tabState.value)}
</WaitForQuery>
</div>
) : null}
<div className={styles.nodesContainer}>
{tabState.value === tabs.nodes.id && (
<div className={styles.filters}>
<ExecutionFilters {...filterState} />
</div>
)}
<WaitForQuery
errorComponent={DataError}
loadingComponent={LoadingComponent}
query={nodeExecutionsQuery}
>
{() => renderTab(tabState.value)}
</WaitForQuery>
</div>
</NodeExecutionsByIdContext.Provider>
</NodeExecutionDetailsContextProvider>
</>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,25 +5,37 @@ import { Admin } from 'flyteidl';
import { Workflow } from 'models/Workflow/types';
import * as React from 'react';
import { useQuery, useQueryClient } from 'react-query';
import { NodeExecution } from 'models/Execution/types';
import { useNodeExecutionContext } from '../contextProvider/NodeExecutionDetails';
import { ScaleProvider } from './Timeline/scaleContext';
import { ExecutionTabContent } from './ExecutionTabContent';

export interface ExecutionTabProps {
tabType: string;
abortMetadata?: Admin.IAbortMetadata;
filteredNodeExecutions: NodeExecution[];
}

/** Contains the available ways to visualize the nodes of a WorkflowExecution */
export const ExecutionTab: React.FC<ExecutionTabProps> = ({ tabType, abortMetadata }) => {
export const ExecutionTab: React.FC<ExecutionTabProps> = ({
tabType,
abortMetadata,
filteredNodeExecutions,
}) => {
const queryClient = useQueryClient();
const { workflowId } = useNodeExecutionContext();
const workflowQuery = useQuery<Workflow, Error>(makeWorkflowQuery(queryClient, workflowId));

return (
<ScaleProvider>
<WaitForQuery errorComponent={DataError} query={workflowQuery}>
{() => <ExecutionTabContent tabType={tabType} abortMetadata={abortMetadata} />}
{() => (
<ExecutionTabContent
tabType={tabType}
abortMetadata={abortMetadata}
filteredNodeExecutions={filteredNodeExecutions}
/>
)}
</WaitForQuery>
</ScaleProvider>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { DetailsPanel } from 'components/common/DetailsPanel';
import { makeNodeExecutionDynamicWorkflowQuery } from 'components/Workflow/workflowQueries';
import { WorkflowGraph } from 'components/WorkflowGraph/WorkflowGraph';
import { TaskExecutionPhase } from 'models/Execution/enums';
import { NodeExecutionIdentifier } from 'models/Execution/types';
import { NodeExecution, NodeExecutionIdentifier } from 'models/Execution/types';
import { startNodeId, endNodeId } from 'models/Node/constants';
import { Admin } from 'flyteidl';
import * as React from 'react';
Expand All @@ -25,6 +25,7 @@ import { convertToPlainNodes, TimeZone } from './Timeline/helpers';
export interface ExecutionTabContentProps {
tabType: string;
abortMetadata?: Admin.IAbortMetadata;
filteredNodeExecutions: NodeExecution[];
}

const useStyles = makeStyles(() => ({
Expand All @@ -43,6 +44,7 @@ const useStyles = makeStyles(() => ({
export const ExecutionTabContent: React.FC<ExecutionTabContentProps> = ({
tabType,
abortMetadata,
filteredNodeExecutions,
}) => {
const styles = useStyles();
const { compiledWorkflowClosure } = useNodeExecutionContext();
Expand Down Expand Up @@ -171,6 +173,7 @@ export const ExecutionTabContent: React.FC<ExecutionTabContentProps> = ({
initialNodes={initialNodes}
selectedExecution={selectedExecution}
setSelectedExecution={onExecutionSelectionChanged}
filteredNodeExecutions={filteredNodeExecutions}
/>
);
default:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { fireEvent, render, screen, waitFor } from '@testing-library/react';
import { fireEvent, render, waitFor } 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';
Expand All @@ -21,6 +21,14 @@ jest.mock('chartjs-plugin-datalabels', () => ({
ChartDataLabels: null,
}));

jest.mock('components/Executions/Tables/NodeExecutionRow', () => ({
NodeExecutionRow: jest.fn(({ children, execution }) => (
<div data-testid="node-execution-row">
<span id="node-execution-col-id">{execution?.id?.nodeId}</span>
{children}
</div>
)),
}));
// ExecutionNodeViews uses query params for NE list, so we must match them
// for the list to be returned properly
const baseQueryParams = {
Expand All @@ -29,7 +37,7 @@ const baseQueryParams = {
'sort_by.key': 'created_at',
};

describe.skip('ExecutionNodeViews', () => {
describe('ExecutionNodeViews', () => {
let queryClient: QueryClient;
let execution: Execution;
let fixture: ReturnType<typeof oneFailedTaskWorkflow.generate>;
Expand Down Expand Up @@ -64,35 +72,43 @@ describe.skip('ExecutionNodeViews', () => {
const failedNodeName = nodeExecutions.failedNode.data.id.nodeId;
const succeededNodeName = nodeExecutions.pythonNode.data.id.nodeId;

const { getByText, queryByText } = renderViews();
const nodesTab = await waitFor(() => getByText(tabs.nodes.label));
const graphTab = await waitFor(() => getByText(tabs.graph.label));
const { getByText, queryByText, getByLabelText } = renderViews();

await waitFor(() => getByText(tabs.nodes.label));

const nodesTab = getByText(tabs.nodes.label);
const graphTab = getByText(tabs.graph.label);

// Ensure we are on Nodes tab
fireEvent.click(nodesTab);
await waitFor(() => getByText(succeededNodeName));
await waitFor(() => queryByText(succeededNodeName));

const statusButton = await waitFor(() => getByText(filterLabels.status));

// Apply 'Failed' filter and wait for list to include only the failed item
fireEvent.click(statusButton);
const failedFilter = await waitFor(() =>
screen.getByLabelText(nodeExecutionStatusFilters.failed.label),
getByLabelText(nodeExecutionStatusFilters.failed.label),
);

// Wait for succeeded task to disappear and ensure failed task remains
fireEvent.click(failedFilter);
await waitFor(() => queryByText(succeededNodeName) == null);
await waitFor(() => expect(getByText(failedNodeName)).toBeInTheDocument());
await waitFor(() => queryByText(failedNodeName));

expect(queryByText(succeededNodeName)).not.toBeInTheDocument();
expect(getByText(failedNodeName)).toBeInTheDocument();

// Switch to the Graph tab
fireEvent.click(statusButton);
fireEvent.click(graphTab);
await waitFor(() => queryByText(failedNodeName) == null);
await waitFor(() => queryByText(succeededNodeName));

expect(queryByText(succeededNodeName)).toBeInTheDocument();

// Switch back to Nodes Tab and verify filter still applied
fireEvent.click(nodesTab);
await waitFor(() => getByText(failedNodeName));
expect(queryByText(succeededNodeName)).toBeNull();
await waitFor(() => queryByText(failedNodeName));
expect(queryByText(succeededNodeName)).not.toBeInTheDocument();
expect(queryByText(failedNodeName)).toBeInTheDocument();
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
import { render, waitFor } from '@testing-library/react';
import { NodeExecutionDetailsContextProvider } from 'components/Executions/contextProvider/NodeExecutionDetails';
import { NodeExecutionsByIdContext } from 'components/Executions/contexts';
import { basicPythonWorkflow } from 'mocks/data/fixtures/basicPythonWorkflow';
import { mockWorkflowId } from 'mocks/data/fixtures/types';
import { insertFixture } from 'mocks/data/insertFixture';
import { mockServer } from 'mocks/server';
import * as React from 'react';
import { QueryClient, QueryClientProvider } from 'react-query';
import { createTestQueryClient } from 'test/utils';
import { ExecutionTabContent } from '../ExecutionTabContent';
import { tabs } from '../constants';

jest.mock('components/Workflow/workflowQueries');
const { fetchWorkflow } = require('components/Workflow/workflowQueries');

jest.mock('components/common/DetailsPanel', () => ({
DetailsPanel: jest.fn(({ children }) => <div data-testid="details-panel">{children}</div>),
}));

jest.mock('components/Executions/Tables/NodeExecutionsTable', () => ({
NodeExecutionsTable: jest.fn(({ children }) => (
<div data-testid="node-executions-table">{children}</div>
)),
}));
jest.mock('components/Executions/ExecutionDetails/Timeline/ExecutionTimeline', () => ({
ExecutionTimeline: jest.fn(({ children }) => (
<div data-testid="execution-timeline">{children}</div>
)),
}));
jest.mock('components/Executions/ExecutionDetails/Timeline/ExecutionTimelineFooter', () => ({
ExecutionTimelineFooter: jest.fn(({ children }) => (
<div data-testid="execution-timeline-footer">{children}</div>
)),
}));
jest.mock('components/WorkflowGraph/WorkflowGraph', () => ({
WorkflowGraph: jest.fn(({ children }) => <div data-testid="workflow-graph">{children}</div>),
}));

describe('Executions > ExecutionDetails > ExecutionTabContent', () => {
let queryClient: QueryClient;
let fixture: ReturnType<typeof basicPythonWorkflow.generate>;

beforeEach(() => {
queryClient = createTestQueryClient();
fixture = basicPythonWorkflow.generate();
insertFixture(mockServer, fixture);
fetchWorkflow.mockImplementation(() => Promise.resolve(fixture.workflows.top));
});

const renderTabContent = ({ tabType, nodeExecutionsById }) => {
return render(
<QueryClientProvider client={queryClient}>
<NodeExecutionDetailsContextProvider workflowId={mockWorkflowId}>
<NodeExecutionsByIdContext.Provider value={nodeExecutionsById}>
<ExecutionTabContent tabType={tabType} filteredNodeExecutions={[]} />
</NodeExecutionsByIdContext.Provider>
</NodeExecutionDetailsContextProvider>
</QueryClientProvider>,
);
};

it('renders NodeExecutionsTable when the Nodes tab is selected', async () => {
const { queryByTestId } = renderTabContent({
tabType: tabs.nodes.id,
nodeExecutionsById: {},
});

await waitFor(() => queryByTestId('node-executions-table'));
expect(queryByTestId('node-executions-table')).toBeInTheDocument();
});

it('renders WorkflowGraph when the Graph tab is selected', async () => {
const { queryByTestId } = renderTabContent({
tabType: tabs.graph.id,
nodeExecutionsById: {},
});

await waitFor(() => queryByTestId('workflow-graph'));
expect(queryByTestId('workflow-graph')).toBeInTheDocument();
});

it('renders ExecutionTimeline when the Timeline tab is selected', async () => {
const { queryByTestId } = renderTabContent({
tabType: tabs.timeline.id,
nodeExecutionsById: {},
});

await waitFor(() => queryByTestId('execution-timeline'));
expect(queryByTestId('execution-timeline')).toBeInTheDocument();
});
});
Loading

0 comments on commit 5ab40b1

Please sign in to comment.