Skip to content

Commit

Permalink
Merge pull request galaxyproject#19180 from ElectronicBlueberry/fix-t…
Browse files Browse the repository at this point in the history 
…est-warning-task-monitor

Fix Pesky warning with PersistentTaskProgressMonitorAlert.test.ts
  • Loading branch information
@davelopez
davelopez authored Nov 21, 2024
2 parents 43984c1 + 1cad7f7 commit 701d0ff
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 50 deletions.
2 changes: 1 addition & 1 deletion client/src/components/Common/PersistentTaskProgressMonitorAlert.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const FAKE_MONITOR: TaskMonitor = {
isCompleted: ref(false),
hasFailed: ref(false),
requestHasFailed: ref(false),
status: ref(),
taskStatus: ref(""),
expirationTime: FAKE_EXPIRATION_TIME,
isFinalState: jest.fn(),
loadStatus: jest.fn(),
Expand Down
18 changes: 9 additions & 9 deletions client/src/composables/genericTaskMonitor.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export interface TaskMonitor {
* The meaning of the status string is up to the monitor implementation.
* In case of an error, this will be the error message.
*/
status: Readonly<Ref<string | undefined>>;
taskStatus: Readonly<Ref<string | undefined>>;

/**
* Loads the status of the task from a stored value.
Expand Down Expand Up @@ -96,19 +96,19 @@ export function useGenericMonitor(options: {
let pollDelay = options.defaultPollDelay ?? DEFAULT_POLL_DELAY;

const isRunning = ref(false);
const status = ref<string>();
const taskStatus = ref<string>();
const requestId = ref<string>();
const requestHasFailed = ref(false);

const isCompleted = computed(() => options.completedCondition(status.value));
const hasFailed = computed(() => options.failedCondition(status.value));
const isCompleted = computed(() => options.completedCondition(taskStatus.value));
const hasFailed = computed(() => options.failedCondition(taskStatus.value));

function isFinalState(status?: string) {
return options.completedCondition(status) || options.failedCondition(status);
}

function loadStatus(storedStatus: string) {
status.value = storedStatus;
taskStatus.value = storedStatus;
}

async function waitForTask(taskId: string, pollDelayInMs?: number) {
Expand All @@ -122,7 +122,7 @@ export function useGenericMonitor(options: {
async function fetchTaskStatus(taskId: string) {
try {
const result = await options.fetchStatus(taskId);
status.value = result;
taskStatus.value = result;
if (isCompleted.value || hasFailed.value) {
isRunning.value = false;
} else {
Expand All @@ -141,7 +141,7 @@ export function useGenericMonitor(options: {
}

function handleError(err: string) {
status.value = err.toString();
taskStatus.value = err.toString();
requestHasFailed.value = true;
isRunning.value = false;
resetTimeout();
Expand All @@ -156,7 +156,7 @@ export function useGenericMonitor(options: {

function resetState() {
resetTimeout();
status.value = undefined;
taskStatus.value = undefined;
requestHasFailed.value = false;
isRunning.value = false;
}
Expand All @@ -169,7 +169,7 @@ export function useGenericMonitor(options: {
isCompleted: readonly(isCompleted),
hasFailed: readonly(hasFailed),
requestHasFailed: readonly(requestHasFailed),
status: readonly(status),
taskStatus: readonly(taskStatus),
expirationTime: options.expirationTime,
};
}
6 changes: 3 additions & 3 deletions client/src/composables/persistentProgressMonitor.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ jest.mock("@vueuse/core", () => ({

function useMonitorMock(): TaskMonitor {
const isRunning = ref(false);
const status = ref();
const taskStatus = ref();

return {
waitForTask: jest.fn().mockImplementation(() => {
Expand All @@ -30,11 +30,11 @@ function useMonitorMock(): TaskMonitor {
isCompleted: ref(false),
hasFailed: ref(false),
requestHasFailed: ref(false),
status,
taskStatus,
expirationTime: 1000,
isFinalState: jest.fn(),
loadStatus(storedStatus) {
status.value = storedStatus;
taskStatus.value = storedStatus;
},
};
}
Expand Down
14 changes: 7 additions & 7 deletions client/src/composables/persistentProgressMonitor.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ export interface MonitoringData {
* The meaning of the status string is up to the monitor implementation.
* In case of an error, this will be the error message.
*/
status?: string;
taskStatus?: string;
}

/**
Expand All @@ -120,7 +120,7 @@ export function usePersistentProgressTaskMonitor(
isCompleted,
hasFailed,
requestHasFailed,
status,
taskStatus,
expirationTime,
} = useMonitor;

Expand Down Expand Up @@ -152,12 +152,12 @@ export function usePersistentProgressTaskMonitor(
});

watch(
status,
() => taskStatus.value,
(newStatus) => {
if (newStatus && currentMonitoringData.value) {
currentMonitoringData.value = {
...currentMonitoringData.value,
status: newStatus,
taskStatus: newStatus,
};
}
},
Expand All @@ -173,10 +173,10 @@ export function usePersistentProgressTaskMonitor(
throw new Error("No monitoring data provided or stored. Cannot start monitoring progress.");
}

if (isFinalState(currentMonitoringData.value.status)) {
if (isFinalState(currentMonitoringData.value.taskStatus)) {
// The task has already finished no need to start monitoring again.
// Instead, reload the stored status to update the UI.
return loadStatus(currentMonitoringData.value.status!);
return loadStatus(currentMonitoringData.value.taskStatus!);
}

if (hasExpired.value) {
Expand Down Expand Up @@ -240,7 +240,7 @@ export function usePersistentProgressTaskMonitor(
* The meaning of the status string is up to the monitor implementation.
* In case of an error, this will be the error message.
*/
status,
status: taskStatus,

/**
* True if the monitoring data can expire.
Expand Down
28 changes: 14 additions & 14 deletions client/src/composables/shortTermStorageMonitor.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,72 +31,72 @@ describe("useShortTermStorageMonitor", () => {
});

it("should indicate the task is running when it is still not ready", async () => {
const { waitForTask, isRunning, status } = useShortTermStorageMonitor();
const { waitForTask, isRunning, taskStatus } = useShortTermStorageMonitor();

expect(isRunning.value).toBe(false);
waitForTask(PENDING_TASK_ID);
await flushPromises();
expect(isRunning.value).toBe(true);
expect(status.value).toBe("PENDING");
expect(taskStatus.value).toBe("PENDING");
});

it("should indicate the task is successfully completed when the state is ready", async () => {
const { waitForTask, isRunning, isCompleted, status } = useShortTermStorageMonitor();
const { waitForTask, isRunning, isCompleted, taskStatus } = useShortTermStorageMonitor();

expect(isCompleted.value).toBe(false);
waitForTask(COMPLETED_TASK_ID);
await flushPromises();
expect(isCompleted.value).toBe(true);
expect(isRunning.value).toBe(false);
expect(status.value).toBe("READY");
expect(taskStatus.value).toBe("READY");
});

it("should indicate the task status request failed when the request failed", async () => {
const { waitForTask, requestHasFailed, isRunning, isCompleted, status } = useShortTermStorageMonitor();
const { waitForTask, requestHasFailed, isRunning, isCompleted, taskStatus } = useShortTermStorageMonitor();

expect(requestHasFailed.value).toBe(false);
waitForTask(REQUEST_FAILED_TASK_ID);
await flushPromises();
expect(requestHasFailed.value).toBe(true);
expect(isRunning.value).toBe(false);
expect(isCompleted.value).toBe(false);
expect(status.value).toBe("Request failed");
expect(taskStatus.value).toBe("Request failed");
});

it("should load the status from the stored monitoring data", async () => {
const { loadStatus, isRunning, isCompleted, hasFailed, status } = useShortTermStorageMonitor();
const { loadStatus, isRunning, isCompleted, hasFailed, taskStatus } = useShortTermStorageMonitor();
const storedStatus = "READY";

loadStatus(storedStatus);

expect(status.value).toBe(storedStatus);
expect(taskStatus.value).toBe(storedStatus);
expect(isRunning.value).toBe(false);
expect(isCompleted.value).toBe(true);
expect(hasFailed.value).toBe(false);
});

describe("isFinalState", () => {
it("should indicate is final state when the task is completed", async () => {
const { waitForTask, isFinalState, isRunning, isCompleted, hasFailed, status } =
const { waitForTask, isFinalState, isRunning, isCompleted, hasFailed, taskStatus } =
useShortTermStorageMonitor();

expect(isFinalState(status.value)).toBe(false);
expect(isFinalState(taskStatus.value)).toBe(false);
waitForTask(COMPLETED_TASK_ID);
await flushPromises();
expect(isFinalState(status.value)).toBe(true);
expect(isFinalState(taskStatus.value)).toBe(true);
expect(isRunning.value).toBe(false);
expect(isCompleted.value).toBe(true);
expect(hasFailed.value).toBe(false);
});

it("should indicate is final state when the task has failed", async () => {
const { waitForTask, isFinalState, isRunning, isCompleted, hasFailed, status } =
const { waitForTask, isFinalState, isRunning, isCompleted, hasFailed, taskStatus } =
useShortTermStorageMonitor();

expect(isFinalState(status.value)).toBe(false);
expect(isFinalState(taskStatus.value)).toBe(false);
waitForTask(REQUEST_FAILED_TASK_ID);
await flushPromises();
expect(isFinalState(status.value)).toBe(true);
expect(isFinalState(taskStatus.value)).toBe(true);
expect(isRunning.value).toBe(false);
expect(isCompleted.value).toBe(false);
expect(hasFailed.value).toBe(true);
Expand Down
32 changes: 16 additions & 16 deletions client/src/composables/taskMonitor.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,81 +35,81 @@ describe("useTaskMonitor", () => {
});

it("should indicate the task is running when it is still pending", async () => {
const { waitForTask, isRunning, status } = useTaskMonitor();
const { waitForTask, isRunning, taskStatus } = useTaskMonitor();

expect(isRunning.value).toBe(false);
waitForTask(PENDING_TASK_ID);
await flushPromises();
expect(isRunning.value).toBe(true);
expect(status.value).toBe("PENDING");
expect(taskStatus.value).toBe("PENDING");
});

it("should indicate the task is successfully completed when the state is SUCCESS", async () => {
const { waitForTask, isRunning, isCompleted, status } = useTaskMonitor();
const { waitForTask, isRunning, isCompleted, taskStatus } = useTaskMonitor();

expect(isCompleted.value).toBe(false);
waitForTask(COMPLETED_TASK_ID);
await flushPromises();
expect(isCompleted.value).toBe(true);
expect(isRunning.value).toBe(false);
expect(status.value).toBe("SUCCESS");
expect(taskStatus.value).toBe("SUCCESS");
});

it("should indicate the task has failed when the state is FAILED", async () => {
const { waitForTask, isRunning, hasFailed, status } = useTaskMonitor();
const { waitForTask, isRunning, hasFailed, taskStatus } = useTaskMonitor();

expect(hasFailed.value).toBe(false);
waitForTask(FAILED_TASK_ID);
await flushPromises();
expect(hasFailed.value).toBe(true);
expect(isRunning.value).toBe(false);
expect(status.value).toBe("FAILURE");
expect(taskStatus.value).toBe("FAILURE");
});

it("should indicate the task status request failed when the request failed", async () => {
const { waitForTask, requestHasFailed, isRunning, isCompleted, status } = useTaskMonitor();
const { waitForTask, requestHasFailed, isRunning, isCompleted, taskStatus } = useTaskMonitor();

expect(requestHasFailed.value).toBe(false);
waitForTask(REQUEST_FAILED_TASK_ID);
await flushPromises();
expect(requestHasFailed.value).toBe(true);
expect(isRunning.value).toBe(false);
expect(isCompleted.value).toBe(false);
expect(status.value).toBe("Request failed");
expect(taskStatus.value).toBe("Request failed");
});

it("should load the status from the stored monitoring data", async () => {
const { loadStatus, isRunning, isCompleted, hasFailed, status } = useTaskMonitor();
const { loadStatus, isRunning, isCompleted, hasFailed, taskStatus } = useTaskMonitor();
const storedStatus = "SUCCESS";

loadStatus(storedStatus);

expect(status.value).toBe(storedStatus);
expect(taskStatus.value).toBe(storedStatus);
expect(isRunning.value).toBe(false);
expect(isCompleted.value).toBe(true);
expect(hasFailed.value).toBe(false);
});

describe("isFinalState", () => {
it("should indicate is final state when the task is completed", async () => {
const { waitForTask, isFinalState, isRunning, isCompleted, hasFailed, status } = useTaskMonitor();
const { waitForTask, isFinalState, isRunning, isCompleted, hasFailed, taskStatus } = useTaskMonitor();

expect(isFinalState(status.value)).toBe(false);
expect(isFinalState(taskStatus.value)).toBe(false);
waitForTask(COMPLETED_TASK_ID);
await flushPromises();
expect(isFinalState(status.value)).toBe(true);
expect(isFinalState(taskStatus.value)).toBe(true);
expect(isRunning.value).toBe(false);
expect(isCompleted.value).toBe(true);
expect(hasFailed.value).toBe(false);
});

it("should indicate is final state when the task has failed", async () => {
const { waitForTask, isFinalState, isRunning, isCompleted, hasFailed, status } = useTaskMonitor();
const { waitForTask, isFinalState, isRunning, isCompleted, hasFailed, taskStatus } = useTaskMonitor();

expect(isFinalState(status.value)).toBe(false);
expect(isFinalState(taskStatus.value)).toBe(false);
waitForTask(FAILED_TASK_ID);
await flushPromises();
expect(isFinalState(status.value)).toBe(true);
expect(isFinalState(taskStatus.value)).toBe(true);
expect(isRunning.value).toBe(false);
expect(isCompleted.value).toBe(false);
expect(hasFailed.value).toBe(true);
Expand Down

0 comments on commit 701d0ff

Please sign in to comment.