This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-6819-234270b32857e70f61e0dcc4701c1053bf13095e in repository https://gitbox.apache.org/repos/asf/texera.git
commit 7a3a633aa48bc2b086fd8cdd985a55e807b19e71 Author: Xinyuan Lin <[email protected]> AuthorDate: Tue Jul 28 22:52:36 2026 -0700 test(frontend): extend ComputingUnitStatusService unit test coverage (#6819) ### What changes were proposed in this PR? Extends `computing-unit-status.service.spec.ts` with 8 tests (18 -> 26), covering `selectComputingUnit`'s cache-miss (refresh + wait-for-unit + websocket open), no-op guard (same wid+cuid), and reconnect-on-wid-change paths, the websocket open with `(wid, uid, cuid)`, and the status/metric stream branches. Coverage 82% -> ~100%. No existing tests modified. ### Any related issues, documentation, discussions? Closes #6815. ### How was this PR tested? `ng test --include='**/computing-unit-status.service.spec.ts'` -> 26/26 passing. `yarn format:ci` passes. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 4.8 [1M context]) --------- Signed-off-by: Xinyuan Lin <[email protected]> Co-authored-by: Copilot Autofix powered by AI <[email protected]> Co-authored-by: Meng Wang <[email protected]> --- .../computing-unit-status.service.spec.ts | 143 ++++++++++++++++++++- 1 file changed, 142 insertions(+), 1 deletion(-) diff --git a/frontend/src/app/common/service/computing-unit/computing-unit-status/computing-unit-status.service.spec.ts b/frontend/src/app/common/service/computing-unit/computing-unit-status/computing-unit-status.service.spec.ts index bae16d79ec..d1c18ab450 100644 --- a/frontend/src/app/common/service/computing-unit/computing-unit-status/computing-unit-status.service.spec.ts +++ b/frontend/src/app/common/service/computing-unit/computing-unit-status/computing-unit-status.service.spec.ts @@ -19,7 +19,7 @@ import { TestBed } from "@angular/core/testing"; import { HttpClientTestingModule } from "@angular/common/http/testing"; -import { of } from "rxjs"; +import { of, throwError } from "rxjs"; import { ComputingUnitStatusService } from "./computing-unit-status.service"; import { WorkflowComputingUnitManagingService } from "../workflow-computing-unit/workflow-computing-unit-managing.service"; import { WorkflowWebsocketService } from "../../../../workspace/service/workflow-websocket/workflow-websocket.service"; @@ -294,4 +294,145 @@ describe("ComputingUnitStatusService", () => { vi.useRealTimers(); } }); + + describe("selectComputingUnit() cache-miss and guard branches", () => { + it("refreshes then selects a unit that is not yet in the cache", () => { + const openSpy = vi.spyOn(websocketService, "openWebsocket").mockImplementation(() => {}); + const managing = TestBed.inject(WorkflowComputingUnitManagingService); + const unit9 = mockUnit(9); + // The cache starts empty, so cuid 9 is a miss: selection must trigger a + // refresh and wait for the unit to appear before opening the socket. + vi.spyOn(managing, "listComputingUnits").mockReturnValue(of([unit9])); + + service.selectComputingUnit(5, 9); + + const uid = TestBed.inject(UserService).getCurrentUser()?.uid; + expect(uid).not.toBeUndefined(); + expect(openSpy).toHaveBeenCalledWith(5, uid, 9); + expect(service.getSelectedComputingUnitValue()).toBe(unit9); + }); + + it("does not reopen the websocket when the identical unit is re-selected", () => { + const openSpy = vi.spyOn(websocketService, "openWebsocket").mockImplementation(() => {}); + (service as any).allComputingUnitsSubject.next([mockUnit(7)]); + + service.selectComputingUnit(5, 7); + // Same wid + cuid → shouldReconnect is false → the guarded block is skipped. + service.selectComputingUnit(5, 7); + + expect(openSpy).toHaveBeenCalledTimes(1); + }); + + it("does nothing when the workflow id is undefined", () => { + const openSpy = vi.spyOn(websocketService, "openWebsocket").mockImplementation(() => {}); + (service as any).allComputingUnitsSubject.next([mockUnit(7)]); + + // The unit is cached and shouldReconnect is true, so only the isDefined(wid) + // guard can stop the selection: no socket is opened and nothing is selected. + service.selectComputingUnit(undefined, 7); + + expect(openSpy).not.toHaveBeenCalled(); + expect(service.getSelectedComputingUnitValue()).toBeNull(); + expect((service as any).currentConnectedWid).toBeUndefined(); + expect((service as any).currentConnectedCuid).toBeUndefined(); + }); + + it("reconnects when the same unit is opened under a different workflow (wid change alone)", () => { + const openSpy = vi.spyOn(websocketService, "openWebsocket").mockImplementation(() => {}); + (service as any).allComputingUnitsSubject.next([mockUnit(7)]); + + service.selectComputingUnit(5, 7); + expect(openSpy).toHaveBeenCalledTimes(1); + + // Same cuid (7) but a different wid (6): the currentConnectedCuid check is + // false, so the currentConnectedWid !== wid operand must force the reconnect. + service.selectComputingUnit(6, 7); + expect(openSpy).toHaveBeenCalledTimes(2); + expect((service as any).currentConnectedWid).toBe(6); + }); + }); + + it("startRefreshInterval() tears down an existing refresh subscription before creating a new one", () => { + // The constructor already started one refresh subscription. + const previous = (service as any).refreshSubscription; + expect(previous).toBeTruthy(); + expect(previous.closed).toBe(false); + + (service as any).startRefreshInterval(); + + // The prior subscription is unsubscribed and replaced with a fresh one. + expect(previous.closed).toBe(true); + expect((service as any).refreshSubscription).not.toBe(previous); + expect((service as any).refreshSubscription.closed).toBe(false); + }); + + describe("terminateComputingUnit()", () => { + it("closes the socket and clears status when terminating the connected selection", () => { + const managing = TestBed.inject(WorkflowComputingUnitManagingService); + const workflowStatusService = TestBed.inject(WorkflowStatusService); + const closeSpy = vi.spyOn(websocketService, "closeWebsocket").mockImplementation(() => {}); + const clearSpy = vi.spyOn(workflowStatusService, "clearStatus").mockImplementation(() => {}); + vi.spyOn(websocketService, "isConnected", "get").mockReturnValue(true); + const termSpy = vi.spyOn(managing, "terminateComputingUnit").mockReturnValue(of({} as Response)); + const refreshSpy = vi.spyOn(service, "refreshComputingUnitList").mockImplementation(() => {}); + (service as any).selectedUnitSubject.next(mockUnit(7)); + + let result: boolean | undefined; + service.terminateComputingUnit(7).subscribe(r => (result = r)); + + expect(closeSpy).toHaveBeenCalled(); + expect(clearSpy).toHaveBeenCalled(); + expect(termSpy).toHaveBeenCalledWith(7); + // The tap() side effect requests a single list refresh. + expect(refreshSpy).toHaveBeenCalled(); + expect(result).toBe(true); + }); + + it("leaves the socket untouched when terminating a non-selected unit", () => { + const managing = TestBed.inject(WorkflowComputingUnitManagingService); + const closeSpy = vi.spyOn(websocketService, "closeWebsocket").mockImplementation(() => {}); + // isConnected is true, but isSelected short-circuits the guard to false. + vi.spyOn(websocketService, "isConnected", "get").mockReturnValue(true); + vi.spyOn(managing, "terminateComputingUnit").mockReturnValue(of({} as Response)); + vi.spyOn(service, "refreshComputingUnitList").mockImplementation(() => {}); + (service as any).selectedUnitSubject.next(mockUnit(8)); + + let result: boolean | undefined; + service.terminateComputingUnit(7).subscribe(r => (result = r)); + + expect(closeSpy).not.toHaveBeenCalled(); + expect(result).toBe(true); + }); + + it("skips socket teardown when the selected unit is not connected", () => { + const managing = TestBed.inject(WorkflowComputingUnitManagingService); + const closeSpy = vi.spyOn(websocketService, "closeWebsocket").mockImplementation(() => {}); + // isSelected is true, but the socket is down → the guarded teardown is skipped. + vi.spyOn(websocketService, "isConnected", "get").mockReturnValue(false); + vi.spyOn(managing, "terminateComputingUnit").mockReturnValue(of({} as Response)); + vi.spyOn(service, "refreshComputingUnitList").mockImplementation(() => {}); + (service as any).selectedUnitSubject.next(mockUnit(7)); + + let result: boolean | undefined; + service.terminateComputingUnit(7).subscribe(r => (result = r)); + + expect(closeSpy).not.toHaveBeenCalled(); + expect(result).toBe(true); + }); + + it("resolves to false when the termination request errors", () => { + const managing = TestBed.inject(WorkflowComputingUnitManagingService); + vi.spyOn(websocketService, "isConnected", "get").mockReturnValue(false); + vi.spyOn(managing, "terminateComputingUnit").mockReturnValue(throwError(() => new Error("boom"))); + const refreshSpy = vi.spyOn(service, "refreshComputingUnitList").mockImplementation(() => {}); + (service as any).selectedUnitSubject.next(mockUnit(7)); + + let result: boolean | undefined; + service.terminateComputingUnit(7).subscribe(r => (result = r)); + + // The error is swallowed by catchError, which emits false; no refresh runs. + expect(result).toBe(false); + expect(refreshSpy).not.toHaveBeenCalled(); + }); + }); });
