Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixes await bug in create new run #553

Merged
merged 1 commit into from Dec 18, 2018
Merged

Fixes await bug in create new run #553

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 0 additions & 2 deletions frontend/src/pages/NewRun.test.tsx
View file
Open in desktop
Expand Up @@ -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);
});
Expand Down
13 changes: 9 additions & 4 deletions frontend/src/pages/NewRun.tsx
View file
Open in desktop
Expand Up @@ -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(
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/pages/__snapshots__/NewRun.test.tsx.snap
View file
Open in desktop
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down