mengw15 commented on code in PR #7911:
URL: https://github.com/apache/texera/pull/7911#discussion_r3842310401


##########
frontend/src/app/workspace/service/agent/agent.service.spec.ts:
##########
@@ -493,6 +494,135 @@ describe("AgentService", () => {
         expect(service.isAgentActivelyConnected("agent-1")).toBe(true);
         expect(service.getActivelyConnectedAgentIds()).toEqual(["agent-1"]);
       });
+
+      it("keeps an already-open socket on re-activation and replaces a 
non-open one", () => {
+        seedAgent("agent-1");
+        service.activateAgent("agent-1");
+        const tracking = (service as any).agentStateTracking.get("agent-1");
+        const ws = FakeWebSocket.latest();
+
+        // deactivateAgent always drops the socket, so the "inactive but still
+        // holding a socket" shape that activateAgent's readyState guard 
defends
+        // against is set up directly.
+        ws.readyState = FakeWebSocket.OPEN;
+        tracking.isActive = false;
+        expect(service.activateAgent("agent-1")).toBe(true);
+        expect(FakeWebSocket.instances.length).toBe(1);
+        expect(tracking.websocket).toBe(ws);
+
+        // The same shape with a socket that is no longer OPEN reconnects 
instead.
+        ws.readyState = FakeWebSocket.CLOSED;
+        tracking.isActive = false;
+        expect(service.activateAgent("agent-1")).toBe(true);
+        expect(FakeWebSocket.instances.length).toBe(2);
+        expect(tracking.websocket).toBe(FakeWebSocket.latest());
+      });
+
+      it("ignores deactivation of an unknown agent and of an already-inactive 
one", () => {
+        service.deactivateAgent("nope");
+        expect((service as any).agentStateTracking.has("nope")).toBe(false);
+
+        seedAgent("agent-1");
+        service.activateAgent("agent-1");
+        const ws = FakeWebSocket.latest();
+        service.deactivateAgent("agent-1");
+        expect(ws.close).toHaveBeenCalledTimes(1);
+
+        const tracking = (service as any).agentStateTracking.get("agent-1");
+        const stopPolling = tracking.stopPolling$;
+        service.deactivateAgent("agent-1");
+
+        // The second call returns at the isActive guard: nothing is torn down 
twice.
+        expect(ws.close).toHaveBeenCalledTimes(1);
+        expect(tracking.stopPolling$).toBe(stopPolling);
+      });
+
+      it("deactivates cleanly when the socket was already dropped by a close 
event", () => {
+        seedAgent("agent-1");
+        service.activateAgent("agent-1");
+        const ws = FakeWebSocket.latest();
+        const tracking = (service as any).agentStateTracking.get("agent-1");
+
+        // A normal close clears tracking.websocket but leaves the agent 
active.
+        ws.onclose!({ code: 1000 });
+        expect(tracking.websocket).toBeUndefined();
+        expect(tracking.isActive).toBe(true);
+
+        const stopPolling = tracking.stopPolling$;
+        service.deactivateAgent("agent-1");
+
+        expect(ws.close).not.toHaveBeenCalled();
+        expect(tracking.isActive).toBe(false);
+        expect(tracking.stopPolling$).not.toBe(stopPolling);
+      });
+    });
+
+    describe("connection setup", () => {
+      /** Swap window.location for the duration of fn; jsdom's own is not 
writable. */
+      const withLocation = <T>(overrides: Partial<Location>, fn: () => T): T 
=> {
+        const original = window.location;
+        Object.defineProperty(window, "location", {
+          configurable: true,
+          value: { ...original, ...overrides },

Review Comment:
   `Location`'s attributes are `[LegacyUnforgeable]`: non-configurable, but 
still enumerable, so the spread does copy them. Probed on this jsdom build — 
`Object.keys(window.location)` returns all 13 members, and 
`{...window.location}.href` is `"http://localhost:3000/"`. Keeping the helper 
as is; it mirrors `virtual-environment.service.spec.ts`.



-- 
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]

Reply via email to