Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Execution View Test #612

Merged
merged 9 commits into from
Oct 11, 2022
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}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if the WaitForQuery component already takes in a loading component, why do we need the extra check on line 148?

>
{() => (
<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