PipelineSelector, RecurringRunsManager, and 404Page tests (#319)

* pipeline selector tests

* 404 page tests

* wip more fixes

* recurring runs manager tests

* restore refresh, add test for it

* pr comments

* fix tests after rebase

frontend/package.json

@@ -51,7 +51,7 @@
     "@types/d3": "^5.0.0",
     "@types/d3-dsv": "^1.0.33",
     "@types/dagre": "^0.7.40",
-    "@types/enzyme": "^3.1.14",
+    "@types/enzyme": "^3.1.15",
     "@types/enzyme-adapter-react-16": "^1.0.3",
     "@types/express": "^4.16.0",
     "@types/http-proxy-middleware": "^0.17.5",
@@ -66,7 +66,7 @@
     "@types/react-virtualized": "^9.18.7",
     "backstopjs": "^3.5.16",
     "coveralls": "^3.0.2",
-    "enzyme": "^3.6.0",
+    "enzyme": "^3.7.0",
     "enzyme-adapter-react-16": "^1.5.0",
     "enzyme-to-json": "^3.3.4",
     "react-router-test-context": "^0.1.0",

frontend/src/pages/404.test.tsx

@@ -0,0 +1,39 @@
+/*
+ * 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 Page404 from './404';
+import { PageProps } from './Page';
+import { shallow } from 'enzyme';
+
+describe('404', () => {
+  function generateProps(): PageProps {
+    return {
+      history: {} as any,
+      location: { pathname: 'some bad page' } as any,
+      match: {} as any,
+      toolbarProps: {} as any,
+      updateBanner: jest.fn(),
+      updateDialog: jest.fn(),
+      updateSnackbar: jest.fn(),
+      updateToolbar: jest.fn(),
+    };
+  }
+
+  it('renders a 404 page', () => {
+    expect(shallow(<Page404 {...generateProps()} />)).toMatchSnapshot();
+  });
+});

frontend/src/pages/PipelineSelector.test.tsx

@@ -0,0 +1,105 @@
+/*
+ * 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 PipelineSelector, { PipelineSelectorProps } from './PipelineSelector';
+import TestUtils from '../TestUtils';
+import { ApiPipeline } from '../apis/pipeline';
+import { ListRequest, Apis } from '../lib/Apis';
+import { shallow } from 'enzyme';
+
+describe('PipelineSelector', () => {
+  class TestPipelineSelector extends PipelineSelector {
+    public async _loadPipelines(request: ListRequest): Promise<string> {
+      return super._loadPipelines(request);
+    }
+    public _pipelineSelectionChanged(selectedIds: string[]): void {
+      return super._pipelineSelectionChanged(selectedIds);
+    }
+  }
+
+  const updateDialogSpy = jest.fn();
+  const pipelineSelectionChangedCbSpy = jest.fn();
+  const listPipelinesSpy = jest.spyOn(Apis.pipelineServiceApi, 'listPipelines');
+  const PIPELINES: ApiPipeline[] = [{
+    created_at: new Date(2018, 10, 9, 8, 7, 6),
+    description: 'test pipeline description',
+    name: 'test pipeline name',
+  }];
+
+  function generateProps(): PipelineSelectorProps {
+    return {
+      history: {} as any,
+      location: '' as any,
+      match: {} as any,
+      pipelineSelectionChanged: pipelineSelectionChangedCbSpy,
+      updateDialog: updateDialogSpy,
+    };
+  }
+
+  beforeEach(() => {
+    listPipelinesSpy.mockReset();
+    listPipelinesSpy.mockImplementation(() => ({ pipelines: PIPELINES }));
+    updateDialogSpy.mockReset();
+    pipelineSelectionChangedCbSpy.mockReset();
+  });
+
+  it('calls API to load pipelines', async () => {
+    const tree = shallow(<TestPipelineSelector {...generateProps()} />);
+    await (tree.instance() as TestPipelineSelector)._loadPipelines({});
+    expect(listPipelinesSpy).toHaveBeenCalledTimes(1);
+    expect(listPipelinesSpy).toHaveBeenLastCalledWith(undefined, undefined, undefined);
+    expect(tree.state('pipelines')).toEqual(PIPELINES);
+    expect(tree).toMatchSnapshot();
+    tree.unmount();
+  });
+
+  it('shows error dialog if listing fails', async () => {
+    TestUtils.makeErrorResponseOnce(listPipelinesSpy, 'woops!');
+    jest.spyOn(console, 'error').mockImplementation();
+    const tree = shallow(<TestPipelineSelector {...generateProps()} />);
+    await (tree.instance() as TestPipelineSelector)._loadPipelines({});
+    expect(listPipelinesSpy).toHaveBeenCalledTimes(1);
+    expect(updateDialogSpy).toHaveBeenLastCalledWith(expect.objectContaining({
+      content: 'List pipelines request failed with:\nwoops!',
+      title: 'Error retrieving pipelines',
+    }));
+    expect(tree.state('pipelines')).toEqual([]);
+    tree.unmount();
+  });
+
+  it('calls selection callback when a pipeline is selected', async () => {
+    const tree = shallow(<TestPipelineSelector {...generateProps()} />);
+    await (tree.instance() as TestPipelineSelector)._loadPipelines({});
+    expect(tree.state('selectedIds')).toEqual([]);
+    (tree.instance() as TestPipelineSelector)._pipelineSelectionChanged(['pipeline-id']);
+    expect(pipelineSelectionChangedCbSpy).toHaveBeenLastCalledWith('pipeline-id');
+    expect(tree.state('selectedIds')).toEqual(['pipeline-id']);
+    tree.unmount();
+  });
+
+  it('logs error if more than one pipeline is selected', async () => {
+    const tree = shallow(<TestPipelineSelector {...generateProps()} />);
+    const consoleSpy = jest.spyOn(console, 'error').mockImplementation();
+    await (tree.instance() as TestPipelineSelector)._loadPipelines({});
+    expect(tree.state('selectedIds')).toEqual([]);
+    (tree.instance() as TestPipelineSelector)._pipelineSelectionChanged(['pipeline-id', 'pipeline2-id']);
+    expect(pipelineSelectionChangedCbSpy).not.toHaveBeenCalled();
+    expect(tree.state('selectedIds')).toEqual([]);
+    expect(consoleSpy).toHaveBeenCalled();
+    tree.unmount();
+  });
+});

frontend/src/pages/PipelineSelector.tsx

@@ -23,7 +23,7 @@ import { logger, formatDateString, errorToMessage } from '../lib/Utils';
 import { ApiPipeline } from '../apis/pipeline';
 import { DialogProps } from '../components/Router';
 
-interface PipelineSelectorProps extends RouteComponentProps {
+export interface PipelineSelectorProps extends RouteComponentProps {
   pipelineSelectionChanged: (selectedPipelineId: string) => void;
   updateDialog: (dialogProps: DialogProps) => void;
 }
@@ -36,7 +36,6 @@ interface PipelineSelectorState {
 
 class PipelineSelector extends React.Component<PipelineSelectorProps, PipelineSelectorState> {
   protected _isMounted = true;
-  private _tableRef = React.createRef<CustomTable>();
 
   constructor(props: any) {
     super(props);
@@ -74,8 +73,7 @@ class PipelineSelector extends React.Component<PipelineSelectorProps, PipelineSe
         <Toolbar actions={toolbarActions} breadcrumbs={[]} pageTitle='Choose a pipeline' />
         <CustomTable columns={columns} rows={rows} selectedIds={selectedIds} useRadioButtons={true}
           updateSelection={this._pipelineSelectionChanged.bind(this)}
-          initialSortColumn={PipelineSortKeys.CREATED_AT} ref={this._tableRef}
-          reload={this._loadPipelines.bind(this)}
+          initialSortColumn={PipelineSortKeys.CREATED_AT} reload={this._loadPipelines.bind(this)}
           emptyMessage={'No pipelines found. Upload a pipeline and then try again.'} />
       </React.Fragment>
     );
@@ -85,19 +83,13 @@ class PipelineSelector extends React.Component<PipelineSelectorProps, PipelineSe
     this._isMounted = false;
   }
 
-  public async refresh(): Promise<void> {
-    if (this._tableRef.current) {
-      await this._tableRef.current.reload();
-    }
-  }
-
   protected setStateSafe(newState: Partial<PipelineSelectorState>, cb?: () => void): void {
     if (this._isMounted) {
       this.setState(newState as any, cb);
     }
   }
 
-  private _pipelineSelectionChanged(selectedIds: string[]): void {
+  protected _pipelineSelectionChanged(selectedIds: string[]): void {
     if (!Array.isArray(selectedIds) || selectedIds.length !== 1) {
       logger.error(`${selectedIds.length} pipelines were selected somehow`, selectedIds);
       return;
@@ -106,7 +98,7 @@ class PipelineSelector extends React.Component<PipelineSelectorProps, PipelineSe
     this.setStateSafe({ selectedIds });
   }
 
-  private async _loadPipelines(request: ListRequest): Promise<string> {
+  protected async _loadPipelines(request: ListRequest): Promise<string> {
     let pipelines: ApiPipeline[] = [];
     let nextPageToken = '';
     try {