modify requested chances 12/23

frontend/package.json

@@ -88,6 +88,7 @@
     "prettier": "1.18.2",
     "react-router-test-context": "^0.1.0",
     "react-test-renderer": "^16.5.2",
+    "snapshot-diff": "^0.6.1",
     "swagger-ts-client": "^0.9.6",
     "ts-node": "^7.0.1",
     "ts-node-dev": "^1.0.0-pre.30",

frontend/server/k8s-helper.ts

@@ -184,7 +184,7 @@ export async function getTensorboardInstance(
  * and returns the deleted podAddress
  */
 
-export async function deleteTensorboardInstance(logdir: string, tfversion: string): Promise<void> {
+export async function deleteTensorboardInstance(logdir: string): Promise<void> {
   if (!k8sV1CustomObjectClient) {
     throw new Error('Cannot access kubernetes Custom Object API');
   }

frontend/server/server.ts

@@ -294,12 +294,14 @@ const getTensorboardHandler = async (req, res) => {
     res.status(500).send('Cannot talk to Kubernetes master');
     return;
   }
-  const logdir = decodeURIComponent(req.query.logdir);
-  if (!logdir) {
+
+  if (!req.query.logdir) {
     res.status(404).send('logdir argument is required');
     return;
   }
 
+  const logdir = decodeURIComponent(req.query.logdir);
+
   try {
     res.send(await k8sHelper.getTensorboardInstance(logdir));
   } catch (err) {
@@ -312,19 +314,20 @@ const createTensorboardHandler = async (req, res) => {
     res.status(500).send('Cannot talk to Kubernetes master');
     return;
   }
-  const logdir = decodeURIComponent(req.query.logdir);
-  const tfversion = decodeURIComponent(req.query.tfversion);
 
-  if (!logdir) {
+  if (!req.query.logdir) {
     res.status(404).send('logdir argument is required');
     return;
   }
 
-  if (!tfversion) {
+  if (!req.query.tfversion) {
     res.status(404).send('tensorflow version argument is required');
     return;
   }
 
+  const logdir = decodeURIComponent(req.query.logdir);
+  const tfversion = decodeURIComponent(req.query.tfversion);
+
   try {
     await k8sHelper.newTensorboardInstance(logdir, tfversion, podTemplateSpec);
     const tensorboardAddress = await k8sHelper.waitForTensorboardInstance(logdir, 60 * 1000);
@@ -339,11 +342,16 @@ const deleteTensorboardHandler = async (req, res) => {
     res.status(500).send('Cannot talk to Kubernetes master');
     return;
   }
+
+  if (!req.query.logdir) {
+    res.status(404).send('logdir argument is required');
+    return;
+  }
+
   const logdir = decodeURIComponent(req.query.logdir);
-  const tfversion = decodeURIComponent(req.query.tfversion);
 
   try {
-    await k8sHelper.deleteTensorboardInstance(logdir, tfversion);
+    await k8sHelper.deleteTensorboardInstance(logdir);
     res.send('Tensorboard deleted.');
   } catch (err) {
     res.status(500).send('Failed to delete Tensorboard app: ' + JSON.stringify(err));

frontend/src/components/viewers/Tensorboard.test.tsx

@@ -22,6 +22,8 @@ import { PlotType } from './Viewer';
 import { ReactWrapper, ShallowWrapper, shallow, mount } from 'enzyme';
 
 describe('Tensorboard', () => {
+  const snapshotDiff = require('snapshot-diff');
+
   let tree: ReactWrapper | ShallowWrapper;
   beforeEach(() => {
     jest.clearAllMocks();
@@ -37,19 +39,30 @@ describe('Tensorboard', () => {
     jest.restoreAllMocks();
   });
 
-  it('does not break on no config', async () => {
+  it('base component snapshot', async () => {
     Apis.getTensorboardApp = jest.fn(() => Promise.resolve({ podAddress: '', tfVersion: '' }));
     tree = shallow(<TensorboardViewer configs={[]} />);
     await TestUtils.flushPromises();
     expect(tree).toMatchSnapshot();
   });
 
+  it('does not break on no config', async () => {
+    Apis.getTensorboardApp = jest.fn(() => Promise.resolve({ podAddress: '', tfVersion: '' }));
+    tree = shallow(<TensorboardViewer configs={[]} />);
+    const initialHtml = tree.debug();
+
+    await TestUtils.flushPromises();
+    expect(snapshotDiff(tree.debug(), initialHtml)).toMatchSnapshot();
+  });
+
   it('does not break on empty data', async () => {
     Apis.getTensorboardApp = jest.fn(() => Promise.resolve({ podAddress: '', tfVersion: '' }));
     const config = { type: PlotType.TENSORBOARD, url: '' };
     tree = shallow(<TensorboardViewer configs={[config]} />);
+    const initialHtml = tree.debug();
+
     await TestUtils.flushPromises();
-    expect(tree).toMatchSnapshot();
+    expect(snapshotDiff(tree.debug(), initialHtml)).toMatchSnapshot();
   });
 
   it('shows a link to the tensorboard instance if exists', async () => {
@@ -58,6 +71,7 @@ describe('Tensorboard', () => {
       Promise.resolve({ podAddress: 'test/address', tfVersion: '1.14.0' }),
     );
     tree = shallow(<TensorboardViewer configs={[config]} />);
+
     await TestUtils.flushPromises();
     expect(tree).toMatchSnapshot();
   });
@@ -67,9 +81,10 @@ describe('Tensorboard', () => {
     const getAppMock = () => Promise.resolve({ podAddress: '', tfVersion: '' });
     const spy = jest.spyOn(Apis, 'getTensorboardApp').mockImplementation(getAppMock);
     tree = shallow(<TensorboardViewer configs={[config]} />);
+    const initialHtml = tree.debug();
 
     await TestUtils.flushPromises();
-    expect(tree).toMatchSnapshot();
+    expect(snapshotDiff(tree.debug(), initialHtml)).toMatchSnapshot();
     expect(spy).toHaveBeenCalledWith(config.url);
   });
 
@@ -131,7 +146,7 @@ describe('Tensorboard', () => {
     expect(Apis.startTensorboardApp).toHaveBeenCalledWith('http%3A%2F%2Ftest%2Furl', '2.0.0');
   });
 
-  it('delete the tensorboard instance, confirm in the dialog, \
+  it('delete the tensorboard instance, confirm in the dialog,\
     then return back to previous page', async () => {
     Apis.getTensorboardApp = jest.fn(() =>
       Promise.resolve({ podAddress: 'podaddress', tfVersion: '1.14.0' }),
@@ -149,10 +164,7 @@ describe('Tensorboard', () => {
       .find('Button')
       .simulate('click');
     tree.find('BusyButton').simulate('click');
-    expect(Apis.deleteTensorboardApp).toHaveBeenCalledWith(
-      encodeURIComponent('http://test/url'),
-      tree.state('tensorflowVersion'),
-    );
+    expect(Apis.deleteTensorboardApp).toHaveBeenCalledWith(encodeURIComponent('http://test/url'));
     await TestUtils.flushPromises();
     tree.update();
     // the tree has returned to 'start tensorboard' page
@@ -192,7 +204,7 @@ describe('Tensorboard', () => {
       .find('#cancel')
       .find('Button')
       .simulate('click');
-    tree.update();
+
     expect(tree.findWhere(el => el.text() === 'Open Tensorboard').exists()).toBeTruthy();
     expect(tree.findWhere(el => el.text() === 'Delete Tensorboard').exists()).toBeTruthy();
   });

frontend/src/components/viewers/Tensorboard.tsx

@@ -31,6 +31,23 @@ import DialogActions from '@material-ui/core/DialogActions';
 import DialogContent from '@material-ui/core/DialogContent';
 import DialogContentText from '@material-ui/core/DialogContentText';
 import DialogTitle from '@material-ui/core/DialogTitle';
+import { classes, stylesheet } from 'typestyle';
+
+export const css = stylesheet({
+  button: {
+    marginBottom: 20,
+    width: 150,
+  },
+  shortButton: {
+    width: 50,
+  },
+  formControl: {
+    minWidth: 120,
+  },
+  select: {
+    minHeight: 50,
+  },
+});
 
 export interface TensorboardViewerConfig extends ViewerConfig {
   url: string;
@@ -71,7 +88,7 @@ class TensorboardViewer extends Viewer<TensorboardViewerProps, TensorboardViewer
     this._checkTensorboardApp();
   }
 
-  public onChangeFunc = (e: React.ChangeEvent<{ name?: string; value: unknown }>): void => {
+  public handleVersionSelect = (e: React.ChangeEvent<{ name?: string; value: unknown }>): void => {
     this.setState({ tensorflowVersion: e.target.value as string });
   };
 
@@ -89,63 +106,63 @@ class TensorboardViewer extends Viewer<TensorboardViewerProps, TensorboardViewer
       <div>
         {this.state.podAddress && (
           <div>
-            <div className={padding(20, 'b')}>{`Tensorboard is running for this output.`}</div>
+            <div
+              className={padding(20, 'b')}
+            >{`Tensorboard ${this.state.tensorflowVersion} is running for this output.`}</div>
             <a
               href={'apis/v1beta1/_proxy/' + podAddress}
               target='_blank'
               className={commonCss.unstyled}
             >
               <Button
-                className={commonCss.buttonAction}
+                className={classes(commonCss.buttonAction, css.button)}
                 disabled={this.state.busy}
-                style={{ marginBottom: 20, width: 150 }}
+                color={'primary'}
               >
                 Open Tensorboard
               </Button>
             </a>
 
             <div>
               <Button
-                className={commonCss.buttonAction}
+                className={css.button}
                 disabled={this.state.busy}
                 id={'delete'}
                 title={`stop tensorboard and delete its instance`}
                 onClick={this._handleDeleteOpen.bind(this)}
-                style={{ marginBottom: 20, width: 150 }}
+                color={'default'}
               >
                 Delete Tensorboard
               </Button>
               <Dialog
                 open={this.state.deleteDialogOpen}
-                onClose={this._handleDeleteClose.bind(this)}
+                onClose={this._handleDeleteClose}
                 aria-labelledby='dialog-title'
               >
-                <DialogTitle style={{ cursor: 'move' }} id='dialog-title'>
+                <DialogTitle id='dialog-title'>
                   {`Stop Tensorboard ${this.state.tensorflowVersion}?`}
                 </DialogTitle>
                 <DialogContent>
                   <DialogContentText>
                     You can stop the current running tensorboard. The tensorboard viewer will also
-                    be deleted from your GKE workloads.
+                    be deleted from your workloads.
                   </DialogContentText>
                 </DialogContent>
                 <DialogActions>
                   <Button
+                    className={css.shortButton}
                     id={'cancel'}
                     autoFocus={true}
-                    onClick={this._handleDeleteClose.bind(this)}
-                    style={{ width: 50 }}
+                    onClick={this._handleDeleteClose}
                     color='primary'
                   >
                     Cancel
                   </Button>
                   <BusyButton
-                    id={'confirm'}
-                    className={commonCss.buttonAction}
+                    className={classes(commonCss.buttonAction, css.shortButton)}
                     onClick={this._deleteTensorboard.bind(this)}
                     busy={this.state.busy}
                     color='primary'
-                    style={{ width: 50 }}
                     title={`Stop`}
                   />
                 </DialogActions>
@@ -157,15 +174,14 @@ class TensorboardViewer extends Viewer<TensorboardViewerProps, TensorboardViewer
         {!this.state.podAddress && (
           <div>
             <div className={padding(30, 'b')}>
-              <FormControl className='Launch Tensorboard' style={{ minWidth: 120 }}>
+              class
+              <FormControl className={css.formControl}>
                 <InputLabel htmlFor='grouped-select'>TF Version</InputLabel>
                 <Select
+                  className={css.select}
                   value={this.state.tensorflowVersion}
                   input={<Input id='grouped-select' />}
-                  onChange={this.onChangeFunc}
-                  style={{
-                    minHeight: 40,
-                  }}
+                  onChange={this.handleVersionSelect}
                 >
                   <ListSubheader>Tensoflow 1.x</ListSubheader>
                   <MenuItem value={'1.4.0'}>TensorFlow 1.4.0</MenuItem>
@@ -234,22 +250,19 @@ class TensorboardViewer extends Viewer<TensorboardViewerProps, TensorboardViewer
     });
   }
 
-  private async _deleteTensorboard(): Promise<void> {
+  private _deleteTensorboard = async () => {
     // delete the already opened Tensorboard, clear the podAddress recorded in frontend,
     // and return to the select & start tensorboard page
     this.setState({ busy: true }, async () => {
-      await Apis.deleteTensorboardApp(
-        encodeURIComponent(this._buildUrl()),
-        encodeURIComponent(this.state.tensorflowVersion),
-      );
+      await Apis.deleteTensorboardApp(encodeURIComponent(this._buildUrl()));
       this.setState({
         busy: false,
         deleteDialogOpen: false,
         podAddress: '',
         tensorflowVersion: '',
       });
     });
-  }
+  };
 }
 
 export default TensorboardViewer;