(texera) 01/01: test(frontend): extend ComputingUnitStatusService unit test coverage (#6819)

[gi...@apache.org]

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 <xi...@uci.edu>
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 <xi...@uci.edu>
    Co-authored-by: Copilot Autofix powered by AI <17...@users.noreply.github.com>
    Co-authored-by: Meng Wang <me...@uci.edu>
---
 .../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();
+    });
+  });
 });