Skip to content

Commit

Permalink
refactor fit to have loading state in store + tests + lint
Browse files Browse the repository at this point in the history
  • Loading branch information
M-Kusumgar committed Jul 5, 2023
1 parent af4165f commit 68b93b1
Show file tree
Hide file tree
Showing 13 changed files with 116 additions and 48 deletions.
6 changes: 3 additions & 3 deletions app/static/src/app/components/LoadingButton.vue
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@
</button>
</template>
<script lang="ts">
import { defineComponent } from 'vue';
import { defineComponent } from "vue";
export default defineComponent({
props: {
loading: Boolean,
isDisabled: Boolean
}
})
</script>
});
</script>
32 changes: 14 additions & 18 deletions app/static/src/app/components/fit/FitTab.vue
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
</template>

<script lang="ts">
import { computed, ref } from "vue";
import { computed } from "vue";
import { useStore } from "vuex";
import VueFeather from "vue-feather";
import FitPlot from "./FitPlot.vue";
Expand All @@ -43,12 +43,12 @@ import LoadingButton from "../LoadingButton.vue";
export default {
name: "FitTab",
components: {
LoadingSpinner,
FitPlot,
ActionRequiredMessage,
VueFeather,
LoadingButton
},
LoadingSpinner,
FitPlot,
ActionRequiredMessage,
VueFeather,
LoadingButton
},
setup() {
const store = useStore();
const namespace = "modelFit";
Expand All @@ -57,24 +57,20 @@ export default {
const canFitModel = computed(() => allTrue(fitRequirements.value));
const compileRequired = computed(() => store.state.model.compileRequired);
const fitUpdateRequired = computed(() => store.state.modelFit.fitUpdateRequired);
const loading = ref(false);
const loading = computed(() => store.state.modelFit.loading);
const fitModel = () => {
loading.value = true;
store.dispatch(`${namespace}/${ModelFitAction.FitModel}`)
store.commit(`${namespace}/${ModelFitMutation.SetLoading}`, true);
store.dispatch(`${namespace}/${ModelFitAction.FitModel}`);
};
const cancelFit = () => {
store.commit(`${namespace}/${ModelFitMutation.SetFitting}`, false)
loading.value = false;
store.commit(`${namespace}/${ModelFitMutation.SetFitting}`, false);
store.commit(`${namespace}/${ModelFitMutation.SetLoading}`, false);
};
const iterations = computed(() => store.state.modelFit.iterations);
const converged = computed(() => {
if (store.state.modelFit.converged) {
loading.value = false
}
return store.state.modelFit.converged
});
const converged = computed(() => store.state.modelFit.converged);
const fitting = computed(() => store.state.modelFit.fitting);
const cancelled = computed(() => iterations.value && !fitting.value && !converged.value);
const sumOfSquares = computed(() => store.state.modelFit.sumOfSquares);
Expand Down
16 changes: 8 additions & 8 deletions app/static/src/app/components/sensitivity/SensitivityTab.vue
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,13 @@ import { SensitivityMutation } from "../../store/sensitivity/mutations";
export default defineComponent({
name: "SensitivityTab",
components: {
ErrorInfo,
LoadingSpinner,
SensitivitySummaryPlot,
ActionRequiredMessage,
SensitivityTracesPlot,
LoadingButton
},
ErrorInfo,
LoadingSpinner,
SensitivitySummaryPlot,
ActionRequiredMessage,
SensitivityTracesPlot,
LoadingButton
},
setup() {
const store = useStore();
Expand All @@ -69,7 +69,7 @@ export default defineComponent({
// to react to loading being true
setTimeout(() => {
store.dispatch(`sensitivity/${SensitivityAction.RunSensitivity}`);
}, 100)
}, 100);
};
const sensitivityProgressMsg = computed(() => {
Expand Down
3 changes: 2 additions & 1 deletion app/static/src/app/store/modelFit/modelFit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ export const defaultState: ModelFitState = {
sumOfSquares: null,
paramsToVary: [],
inputs: null,
result: null
result: null,
loading: false
};

export const modelFit = {
Expand Down
10 changes: 9 additions & 1 deletion app/static/src/app/store/modelFit/mutations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ export enum ModelFitMutation {
SetInputs = "SetInputs",
SetParamsToVary = "SetParamsToVary",
SetSumOfSquares = "SetSumOfSquares",
SetFitUpdateRequired = "SetFitUpdateRequired"
SetFitUpdateRequired = "SetFitUpdateRequired",
SetLoading = "SetLoading"
}

export const mutations: MutationTree<ModelFitState> = {
Expand All @@ -18,6 +19,9 @@ export const mutations: MutationTree<ModelFitState> = {

[ModelFitMutation.SetResult](state: ModelFitState, payload: SimplexResult) {
state.converged = payload.converged;
if (payload.converged) {
state.loading = false;
}
state.iterations = payload.iterations;
state.sumOfSquares = payload.value;
const inputs = {
Expand All @@ -43,6 +47,10 @@ export const mutations: MutationTree<ModelFitState> = {
state.sumOfSquares = payload;
},

[ModelFitMutation.SetLoading](state: ModelFitState, payload: boolean) {
state.loading = payload;
},

[ModelFitMutation.SetFitUpdateRequired](state: ModelFitState, payload: null | Partial<FitUpdateRequiredReasons>) {
if (payload === null) {
state.fitUpdateRequired = {
Expand Down
1 change: 1 addition & 0 deletions app/static/src/app/store/modelFit/state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export interface FitUpdateRequiredReasons {

export interface ModelFitState {
fitting: boolean,
loading: boolean,
fitUpdateRequired: FitUpdateRequiredReasons,
iterations: number | null,
converged: boolean | null,
Expand Down
2 changes: 2 additions & 0 deletions app/static/tests/e2e/sensitivity.etest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ test.describe("Sensitivity tests", () => {

// re-run - message should be removed
await page.click("#run-sens-btn");
await new Promise((r) => setTimeout(r, 101));
await expect(await page.innerText(".action-required-msg")).toBe("");

// switch to Value at Time - expect axes to change
Expand Down Expand Up @@ -204,6 +205,7 @@ test.describe("Sensitivity tests", () => {
await page.fill(":nth-match(#model-params input, 3)", "1000000");
await page.fill(":nth-match(#model-params input, 4)", "1.5");
await page.click("#run-sens-btn");
await new Promise((r) => setTimeout(r, 101));
await expect(await page.locator(".wodin-plot-data-summary-series")).toHaveCount(66, { timeout });

// current parameters
Expand Down
2 changes: 2 additions & 0 deletions app/static/tests/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ export const mockFitDataState = (state:Partial<FitDataState> = {}): FitDataState
export const mockSensitivityState = (state: Partial<SensitivityState> = {}): SensitivityState => {
return {
running: false,
loading: false,
paramSettings: {
parameterToVary: null,
scaleType: SensitivityScaleType.Arithmetic,
Expand Down Expand Up @@ -195,6 +196,7 @@ export const mockBasicState = (state: Partial<BasicState> = {}): BasicState => {
export const mockModelFitState = (state: Partial<ModelFitState> = {}): ModelFitState => {
return {
fitting: false,
loading: false,
fitUpdateRequired: {
modelChanged: false,
dataChanged: false,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { shallowMount } from "@vue/test-utils";
import { mount, shallowMount } from "@vue/test-utils";
import Vuex from "vuex";
import { ModelState } from "../../../../src/app/store/model/state";
import SensitivityTab from "../../../../src/app/components/sensitivity/SensitivityTab.vue";
Expand All @@ -12,11 +12,13 @@ import ErrorInfo from "../../../../src/app/components/ErrorInfo.vue";
import { AppState, AppType } from "../../../../src/app/store/appState/state";
import { ModelGetter } from "../../../../src/app/store/model/getters";
import LoadingSpinner from "../../../../src/app/components/LoadingSpinner.vue";
import { SensitivityMutation } from "../../../../src/app/store/sensitivity/mutations";

jest.mock("plotly.js-basic-dist-min", () => {});

describe("SensitivityTab", () => {
const mockRunSensitivity = jest.fn();
const mockSetLoading = jest.fn();

const getWrapper = (appType = AppType.Basic, modelState: Partial<ModelState> = {},
sensitivityState: Partial<SensitivityState> = {}, batchPars: any = {}, hasRunner = true) => {
Expand Down Expand Up @@ -67,13 +69,23 @@ describe("SensitivityTab", () => {
},
actions: {
[SensitivityAction.RunSensitivity]: mockRunSensitivity
},
mutations: {
[SensitivityMutation.SetLoading]: mockSetLoading
}
}
}
});
return shallowMount(SensitivityTab, {
return mount(SensitivityTab, {
global: {
plugins: [store]
plugins: [store],
stubs: [
"action-required-message",
"sensitivity-traces-plot",
"sensitivity-summary-plot",
"loading-spinner",
"error-info"
]
}
});
};
Expand Down Expand Up @@ -158,6 +170,16 @@ describe("SensitivityTab", () => {
expect(wrapper.find("button").element.disabled).toBe(true);
});

it("disables run button when loading is true", () => {
const wrapper = getWrapper(AppType.Fit, {}, { loading: true });
expect(wrapper.find("button").element.disabled).toBe(true);
});

it("disables run button when running is true", () => {
const wrapper = getWrapper(AppType.Fit, {}, { running: true });
expect(wrapper.find("button").element.disabled).toBe(true);
});

it("renders expected update message when required action is Compile", () => {
const sensitivityState = {
result: {
Expand Down Expand Up @@ -238,10 +260,13 @@ describe("SensitivityTab", () => {
expect(runningMsg.findComponent(LoadingSpinner).props("size")).toBe("xs");
});

it("dispatches sensitivity run when button is clicked", () => {
it("commits set loading and dispatches sensitivity run when button is clicked", async () => {
const wrapper = getWrapper();
expect(mockRunSensitivity).not.toHaveBeenCalled();
expect(mockSetLoading).not.toHaveBeenCalled();
wrapper.find("button").trigger("click");
await new Promise((r) => setTimeout(r, 101));
expect(mockRunSensitivity).toHaveBeenCalledTimes(1);
expect(mockSetLoading).toHaveBeenCalledTimes(1);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import SensitivityTracesPlot from "../../../../src/app/components/sensitivity/Se
import { FitDataGetter } from "../../../../src/app/store/fitData/getters";
import { mockRunState } from "../../../mocks";
import { getters as runGetters } from "../../../../src/app/store/run/getters";
import { SensitivityMutation } from "../../../../src/app/store/sensitivity/mutations";

jest.mock("plotly.js-basic-dist-min", () => {});

Expand Down Expand Up @@ -380,6 +381,8 @@ const mockParameterSetCentralResults = {
"Set 2": { solution: mockParameterSetCentralSln }
} as any;

const mockSetLoading = jest.fn();

describe("SensitivityTracesPlot", () => {
const getWrapper = (sensitivityHasSolutions = true, fadePlot = false, sensitivityHasData = false,
stochastic = false, hasSelectedVariables = true, hasParameterSets = false) => {
Expand All @@ -389,19 +392,6 @@ describe("SensitivityTracesPlot", () => {
model: {
paletteModel: mockPalette,
selectedVariables: hasSelectedVariables ? selectedVariables : []
},
sensitivity: {
result: {
batch: {
solutions: sensitivityHasSolutions ? mockSolutions : null,
allFitData: sensitivityHasData ? mockAllFitData : undefined,
pars: {
name: "alpha",
values: [1.11111, 2.22222]
}
}
},
parameterSetResults: hasParameterSets ? mockParameterSetResults : {}
}
} as any,
modules: {
Expand All @@ -425,6 +415,25 @@ describe("SensitivityTracesPlot", () => {
parameterSetResults: hasParameterSets ? mockParameterSetCentralResults : {}
}),
getters: runGetters
},
sensitivity: {
namespaced: true,
state: {
result: {
batch: {
solutions: sensitivityHasSolutions ? mockSolutions : null,
allFitData: sensitivityHasData ? mockAllFitData : undefined,
pars: {
name: "alpha",
values: [1.11111, 2.22222]
}
}
},
parameterSetResults: hasParameterSets ? mockParameterSetResults : {}
},
mutations: {
[SensitivityMutation.SetLoading]: mockSetLoading
}
}
}
});
Expand Down Expand Up @@ -540,4 +549,14 @@ describe("SensitivityTracesPlot", () => {
const wrapper = getWrapper(true, false);
expect(wrapper.findComponent(WodinPlot).props("fadePlot")).toBe(false);
});

it("commits set loading when plotData is run", () => {
const wrapper = getWrapper();
expect(mockSetLoading).toHaveBeenCalledTimes(0);
const wodinPlot = wrapper.findComponent(WodinPlot);
const plotData = wodinPlot.props("plotData");
console.log(plotData)
plotData(0, 1, 100);
expect(mockSetLoading).toHaveBeenCalledTimes(1)
});
});
2 changes: 2 additions & 0 deletions app/static/tests/unit/serialiser.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ describe("serialise", () => {
};
const sensitivityState = {
running: true,
loading: false,
paramSettings: {
parameterToVary: "alpha",
scaleType: SensitivityScaleType.Arithmetic,
Expand Down Expand Up @@ -188,6 +189,7 @@ describe("serialise", () => {

const modelFitState = {
fitting: false,
loading: false,
fitUpdateRequired: {
modelChanged: false,
dataChanged: false,
Expand Down
6 changes: 6 additions & 0 deletions app/static/tests/unit/store/modelFit/mutations.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@ describe("ModelFit mutations", () => {
expect(state.fitting).toBe(true);
});

it("sets loading", () => {
const state = mockModelFitState();
mutations.SetLoading(state, true);
expect(state.loading).toBe(true);
});

it("sets model fit inputs", () => {
const state = mockModelFitState();
mutations.SetInputs(state, mockInputs);
Expand Down
6 changes: 6 additions & 0 deletions app/static/tests/unit/store/sensitivity/mutations.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,12 @@ describe("Sensitivity mutations", () => {
expect(state.running).toBe(true);
});

it("sets running", () => {
const state = mockSensitivityState();
mutations.SetLoading(state, true);
expect(state.loading).toBe(true);
});

it("saves result when parameter set added", () => {
const mockResult = { batch: "fake batch" } as any;
const state = mockSensitivityState({
Expand Down

0 comments on commit 68b93b1

Please sign in to comment.