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]