diff --git a/frontend/src/pages/AllRunsList.test.tsx b/frontend/src/pages/AllRunsList.test.tsx index 012df5adef9..89b84cdf407 100644 --- a/frontend/src/pages/AllRunsList.test.tsx +++ b/frontend/src/pages/AllRunsList.test.tsx @@ -36,6 +36,7 @@ describe('AllRunsList', () => { updateToolbar: updateToolbarSpy, }; + // TODO: why is this called mount if it is using shallow? function mountComponent(): ShallowWrapper { const tree = shallow(); // Necessary since the component calls updateToolbar with the toolbar props, diff --git a/frontend/src/pages/ExperimentDetails.test.tsx b/frontend/src/pages/ExperimentDetails.test.tsx new file mode 100644 index 00000000000..d08763bd596 --- /dev/null +++ b/frontend/src/pages/ExperimentDetails.test.tsx @@ -0,0 +1,441 @@ +/* + * Copyright 2018 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as React from 'react'; +import ExperimentDetails from './ExperimentDetails'; +import TestUtils from '../TestUtils'; +import { ReactWrapper, ShallowWrapper, shallow } from 'enzyme'; +import { Apis } from '../lib/Apis'; +import { PageProps } from './Page'; +import { range } from 'lodash'; +import { ApiExperiment } from '../apis/experiment'; +import { ApiResourceType } from '../apis/job'; +import { RoutePage, RouteParams } from '../components/Router'; +import { ToolbarProps } from '../components/Toolbar'; +import { QUERY_PARAMS } from '../lib/URLParser'; + +describe('ExperimentDetails', () => { + + let tree: ReactWrapper | ShallowWrapper; + + const consoleLogSpy = jest.spyOn(console, 'log').mockImplementation(() => null); + const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => null); + + const updateToolbarSpy = jest.fn(); + const updateBannerSpy = jest.fn(); + const updateDialogSpy = jest.fn(); + const updateSnackbarSpy = jest.fn(); + const historyPushSpy = jest.fn(); + const getExperimentSpy = jest.spyOn(Apis.experimentServiceApi, 'getExperiment'); + const listJobsSpy = jest.spyOn(Apis.jobServiceApi, 'listJobs'); + const listRunsSpy = jest.spyOn(Apis.runServiceApi, 'listRuns'); + + const MOCK_EXPERIMENT = newMockExperiment(); + + function newMockExperiment(): ApiExperiment { + return { + description: 'mock experiment description', + id: 'some-mock-experiment-id', + name: 'some mock experiment name', + }; + } + + function generateProps(): PageProps { + return { + history: { push: historyPushSpy } as any, + location: '' as any, + // 'eid' here corresponds to RouteParams.experimentId + match: { params: { eid: MOCK_EXPERIMENT.id } } as any, + toolbarProps: ExperimentDetails.prototype.getInitialToolbarState(), + updateBanner: updateBannerSpy, + updateDialog: updateDialogSpy, + updateSnackbar: updateSnackbarSpy, + updateToolbar: updateToolbarSpy, + }; + } + + async function mockNJobs(n: number): Promise { + listJobsSpy.mockImplementation(() => ({ + jobs: range(n).map(i => ({ id: 'test-job-id' + i, enabled: true, name: 'test job name' + i })), + })); + await listJobsSpy; + await TestUtils.flushPromises(); + } + + async function mockNRuns(n: number): Promise { + listRunsSpy.mockImplementation(() => ({ + runs: range(n).map(i => ({ id: 'test-run-id' + i, name: 'test run name' + i })), + })); + await listRunsSpy; + await TestUtils.flushPromises(); + } + + beforeEach(async () => { + // Reset mocks + consoleLogSpy.mockReset(); + consoleErrorSpy.mockReset(); + updateBannerSpy.mockReset(); + updateDialogSpy.mockReset(); + updateSnackbarSpy.mockReset(); + updateToolbarSpy.mockReset(); + getExperimentSpy.mockReset(); + historyPushSpy.mockReset(); + listJobsSpy.mockReset(); + listRunsSpy.mockReset(); + + getExperimentSpy.mockImplementation(() => newMockExperiment()); + + await mockNJobs(0); + await mockNRuns(0); + }); + + afterEach(() => { + tree.unmount(); + }); + + it('renders a page with no runs or recurring runs', async () => { + tree = shallow(); + await TestUtils.flushPromises(); + expect(updateBannerSpy).toHaveBeenCalledTimes(1); + expect(updateBannerSpy).toHaveBeenLastCalledWith({}); + expect(tree).toMatchSnapshot(); + }); + + it('uses the experiment ID in props as the page title if the experiment has no name', async () => { + const experiment = newMockExperiment(); + experiment.name = ''; + + const props = generateProps(); + props.match = { params: { [RouteParams.experimentId]: 'test exp ID' } } as any; + + getExperimentSpy.mockImplementationOnce(() => experiment); + + tree = shallow(); + await TestUtils.flushPromises(); + expect(updateToolbarSpy).toHaveBeenLastCalledWith(expect.objectContaining({ + pageTitle: 'test exp ID', + pageTitleTooltip: 'test exp ID' + })); + }); + + it('uses the experiment name as the page title', async () => { + const experiment = newMockExperiment(); + experiment.name = 'A Test Experiment'; + + getExperimentSpy.mockImplementationOnce(() => experiment); + + tree = shallow(); + await TestUtils.flushPromises(); + expect(updateToolbarSpy).toHaveBeenLastCalledWith(expect.objectContaining({ + pageTitle: 'A Test Experiment', + pageTitleTooltip: 'A Test Experiment' + })); + }); + + it('uses an empty string if the experiment has no description', async () => { + const experiment = newMockExperiment(); + delete experiment.description; + + getExperimentSpy.mockImplementationOnce(() => experiment); + + tree = shallow(); + await TestUtils.flushPromises(); + expect(tree).toMatchSnapshot(); + }); + + it('removes all description text after second newline and replaces with an ellipsis', async () => { + const experiment = newMockExperiment(); + experiment.description = 'Line 1\nLine 2\nLine 3\nLine 4'; + + getExperimentSpy.mockImplementationOnce(() => experiment); + + tree = shallow(); + await TestUtils.flushPromises(); + expect(tree).toMatchSnapshot(); + }); + + it('opens the expanded description modal when the expand button is clicked', async () => { + tree = TestUtils.mountWithRouter(); + await TestUtils.flushPromises(); + + tree.update(); + + tree.find('#expandExperimentDescriptionBtn').at(0).simulate('click'); + await TestUtils.flushPromises(); + expect(updateDialogSpy).toHaveBeenCalledWith({ + content: MOCK_EXPERIMENT.description, + title: 'Experiment description', + }); + }); + + it('calls getExperiment with the experiment ID in props', async () => { + const props = generateProps(); + props.match = { params: { eid: 'test exp ID' } } as any; + tree = shallow(); + await TestUtils.flushPromises(); + expect(getExperimentSpy).toHaveBeenCalledTimes(1); + expect(getExperimentSpy).toHaveBeenCalledWith('test exp ID'); + }); + + it('shows an error banner if fetching the experiment fails', async () => { + TestUtils.makeErrorResponseOnce(getExperimentSpy, 'test error'); + + tree = shallow(); + await TestUtils.flushPromises(); + + expect(updateBannerSpy).toHaveBeenLastCalledWith(expect.objectContaining({ + additionalInfo: 'test error', + message: 'Error: failed to retrieve experiment: ' + MOCK_EXPERIMENT.id + + '. Click Details for more information.', + mode: 'error', + })); + expect(consoleErrorSpy.mock.calls[0][0]).toBe( + 'Error loading experiment: ' + MOCK_EXPERIMENT.id + ); + }); + + it('fetches this experiment\'s recurring runs', async () => { + await mockNJobs(1); + + tree = shallow(); + await TestUtils.flushPromises(); + + expect(listJobsSpy).toHaveBeenCalledTimes(1); + expect(listJobsSpy).toHaveBeenLastCalledWith( + undefined, + 100, + '', + ApiResourceType.EXPERIMENT.toString(), + MOCK_EXPERIMENT.id, + ); + expect(tree.state('activeRecurringRunsCount')).toBe(1); + expect(tree).toMatchSnapshot(); + }); + + it('shows an error banner if fetching the experiment\'s recurring runs fails', async () => { + TestUtils.makeErrorResponseOnce(listJobsSpy, 'test error'); + + tree = shallow(); + await TestUtils.flushPromises(); + + expect(updateBannerSpy).toHaveBeenLastCalledWith(expect.objectContaining({ + additionalInfo: 'test error', + message: 'Error: failed to retrieve recurring runs for experiment: ' + MOCK_EXPERIMENT.id + + '. Click Details for more information.', + mode: 'error', + })); + expect(consoleErrorSpy.mock.calls[0][0]).toBe( + 'Error fetching recurring runs for experiment: ' + MOCK_EXPERIMENT.id + ); + }); + + it('only counts enabled recurring runs as active', async () => { + const jobs = [ + { id: 'enabled-job-1-id', enabled: true, name: 'enabled-job-1' }, + { id: 'enabled-job-2-id', enabled: true, name: 'enabled-job-2' }, + { id: 'disabled-job-1-id', enabled: false, name: 'disabled-job-1' }, + ]; + listJobsSpy.mockImplementationOnce(() => ({ jobs })); + await listJobsSpy; + + tree = shallow(); + await TestUtils.flushPromises(); + + expect(tree.state('activeRecurringRunsCount')).toBe(2); + }); + + it('opens the recurring run manager modal when \'manage\' is clicked', async () => { + await mockNJobs(1); + tree = TestUtils.mountWithRouter(); + await TestUtils.flushPromises(); + + tree.update(); + + tree.find('#manageExperimentRecurringRunsBtn').at(0).simulate('click'); + await TestUtils.flushPromises(); + expect(tree.state('recurringRunsManagerOpen')).toBe(true); + }); + + it('closes the recurring run manager modal', async () => { + await mockNJobs(1); + tree = TestUtils.mountWithRouter(); + await TestUtils.flushPromises(); + + tree.update(); + + tree.find('#manageExperimentRecurringRunsBtn').at(0).simulate('click'); + await TestUtils.flushPromises(); + expect(tree.state('recurringRunsManagerOpen')).toBe(true); + + tree.find('#closeExperimentRecurringRunManagerBtn').at(0).simulate('click'); + await TestUtils.flushPromises(); + expect(tree.state('recurringRunsManagerOpen')).toBe(false); + + }); + + it('refreshes the number of active recurring runs when the recurring run manager is closed', async () => { + await mockNJobs(1); + tree = TestUtils.mountWithRouter(); + await TestUtils.flushPromises(); + + tree.update(); + + // Called when the page initially loads to display the number of active recurring runs + expect(listJobsSpy).toHaveBeenCalledTimes(1); + + tree.find('#manageExperimentRecurringRunsBtn').at(0).simulate('click'); + await TestUtils.flushPromises(); + expect(tree.state('recurringRunsManagerOpen')).toBe(true); + + // Called in the recurring run manager to list the recurring runs + expect(listJobsSpy).toHaveBeenCalledTimes(2); + + tree.find('#closeExperimentRecurringRunManagerBtn').at(0).simulate('click'); + await TestUtils.flushPromises(); + expect(tree.state('recurringRunsManagerOpen')).toBe(false); + + // Called a third time when the manager is closed to update the number of active recurring runs + expect(listJobsSpy).toHaveBeenCalledTimes(3); + + }); + + it('clears the error banner on refresh', async () => { + TestUtils.makeErrorResponseOnce(getExperimentSpy, 'test error'); + + tree = shallow(); + await TestUtils.flushPromises(); + + // Verify that error banner is being shown + expect(updateBannerSpy).toHaveBeenLastCalledWith(expect.objectContaining({ mode: 'error' })); + + (tree.instance() as ExperimentDetails).refresh(); + + // Error banner should be cleared + expect(updateBannerSpy).toHaveBeenLastCalledWith({}); + + }); + + it('navigates to the compare runs page', async () => { + const runs = [ + { id: 'run-1-id', name: 'run-1' }, + { id: 'run-2-id', name: 'run-2' }, + ]; + listRunsSpy.mockImplementationOnce(() => ({ runs })); + await listRunsSpy; + + tree = TestUtils.mountWithRouter(); + await TestUtils.flushPromises(); + tree.update(); + + tree.find('.tableRow').at(0).simulate('click'); + tree.find('.tableRow').at(1).simulate('click'); + + const compareBtn = (tree.state('runListToolbarProps') as ToolbarProps) + .actions.find(b => b.title === 'Compare runs'); + await compareBtn!.action(); + + expect(historyPushSpy).toHaveBeenCalledWith( + RoutePage.COMPARE + `?${QUERY_PARAMS.runlist}=run-1-id,run-2-id`); + }); + + it('navigates to the new run page and passes this experiment\s ID as a query param', async () => { + tree = shallow(); + await TestUtils.flushPromises(); + tree.update(); + + const newRunBtn = (tree.state('runListToolbarProps') as ToolbarProps) + .actions.find(b => b.title === 'Create run'); + await newRunBtn!.action(); + + expect(historyPushSpy).toHaveBeenCalledWith( + RoutePage.NEW_RUN + `?${QUERY_PARAMS.experimentId}=${MOCK_EXPERIMENT.id}`); + }); + + it('navigates to the new run page with query param indicating it will be a recurring run', async () => { + tree = shallow(); + await TestUtils.flushPromises(); + tree.update(); + + const newRecurringRunBtn = (tree.state('runListToolbarProps') as ToolbarProps) + .actions.find(b => b.title === 'Create recurring run'); + await newRecurringRunBtn!.action(); + + expect(historyPushSpy).toHaveBeenCalledWith( + RoutePage.NEW_RUN + + `?${QUERY_PARAMS.experimentId}=${MOCK_EXPERIMENT.id}` + + `&${QUERY_PARAMS.isRecurring}=1`); + }); + + it('supports cloning a selected run', async () => { + const runs = [{ id: 'run-1-id', name: 'run-1' }]; + listRunsSpy.mockImplementationOnce(() => ({ runs })); + await listRunsSpy; + + tree = TestUtils.mountWithRouter(); + await TestUtils.flushPromises(); + tree.update(); + + // Select the run to clone + tree.find('.tableRow').simulate('click'); + + const cloneBtn = (tree.state('runListToolbarProps') as ToolbarProps) + .actions.find(b => b.title === 'Clone'); + await cloneBtn!.action(); + + expect(historyPushSpy).toHaveBeenCalledWith( + RoutePage.NEW_RUN + `?${QUERY_PARAMS.cloneFromRun}=run-1-id`); + }); + + it('enables the compare runs button only when between 2 and 10 runs are selected', async () => { + await mockNRuns(12); + + tree = TestUtils.mountWithRouter(); + await TestUtils.flushPromises(); + tree.update(); + + const compareBtn = (tree.state('runListToolbarProps') as ToolbarProps) + .actions.find(b => b.title === 'Compare runs'); + + for (let i = 0; i < 12; i++) { + if (i < 2 || i > 10) { + expect(compareBtn!.disabled).toBe(true); + } else { + expect(compareBtn!.disabled).toBe(false); + } + tree.find('.tableRow').at(i).simulate('click'); + } + }); + + it('enables the clone run button only when 1 run is selected', async () => { + await mockNRuns(4); + + tree = TestUtils.mountWithRouter(); + await TestUtils.flushPromises(); + tree.update(); + + const cloneBtn = (tree.state('runListToolbarProps') as ToolbarProps) + .actions.find(b => b.title === 'Clone'); + + for (let i = 0; i < 4; i++) { + if (i === 1) { + expect(cloneBtn!.disabled).toBe(false); + } else { + expect(cloneBtn!.disabled).toBe(true); + } + tree.find('.tableRow').at(i).simulate('click'); + } + }); +}); diff --git a/frontend/src/pages/ExperimentDetails.tsx b/frontend/src/pages/ExperimentDetails.tsx index bf10b247f10..5e0054f0136 100644 --- a/frontend/src/pages/ExperimentDetails.tsx +++ b/frontend/src/pages/ExperimentDetails.tsx @@ -170,13 +170,14 @@ class ExperimentDetails extends Page<{}, ExperimentDetailsState> { tooltip: 'Refresh', }], breadcrumbs: [{ displayName: 'Experiments', href: RoutePage.EXPERIMENTS }], - pageTitle: this.props.match.params[RouteParams.experimentId], + // TODO: determine what to show if no props. + pageTitle: this.props ? this.props.match.params[RouteParams.experimentId] : '', }; } public render(): JSX.Element { const { activeRecurringRunsCount, experiment } = this.state; - const description = experiment ? experiment.description || '' : ''; + const description = experiment ? (experiment.description || '') : ''; return (
@@ -194,7 +195,7 @@ class ExperimentDetails extends Page<{}, ExperimentDetailsState> {
{activeRecurringRunsCount + ' active'}
- @@ -203,7 +204,7 @@ class ExperimentDetails extends Page<{}, ExperimentDetailsState> {
Experiment description - @@ -257,8 +260,7 @@ class ExperimentDetails extends Page<{}, ExperimentDetailsState> { try { const experiment = await Apis.experimentServiceApi.getExperiment(experimentId); - const pageTitle = (experiment && experiment.name) ? - experiment.name : this.props.match.params[RouteParams.experimentId]; + const pageTitle = experiment.name || this.props.match.params[RouteParams.experimentId]; this.props.updateToolbar({ actions: this.props.toolbarProps.actions, @@ -267,24 +269,37 @@ class ExperimentDetails extends Page<{}, ExperimentDetailsState> { pageTitleTooltip: pageTitle, }); - // TODO: get ALL jobs in the experiment - const recurringRuns = await Apis.jobServiceApi.listJobs( - undefined, - 100, - '', - ApiResourceType.EXPERIMENT.toString(), - experimentId, - ); - const activeRecurringRunsCount = - (recurringRuns.jobs || []).filter(j => j.enabled === true).length; - this.setState( - { activeRecurringRunsCount, experiment }, - () => this._runlistRef.current && this._runlistRef.current.refresh() - ); + let activeRecurringRunsCount = -1; + + // Fetch this experiment's jobs + try { + // TODO: get ALL jobs in the experiment + const recurringRuns = await Apis.jobServiceApi.listJobs( + undefined, + 100, + '', + ApiResourceType.EXPERIMENT.toString(), + experimentId, + ); + activeRecurringRunsCount = + (recurringRuns.jobs || []).filter(j => j.enabled === true).length; + + } catch (err) { + await this.showPageError( + `Error: failed to retrieve recurring runs for experiment: ${experimentId}.`, err); + logger.error(`Error fetching recurring runs for experiment: ${experimentId}`, err); + } + + this.setStateSafe({ activeRecurringRunsCount, experiment }); + } catch (err) { await this.showPageError(`Error: failed to retrieve experiment: ${experimentId}.`, err); logger.error(`Error loading experiment: ${experimentId}`, err); } + + if (this._runlistRef.current) { + this._runlistRef.current.refresh(); + } } private _compareRuns(): void { diff --git a/frontend/src/pages/__snapshots__/ExperimentDetails.test.tsx.snap b/frontend/src/pages/__snapshots__/ExperimentDetails.test.tsx.snap new file mode 100644 index 00000000000..28556b6a3a3 --- /dev/null +++ b/frontend/src/pages/__snapshots__/ExperimentDetails.test.tsx.snap @@ -0,0 +1,1336 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`ExperimentDetails fetches this experiment's recurring runs 1`] = ` +
+
+
+ +
+
+ Recurring run configs +
+
+ 1 active +
+ + Manage + +
+
+ +
+ + Experiment description + + + + + + +
+
+ mock experiment description +
+
+
+ + + + + + + + + Close + + + +
+
+`; + +exports[`ExperimentDetails removes all description text after second newline and replaces with an ellipsis 1`] = ` +
+
+
+ +
+
+ Recurring run configs +
+
+ 0 active +
+ + Manage + +
+
+ +
+ + Experiment description + + + + + + +
+
+ Line 1 +
+
+ Line 2 +
+ ... +
+
+ + + + + + + + + Close + + + +
+
+`; + +exports[`ExperimentDetails renders a page with no runs or recurring runs 1`] = ` +
+
+
+ +
+
+ Recurring run configs +
+
+ 0 active +
+ + Manage + +
+
+ +
+ + Experiment description + + + + + + +
+
+ mock experiment description +
+
+
+ + + + + + + + + Close + + + +
+
+`; + +exports[`ExperimentDetails uses an empty string if the experiment has no description 1`] = ` +
+
+
+ +
+
+ Recurring run configs +
+
+ 0 active +
+ + Manage + +
+
+ +
+ + Experiment description + + + + + + +
+
+ +
+ + + + + + + + + Close + + + +
+
+`;