Fixes await bug in create new run (#553)

kubeflow · Dec 18, 2018 · 302e93c · 302e93c
1 parent f1fb2df
commit 302e93c
Show file tree

Hide file tree

Showing 3 changed files with 11 additions and 8 deletions.
diff --git a/frontend/src/pages/NewRun.test.tsx b/frontend/src/pages/NewRun.test.tsx
@@ -1021,8 +1021,6 @@ describe('NewRun', () => {
       await TestUtils.flushPromises();
 
       tree.find('#createNewRunBtn').simulate('click');
-      // The create APIs are called in a callback triggered by clicking 'Create', so we wait again
-      await TestUtils.flushPromises();
 
       expect(tree.state('isBeingCreated')).toBe(true);
     });

diff --git a/frontend/src/pages/NewRun.tsx b/frontend/src/pages/NewRun.tsx
@@ -573,17 +573,22 @@ class NewRun extends Page<{}, NewRunState> {
     }
 
     this.setStateSafe({ isBeingCreated: true }, async () => {
+      // TODO: there was previously a bug here where the await wasn't being applied to the API
+      // calls, so a run creation could fail, and the success path would still be taken. We need
+      // tests for this and other similar situations.
       try {
-        await isRecurringRun
-          ? Apis.jobServiceApi.createJob(newRun)
-          : Apis.runServiceApi.createRun(newRun);
+        isRecurringRun
+          ? await Apis.jobServiceApi.createJob(newRun)
+          : await Apis.runServiceApi.createRun(newRun);
       } catch (err) {
         const errorMessage = await errorToMessage(err);
         this.showErrorDialog('Run creation failed', errorMessage);
         logger.error('Error creating Run:', err);
-        this.setStateSafe({ isBeingCreated: false });
         return;
+      } finally {
+        this.setStateSafe({ isBeingCreated: false });
       }
+
       if (this.state.experiment) {
         this.props.history.push(
           RoutePage.EXPERIMENT_DETAILS.replace(

diff --git a/frontend/src/pages/__snapshots__/NewRun.test.tsx.snap b/frontend/src/pages/__snapshots__/NewRun.test.tsx.snap
@@ -2298,7 +2298,7 @@ exports[`NewRun creating a new run copies pipeline from run in the create API ca
       className="flex"
     >
       <BusyButton
-        busy={true}
+        busy={false}
         className="buttonAction"
         disabled={false}
         id="createNewRunBtn"
@@ -2745,7 +2745,7 @@ exports[`NewRun creating a new run updates the pipeline in state when a user fil
       className="flex"
     >
       <BusyButton
-        busy={true}
+        busy={false}
         className="buttonAction"
         disabled={false}
         id="createNewRunBtn"