From a3691ced2e0bbf491c6ce003c31c84ec43454586 Mon Sep 17 00:00:00 2001
From: Ahmed Awan <qe66653@umbc.edu>
Date: Wed, 17 Apr 2024 13:45:40 -0500
Subject: [PATCH] refactor `WorkflowInvocationState` and improve its jest

Added a mock implementation for the store to detect fetches and use those to check if invocation or job states are terminal
---
 .../components/Workflow/InvocationsList.vue   |   9 +-
 .../WorkflowInvocationState.test.ts           | 131 +++++++--
 .../WorkflowInvocationState.vue               | 253 +++++++++---------
 3 files changed, 237 insertions(+), 156 deletions(-)

diff --git a/client/src/components/Workflow/InvocationsList.vue b/client/src/components/Workflow/InvocationsList.vue
index 829f68f51a56..23f1d485415a 100644
--- a/client/src/components/Workflow/InvocationsList.vue
+++ b/client/src/components/Workflow/InvocationsList.vue
@@ -2,7 +2,14 @@
     <div class="invocations-list" aria-labelledby="invocations-title">
         <div class="grid-header d-flex justify-content-between pb-2 flex-column">
             <div class="d-flex">
-                <Heading h1 separator inline size="xl" class="flex-grow-1 m-0" data-description="grid title">
+                <Heading
+                    id="invocations-title"
+                    h1
+                    separator
+                    inline
+                    size="xl"
+                    class="flex-grow-1 m-0"
+                    data-description="grid title">
                     <span v-localize>{{ title }}</span>
                 </Heading>
             </div>
diff --git a/client/src/components/WorkflowInvocationState/WorkflowInvocationState.test.ts b/client/src/components/WorkflowInvocationState/WorkflowInvocationState.test.ts
index 751bdc5dd32a..7189ccf76f4e 100644
--- a/client/src/components/WorkflowInvocationState/WorkflowInvocationState.test.ts
+++ b/client/src/components/WorkflowInvocationState/WorkflowInvocationState.test.ts
@@ -1,39 +1,91 @@
 import { createTestingPinia } from "@pinia/testing";
 import { shallowMount, Wrapper } from "@vue/test-utils";
 import flushPromises from "flush-promises";
-import { setActivePinia } from "pinia";
+import { PiniaVuePlugin, setActivePinia } from "pinia";
 import { getLocalVue } from "tests/jest/helpers";
 
-import type { WorkflowInvocation } from "@/api/invocations";
-
 import invocationData from "../Workflow/test/json/invocation.json";
 
 import WorkflowInvocationState from "./WorkflowInvocationState.vue";
 
 const localVue = getLocalVue();
+localVue.use(PiniaVuePlugin);
+
+jest.mock("vue-router/composables", () => ({
+    useRoute: jest.fn().mockReturnValue({ path: "" }),
+}));
 
 const selectors = {
     invocationSummary: ".invocation-summary",
 };
 
-const invocationJobsSummaryById = {
-    id: "d9833097445452b0",
+/** Invocation data to be expected in the store */
+const invocationById = {
+    [invocationData.id]: invocationData,
+    "not-fetched-invocation": null,
+    "non-terminal-id": {
+        ...invocationData,
+        id: "non-terminal-id",
+        state: "new",
+    },
+    "non-terminal-jobs": {
+        ...invocationData,
+        id: "non-terminal-jobs",
+    },
+};
+
+// Jobs summary constants
+const invocationDataJobsSummary = {
     model: "WorkflowInvocation",
     states: {},
     populated_state: "ok",
 };
+const invocationJobsSummaryById = {
+    [invocationData.id]: invocationDataJobsSummary,
+    "not-fetched-invocation": null,
+    "non-terminal-invocation": invocationDataJobsSummary,
+    "non-terminal-jobs": {
+        ...invocationDataJobsSummary,
+        states: {
+            running: 1,
+        },
+    },
+};
 
-async function mountWorkflowInvocationState(invocation: WorkflowInvocation | null) {
+// Mock the invocation store to return the expected invocation data given the invocation ID
+jest.mock("@/stores/invocationStore", () => {
+    const originalModule = jest.requireActual("@/stores/invocationStore");
+    const mockFetchInvocationForId = jest.fn();
+    const mockFetchInvocationJobsSummaryForId = jest.fn();
+    return {
+        ...originalModule,
+        useInvocationStore: () => ({
+            ...originalModule.useInvocationStore(),
+            getInvocationById: jest.fn().mockImplementation((invocationId) => {
+                return invocationById[invocationId];
+            }),
+            getInvocationJobsSummaryById: jest.fn().mockImplementation((invocationId) => {
+                return invocationJobsSummaryById[invocationId];
+            }),
+            fetchInvocationForId: mockFetchInvocationForId,
+            fetchInvocationJobsSummaryForId: mockFetchInvocationJobsSummaryForId,
+        }),
+        mockFetchInvocationForId,
+        mockFetchInvocationJobsSummaryForId,
+    };
+});
+
+/** Mount the WorkflowInvocationState component with the given invocation ID
+ * @param invocationId The invocation ID to be passed as a prop
+ * @returns The mounted wrapper
+ */
+async function mountWorkflowInvocationState(invocationId: string) {
     const pinia = createTestingPinia();
     setActivePinia(pinia);
 
-    const wrapper = shallowMount(WorkflowInvocationState, {
+    const wrapper = shallowMount(WorkflowInvocationState as object, {
         propsData: {
-            invocationId: invocationData.id,
-        },
-        computed: {
-            invocation: () => invocation,
-            jobStatesSummary: () => invocationJobsSummaryById,
+            invocationId,
         },
         pinia,
         localVue,
@@ -44,29 +96,60 @@ async function mountWorkflowInvocationState(invocation: WorkflowInvocation | nul
 
 describe("WorkflowInvocationState.vue", () => {
     it("determines that invocation and job states are terminal with terminal invocation", async () => {
-        const wrapper = await mountWorkflowInvocationState(invocationData as WorkflowInvocation);
+        const wrapper = await mountWorkflowInvocationState(invocationData.id);
         expect(isInvocationAndJobTerminal(wrapper)).toBe(true);
+
+        // Neither the invocation nor the jobs summary should be fetched for terminal invocations
+        assertInvocationFetched(0);
+        assertJobsSummaryFetched(0);
     });
 
-    it("determines that invocation and job states are not terminal with no invocation", async () => {
-        const wrapper = await mountWorkflowInvocationState(null);
+    it("determines that invocation and job states are not terminal with no fetched invocation", async () => {
+        const wrapper = await mountWorkflowInvocationState("not-fetched-invocation");
         expect(isInvocationAndJobTerminal(wrapper)).toBe(false);
+
+        // Both, the invocation and jobs summary should be fetched once if the invocation is not in the store
+        assertInvocationFetched(1);
+        assertJobsSummaryFetched(1);
     });
 
-    it("determines that invocation and job states are not terminal with non-terminal invocation", async () => {
-        const invocation = {
-            ...invocationData,
-            state: "new",
-        } as WorkflowInvocation;
-        const wrapper = await mountWorkflowInvocationState(invocation);
+    it("determines that invocation is not terminal with non-terminal state", async () => {
+        const wrapper = await mountWorkflowInvocationState("non-terminal-invocation");
         expect(isInvocationAndJobTerminal(wrapper)).toBe(false);
+
+        // Only the invocation should be fetched for non-terminal invocations
+        assertInvocationFetched(1);
+        assertJobsSummaryFetched(0);
+    });
+
+    it("determines that job states are not terminal with non-terminal jobs but scheduled invocation", async () => {
+        const wrapper = await mountWorkflowInvocationState("non-terminal-jobs");
+        expect(isInvocationAndJobTerminal(wrapper)).toBe(false);
+
+        // Only the jobs summary should be fetched, not the invocation since it is in scheduled/terminal state
+        assertInvocationFetched(0);
+        assertJobsSummaryFetched(1);
     });
 });
 
+/**
+ * This is a somewhat hacky way to determine if the invocation and job states are terminal without
+ * exposing the internals of the component. This is just to restore the previous behavior of the test
+ * and it only uses the wrapper to check the props of the invocation summary component.
+ */
 function isInvocationAndJobTerminal(wrapper: Wrapper<Vue>): boolean {
     const invocationSummary = wrapper.find(selectors.invocationSummary);
-    // This is a somewhat hacky way to determine if the invocation and job states are terminal without
-    // exposing the internals of the component. This is just to restore the previous behavior of the test
-    // but it would be better to test this in a more appropriate way.
     return invocationSummary.exists() && invocationSummary.html().includes('invocationandjobterminal="true"');
 }
+
+/** Asserts that the invocation was fetched in the store the given number of times */
+function assertInvocationFetched(count = 1) {
+    const { mockFetchInvocationForId } = jest.requireMock("@/stores/invocationStore");
+    expect(mockFetchInvocationForId).toHaveBeenCalledTimes(count);
+}
+
+/** Asserts that the jobs summary was fetched in the store the given number of times */
+function assertJobsSummaryFetched(count = 1) {
+    const { mockFetchInvocationJobsSummaryForId } = jest.requireMock("@/stores/invocationStore");
+    expect(mockFetchInvocationJobsSummaryForId).toHaveBeenCalledTimes(count);
+}
diff --git a/client/src/components/WorkflowInvocationState/WorkflowInvocationState.vue b/client/src/components/WorkflowInvocationState/WorkflowInvocationState.vue
index b2fe9deec3df..ada6cae30519 100644
--- a/client/src/components/WorkflowInvocationState/WorkflowInvocationState.vue
+++ b/client/src/components/WorkflowInvocationState/WorkflowInvocationState.vue
@@ -1,6 +1,108 @@
+<script setup lang="ts">
+import { library } from "@fortawesome/fontawesome-svg-core";
+import { faArrowLeft } from "@fortawesome/free-solid-svg-icons";
+import { FontAwesomeIcon } from "@fortawesome/vue-fontawesome";
+import { BAlert, BButton, BTab, BTabs } from "bootstrap-vue";
+import { computed, onMounted, onUnmounted, ref } from "vue";
+import { RouterLink } from "vue-router";
+import { useRoute } from "vue-router/composables";
+
+import { InvocationJobsSummary, WorkflowInvocationElementView } from "@/api/invocations";
+import { useInvocationStore } from "@/stores/invocationStore";
+
+import { cancelWorkflowScheduling } from "./services";
+import { isTerminal, jobCount } from "./util";
+
+import WorkflowInvocationDetails from "./WorkflowInvocationDetails.vue";
+import WorkflowInvocationExportOptions from "./WorkflowInvocationExportOptions.vue";
+import WorkflowInvocationSummary from "./WorkflowInvocationSummary.vue";
+import LoadingSpan from "@/components/LoadingSpan.vue";
+
+library.add(faArrowLeft);
+
+interface Props {
+    invocationId: string;
+    index?: number;
+    isSubworkflow?: boolean;
+}
+
+const props = withDefaults(defineProps<Props>(), {
+    index: undefined,
+    isSubworkflow: false,
+});
+
+const emit = defineEmits<{
+    (e: "invocation-cancelled"): void;
+}>();
+
+const route = useRoute();
+
+const invocationStore = useInvocationStore();
+
+const stepStatesInterval = ref<any>(undefined);
+const jobStatesInterval = ref<any>(undefined);
+
+const invocation = computed(
+    () => invocationStore.getInvocationById(props.invocationId) as WorkflowInvocationElementView
+);
+const invocationState = computed(() => invocation.value?.state || "new");
+const invocationAndJobTerminal = computed(() => invocationSchedulingTerminal.value && jobStatesTerminal.value);
+const invocationSchedulingTerminal = computed(() => {
+    return (
+        invocationState.value == "scheduled" ||
+        invocationState.value == "cancelled" ||
+        invocationState.value == "failed"
+    );
+});
+const jobStatesTerminal = computed(() => {
+    if (invocationSchedulingTerminal.value && jobCount(jobStatesSummary.value as InvocationJobsSummary) === 0) {
+        // no jobs for this invocation (think subworkflow or just inputs)
+        return true;
+    }
+    return !!jobStatesSummary.value && isTerminal(jobStatesSummary.value as InvocationJobsSummary);
+});
+const jobStatesSummary = computed(() => {
+    const jobsSummary = invocationStore.getInvocationJobsSummaryById(props.invocationId);
+    return (!jobsSummary ? null : jobsSummary) as InvocationJobsSummary;
+});
+const isInvocationRoute = computed(() => route.path.includes(`/workflows/invocations/${props.invocationId}`));
+
+onMounted(() => {
+    pollStepStatesUntilTerminal();
+    pollJobStatesUntilTerminal();
+});
+
+onUnmounted(() => {
+    clearTimeout(stepStatesInterval.value);
+    clearTimeout(jobStatesInterval.value);
+});
+
+async function pollStepStatesUntilTerminal() {
+    if (!invocation.value || !invocationSchedulingTerminal.value) {
+        await invocationStore.fetchInvocationForId({ id: props.invocationId });
+        stepStatesInterval.value = setTimeout(pollStepStatesUntilTerminal, 3000);
+    }
+}
+async function pollJobStatesUntilTerminal() {
+    if (!jobStatesTerminal.value) {
+        await invocationStore.fetchInvocationJobsSummaryForId({ id: props.invocationId });
+        jobStatesInterval.value = setTimeout(pollJobStatesUntilTerminal, 3000);
+    }
+}
+function onError(e: any) {
+    console.error(e);
+}
+function onCancel() {
+    emit("invocation-cancelled");
+}
+function cancelWorkflowSchedulingLocal() {
+    cancelWorkflowScheduling(props.invocationId).then(onCancel).catch(onError);
+}
+</script>
+
 <template>
-    <b-tabs v-if="invocation" :class="{ 'position-relative': isInvocationRoute }">
-        <b-tab title="Summary" active>
+    <BTabs v-if="invocation" :class="{ 'position-relative': isInvocationRoute }">
+        <BTab title="Summary" active>
             <WorkflowInvocationSummary
                 class="invocation-summary"
                 :invocation="invocation"
@@ -9,141 +111,30 @@
                 :invocation-scheduling-terminal="invocationSchedulingTerminal"
                 :job-states-summary="jobStatesSummary"
                 :is-subworkflow="isSubworkflow"
-                @invocation-cancelled="cancelWorkflowScheduling" />
-        </b-tab>
-        <b-tab title="Details">
+                @invocation-cancelled="cancelWorkflowSchedulingLocal" />
+        </BTab>
+        <BTab title="Details">
             <WorkflowInvocationDetails :invocation="invocation" />
-        </b-tab>
-        <!-- <b-tab title="Workflow Overview">
+        </BTab>
+        <!-- <BTab title="Workflow Overview">
             <p>TODO: Insert readonly version of workflow editor here</p>
-        </b-tab> -->
-        <b-tab title="Export">
+        </BTab> -->
+        <BTab title="Export">
             <div v-if="invocationAndJobTerminal">
                 <WorkflowInvocationExportOptions :invocation-id="invocation.id" />
             </div>
-            <b-alert v-else variant="info" show>
+            <BAlert v-else variant="info" show>
                 <LoadingSpan message="Waiting to complete invocation" />
-            </b-alert>
-        </b-tab>
-        <router-link to="/workflows/invocations">
-            <b-button v-if="isInvocationRoute" class="position-absolute text-decoration-none" style="top: 0; right: 0">
-                <i class="fa fa-arrow-left mr-1" />
+            </BAlert>
+        </BTab>
+        <RouterLink v-if="isInvocationRoute" to="/workflows/invocations">
+            <BButton class="position-absolute text-decoration-none" style="top: 0; right: 0">
+                <FontAwesomeIcon :icon="faArrowLeft" class="mr-1" />
                 Go to Invocations List
-            </b-button>
-        </router-link>
-    </b-tabs>
-    <b-alert v-else variant="info" show>
+            </BButton>
+        </RouterLink>
+    </BTabs>
+    <BAlert v-else variant="info" show>
         <LoadingSpan message="Loading invocation" />
-    </b-alert>
+    </BAlert>
 </template>
-<script>
-import LoadingSpan from "components/LoadingSpan";
-
-import { useInvocationStore } from "@/stores/invocationStore";
-
-import { cancelWorkflowScheduling } from "./services";
-import { isTerminal, jobCount } from "./util";
-
-import WorkflowInvocationDetails from "./WorkflowInvocationDetails.vue";
-import WorkflowInvocationExportOptions from "./WorkflowInvocationExportOptions.vue";
-import WorkflowInvocationSummary from "./WorkflowInvocationSummary.vue";
-
-export default {
-    components: {
-        LoadingSpan,
-        WorkflowInvocationSummary,
-        WorkflowInvocationDetails,
-        WorkflowInvocationExportOptions,
-    },
-    props: {
-        invocationId: {
-            type: String,
-            required: true,
-        },
-        index: {
-            type: Number,
-            required: false,
-            default: null,
-        },
-        isSubworkflow: {
-            type: Boolean,
-            default: false,
-        },
-    },
-    setup() {
-        const invocationStore = useInvocationStore();
-        return {
-            invocationStore,
-        };
-    },
-    data() {
-        return {
-            stepStatesInterval: null,
-            jobStatesInterval: null,
-        };
-    },
-    computed: {
-        invocation: function () {
-            return this.invocationStore.getInvocationById(this.invocationId);
-        },
-        invocationState: function () {
-            return this.invocation?.state || "new";
-        },
-        invocationAndJobTerminal: function () {
-            return !!(this.invocationSchedulingTerminal && this.jobStatesTerminal);
-        },
-        invocationSchedulingTerminal: function () {
-            return (
-                this.invocationState == "scheduled" ||
-                this.invocationState == "cancelled" ||
-                this.invocationState == "failed"
-            );
-        },
-        jobStatesTerminal: function () {
-            if (this.invocationSchedulingTerminal && jobCount(this.jobStatesSummary) === 0) {
-                // no jobs for this invocation (think subworkflow or just inputs)
-                return true;
-            }
-            return this.jobStatesSummary && isTerminal(this.jobStatesSummary);
-        },
-        jobStatesSummary() {
-            const jobsSummary = this.invocationStore.getInvocationJobsSummaryById(this.invocationId);
-            return !jobsSummary ? null : jobsSummary;
-        },
-        isInvocationRoute() {
-            return this.$route.path.includes(`/workflows/invocations/${this.invocationId}`);
-        },
-    },
-    created: function () {
-        this.pollStepStatesUntilTerminal();
-        this.pollJobStatesUntilTerminal();
-    },
-    beforeDestroy: function () {
-        clearTimeout(this.jobStatesInterval);
-        clearTimeout(this.stepStatesInterval);
-    },
-    methods: {
-        pollStepStatesUntilTerminal: async function () {
-            if (!this.invocation || !this.invocationSchedulingTerminal) {
-                await this.invocationStore.fetchInvocationForId({ id: this.invocationId });
-                this.stepStatesInterval = setTimeout(this.pollStepStatesUntilTerminal, 3000);
-            }
-        },
-        pollJobStatesUntilTerminal: async function () {
-            if (!this.jobStatesTerminal) {
-                await this.invocationStore.fetchInvocationJobsSummaryForId({ id: this.invocationId });
-                this.jobStatesInterval = setTimeout(this.pollJobStatesUntilTerminal, 3000);
-            }
-        },
-        onError: function (e) {
-            console.error(e);
-        },
-        onCancel() {
-            this.$emit("invocation-cancelled");
-        },
-        cancelWorkflowScheduling: function () {
-            cancelWorkflowScheduling(this.invocationId).then(this.onCancel).catch(this.onError);
-        },
-    },
-};
-</script>