Copilot commented on code in PR #6818:
URL: https://github.com/apache/texera/pull/6818#discussion_r3635676013
##########
frontend/src/app/dashboard/component/admin/execution/admin-execution.component.spec.ts:
##########
@@ -416,4 +472,68 @@ describe("AdminExecutionComponent methods (#6550)", () => {
});
});
});
+
+ describe("lifecycle polling", () => {
+ // The clock tick and background-refresh intervals are hard-coded in the
component
+ // (1s and 5s respectively); mirror them here to drive the fake timers.
+ const TICK_MS = 1000;
+ const REFRESH_MS = 5000;
+
+ it("ngOnInit loads the current page and starts the 1s clock tick", () => {
+ const firstExec = makeExecution({ workflowId: 5 });
+ vi.mocked(service.getExecutionList).mockReturnValue(of([firstExec]));
+ vi.mocked(service.getTotalWorkflows).mockReturnValue(of(2));
+ const updateSpy = vi.spyOn(component, "updateTimeStatus");
+
+ component.ngOnInit();
+
+ // The initial fetch resolves synchronously (of(...)) and populates the
view.
+ expect(component.listOfExecutions).toEqual([firstExec]);
+ expect(component.totalWorkflows).toBe(2);
+ expect(component.isLoading).toBe(false);
+
+ updateSpy.mockClear();
+ vi.mocked(service.getExecutionList).mockClear();
+
+ // A clock tick recomputes elapsed time client-side without hitting the
service.
+ vi.advanceTimersByTime(TICK_MS);
+
+ expect(updateSpy).toHaveBeenCalled();
+ expect(service.getExecutionList).not.toHaveBeenCalled();
+ });
+
+ it("ngOnInit polls the current page every 5s and leaves the total
untouched when it did not change", () => {
+ vi.mocked(service.getExecutionList).mockReturnValue(of([makeExecution({
workflowId: 5 })]));
+ vi.mocked(service.getTotalWorkflows).mockReturnValue(of(2));
+
+ component.ngOnInit();
+ expect(component.totalWorkflows).toBe(2);
+
+ // Next poll returns fresh rows but the same total.
+ const polledExec = makeExecution({ workflowId: 9, executionStatus:
"RUNNING" });
+ vi.mocked(service.getExecutionList).mockClear();
+ vi.mocked(service.getExecutionList).mockReturnValue(of([polledExec]));
+ vi.mocked(service.getTotalWorkflows).mockReturnValue(of(2));
+
+ vi.advanceTimersByTime(REFRESH_MS);
+
+ expect(service.getExecutionList).toHaveBeenCalledTimes(1);
+ expect(component.listOfExecutions).toEqual([polledExec]);
+ // The total is unchanged, so applyCurrentPage must not reassign/churn
it.
+ expect(component.totalWorkflows).toBe(2);
+ });
+
+ it("ngOnDestroy clears the clock interval so it stops ticking", () => {
+ vi.mocked(service.getExecutionList).mockReturnValue(of([]));
+ vi.mocked(service.getTotalWorkflows).mockReturnValue(of(0));
+ component.ngOnInit();
+
+ const updateSpy = vi.spyOn(component, "updateTimeStatus");
+ component.ngOnDestroy();
+
+ vi.advanceTimersByTime(TICK_MS * 3);
+
+ expect(updateSpy).not.toHaveBeenCalled();
+ });
Review Comment:
The issue/PR description calls out `ngOnDestroy` “interval cleanup”, but
this test only advances time by 3 seconds, so it can’t catch a leak in the 5s
polling subscription (it would only fire at/after 5s). Advancing beyond
`REFRESH_MS` and asserting no service refetch occurs after destroy would lock
in cleanup for both the 1s clock tick and the 5s poll.
##########
frontend/src/app/dashboard/component/admin/execution/admin-execution.component.spec.ts:
##########
@@ -416,4 +472,68 @@ describe("AdminExecutionComponent methods (#6550)", () => {
});
});
});
+
+ describe("lifecycle polling", () => {
+ // The clock tick and background-refresh intervals are hard-coded in the
component
+ // (1s and 5s respectively); mirror them here to drive the fake timers.
+ const TICK_MS = 1000;
+ const REFRESH_MS = 5000;
+
+ it("ngOnInit loads the current page and starts the 1s clock tick", () => {
+ const firstExec = makeExecution({ workflowId: 5 });
+ vi.mocked(service.getExecutionList).mockReturnValue(of([firstExec]));
+ vi.mocked(service.getTotalWorkflows).mockReturnValue(of(2));
+ const updateSpy = vi.spyOn(component, "updateTimeStatus");
+
+ component.ngOnInit();
+
+ // The initial fetch resolves synchronously (of(...)) and populates the
view.
+ expect(component.listOfExecutions).toEqual([firstExec]);
+ expect(component.totalWorkflows).toBe(2);
+ expect(component.isLoading).toBe(false);
+
+ updateSpy.mockClear();
+ vi.mocked(service.getExecutionList).mockClear();
+
+ // A clock tick recomputes elapsed time client-side without hitting the
service.
+ vi.advanceTimersByTime(TICK_MS);
+
+ expect(updateSpy).toHaveBeenCalled();
+ expect(service.getExecutionList).not.toHaveBeenCalled();
+ });
+
+ it("ngOnInit polls the current page every 5s and leaves the total
untouched when it did not change", () => {
+ vi.mocked(service.getExecutionList).mockReturnValue(of([makeExecution({
workflowId: 5 })]));
+ vi.mocked(service.getTotalWorkflows).mockReturnValue(of(2));
+
+ component.ngOnInit();
+ expect(component.totalWorkflows).toBe(2);
+
+ // Next poll returns fresh rows but the same total.
+ const polledExec = makeExecution({ workflowId: 9, executionStatus:
"RUNNING" });
+ vi.mocked(service.getExecutionList).mockClear();
+ vi.mocked(service.getExecutionList).mockReturnValue(of([polledExec]));
+ vi.mocked(service.getTotalWorkflows).mockReturnValue(of(2));
+
+ vi.advanceTimersByTime(REFRESH_MS);
+
+ expect(service.getExecutionList).toHaveBeenCalledTimes(1);
+ expect(component.listOfExecutions).toEqual([polledExec]);
+ // The total is unchanged, so applyCurrentPage must not reassign/churn
it.
+ expect(component.totalWorkflows).toBe(2);
+ });
Review Comment:
This test claims it verifies that `applyCurrentPage` “leaves the total
untouched” when the backend total doesn’t change, but
`expect(component.totalWorkflows).toBe(2)` will pass even if `totalWorkflows`
is reassigned to the same value. As written, it doesn’t exercise/lock in the
`if (result.total !== this.totalWorkflows)` branch behavior, so the test is
weaker than its name/comment suggests. Consider spying on a setter (via
`Object.defineProperty`) to assert the property is not reassigned during the
poll when `total` is unchanged.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
