From 66883b0eedb9fb5d3a3e5ad95461cf22184ba033 Mon Sep 17 00:00:00 2001
From: Riley Bauer <34456002+rileyjbauer@users.noreply.github.com>
Date: Tue, 6 Aug 2019 07:09:54 -0700
Subject: [PATCH] Fixes cloning of recurring runs (#1712)
* Fixes cloning of recurring runs
Simplifies NewRun a little
Creates NewRunParameters component
* Fixes all NewRun tests
* All tests pass
* Adds tests for NewRunParameters
* Adds clone test to RecurringRunDetails
* Clean up
---
.../src/components/NewRunParameters.test.tsx | 55 +
frontend/src/components/NewRunParameters.tsx | 54 +
frontend/src/components/Router.tsx | 1 +
.../NewRunParameters.test.tsx.snap | 45 +
frontend/src/lib/Buttons.ts | 24 +-
frontend/src/lib/RunUtils.ts | 26 +-
frontend/src/pages/NewRun.test.tsx | 98 +-
frontend/src/pages/NewRun.tsx | 253 +--
.../src/pages/RecurringRunDetails.test.tsx | 18 +-
frontend/src/pages/RecurringRunDetails.tsx | 5 +-
.../pages/__snapshots__/NewRun.test.tsx.snap | 1414 +++--------------
11 files changed, 614 insertions(+), 1379 deletions(-)
create mode 100644 frontend/src/components/NewRunParameters.test.tsx
create mode 100644 frontend/src/components/NewRunParameters.tsx
create mode 100644 frontend/src/components/__snapshots__/NewRunParameters.test.tsx.snap
diff --git a/frontend/src/components/NewRunParameters.test.tsx b/frontend/src/components/NewRunParameters.test.tsx
new file mode 100644
index 00000000000..9e0e4e85122
--- /dev/null
+++ b/frontend/src/components/NewRunParameters.test.tsx
@@ -0,0 +1,55 @@
+/*
+ * Copyright 2019 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 { shallow } from 'enzyme';
+import NewRunParameters, { NewRunParametersProps } from './NewRunParameters';
+
+describe('NewRunParameters', () => {
+ it('shows parameters', () => {
+ const props = {
+ handleParamChange: jest.fn(),
+ initialParams: [{ name: 'testParam', value: 'testVal' }],
+ titleMessage: 'Specify parameters required by the pipeline',
+ } as NewRunParametersProps;
+ expect(shallow()).toMatchSnapshot();
+ });
+
+ it('does not display any text fields if there are parameters', () => {
+ const props = {
+ handleParamChange: jest.fn(),
+ initialParams: [],
+ titleMessage: 'This pipeline has no parameters',
+ } as NewRunParametersProps;
+ expect(shallow()).toMatchSnapshot();
+ });
+
+ it('fires handleParamChange callback on change', () => {
+ const handleParamChange = jest.fn();
+ const props = {
+ handleParamChange,
+ initialParams: [
+ { name: 'testParam1', value: 'testVal1' },
+ { name: 'testParam2', value: 'testVal2' }
+ ],
+ titleMessage: 'Specify parameters required by the pipeline',
+ } as NewRunParametersProps;
+ const tree = shallow();
+ tree.find('#newRunPipelineParam1').simulate('change', { target: { value: 'test param value' } });
+ expect(handleParamChange).toHaveBeenCalledTimes(1);
+ expect(handleParamChange).toHaveBeenLastCalledWith(1, 'test param value');
+ });
+});
diff --git a/frontend/src/components/NewRunParameters.tsx b/frontend/src/components/NewRunParameters.tsx
new file mode 100644
index 00000000000..52a2c653063
--- /dev/null
+++ b/frontend/src/components/NewRunParameters.tsx
@@ -0,0 +1,54 @@
+/*
+ * 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 { commonCss } from '../Css';
+import TextField from '@material-ui/core/TextField';
+import { ApiParameter } from '../apis/pipeline';
+
+export interface NewRunParametersProps {
+ initialParams: ApiParameter[];
+ titleMessage: string;
+ handleParamChange: (index: number, value: string) => void;
+}
+
+class NewRunParameters extends React.Component {
+ constructor(props: any) {
+ super(props);
+ }
+
+ public render(): JSX.Element | null {
+ const { handleParamChange, initialParams, titleMessage } = this.props;
+
+ return (
+
@@ -548,7 +537,7 @@ exports[`NewRun changes the exit button's text if query params indicate this is
"href": "/experiments/details/some-mock-experiment-id",
},
],
- "pageTitle": "Start a new run",
+ "pageTitle": "Start a run",
},
],
],
@@ -678,7 +667,7 @@ exports[`NewRun changes the exit button's text if query params indicate this is
"href": "/experiments/details/some-mock-experiment-id",
},
],
- "pageTitle": "Start a new run",
+ "pageTitle": "Start a run",
},
],
],
@@ -775,20 +764,17 @@ exports[`NewRun changes the exit button's text if query params indicate this is
control={
}
+ id="recurringToggle"
label="Recurring"
onChange={[Function]}
/>
-
- Run parameters
-
-
- Parameters will appear after you select a pipeline
-
+
@@ -968,7 +954,7 @@ exports[`NewRun changes title and form if the new run will recur, based on the r
"href": "/experiments/details/some-mock-experiment-id",
},
],
- "pageTitle": "Start a new run",
+ "pageTitle": "Start a run",
},
],
Array [
@@ -1046,887 +1032,7 @@ exports[`NewRun changes title and form if the new run will recur, based on the r
location={
Object {
"pathname": "/runs/new",
- "search": "?experimentId=some-mock-experiment-id",
- }
- }
- match=""
- selectionChanged={[Function]}
- title="Choose an experiment"
- toolbarProps={
- Object {
- "actions": Array [],
- "breadcrumbs": Array [
- Object {
- "displayName": "Experiments",
- "href": "/experiments",
- },
- ],
- "pageTitle": "Start a new run",
- }
- }
- updateBanner={
- [MockFunction] {
- "calls": Array [
- Array [
- Object {},
- ],
- ],
- }
- }
- updateDialog={[MockFunction]}
- updateSnackbar={[MockFunction]}
- updateToolbar={
- [MockFunction] {
- "calls": Array [
- Array [
- Object {
- "actions": Array [],
- "breadcrumbs": Array [
- Object {
- "displayName": "Experiments",
- "href": "/experiments",
- },
- ],
- "pageTitle": "Start a new run",
- },
- ],
- Array [
- Object {
- "actions": Array [],
- "breadcrumbs": Array [
- Object {
- "displayName": "Experiments",
- "href": "/experiments",
- },
- Object {
- "displayName": "some mock experiment name",
- "href": "/experiments/details/some-mock-experiment-id",
- },
- ],
- "pageTitle": "Start a new run",
- },
- ],
- Array [
- Object {
- "pageTitle": "Start a recurring run",
- },
- ],
- ],
- }
- }
- />
-
-
-
- Cancel
-
-
- Use this experiment
-
-
-
-
-
-
- This run will be associated with the following experiment
-
`;
-exports[`NewRun cloning from a run shows switching controls when run has embedded pipeline, selects that pipeline by default, and hides pipeline selector 1`] = `
+exports[`NewRun changes title and form to default state if the new run is a one-off, based on the radio buttons 1`] = `
@@ -2131,31 +1256,37 @@ exports[`NewRun cloning from a run shows switching controls when run has embedde
>
Run details
@@ -2564,7 +1704,7 @@ exports[`NewRun fetches the associated pipeline if one is present in the query p
disabled={true}
label="Pipeline"
required={true}
- value="some mock pipeline name"
+ value="original mock pipeline name"
variant="outlined"
/>
}
+ id="recurringToggle"
label="Recurring"
onChange={[Function]}
/>
-
- Run parameters
-
-
- This pipeline has no parameters
-
+
@@ -3083,7 +2220,7 @@ exports[`NewRun renders the new run page 1`] = `
"href": "/experiments/details/some-mock-experiment-id",
},
],
- "pageTitle": "Start a new run",
+ "pageTitle": "Start a run",
},
],
],
@@ -3213,7 +2350,7 @@ exports[`NewRun renders the new run page 1`] = `
"href": "/experiments/details/some-mock-experiment-id",
},
],
- "pageTitle": "Start a new run",
+ "pageTitle": "Start a run",
},
],
],
@@ -3310,20 +2447,17 @@ exports[`NewRun renders the new run page 1`] = `
control={
}
+ id="recurringToggle"
label="Recurring"
onChange={[Function]}
/>
-
- Run parameters
-
-
- Parameters will appear after you select a pipeline
-
+
@@ -3722,9 +2856,9 @@ exports[`NewRun starting a new recurring run includes additional trigger input f
control={
}
+ id="recurringToggle"
label="Recurring"
onChange={[Function]}
/>
@@ -3739,14 +2873,11 @@ exports[`NewRun starting a new recurring run includes additional trigger input f
-
- Run parameters
-
-
- Parameters will appear after you select a pipeline
-
+
@@ -3790,32 +2921,18 @@ exports[`NewRun starting a new run copies pipeline from run in the start API cal
>
Run details
@@ -4216,7 +3311,7 @@ exports[`NewRun starting a new run copies pipeline from run in the start API cal
`;
-exports[`NewRun starting a new run updates the pipeline in state when a user fills in its params 1`] = `
+exports[`NewRun starting a new run updates the parameters in state on handleParamChange 1`] = `
@@ -4257,7 +3352,7 @@ exports[`NewRun starting a new run updates the pipeline in state when a user fil
disabled={true}
label="Pipeline"
required={true}
- value="some mock pipeline name"
+ value="original mock pipeline name"
variant="outlined"
/>
}
+ id="recurringToggle"
label="Recurring"
onChange={[Function]}
/>
-
@@ -4842,7 +3911,7 @@ exports[`NewRun starting a new run updates the pipeline params as user selects d
"href": "/experiments/details/some-mock-experiment-id",
},
],
- "pageTitle": "Start a new run",
+ "pageTitle": "Start a run",
},
],
],
@@ -4972,7 +4041,7 @@ exports[`NewRun starting a new run updates the pipeline params as user selects d
"href": "/experiments/details/some-mock-experiment-id",
},
],
- "pageTitle": "Start a new run",
+ "pageTitle": "Start a run",
},
],
],
@@ -5069,20 +4138,17 @@ exports[`NewRun starting a new run updates the pipeline params as user selects d
control={
}
+ id="recurringToggle"
label="Recurring"
onChange={[Function]}
/>
-
- Run parameters
-
-
- Parameters will appear after you select a pipeline
-
+
@@ -5155,7 +4221,7 @@ exports[`NewRun starting a new run updates the pipeline params as user selects d
disabled={true}
label="Pipeline"
required={true}
- value="some mock pipeline name"
+ value="original mock pipeline name"
variant="outlined"
/>
}
+ id="recurringToggle"
label="Recurring"
onChange={[Function]}
/>
-
- Run parameters
-
-
- Specify parameters required by the pipeline
-
-
-
-
-
+ "name": "param-2",
+ "value": "prefilled value 2",
+ },
+ ]
+ }
+ titleMessage="Specify parameters required by the pipeline"
+ />
@@ -5609,7 +4649,7 @@ exports[`NewRun starting a new run updates the pipeline params as user selects d
disabled={true}
label="Pipeline"
required={true}
- value="some mock pipeline name"
+ value="original mock pipeline name"
variant="outlined"
/>
}
+ id="recurringToggle"
label="Recurring"
onChange={[Function]}
/>
-
- Run parameters
-
-
- This pipeline has no parameters
-
+
@@ -6136,7 +5173,7 @@ exports[`NewRun updates the run's state with the associated experiment if one is
"href": "/experiments/details/some-mock-experiment-id",
},
],
- "pageTitle": "Start a new run",
+ "pageTitle": "Start a run",
},
],
],
@@ -6266,7 +5303,7 @@ exports[`NewRun updates the run's state with the associated experiment if one is
"href": "/experiments/details/some-mock-experiment-id",
},
],
- "pageTitle": "Start a new run",
+ "pageTitle": "Start a run",
},
],
],
@@ -6363,20 +5400,17 @@ exports[`NewRun updates the run's state with the associated experiment if one is
control={
}
+ id="recurringToggle"
label="Recurring"
onChange={[Function]}
/>
-
- Run parameters
-
-
- Parameters will appear after you select a pipeline
-