Copilot commented on code in PR #7908:
URL: https://github.com/apache/texera/pull/7908#discussion_r3841930111
##########
frontend/src/app/workspace/service/agent/agent.service.spec.ts:
##########
@@ -965,4 +965,339 @@ describe("AgentService", () => {
expect(target).toEqual({ agentId: "agent-1", messageId: "m1", stepId: 4
});
});
});
+
+ //
---------------------------------------------------------------------------
+ // State accessors: the untracked side of each (an id with no tracking entry)
+ //
---------------------------------------------------------------------------
+ describe("state accessors without tracking", () => {
+ it("getAgentState returns the tracked value, or UNAVAILABLE for an unknown
id", () => {
+ // Subscribing to the observable getter is what creates the tracking
entry.
+ service.getAgentStateObservable("agent-1").subscribe();
+ (service as
any).agentStateTracking.get("agent-1").stateSubject.next(AgentState.GENERATING);
+
+ let tracked: AgentState | undefined;
+ service.getAgentState("agent-1").subscribe(s => (tracked = s));
+ expect(tracked).toBe(AgentState.GENERATING);
+
+ let unknown: AgentState | undefined;
+ service.getAgentState("ghost").subscribe(s => (unknown = s));
+ expect(unknown).toBe(AgentState.UNAVAILABLE);
+ });
+
+ it("isAgentConnected is false for an unavailable/unknown agent and true
otherwise", () => {
+ let unknown: boolean | undefined;
+ service.isAgentConnected("ghost").subscribe(v => (unknown = v));
+ expect(unknown).toBe(false);
+
+ service.getAgentStateObservable("agent-1").subscribe();
+ (service as
any).agentStateTracking.get("agent-1").stateSubject.next(AgentState.AVAILABLE);
+ let connected: boolean | undefined;
+ service.isAgentConnected("agent-1").subscribe(v => (connected = v));
+ expect(connected).toBe(true);
+ });
+
+ it("getHeadId / getHeadIdObservable return the tracked HEAD, or null when
untracked", () => {
+ const emitted: (string | null)[] = [];
+ service.getHeadIdObservable("agent-1").subscribe(v => emitted.push(v));
+ (service as
any).agentStateTracking.get("agent-1").headIdSubject.next("m1-2");
+
+ expect(emitted).toEqual([null, "m1-2"]);
+ expect(service.getHeadId("agent-1")).toBe("m1-2");
+ expect(service.getHeadId("ghost")).toBeNull();
+ });
+
+ it("getVisibleSteps returns the tracked snapshot, or [] when untracked",
() => {
+ service.getReActStepsObservable("agent-1").subscribe();
+ const step = { messageId: "m1", stepId: 0 } as unknown as ReActStep;
+ (service as
any).agentStateTracking.get("agent-1").reActStepsSubject.next([step]);
+
+ expect(service.getVisibleSteps("agent-1")).toEqual([step]);
+ expect(service.getVisibleSteps("ghost")).toEqual([]);
+ });
+
+ it("getWorkflowObservable emits the tracked workflow, or null when
untracked", () => {
+ service.getAgentStateObservable("agent-1").subscribe();
+ (service as
any).agentStateTracking.get("agent-1").workflowSubject.next(stubWorkflow);
+ let tracked: Workflow | null | undefined;
+ service.getWorkflowObservable("agent-1").subscribe(w => (tracked = w));
+ expect(tracked).toBe(stubWorkflow);
+
+ let untracked: Workflow | null | undefined = stubWorkflow;
+ service.getWorkflowObservable("ghost").subscribe(w => (untracked = w));
+ expect(untracked).toBeNull();
+ });
+
+ it("getAgentWorkflowId walks the delegate chain: missing agent, no
delegate, full", () => {
+ expect(service.getAgentWorkflowId("ghost")).toBeUndefined();
+
+ (service as any).agents.set("no-delegate", { id: "no-delegate", name:
"x" });
+ expect(service.getAgentWorkflowId("no-delegate")).toBeUndefined();
+
+ seedAgent("with-wf", 42);
+ expect(service.getAgentWorkflowId("with-wf")).toBe(42);
+ });
+
+ it("getAgentCount reports the number of registered agents", () => {
+ let empty: number | undefined;
+ service.getAgentCount().subscribe(c => (empty = c));
+ expect(empty).toBe(0);
+
+ seedAgent("agent-1");
+ seedAgent("agent-2");
+ let count: number | undefined;
+ service.getAgentCount().subscribe(c => (count = c));
+ expect(count).toBe(2);
+ });
+ });
+
+ describe("setHoveredMessage guards", () => {
+ it("emits empty arrays for a non-null step that carries no operator
access", () => {
+ let latest:
+ | { viewedOperatorIds: string[]; addedOperatorIds: string[];
modifiedOperatorIds: string[] }
+ | undefined;
+ service.getHoveredMessageOperatorsObservable("agent-1").subscribe(v =>
(latest = v));
+
+ // A real step with no operatorAccess map takes the else-branch,
distinct from
+ // the null-step case already covered above.
+ service.setHoveredMessage("agent-1", { messageId: "m1" } as unknown as
ReActStep);
+
+ expect(latest).toEqual({ viewedOperatorIds: [], addedOperatorIds: [],
modifiedOperatorIds: [] });
+ });
+
+ it("is a no-op when the agent has no tracking", () => {
+ // No tracking entry exists for this id, so the method must return
without throwing.
+ expect(() => service.setHoveredMessage("ghost", { messageId: "m1" } as
unknown as ReActStep)).not.toThrow();
+ });
+ });
+
+ describe("getReActStepsByOperatorAccess without operator access", () => {
+ it("skips steps whose operatorAccess is absent", () => {
+ let result: { viewedBy: ReActStep[]; modifiedBy: ReActStep[] } |
undefined;
+ service.getReActStepsByOperatorAccess("agent-1", "op-7").subscribe(r =>
(result = r));
+
+ httpMock
+ .expectOne(r => r.method === "GET" && r.url ===
"/api/agents/agent-1/react-steps")
+ .flush({
+ state: "AVAILABLE",
+ steps: [{ messageId: "no-access", timestamp:
"2026-06-11T00:00:00.000Z" }],
+ });
+
+ expect(result).toEqual({ viewedBy: [], modifiedBy: [] });
+ });
+ });
+
+ describe("mapStateToAgentState", () => {
+ it("maps STOPPING, UNAVAILABLE and unrecognised backend states", () => {
+ let mapped: AgentInfo[] | undefined;
+ service.getAllAgents().subscribe(a => (mapped = a));
+
+ httpMock
+ .expectOne(r => r.method === "GET" && r.url === "/api/agents")
+ .flush({
+ agents: [
+ { ...apiAgent, id: "s", state: "STOPPING" },
+ { ...apiAgent, id: "u", state: "UNAVAILABLE" },
+ { ...apiAgent, id: "z", state: "NOT_A_REAL_STATE" },
+ ],
+ });
+
+ const byId = new Map(mapped!.map(a => [a.id, a.state]));
+ expect(byId.get("s")).toBe(AgentState.STOPPING);
+ expect(byId.get("u")).toBe(AgentState.UNAVAILABLE);
+ // Anything unrecognised falls through the default arm to UNAVAILABLE.
+ expect(byId.get("z")).toBe(AgentState.UNAVAILABLE);
+ });
+ });
+
+ //
---------------------------------------------------------------------------
+ // HTTP failure paths and their fallback chains
+ //
---------------------------------------------------------------------------
+ describe("createAgent failure fallback chain", () => {
+ it("prefers the nested error.error message", () => {
+ let message: string | undefined;
+ service.createAgent("gpt-5-mini").subscribe({ error: (e: unknown) =>
(message = (e as Error).message) });
+ httpMock
+ .expectOne(r => r.method === "POST" && r.url === "/api/agents")
+ .flush({ error: "nested boom" }, { status: 400, statusText: "Bad
Request" });
+
+ expect(message).toBe("nested boom");
+ expect(notification.error).toHaveBeenCalledWith("nested boom");
+ });
+
+ it("falls back to err.message when there is no nested error", () => {
+ let message: string | undefined;
+ service.createAgent("gpt-5-mini").subscribe({ error: (e: unknown) =>
(message = (e as Error).message) });
+ // A body with no `.error` field leaves err.error?.error undefined, so
the
+ // HttpErrorResponse's own message is used.
+ httpMock
+ .expectOne(r => r.method === "POST" && r.url === "/api/agents")
+ .flush({}, { status: 500, statusText: "Server Error" });
+
+ expect(message).toContain("Http failure response");
+ expect(notification.error).toHaveBeenCalledWith(message);
+ });
+
+ it("falls back to the default label when the error carries neither field",
() => {
+ // An HttpErrorResponse always has a message, so the default literal is
only
+ // reachable for a non-HTTP error; a bare object exercises that last arm.
+ vi.spyOn((service as any).http, "post").mockReturnValue(throwError(() =>
({})));
+
+ let message: string | undefined;
+ service.createAgent("gpt-5-mini").subscribe({ error: (e: unknown) =>
(message = (e as Error).message) });
+
+ expect(message).toBe("Failed to create agent");
+ expect(notification.error).toHaveBeenCalledWith("Failed to create
agent");
+ });
Review Comment:
The `vi.spyOn((service as any).http, "post")` mock is never restored. Vitest
does not enable `restoreMocks` here (see frontend/vitest.config.ts), so this
spy can leak across specs (especially if TestBed teardown is disabled) and
cause later tests that rely on real `HttpTestingController` POSTs to fail
unexpectedly. Restore the spy in this test (or add a global
`vi.restoreAllMocks()` in `afterEach`).
This issue also appears on line 1164 of the same file.
--
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]
