mengw15 commented on code in PR #7911:
URL: https://github.com/apache/texera/pull/7911#discussion_r3842311270
##########
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 },
+ });
+ try {
+ return fn();
+ } finally {
+ Object.defineProperty(window, "location", { configurable: true,
value: original });
+ }
+ };
+
+ it("switches to the wss scheme when the page is served over https", ()
=> {
+ seedAgent("agent-1");
+
+ withLocation({ protocol: "https:", host: "texera.example.org" }, () =>
service.activateAgent("agent-1"));
+
+
expect(FakeWebSocket.latest().url).toBe("wss://texera.example.org/api/agents/agent-1/react");
+ });
+
+ it("logs a payload that is not valid JSON instead of throwing out of the
handler", () => {
+ const errSpy = vi.spyOn(console, "error").mockImplementation(() => {});
+ seedAgent("agent-1");
+ service.activateAgent("agent-1");
+ const ws = FakeWebSocket.latest();
+
+ expect(() => ws.onmessage!({ data: "<html>not json</html>"
})).not.toThrow();
+
+ expect(errSpy).toHaveBeenCalledWith("Failed to parse agent WebSocket
message:", expect.any(SyntaxError));
+ // The connection is left intact.
+ expect((service as
any).agentStateTracking.get("agent-1").websocket).toBe(ws);
+ });
+
+ it("logs socket transport errors", () => {
+ const errSpy = vi.spyOn(console, "error").mockImplementation(() => {});
+ seedAgent("agent-1");
+ service.activateAgent("agent-1");
+ const event = { type: "error" };
+
+ FakeWebSocket.latest().onerror!(event);
+
+ expect(errSpy).toHaveBeenCalledWith("Agent agent-1 WebSocket error:",
event);
+ });
+
+ it("ignores a close event from a socket that has already been replaced",
() => {
+ seedAgent("agent-1");
+ service.activateAgent("agent-1");
+ const stale = FakeWebSocket.latest();
+ const states: AgentState[] = [];
+ service.getAgentStateObservable("agent-1").subscribe(s =>
states.push(s));
Review Comment:
Each test gets a fresh `AgentService` from a fresh `TestBed`, so a later
test cannot emit into an earlier test's captured array. Probed: the injected
instance differs between two tests in one describe, and the first test's array
stays at length 1 while the second subscribes and emits on its own instance.
Same as the 47 pre-existing tests in this file. (Applies to the four identical
comments below.)
--
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]
