From 0e2030c6450d646e72c402cf561cea8aaab7af7c Mon Sep 17 00:00:00 2001 From: Yasser Elsayed Date: Tue, 6 Nov 2018 14:09:31 -0800 Subject: [PATCH] ExperimentList tests, use immer.js (#86) * experiment list tests * use produce in more places * remove extra test * remove extra import --- frontend/package-lock.json | 5 + frontend/package.json | 1 + frontend/src/TestUtils.tsx | 4 +- frontend/src/components/CustomTable.tsx | 7 +- .../__snapshots__/CustomTable.test.tsx.snap | 12 + frontend/src/pages/ExperimentList.test.tsx | 334 ++++++++++++++++++ frontend/src/pages/ExperimentList.tsx | 65 ++-- frontend/src/pages/PipelineList.test.tsx | 19 +- frontend/src/pages/PipelineList.tsx | 10 +- .../ExperimentList.test.tsx.snap | 298 ++++++++++++++++ 10 files changed, 710 insertions(+), 45 deletions(-) create mode 100644 frontend/src/pages/ExperimentList.test.tsx create mode 100644 frontend/src/pages/__snapshots__/ExperimentList.test.tsx.snap diff --git a/frontend/package-lock.json b/frontend/package-lock.json index 2cdf9e6972d..c4dbd516412 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -6482,6 +6482,11 @@ "resolved": "https://registry.npmjs.org/iferr/-/iferr-0.1.5.tgz", "integrity": "sha1-xg7taebY/bazEEofy8ocGS3FtQE=" }, + "immer": { + "version": "1.7.4", + "resolved": "https://registry.npmjs.org/immer/-/immer-1.7.4.tgz", + "integrity": "sha512-mZpvQe9LXc+lg3hhcT/2M6Apej43FSs6hS2Bt4HrQxyI6hVJugukLgH2KdSBL/fIOmahHmQvpWU37BWzFLDp1Q==" + }, "import-lazy": { "version": "2.1.0", "resolved": "https://registry.npmjs.org/import-lazy/-/import-lazy-2.1.0.tgz", diff --git a/frontend/package.json b/frontend/package.json index 4114569c0f4..c6a8ed776fe 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -11,6 +11,7 @@ "d3-dsv": "^1.0.10", "dagre": "^0.8.2", "http-proxy-middleware": "^0.19.0", + "immer": "^1.7.4", "js-yaml": "^3.12.0", "portable-fetch": "^3.0.0", "re-resizable": "^4.9.0", diff --git a/frontend/src/TestUtils.tsx b/frontend/src/TestUtils.tsx index 42af1c8d43a..4850da50ee7 100644 --- a/frontend/src/TestUtils.tsx +++ b/frontend/src/TestUtils.tsx @@ -25,12 +25,12 @@ export default class TestUtils { * Mounts the given component with a fake router and returns the mounted tree */ // tslint:disable-next-line:variable-name - public static mountWithRouter(ComponentClass: React.ComponentClass, props: any) { + public static mountWithRouter(component: React.ReactElement) { const childContextTypes = { router: object, }; const context = createRouterContext(); - const tree = mount(, { context, childContextTypes }); + const tree = mount(component, { context, childContextTypes }); return tree; } diff --git a/frontend/src/components/CustomTable.tsx b/frontend/src/components/CustomTable.tsx index a6a9a3e6393..108286e875f 100644 --- a/frontend/src/components/CustomTable.tsx +++ b/frontend/src/components/CustomTable.tsx @@ -16,17 +16,18 @@ import * as React from 'react'; import ArrowRight from '@material-ui/icons/ArrowRight'; +import Checkbox, { CheckboxProps } from '@material-ui/core/Checkbox'; import ChevronLeft from '@material-ui/icons/ChevronLeft'; import ChevronRight from '@material-ui/icons/ChevronRight'; import IconButton from '@material-ui/core/IconButton'; import MenuItem from '@material-ui/core/MenuItem'; import Radio from '@material-ui/core/Radio'; +import Separator from '../atoms/Separator'; import TableSortLabel from '@material-ui/core/TableSortLabel'; import TextField from '@material-ui/core/TextField'; import Tooltip from '@material-ui/core/Tooltip'; import WarningIcon from '@material-ui/icons/WarningRounded'; import { ListRequest } from '../lib/Apis'; -import Checkbox, { CheckboxProps } from '@material-ui/core/Checkbox'; import { TextFieldProps } from '@material-ui/core/TextField'; import { classes, stylesheet } from 'typestyle'; import { fonts, fontsize, dimension, commonCss, color, padding } from '../Css'; @@ -256,6 +257,10 @@ export default class CustomTable extends React.Component )} + {/* Shift cells to account for expand button */} + {!!this.props.getExpandComponent && ( + + )} {this.props.columns.map((col, i) => { const isColumnSortable = !!this.props.columns[i].sortKey; const isCurrentSortColumn = sortBy === this.props.columns[i].sortKey; diff --git a/frontend/src/components/__snapshots__/CustomTable.test.tsx.snap b/frontend/src/components/__snapshots__/CustomTable.test.tsx.snap index 7503dff7b45..6fe84394630 100644 --- a/frontend/src/components/__snapshots__/CustomTable.test.tsx.snap +++ b/frontend/src/components/__snapshots__/CustomTable.test.tsx.snap @@ -234,6 +234,10 @@ exports[`CustomTable renders a collapsed row 1`] = ` onChange={[Function]} /> +
+
+
{ + const updateBannerSpy = jest.fn(); + const updateDialogSpy = jest.fn(); + const updateSnackbarSpy = jest.fn(); + const updateToolbarSpy = jest.fn(); + const historyPushSpy = jest.fn(); + const listExperimentsSpy = jest.spyOn(Apis.experimentServiceApi, 'listExperiment'); + const listRunsSpy = jest.spyOn(Apis.runServiceApi, 'listRuns'); + + function generateProps(): PageProps { + return { + history: { push: historyPushSpy } as any, + location: '' as any, + match: '' as any, + toolbarProps: ExperimentList.prototype.getInitialToolbarState(), + updateBanner: updateBannerSpy, + updateDialog: updateDialogSpy, + updateSnackbar: updateSnackbarSpy, + updateToolbar: updateToolbarSpy, + }; + } + + async function mountWithNExperiments(n: number, nRuns: number) { + listExperimentsSpy.mockImplementation(() => ({ + experiments: range(n).map(i => ({ id: 'test-experiment-id' + i, name: 'test experiment name' + i })), + })); + listRunsSpy.mockImplementation(() => ({ + runs: range(nRuns).map(i => ({ id: 'test-run-id' + i, name: 'test run name' + i })), + })); + const tree = TestUtils.mountWithRouter(); + await listExperimentsSpy; + await listRunsSpy; + await TestUtils.flushPromises(); + tree.update(); // Make sure the tree is updated before returning it + return tree; + } + + beforeEach(() => { + // Reset mocks + updateBannerSpy.mockReset(); + updateDialogSpy.mockReset(); + updateSnackbarSpy.mockReset(); + updateToolbarSpy.mockReset(); + listExperimentsSpy.mockReset(); + listRunsSpy.mockReset(); + }); + + it('renders an empty list with empty state message', () => { + const tree = shallow(); + expect(tree).toMatchSnapshot(); + tree.unmount(); + }); + + it('renders a list of one experiment', async () => { + const tree = shallow(); + tree.setState({ + displayExperiments: [{ + description: 'test experiment description', + expandState: ExpandState.COLLAPSED, + name: 'test experiment name', + }] + }); + await listExperimentsSpy; + await listRunsSpy; + expect(tree).toMatchSnapshot(); + tree.unmount(); + }); + + it('renders a list of one experiment with no description', async () => { + const tree = shallow(); + tree.setState({ + experiments: [{ + expandState: ExpandState.COLLAPSED, + name: 'test experiment name', + }] + }); + await listExperimentsSpy; + await listRunsSpy; + expect(tree).toMatchSnapshot(); + tree.unmount(); + }); + + it('renders a list of one experiment with error', async () => { + const tree = shallow(); + tree.setState({ + experiments: [{ + description: 'test experiment description', + error: 'oops! could not load experiment', + expandState: ExpandState.COLLAPSED, + name: 'test experiment name', + }] + }); + await listExperimentsSpy; + await listRunsSpy; + expect(tree).toMatchSnapshot(); + tree.unmount(); + }); + + it('calls Apis to list experiments, sorted by creation time in descending order', async () => { + const tree = await mountWithNExperiments(1, 1); + expect(listExperimentsSpy).toHaveBeenLastCalledWith('', 10, 'created_at desc'); + expect(listRunsSpy).toHaveBeenLastCalledWith(undefined, 5, 'created_at desc', + ApiResourceType.EXPERIMENT.toString(), 'test-experiment-id0'); + expect(tree.state()).toHaveProperty('displayExperiments', [{ + expandState: ExpandState.COLLAPSED, + id: 'test-experiment-id0', + last5Runs: [{ id: 'test-run-id0', name: 'test run name0' }], + name: 'test experiment name0', + }]); + tree.unmount(); + }); + + it('has a Refresh button, clicking it refreshes the experiment list', async () => { + const tree = await mountWithNExperiments(1, 1); + const instance = tree.instance() as ExperimentList; + expect(listExperimentsSpy.mock.calls.length).toBe(1); + const refreshBtn = instance.getInitialToolbarState().actions.find(b => b.title === 'Refresh'); + expect(refreshBtn).toBeDefined(); + await refreshBtn!.action(); + expect(listExperimentsSpy.mock.calls.length).toBe(2); + expect(listExperimentsSpy).toHaveBeenLastCalledWith('', 10, 'created_at desc'); + expect(updateBannerSpy).toHaveBeenLastCalledWith({}); + tree.unmount(); + }); + + it('shows error banner when listing experiments fails', async () => { + TestUtils.makeErrorResponseOnce(listExperimentsSpy, 'bad stuff happened'); + const tree = TestUtils.mountWithRouter(); + await listExperimentsSpy; + await TestUtils.flushPromises(); + expect(updateBannerSpy).toHaveBeenLastCalledWith(expect.objectContaining({ + additionalInfo: 'bad stuff happened', + message: 'Error: failed to retrieve list of experiments. Click Details for more information.', + mode: 'error', + })); + tree.unmount(); + }); + + it('shows error next to experiment when listing its last 5 runs fails', async () => { + // tslint:disable-next-line:no-console + console.error = jest.spyOn(console, 'error').mockImplementation(); + + listExperimentsSpy.mockImplementationOnce(() => ({ experiments: [{ name: 'exp1' }] })); + TestUtils.makeErrorResponseOnce(listRunsSpy, 'bad stuff happened'); + const tree = TestUtils.mountWithRouter(); + await listExperimentsSpy; + await TestUtils.flushPromises(); + expect(tree.state()).toHaveProperty('displayExperiments', [{ + error: 'Failed to load the last 5 runs of this experiment', + expandState: 0, + name: 'exp1', + }]); + tree.unmount(); + }); + + it('shows error banner when listing experiments fails after refresh', async () => { + const tree = TestUtils.mountWithRouter(); + const instance = tree.instance() as ExperimentList; + const refreshBtn = instance.getInitialToolbarState().actions.find(b => b.title === 'Refresh'); + expect(refreshBtn).toBeDefined(); + TestUtils.makeErrorResponseOnce(listExperimentsSpy, 'bad stuff happened'); + await refreshBtn!.action(); + expect(listExperimentsSpy.mock.calls.length).toBe(2); + expect(listExperimentsSpy).toHaveBeenLastCalledWith('', 10, 'created_at desc'); + expect(updateBannerSpy).toHaveBeenLastCalledWith(expect.objectContaining({ + additionalInfo: 'bad stuff happened', + message: 'Error: failed to retrieve list of experiments. Click Details for more information.', + mode: 'error', + })); + tree.unmount(); + }); + + it('hides error banner when listing experiments fails then succeeds', async () => { + TestUtils.makeErrorResponseOnce(listExperimentsSpy, 'bad stuff happened'); + const tree = TestUtils.mountWithRouter(); + const instance = tree.instance() as ExperimentList; + await listExperimentsSpy; + await TestUtils.flushPromises(); + expect(updateBannerSpy).toHaveBeenLastCalledWith(expect.objectContaining({ + additionalInfo: 'bad stuff happened', + message: 'Error: failed to retrieve list of experiments. Click Details for more information.', + mode: 'error', + })); + updateBannerSpy.mockReset(); + + const refreshBtn = instance.getInitialToolbarState().actions.find(b => b.title === 'Refresh'); + listExperimentsSpy.mockImplementationOnce(() => ({ experiments: [{ name: 'experiment1' }] })); + listRunsSpy.mockImplementationOnce(() => ({ runs: [{ name: 'run1' }] })); + await refreshBtn!.action(); + expect(listExperimentsSpy.mock.calls.length).toBe(2); + expect(updateBannerSpy).toHaveBeenLastCalledWith({}); + tree.unmount(); + }); + + it('can expand an experiment to see its runs', async () => { + const tree = await mountWithNExperiments(1, 1); + tree.find('.tableRow button').at(0).simulate('click'); + expect(tree.state()).toHaveProperty('displayExperiments', [{ + expandState: ExpandState.EXPANDED, + id: 'test-experiment-id0', + last5Runs: [{ id: 'test-run-id0', name: 'test run name0' }], + name: 'test experiment name0', + }]); + tree.unmount(); + }); + + it('renders a list of runs for given experiment', async () => { + const tree = shallow(); + tree.setState({ displayExperiments: [{ last5Runs: [{ id: 'run1id' }, { id: 'run2id' }] }] }); + const runListTree = (tree.instance() as any)._getExpandedExperimentComponent(0); + expect(runListTree.props.runIdListMask).toEqual(['run1id', 'run2id']); + }); + + it('navigates to new experiment page when Create experiment button is clicked', async () => { + const tree = TestUtils.mountWithRouter(); + const createBtn = (tree.instance() as ExperimentList) + .getInitialToolbarState().actions.find(b => b.title === 'Create experiment'); + await createBtn!.action(); + expect(historyPushSpy).toHaveBeenLastCalledWith(RoutePage.NEW_EXPERIMENT); + }); + + it('always has new experiment button enabled', async () => { + const tree = await mountWithNExperiments(1, 1); + const calls = updateToolbarSpy.mock.calls[0]; + expect(calls[0].actions.find((b: any) => b.title === 'Create experiment')).not.toHaveProperty('disabled'); + tree.unmount(); + }); + + it('enables clone button when one run is selected', async () => { + const tree = await mountWithNExperiments(1, 1); + (tree.instance() as any)._runSelectionChanged(['run1']); + expect(updateToolbarSpy).toHaveBeenCalledTimes(2); + expect(updateToolbarSpy.mock.calls[0][0].actions.find((b: any) => b.title === 'Clone run')) + .toHaveProperty('disabled', true); + expect(updateToolbarSpy.mock.calls[1][0].actions.find((b: any) => b.title === 'Clone run')) + .toHaveProperty('disabled', false); + tree.unmount(); + }); + + it('disables clone button when more than one run is selected', async () => { + const tree = await mountWithNExperiments(1, 1); + (tree.instance() as any)._runSelectionChanged(['run1', 'run2']); + expect(updateToolbarSpy).toHaveBeenCalledTimes(2); + expect(updateToolbarSpy.mock.calls[0][0].actions.find((b: any) => b.title === 'Clone run')) + .toHaveProperty('disabled', true); + expect(updateToolbarSpy.mock.calls[1][0].actions.find((b: any) => b.title === 'Clone run')) + .toHaveProperty('disabled', true); + tree.unmount(); + }); + + it('enables compare runs button only when more than one is selected', async () => { + const tree = await mountWithNExperiments(1, 1); + (tree.instance() as any)._runSelectionChanged(['run1']); + (tree.instance() as any)._runSelectionChanged(['run1', 'run2']); + (tree.instance() as any)._runSelectionChanged(['run1', 'run2', 'run3']); + expect(updateToolbarSpy).toHaveBeenCalledTimes(4); + expect(updateToolbarSpy.mock.calls[0][0].actions.find((b: any) => b.title === 'Compare runs')) + .toHaveProperty('disabled', true); + expect(updateToolbarSpy.mock.calls[1][0].actions.find((b: any) => b.title === 'Compare runs')) + .toHaveProperty('disabled', true); + expect(updateToolbarSpy.mock.calls[2][0].actions.find((b: any) => b.title === 'Compare runs')) + .toHaveProperty('disabled', false); + expect(updateToolbarSpy.mock.calls[3][0].actions.find((b: any) => b.title === 'Compare runs')) + .toHaveProperty('disabled', false); + tree.unmount(); + }); + + it('navigates to compare page with the selected run ids', async () => { + const tree = await mountWithNExperiments(1, 1); + (tree.instance() as any)._runSelectionChanged(['run1', 'run2', 'run3']); + const compareBtn = (tree.instance() as ExperimentList) + .getInitialToolbarState().actions.find(b => b.title === 'Compare runs'); + await compareBtn!.action(); + expect(historyPushSpy).toHaveBeenLastCalledWith( + `${RoutePage.COMPARE}?${QUERY_PARAMS.runlist}=run1,run2,run3`); + }); + + it('navigates to new run page with the selected run id for cloning', async () => { + const tree = await mountWithNExperiments(1, 1); + (tree.instance() as any)._runSelectionChanged(['run1']); + const cloneBtn = (tree.instance() as ExperimentList) + .getInitialToolbarState().actions.find(b => b.title === 'Clone run'); + await cloneBtn!.action(); + expect(historyPushSpy).toHaveBeenLastCalledWith( + `${RoutePage.NEW_RUN}?${QUERY_PARAMS.cloneFromRun}=run1`); + }); + + it('renders experiment names as links to their details pages', async () => { + const tree = TestUtils.mountWithRouter((ExperimentList.prototype as any) + ._nameCustomRenderer('experiment name', 'experiment-id')); + expect(tree).toMatchSnapshot(); + }); + + it('renders last 5 runs statuses', async () => { + const tree = shallow((ExperimentList.prototype as any) + ._last5RunsCustomRenderer([ + { status: NodePhase.SUCCEEDED }, + { status: NodePhase.PENDING }, + { status: NodePhase.FAILED }, + { status: NodePhase.UNKNOWN }, + { status: NodePhase.SUCCEEDED }, + ])); + expect(tree).toMatchSnapshot(); + }); +}); diff --git a/frontend/src/pages/ExperimentList.tsx b/frontend/src/pages/ExperimentList.tsx index 25bc19d2af1..58ecb9cf8fd 100644 --- a/frontend/src/pages/ExperimentList.tsx +++ b/frontend/src/pages/ExperimentList.tsx @@ -18,9 +18,10 @@ import * as React from 'react'; import AddIcon from '@material-ui/icons/Add'; import CustomTable, { Column, Row, ExpandState } from '../components/CustomTable'; import RunList from './RunList'; -import { Apis, ExperimentSortKeys, ListRequest, RunSortKeys } from '../lib/Apis'; +import produce from 'immer'; import { ApiListExperimentsResponse, ApiExperiment } from '../apis/experiment'; import { ApiResourceType, ApiRun } from '../apis/run'; +import { Apis, ExperimentSortKeys, ListRequest, RunSortKeys } from '../lib/Apis'; import { Link } from 'react-router-dom'; import { Page } from './Page'; import { RoutePage, RouteParams } from '../components/Router'; @@ -38,7 +39,6 @@ interface DisplayExperiment extends ApiExperiment { interface ExperimentListState { displayExperiments: DisplayExperiment[]; - selectedExperimentIds: string[]; selectedRunIds: string[]; selectedTab: number; sortBy: string; @@ -52,7 +52,6 @@ class ExperimentList extends Page<{}, ExperimentListState> { this.state = { displayExperiments: [], - selectedExperimentIds: [], selectedRunIds: [], selectedTab: 0, sortBy: ExperimentSortKeys.CREATED_AT, @@ -83,7 +82,7 @@ class ExperimentList extends Page<{}, ExperimentListState> { title: 'Clone run', tooltip: 'Create a copy from this run\s initial state', }, { - action: this._reload.bind(this), + action: this.refresh.bind(this), id: 'refreshBtn', title: 'Refresh', tooltip: 'Refresh the list of experiments', @@ -93,19 +92,19 @@ class ExperimentList extends Page<{}, ExperimentListState> { } public render() { - const columns: Column[] = [ - { - customRenderer: this._nameCustomRenderer.bind(this), - flex: 2, - label: 'Experiment name', - sortKey: ExperimentSortKeys.NAME, - }, - { - customRenderer: this._last5RunsCustomRenderer.bind(this), - flex: 1, - label: 'Last 5 runs', - }, - ]; + const columns: Column[] = [{ + customRenderer: this._nameCustomRenderer.bind(this), + flex: 1, + label: 'Experiment name', + sortKey: ExperimentSortKeys.NAME, + }, { + flex: 2, + label: 'Description', + }, { + customRenderer: this._last5RunsCustomRenderer.bind(this), + flex: 1, + label: 'Last 5 runs', + }]; const rows: Row[] = this.state.displayExperiments.map((exp) => { return { @@ -114,6 +113,7 @@ class ExperimentList extends Page<{}, ExperimentListState> { id: exp.id!, otherFields: [ exp.name!, + exp.description!, exp.expandState === ExpandState.EXPANDED ? [] : exp.last5Runs, ] }; @@ -123,8 +123,7 @@ class ExperimentList extends Page<{}, ExperimentListState> {
@@ -134,7 +133,7 @@ class ExperimentList extends Page<{}, ExperimentListState> { public async refresh() { if (this._tableRef.current) { this.clearBanner(); - this._tableRef.current.reload(); + await this._tableRef.current.reload(); } } @@ -193,7 +192,7 @@ class ExperimentList extends Page<{}, ExperimentListState> { private _last5RunsCustomRenderer(runs: ApiRun[]) { return
- {runs.map((run, i) => ( + {(runs || []).map((run, i) => ( {statusToIcon(run.status as NodePhase || NodePhase.UNKNOWN)} @@ -202,12 +201,13 @@ class ExperimentList extends Page<{}, ExperimentListState> { } private _runSelectionChanged(selectedRunIds: string[]) { - const toolbarActions = [...this.props.toolbarProps.actions]; - // Enable/Disable Run compare button - toolbarActions[1].disabled = selectedRunIds.length <= 1 || selectedRunIds.length > 10; - // Enable/Disable Clone button - toolbarActions[2].disabled = selectedRunIds.length !== 1; - this.props.updateToolbar({ breadcrumbs: this.props.toolbarProps.breadcrumbs, actions: toolbarActions }); + const actions = produce(this.props.toolbarProps.actions, draft => { + // Enable/Disable Run compare button + draft[1].disabled = selectedRunIds.length <= 1 || selectedRunIds.length > 10; + // Enable/Disable Clone button + draft[2].disabled = selectedRunIds.length !== 1; + }); + this.props.updateToolbar({ breadcrumbs: this.props.toolbarProps.breadcrumbs, actions }); this.setState({ selectedRunIds }); } @@ -227,11 +227,12 @@ class ExperimentList extends Page<{}, ExperimentListState> { } private _toggleRowExpand(rowIndex: number) { - const displayExperiments = this.state.displayExperiments; - displayExperiments[rowIndex].expandState = - displayExperiments[rowIndex].expandState === ExpandState.COLLAPSED ? - ExpandState.EXPANDED : - ExpandState.COLLAPSED; + const displayExperiments = produce(this.state.displayExperiments, draft => { + draft[rowIndex].expandState = + draft[rowIndex].expandState === ExpandState.COLLAPSED ? + ExpandState.EXPANDED : + ExpandState.COLLAPSED; + }); this.setState({ displayExperiments }); } diff --git a/frontend/src/pages/PipelineList.test.tsx b/frontend/src/pages/PipelineList.test.tsx index 4c45eab44b4..46602d9fc40 100644 --- a/frontend/src/pages/PipelineList.test.tsx +++ b/frontend/src/pages/PipelineList.test.tsx @@ -50,10 +50,10 @@ describe('PipelineList', () => { listPipelinesSpy.mockImplementationOnce(() => ({ pipelines: range(n).map(i => ({ id: 'test-pipeline-id' + i, name: 'test pipeline name' + i })), })); - const tree = TestUtils.mountWithRouter(PipelineList, generateProps()); + const tree = TestUtils.mountWithRouter(); await listPipelinesSpy; await TestUtils.flushPromises(); - tree.update(); // Make sure the tree is updated before searching it + tree.update(); // Make sure the tree is updated before returning it return tree; } @@ -120,7 +120,7 @@ describe('PipelineList', () => { it('calls Apis to list pipelines, sorted by creation time in descending order', async () => { listPipelinesSpy.mockImplementationOnce(() => ({ pipelines: [{ name: 'pipeline1' }] })); - const tree = TestUtils.mountWithRouter(PipelineList, generateProps()); + const tree = TestUtils.mountWithRouter(); await listPipelinesSpy; expect(listPipelinesSpy).toHaveBeenLastCalledWith('', 10, 'created_at desc'); expect(tree.state()).toHaveProperty('pipelines', [{ name: 'pipeline1' }]); @@ -142,7 +142,7 @@ describe('PipelineList', () => { it('shows error banner when listing pipelines fails', async () => { TestUtils.makeErrorResponseOnce(listPipelinesSpy, 'bad stuff happened'); - const tree = TestUtils.mountWithRouter(PipelineList, generateProps()); + const tree = TestUtils.mountWithRouter(); await listPipelinesSpy; await TestUtils.flushPromises(); expect(updateBannerSpy).toHaveBeenLastCalledWith(expect.objectContaining({ @@ -154,7 +154,7 @@ describe('PipelineList', () => { }); it('shows error banner when listing pipelines fails after refresh', async () => { - const tree = TestUtils.mountWithRouter(PipelineList, generateProps()); + const tree = TestUtils.mountWithRouter(); const instance = tree.instance() as PipelineList; const refreshBtn = instance.getInitialToolbarState().actions.find(b => b.title === 'Refresh'); expect(refreshBtn).toBeDefined(); @@ -172,7 +172,7 @@ describe('PipelineList', () => { it('hides error banner when listing pipelines fails then succeeds', async () => { TestUtils.makeErrorResponseOnce(listPipelinesSpy, 'bad stuff happened'); - const tree = TestUtils.mountWithRouter(PipelineList, generateProps()); + const tree = TestUtils.mountWithRouter(); const instance = tree.instance() as PipelineList; await listPipelinesSpy; await TestUtils.flushPromises(); @@ -201,6 +201,13 @@ describe('PipelineList', () => { tree.unmount(); }); + it('always has upload pipeline button enabled', async () => { + const tree = await mountWithNPipelines(1); + const calls = updateToolbarSpy.mock.calls[0]; + expect(calls[0].actions.find((b: any) => b.title === 'Upload pipeline')).not.toHaveProperty('disabled'); + tree.unmount(); + }); + it('enables delete button when one pipeline is selected', async () => { const tree = await mountWithNPipelines(1); tree.find('.tableRow').simulate('click'); diff --git a/frontend/src/pages/PipelineList.tsx b/frontend/src/pages/PipelineList.tsx index 6bee051c266..dea08273b8a 100644 --- a/frontend/src/pages/PipelineList.tsx +++ b/frontend/src/pages/PipelineList.tsx @@ -15,9 +15,10 @@ */ import * as React from 'react'; -import CustomTable, { Column, Row } from '../components/CustomTable'; import AddIcon from '@material-ui/icons/Add'; +import CustomTable, { Column, Row } from '../components/CustomTable'; import UploadPipelineDialog from '../components/UploadPipelineDialog'; +import produce from 'immer'; import { ApiPipeline, ApiListPipelinesResponse } from '../apis/pipeline'; import { Apis, PipelineSortKeys, ListRequest } from '../lib/Apis'; import { Link } from 'react-router-dom'; @@ -142,9 +143,10 @@ class PipelineList extends Page<{}, PipelineListState> { } private _selectionChanged(selectedIds: string[]): void { - const toolbarActions = [...this.props.toolbarProps.actions]; - // Delete pipeline - toolbarActions[2].disabled = selectedIds.length < 1; + const toolbarActions = produce(this.props.toolbarProps.actions, draft => { + // Delete pipeline + draft[2].disabled = selectedIds.length < 1; + }); this.props.updateToolbar({ breadcrumbs: this.props.toolbarProps.breadcrumbs, actions: toolbarActions }); this.setStateSafe({ selectedIds }); } diff --git a/frontend/src/pages/__snapshots__/ExperimentList.test.tsx.snap b/frontend/src/pages/__snapshots__/ExperimentList.test.tsx.snap new file mode 100644 index 00000000000..85b50ffcf9f --- /dev/null +++ b/frontend/src/pages/__snapshots__/ExperimentList.test.tsx.snap @@ -0,0 +1,298 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`ExperimentList renders a list of one experiment 1`] = ` +
+ +
+`; + +exports[`ExperimentList renders a list of one experiment with error 1`] = ` +
+ +
+`; + +exports[`ExperimentList renders a list of one experiment with no description 1`] = ` +
+ +
+`; + +exports[`ExperimentList renders an empty list with empty state message 1`] = ` +
+ +
+`; + +exports[`ExperimentList renders experiment names as links to their details pages 1`] = ` + + + experiment name + + +`; + +exports[`ExperimentList renders last 5 runs statuses 1`] = ` +
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
+`;