Merge pull request #13881 from guerler/revise_provider_test

Fix flaky user histories providers test
galaxyproject · May 11, 2022 · c154561 · c154561
2 parents 58277dc + d7dab7e
commit c154561
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 103 deletions.
diff --git a/client/src/components/providers/UserHistories.test.js b/client/src/components/providers/UserHistories.test.js
@@ -1,32 +1,8 @@
 import Vuex from "vuex";
 import { shallowMount } from "@vue/test-utils";
-import { getLocalVue, wait, waitForLifecyleEvent, watchForChange } from "jest/helpers";
+import { getLocalVue } from "jest/helpers";
 import { historyStore } from "store/historyStore/historyStore";
 import UserHistories from "./UserHistories";
-
-// #region Test Data
-
-let historySummaries;
-let fullHistories;
-let currentHistoryId;
-
-// These history tests can be flaky, so we allow a few retries while we work out
-// what's going on.
-jest.retryTimes(3, { logErrorsBeforeRetry: true });
-
-const resetTestData = () => {
-    const ids = [1, 2, 3, 4];
-    currentHistoryId = ids[0];
-    historySummaries = ids.map((i) => ({ id: i, name: `History #${i}` }));
-    fullHistories = historySummaries.map((h) => ({ ...h, detailView: true }));
-};
-
-const fakeUser = { id: 123234, name: "Bob" };
-
-// #endregion
-
-// #region Mock server responses
-
 import {
     getHistoryList,
     getHistoryById,
@@ -40,78 +16,65 @@ import {
 jest.mock("app");
 jest.mock("store/historyStore/model/queries");
 
-const maxServerDelay = 60;
-const serverDelay = async (min = 0, max = maxServerDelay) => {
-    const delay = min + Math.random() * (max - min);
-    await wait(delay);
+const localVue = getLocalVue();
+
+// generate store for testing
+const localStore = new Vuex.Store({
+    modules: {
+        history: historyStore,
+    },
+});
+
+// test user data
+const fakeUser = { id: 123234, name: "Bob" };
+const createdHistory = { id: 666, name: "I am new" };
+
+// test store data
+let currentHistoryId;
+let histories;
+const setTestData = () => {
+    const ids = [1, 2, 3, 4];
+    currentHistoryId = ids[0];
+    histories = ids.map((i) => ({ id: i, name: `History #${i}` }));
 };
 
+// mock store queries
 getHistoryList.mockImplementation(async () => {
-    await serverDelay();
-    return [...historySummaries];
+    return [...histories];
 });
-
 getCurrentHistoryFromServer.mockImplementation(async () => {
-    await serverDelay();
-    return fullHistories.find((o) => o.id == currentHistoryId);
+    return histories.find((o) => o.id == currentHistoryId);
 });
-
 getHistoryById.mockImplementation(async (id) => {
-    await serverDelay();
-    return fullHistories.find((o) => o.id == id);
+    return histories.find((o) => o.id == id);
 });
-
-const createdHistory = { id: 666, name: "I am new" };
 createNewHistory.mockImplementation(async () => {
-    await serverDelay();
-    historySummaries.push(createdHistory);
-    const newFullHistory = { ...createdHistory, detailView: true };
-    fullHistories.push(newFullHistory);
-    return Object.assign({}, newFullHistory);
+    histories.push(createdHistory);
+    return Object.assign({}, createdHistory);
 });
-
 setCurrentHistoryOnServer.mockImplementation(async (id) => {
-    await serverDelay();
     currentHistoryId = id;
-    const result = fullHistories.find((h) => h.id == id);
+    const result = histories.find((h) => h.id == id);
     return Object.assign({}, result);
 });
-
 updateHistoryFields.mockImplementation(async (id, payload = {}) => {
-    await serverDelay();
-    const existingHistory = fullHistories.find((h) => h.id == id);
+    const existingHistory = histories.find((h) => h.id == id);
     return { ...existingHistory, ...payload };
 });
-
 deleteHistoryById.mockImplementation(async (id) => {
-    await serverDelay();
-    return fullHistories.find((h) => h.id == id);
-});
-
-// #endregion
-
-// #region Mounting
-
-const localVue = getLocalVue();
-
-// Generate store for tesint, just need the one module we talk to
-const historiesStore = new Vuex.Store({
-    modules: {
-        history: historyStore,
-    },
+    return histories.find((h) => h.id == id);
 });
 
-// #endregion
-
+// user histories provider test cases
 describe("UserHistories", () => {
     let wrapper;
     let slotProps;
 
     beforeEach(async () => {
-        resetTestData();
+        setTestData();
         wrapper = shallowMount(UserHistories, {
             localVue,
-            store: historiesStore,
+            store: localStore,
             propsData: {
                 user: fakeUser,
             },
@@ -121,21 +84,20 @@ describe("UserHistories", () => {
                 },
             },
         });
-        await waitForLifecyleEvent(wrapper.vm, "updated");
     });
 
     afterEach(async () => {
-        historiesStore.dispatch("history/resetHistory");
+        localStore.dispatch("history/resetHistory");
         if (wrapper) {
             await wrapper.destroy();
         }
     });
 
-    describe("slotProps: values", () => {
+    describe("slotProp values", () => {
         test("histories", async () => {
             const { histories } = slotProps;
             expect(histories).toBeInstanceOf(Array);
-            expect(histories.length).toEqual(historySummaries.length);
+            expect(histories.length).toEqual(histories.length);
             histories.forEach((h) => {
                 expect(h).toBeInstanceOf(Object);
             });
@@ -152,18 +114,10 @@ describe("UserHistories", () => {
         test("setCurrentHistory", async () => {
             const { setCurrentHistory } = slotProps.handlers;
             expect(setCurrentHistory).toBeInstanceOf(Function);
-
             // set another history as current
-            const nextHistory = historySummaries[1];
-            setCurrentHistory(nextHistory);
-
-            await watchForChange({
-                vm: wrapper.vm,
-                propName: "currentHistory",
-                label: "currenntHistoryChange",
-            });
-
-            // wait for it to register
+            const nextHistory = histories[1];
+            await setCurrentHistory(nextHistory);
+            // verify that current history has changed
             const { currentHistory: changedHistory, histories: newHistories } = slotProps;
             expect(changedHistory.id).toEqual(nextHistory.id);
             expect(changedHistory).toBeInstanceOf(Object);
@@ -173,16 +127,11 @@ describe("UserHistories", () => {
         test("createNewHistory", async () => {
             const { createNewHistory } = slotProps.handlers;
             expect(createNewHistory).toBeInstanceOf(Function);
-
+            // expect initial history id
+            expect(slotProps.currentHistory.id).toEqual(histories[0].id);
             // create new history
             await createNewHistory();
-
-            await watchForChange({
-                vm: wrapper.vm,
-                propName: "currentHistory",
-            });
-
-            // wait for it to come out of the slot
+            // expect new history id
             expect(slotProps.currentHistory.id).toEqual(createdHistory.id);
         });
 
@@ -194,11 +143,8 @@ describe("UserHistories", () => {
             expect(updateHistory).toBeInstanceOf(Function);
             expect(currentHistory).toBeInstanceOf(Object);
             const modifiedHistory = { ...currentHistory, foo: "bar" };
-
             expect(modifiedHistory.id).toBeDefined();
             await updateHistory(modifiedHistory);
-            await waitForLifecyleEvent(wrapper.vm, "updated");
-
             expect(slotProps.histories.find((h) => h.foo == "bar")).toBeTruthy();
         });
 
@@ -207,11 +153,8 @@ describe("UserHistories", () => {
                 currentHistory,
                 handlers: { deleteHistory },
             } = slotProps;
-
             expect(deleteHistory).toBeInstanceOf(Function);
-
             await deleteHistory(currentHistory);
-            await waitForLifecyleEvent(wrapper.vm, "updated");
             expect(slotProps.histories.find((h) => h.id == currentHistory.id)).toBeFalsy();
             expect(slotProps.currentHistory.id).not.toEqual(currentHistory.id);
         });

diff --git a/client/src/store/historyStore/historyStore.js b/client/src/store/historyStore/historyStore.js
@@ -84,20 +84,20 @@ const promises = {
 const actions = {
     async copyHistory({ dispatch }, { history, name, copyAll }) {
         const newHistory = await cloneHistory(history, name, copyAll);
-        dispatch("selectHistory", newHistory);
+        return dispatch("selectHistory", newHistory);
     },
     async createNewHistory({ dispatch }) {
         // create history, then select it as current at the same time
         const newHistory = await createNewHistory();
-        dispatch("selectHistory", newHistory);
+        return dispatch("selectHistory", newHistory);
     },
     async deleteHistory({ dispatch, commit, getters }, { history, purge = false }) {
         const deletedHistory = await deleteHistoryById(history.id, purge);
         commit("deleteHistory", deletedHistory);
         if (getters.firstHistoryId) {
-            await dispatch("setCurrentHistoryId", getters.firstHistoryId);
+            return dispatch("setCurrentHistoryId", getters.firstHistoryId);
         } else {
-            await dispatch("createNewHistory");
+            return dispatch("createNewHistory");
         }
     },
     loadCurrentHistory({ dispatch }) {