From db772d51bc6f733160fdd5d044be055ce0a2104c Mon Sep 17 00:00:00 2001 From: Yasser Elsayed Date: Tue, 27 Nov 2018 23:31:19 -0800 Subject: [PATCH] PipelineDetails page tests (#380) * wip pipeline details tests * pipeline details tests * safe load template * add close panel test * pr comments --- frontend/src/components/Graph.tsx | 5 +- .../__snapshots__/Graph.test.tsx.snap | 6 +- frontend/src/pages/PipelineDetails.test.tsx | 332 +++++++ frontend/src/pages/PipelineDetails.tsx | 82 +- .../PipelineDetails.test.tsx.snap | 810 ++++++++++++++++++ 5 files changed, 1191 insertions(+), 44 deletions(-) create mode 100644 frontend/src/pages/PipelineDetails.test.tsx create mode 100644 frontend/src/pages/__snapshots__/PipelineDetails.test.tsx.snap diff --git a/frontend/src/components/Graph.tsx b/frontend/src/components/Graph.tsx index 5d29609fe27..9fa531b26c5 100644 --- a/frontend/src/components/Graph.tsx +++ b/frontend/src/components/Graph.tsx @@ -119,11 +119,14 @@ interface GraphProps { } export default class Graph extends React.Component { - public render(): JSX.Element { + public render(): JSX.Element | null { const { graph } = this.props; const displayEdges: Edge[] = []; const displayEdgeStartPoints: number[][] = []; + if (!graph.nodes().length) { + return null; + } dagre.layout(graph); // Creates the lines that constitute the edges connecting the graph. diff --git a/frontend/src/components/__snapshots__/Graph.test.tsx.snap b/frontend/src/components/__snapshots__/Graph.test.tsx.snap index 60ae7671100..02d906451e2 100644 --- a/frontend/src/components/__snapshots__/Graph.test.tsx.snap +++ b/frontend/src/components/__snapshots__/Graph.test.tsx.snap @@ -94,11 +94,7 @@ exports[`Graph gracefully renders a graph with a selected node id that does not `; -exports[`Graph handles an empty graph 1`] = ` -
-`; +exports[`Graph handles an empty graph 1`] = `""`; exports[`Graph renders a complex graph with six nodes and seven edges 1`] = `
{ + const updateBannerSpy = jest.fn(); + const updateDialogSpy = jest.fn(); + const updateSnackbarSpy = jest.fn(); + const updateToolbarSpy = jest.fn(); + const historyPushSpy = jest.fn(); + const getPipelineSpy = jest.spyOn(Apis.pipelineServiceApi, 'getPipeline'); + const deletePipelineSpy = jest.spyOn(Apis.pipelineServiceApi, 'deletePipeline'); + const getTemplateSpy = jest.spyOn(Apis.pipelineServiceApi, 'getTemplate'); + const createGraphSpy = jest.spyOn(StaticGraphParser, 'createGraph'); + + let tree: ShallowWrapper | ReactWrapper; + let testPipeline: ApiPipeline = {}; + + function generateProps(): PageProps { + // getInitialToolbarState relies on page props having been populated, so fill those first + const pageProps: PageProps = { + history: { push: historyPushSpy } as any, + location: '' as any, + match: { params: { [RouteParams.pipelineId]: testPipeline.id }, isExact: true, path: '', url: '' }, + toolbarProps: { actions: [], breadcrumbs: [], pageTitle: '' }, + updateBanner: updateBannerSpy, + updateDialog: updateDialogSpy, + updateSnackbar: updateSnackbarSpy, + updateToolbar: updateToolbarSpy, + }; + return Object.assign(pageProps, { + toolbarProps: new PipelineDetails(pageProps).getInitialToolbarState(), + }); + } + + beforeAll(() => jest.spyOn(console, 'error').mockImplementation()); + afterAll(() => jest.resetAllMocks()); + + beforeEach(() => { + testPipeline = { + created_at: new Date(2018, 8, 5, 4, 3, 2), + description: 'test job description', + enabled: true, + id: 'test-job-id', + max_concurrency: '50', + name: 'test job', + pipeline_spec: { + parameters: [{ name: 'param1', value: 'value1' }], + pipeline_id: 'some-pipeline-id', + }, + trigger: { + periodic_schedule: { + end_time: new Date(2018, 10, 9, 8, 7, 6), + interval_second: '3600', + start_time: new Date(2018, 9, 8, 7, 6), + } + }, + } as ApiPipeline; + + historyPushSpy.mockClear(); + updateBannerSpy.mockClear(); + updateDialogSpy.mockClear(); + updateSnackbarSpy.mockClear(); + updateToolbarSpy.mockClear(); + deletePipelineSpy.mockReset(); + getPipelineSpy.mockImplementation(() => Promise.resolve(testPipeline)); + getPipelineSpy.mockClear(); + getTemplateSpy.mockImplementation(() => Promise.resolve('{}')); + getTemplateSpy.mockClear(); + createGraphSpy.mockImplementation(() => new graphlib.Graph()); + createGraphSpy.mockClear(); + }); + + afterEach(() => tree.unmount()); + + it('shows empty pipeline details with no graph', async () => { + TestUtils.makeErrorResponseOnce(createGraphSpy, 'bad graph'); + tree = shallow(); + await getTemplateSpy; + await TestUtils.flushPromises(); + expect(tree).toMatchSnapshot(); + }); + + it('shows load error banner when failing to get pipeline', async () => { + TestUtils.makeErrorResponseOnce(getPipelineSpy, 'woops'); + tree = shallow(); + await getPipelineSpy; + await TestUtils.flushPromises(); + expect(updateBannerSpy).toHaveBeenCalledTimes(2); // Once to clear banner, once to show error + expect(updateBannerSpy).toHaveBeenLastCalledWith(expect.objectContaining({ + additionalInfo: 'woops', + message: 'Cannot retrieve pipeline details. Click Details for more information.', + mode: 'error', + })); + }); + + it('shows load error banner when failing to get pipeline template', async () => { + TestUtils.makeErrorResponseOnce(getTemplateSpy, 'woops'); + tree = shallow(); + await getPipelineSpy; + await TestUtils.flushPromises(); + expect(updateBannerSpy).toHaveBeenCalledTimes(2); // Once to clear banner, once to show error + expect(updateBannerSpy).toHaveBeenLastCalledWith(expect.objectContaining({ + additionalInfo: 'woops', + message: 'Cannot retrieve pipeline template. Click Details for more information.', + mode: 'error', + })); + }); + + it('shows no graph error banner when failing to parse graph', async () => { + TestUtils.makeErrorResponseOnce(createGraphSpy, 'bad graph'); + tree = shallow(); + await getTemplateSpy; + await TestUtils.flushPromises(); + expect(updateBannerSpy).toHaveBeenCalledTimes(2); // Once to clear banner, once to show error + expect(updateBannerSpy).toHaveBeenLastCalledWith(expect.objectContaining({ + additionalInfo: 'bad graph', + message: 'Error: failed to generate Pipeline graph. Click Details for more information.', + mode: 'error', + })); + }); + + it('shows empty pipeline details with empty graph', async () => { + tree = shallow(); + await getTemplateSpy; + await TestUtils.flushPromises(); + expect(tree).toMatchSnapshot(); + }); + + it('sets summary shown state to false when clicking the Hide button', async () => { + tree = mount(); + await getTemplateSpy; + await TestUtils.flushPromises(); + tree.update(); + expect(tree.state('summaryShown')).toBe(true); + tree.find('Paper Button').simulate('click'); + expect(tree.state('summaryShown')).toBe(false); + }); + + it('collapses summary card when summary shown state is false', async () => { + tree = shallow(); + await getTemplateSpy; + await TestUtils.flushPromises(); + tree.setState({ summaryShown: false }); + expect(tree).toMatchSnapshot(); + }); + + it('shows the summary card when clicking Show button', async () => { + tree = mount(); + await getTemplateSpy; + await TestUtils.flushPromises(); + tree.setState({ summaryShown: false }); + tree.find(`.${css.footer} Button`).simulate('click'); + expect(tree.state('summaryShown')).toBe(true); + }); + + it('has a new experiment button', async () => { + tree = shallow(); + await getTemplateSpy; + await TestUtils.flushPromises(); + const instance = tree.instance() as PipelineDetails; + const newExperimentBtn = instance.getInitialToolbarState().actions.find( + b => b.title === 'Start an experiment'); + expect(newExperimentBtn).toBeDefined(); + }); + + it('clicking new experiment button navigates to new experiment page', async () => { + tree = shallow(); + await getTemplateSpy; + await TestUtils.flushPromises(); + const instance = tree.instance() as PipelineDetails; + const newExperimentBtn = instance.getInitialToolbarState().actions.find( + b => b.title === 'Start an experiment'); + await newExperimentBtn!.action(); + expect(historyPushSpy).toHaveBeenCalledTimes(1); + expect(historyPushSpy).toHaveBeenLastCalledWith( + RoutePage.NEW_EXPERIMENT + `?${QUERY_PARAMS.pipelineId}=${testPipeline.id}`); + }); + + it('has a delete button', async () => { + tree = shallow(); + await getTemplateSpy; + await TestUtils.flushPromises(); + const instance = tree.instance() as PipelineDetails; + const deleteBtn = instance.getInitialToolbarState().actions.find( + b => b.title === 'Delete'); + expect(deleteBtn).toBeDefined(); + }); + + it('shows delete confirmation dialog when delete buttin is clicked', async () => { + tree = shallow(); + const deleteBtn = (tree.instance() as PipelineDetails) + .getInitialToolbarState().actions.find(b => b.title === 'Delete'); + await deleteBtn!.action(); + expect(updateDialogSpy).toHaveBeenCalledTimes(1); + expect(updateDialogSpy).toHaveBeenLastCalledWith(expect.objectContaining({ + title: 'Delete this pipeline?', + })); + }); + + it('does not call delete API for selected pipeline when delete dialog is canceled', async () => { + tree = shallow(); + const deleteBtn = (tree.instance() as PipelineDetails) + .getInitialToolbarState().actions.find(b => b.title === 'Delete'); + await deleteBtn!.action(); + const call = updateDialogSpy.mock.calls[0][0]; + const cancelBtn = call.buttons.find((b: any) => b.text === 'Cancel'); + await cancelBtn.onClick(); + expect(deletePipelineSpy).not.toHaveBeenCalled(); + }); + + it('calls delete API when delete dialog is confirmed', async () => { + tree = shallow(); + await getTemplateSpy; + await TestUtils.flushPromises(); + const deleteBtn = (tree.instance() as PipelineDetails) + .getInitialToolbarState().actions.find(b => b.title === 'Delete'); + await deleteBtn!.action(); + const call = updateDialogSpy.mock.calls[0][0]; + const confirmBtn = call.buttons.find((b: any) => b.text === 'Delete'); + await confirmBtn.onClick(); + expect(deletePipelineSpy).toHaveBeenCalledTimes(1); + expect(deletePipelineSpy).toHaveBeenLastCalledWith(testPipeline.id); + }); + + it('shows error dialog if deletion fails', async () => { + tree = shallow(); + TestUtils.makeErrorResponseOnce(deletePipelineSpy, 'woops'); + await getTemplateSpy; + await TestUtils.flushPromises(); + const deleteBtn = (tree.instance() as PipelineDetails) + .getInitialToolbarState().actions.find(b => b.title === 'Delete'); + await deleteBtn!.action(); + const call = updateDialogSpy.mock.calls[0][0]; + const confirmBtn = call.buttons.find((b: any) => b.text === 'Delete'); + await confirmBtn.onClick(); + expect(updateDialogSpy).toHaveBeenCalledTimes(2); // Delete dialog + error dialog + expect(updateDialogSpy).toHaveBeenLastCalledWith(expect.objectContaining({ + content: 'woops', + title: 'Failed to delete pipeline', + })); + }); + + it('shows success snackbar if deletion succeeds', async () => { + tree = shallow(); + await getTemplateSpy; + await TestUtils.flushPromises(); + const deleteBtn = (tree.instance() as PipelineDetails) + .getInitialToolbarState().actions.find(b => b.title === 'Delete'); + await deleteBtn!.action(); + const call = updateDialogSpy.mock.calls[0][0]; + const confirmBtn = call.buttons.find((b: any) => b.text === 'Delete'); + await confirmBtn.onClick(); + expect(updateSnackbarSpy).toHaveBeenCalledTimes(1); + expect(updateSnackbarSpy).toHaveBeenLastCalledWith(expect.objectContaining({ + message: 'Successfully deleted pipeline: ' + testPipeline.name, + open: true, + })); + }); + + it('opens side panel on clicked node, shows message when node is not found in graph', async () => { + tree = shallow(); + await getTemplateSpy; + await TestUtils.flushPromises(); + tree.find('Graph').simulate('click', 'some-node-id'); + expect(tree.state('selectedNodeId')).toBe('some-node-id'); + expect(tree).toMatchSnapshot(); + }); + + it('shows clicked node info in the side panel if it is in the graph', async () => { + const g = new graphlib.Graph(); + const info = new StaticGraphParser.SelectedNodeInfo(); + info.args = ['test arg', 'test arg2']; + info.command = ['test command', 'test command 2']; + info.condition = 'test condition'; + info.image = 'test image'; + info.inputs = [['key1', 'val1'], ['key2', 'val2']]; + info.outputs = [['key3', 'val3'], ['key4', 'val4']]; + info.nodeType = 'container'; + g.setNode('node1', { info, label: 'node1' }); + createGraphSpy.mockImplementation(() => g); + + tree = shallow(); + await getTemplateSpy; + await TestUtils.flushPromises(); + tree.find('Graph').simulate('click', 'node1'); + expect(tree).toMatchSnapshot(); + }); + + it('shows pipeline source code when config tab is clicked', async () => { + tree = shallow(); + await getTemplateSpy; + await TestUtils.flushPromises(); + tree.find('MD2Tabs').simulate('switch', 1); + expect(tree.state('selectedTab')).toBe(1); + expect(tree).toMatchSnapshot(); + }); + + it('closes side panel when close button is clicked', async () => { + tree = shallow(); + await getTemplateSpy; + await TestUtils.flushPromises(); + tree.setState({ selectedNodeId: 'some-node-id' }); + tree.find('SidePanel').simulate('close'); + expect(tree.state('selectedNodeId')).toBe(''); + expect(tree).toMatchSnapshot(); + }); +}); diff --git a/frontend/src/pages/PipelineDetails.tsx b/frontend/src/pages/PipelineDetails.tsx index 8eb12ed7b6a..93cd3af1950 100644 --- a/frontend/src/pages/PipelineDetails.tsx +++ b/frontend/src/pages/PipelineDetails.tsx @@ -27,7 +27,7 @@ import MD2Tabs from '../atoms/MD2Tabs'; import Paper from '@material-ui/core/Paper'; import SidePanel from '../components/SidePanel'; import StaticNodeDetails from '../components/StaticNodeDetails'; -import { ApiPipeline } from '../apis/pipeline'; +import { ApiPipeline, ApiGetTemplateResponse } from '../apis/pipeline'; import { Apis } from '../lib/Apis'; import { Page } from './Page'; import { RoutePage, RouteParams } from '../components/Router'; @@ -52,7 +52,7 @@ interface PipelineDetailsState { const summaryCardWidth = 500; -const css = stylesheet({ +export const css = stylesheet({ containerCss: { $nest: { '& .CodeMirror': { @@ -126,7 +126,7 @@ class PipelineDetails extends Page<{}, PipelineDetailsState> { { onClick: () => this._deleteDialogClosed(false), text: 'Cancel' }, ], onClose: () => this._deleteDialogClosed(false), - title: 'Delete this Pipeline?', + title: 'Delete this pipeline?', }), id: 'deleteBtn', title: 'Delete', @@ -155,7 +155,7 @@ class PipelineDetails extends Page<{}, PipelineDetailsState> {
this.setState({ selectedTab: tab })} + onSwitch={(tab: number) => this.setStateSafe({ selectedTab: tab })} tabs={['Graph', 'Source']} />
@@ -167,7 +167,7 @@ class PipelineDetails extends Page<{}, PipelineDetailsState> {
Summary
-
@@ -179,10 +179,10 @@ class PipelineDetails extends Page<{}, PipelineDetailsState> { )} this.setState({ selectedNodeId: id })} /> + onClick={id => this.setStateSafe({ selectedNodeId: id })} /> this.setState({ selectedNodeId: '' })}> + title={selectedNodeId} onClose={() => this.setStateSafe({ selectedNodeId: '' })}>
{!selectedNodeInfo &&
Unable to retrieve node info
} {!!selectedNodeInfo &&
@@ -192,7 +192,7 @@ class PipelineDetails extends Page<{}, PipelineDetailsState> {
{!summaryShown && ( - )} @@ -238,41 +238,47 @@ class PipelineDetails extends Page<{}, PipelineDetailsState> { this.clearBanner(); const pipelineId = this.props.match.params[RouteParams.pipelineId]; + let pipeline: ApiPipeline; + let templateResponse: ApiGetTemplateResponse; + try { - const [pipeline, templateResponse] = await Promise.all([ - Apis.pipelineServiceApi.getPipeline(pipelineId), - Apis.pipelineServiceApi.getTemplate(pipelineId) - ]); + pipeline = await Apis.pipelineServiceApi.getPipeline(pipelineId); + } catch (err) { + await this.showPageError('Cannot retrieve pipeline details.', err); + logger.error('Cannot retrieve pipeline details.', err); + return; + } - const template: Workflow = JsYaml.safeLoad(templateResponse.template!); - let g: dagre.graphlib.Graph | undefined; - try { - g = StaticGraphParser.createGraph(template); - } catch (err) { - await this.showPageError('Error: failed to generate Pipeline graph.', err); - } + try { + templateResponse = await Apis.pipelineServiceApi.getTemplate(pipelineId); + } catch (err) { + await this.showPageError('Cannot retrieve pipeline template.', err); + logger.error('Cannot retrieve pipeline details.', err); + return; + } - const breadcrumbs = [{ displayName: 'Pipelines', href: RoutePage.PIPELINES }]; - const pageTitle = pipeline.name!; + let template: Workflow | undefined; + let g: dagre.graphlib.Graph | undefined; + try { + template = JsYaml.safeLoad(templateResponse.template || '{}'); + g = StaticGraphParser.createGraph(template!); + } catch (err) { + await this.showPageError('Error: failed to generate Pipeline graph.', err); + } - const toolbarActions = [...this.props.toolbarProps.actions]; - toolbarActions[0].disabled = false; - this.props.updateToolbar({ breadcrumbs, actions: toolbarActions, pageTitle }); + const breadcrumbs = [{ displayName: 'Pipelines', href: RoutePage.PIPELINES }]; + const pageTitle = pipeline.name!; - this.setState({ - graph: g, - pipeline, - template, - templateYaml: templateResponse.template, - }); - } - catch (err) { - await this.showPageError( - `Error: failed to retrieve pipeline or template for ID: ${pipelineId}.`, - err, - ); - logger.error(`Error loading pipeline or template for ID: ${pipelineId}`, err); - } + const toolbarActions = [...this.props.toolbarProps.actions]; + toolbarActions[0].disabled = false; + this.props.updateToolbar({ breadcrumbs, actions: toolbarActions, pageTitle }); + + this.setStateSafe({ + graph: g, + pipeline, + template, + templateYaml: templateResponse.template, + }); } private _createNewExperiment(): void { diff --git a/frontend/src/pages/__snapshots__/PipelineDetails.test.tsx.snap b/frontend/src/pages/__snapshots__/PipelineDetails.test.tsx.snap new file mode 100644 index 00000000000..45d258be928 --- /dev/null +++ b/frontend/src/pages/__snapshots__/PipelineDetails.test.tsx.snap @@ -0,0 +1,810 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`PipelineDetails closes side panel when close button is clicked 1`] = ` +
+
+ +
+
+
+ +
+
+ Summary +
+ + Hide + +
+
+ Uploaded on +
+
+ 9/5/2018, 4:03:02 AM +
+
+ Description +
+
+ test job description +
+
+ + +
+
+ Unable to retrieve node info +
+
+
+
+
+ + + Static pipeline graph + +
+
+
+
+
+
+
+`; + +exports[`PipelineDetails collapses summary card when summary shown state is false 1`] = ` +
+
+ +
+
+
+ + +
+
+ Unable to retrieve node info +
+
+
+
+ + Show summary + +
+ + + Static pipeline graph + +
+
+
+
+
+
+
+`; + +exports[`PipelineDetails opens side panel on clicked node, shows message when node is not found in graph 1`] = ` +
+
+ +
+
+
+ +
+
+ Summary +
+ + Hide + +
+
+ Uploaded on +
+
+ 9/5/2018, 4:03:02 AM +
+
+ Description +
+
+ test job description +
+
+ + +
+
+ Unable to retrieve node info +
+
+
+
+
+ + + Static pipeline graph + +
+
+
+
+
+
+
+`; + +exports[`PipelineDetails shows clicked node info in the side panel if it is in the graph 1`] = ` +
+
+ +
+
+
+ +
+
+ Summary +
+ + Hide + +
+
+ Uploaded on +
+
+ 9/5/2018, 4:03:02 AM +
+
+ Description +
+
+ test job description +
+
+ + +
+
+ +
+
+
+
+
+ + + Static pipeline graph + +
+
+
+
+
+
+
+`; + +exports[`PipelineDetails shows empty pipeline details with empty graph 1`] = ` +
+
+ +
+
+
+ +
+
+ Summary +
+ + Hide + +
+
+ Uploaded on +
+
+ 9/5/2018, 4:03:02 AM +
+
+ Description +
+
+ test job description +
+
+ + +
+
+ Unable to retrieve node info +
+
+
+
+
+ + + Static pipeline graph + +
+
+
+
+
+
+
+`; + +exports[`PipelineDetails shows empty pipeline details with no graph 1`] = ` +
+
+ +
+
+ + No graph to show + +
+
+
+
+`; + +exports[`PipelineDetails shows pipeline source code when config tab is clicked 1`] = ` +
+
+ +
+
+ +
+
+
+
+`;