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-5196-c435aa73434f64fbffe82d5242ded17b56952aab in repository https://gitbox.apache.org/repos/asf/texera.git
commit 2153c17879ca4504da09d4d7dc9648d17f364aa0 Author: Yicong Huang <[email protected]> AuthorDate: Mon May 25 15:35:02 2026 -0700 test(frontend): cover joint-ui.service surface and remove dead code (#5196) ### What changes were proposed in this PR? `joint-ui.service.spec.ts` had ~60 commented test lines plus an `it.todo` placeholder; the commented tests called methods that have since been removed from `JointUIService`, so they're unrevivable. Drop them and add real tests for the public surface the spec was leaving unverified — static helpers, the joint-paper-bound state/color/visibility setters, the port-iteration loops, `getJointOperatorElement`, and `changeOperatorStatistics`. Writing those tests made four pieces of stale `joint-ui.service.ts` itself obvious: a static tooltip-style helper with zero callers (its paired element was removed years ago), two dead `originalName` chains inside `changeOperatorStatistics` (computed, never read), and a `if (!element) return` guard at the very end of `unfoldOperatorDetails` (no-op — nothing follows it). Removed. ### Any related issues, documentation, discussions? Closes #5195. Same shape as #5194. ### How was this PR tested? `yarn ng test --watch=false --include=…` runs 56 tests, all green. `yarn lint` and `yarn format:ci` both clean. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Opus 4.7 --- .../service/joint-ui/joint-ui.service.spec.ts | 850 +++++++++++++-------- .../workspace/service/joint-ui/joint-ui.service.ts | 64 +- 2 files changed, 535 insertions(+), 379 deletions(-) diff --git a/frontend/src/app/workspace/service/joint-ui/joint-ui.service.spec.ts b/frontend/src/app/workspace/service/joint-ui/joint-ui.service.spec.ts index ccefd937de..87b5acd924 100644 --- a/frontend/src/app/workspace/service/joint-ui/joint-ui.service.spec.ts +++ b/frontend/src/app/workspace/service/joint-ui/joint-ui.service.spec.ts @@ -17,327 +17,26 @@ * under the License. */ -// import { mockResultPredicate, mockPoint } from './../workflow-graph/model/mock-workflow-data'; -// import { TestBed, inject } from '@angular/core/testing'; -// import * as joint from 'jointjs'; - -// import { JointUIService, deleteButtonPath, sourceOperatorHandle, targetOperatorHandle } from './joint-ui.service'; -// import { OperatorMetadataService } from '../operator-metadata/operator-metadata.service'; -// import { StubOperatorMetadataService } from '../operator-metadata/stub-operator-metadata.service'; -// import { mockScanPredicate, mockSentimentPredicate } from '../workflow-graph/model/mock-workflow-data'; -// import { mockScanStatistic1, mockScanStatistic2 } from '../workflow-status/mock-workflow-status'; - -// describe('JointUIService', () => { -// let service: JointUIService; - -// beforeEach(() => { -// TestBed.configureTestingModule({ -// providers: [ -// JointUIService, -// { provide: OperatorMetadataService, useClass: StubOperatorMetadataService }, -// ], -// }); -// service = TestBed.get(JointUIService); -// }); - -// it('should be created', inject([JointUIService], (injectedService: JointUIService) => { -// expect(injectedService).toBeTruthy(); -// })); - -// /** -// * Check if the getJointOperatorElement() can successfully creates a JointJS Element -// */ -// it('should create an JointJS Element successfully when the function is called', () => { -// const result = service.getJointOperatorElement( -// mockScanPredicate, mockPoint); -// expect(result).toBeTruthy(); -// }); - -// /** -// * Check if the error in getJointOperatorElement() is correctly thrown -// */ -// it('should throw an error with an non existing operator', () => { -// expect(() => { -// service.getJointOperatorElement( -// { -// operatorID: 'nonexistOperator', -// operatorType: 'nonexistOperatorType', -// operatorProperties: {}, -// inputPorts: [], -// outputPorts: [], -// showAdvanced: true -// }, -// mockPoint -// ); -// }).toThrowError(new RegExp(`doesn't exist`)); -// }); - -// /** -// * Check if the getJointTooltipElement() can successfully creates a JointJS Element -// */ -// it('should create an JointJS Element successfully when the function is called', () => { -// const result = service.getJointOperatorStatusTooltipElement( -// mockScanPredicate, mockPoint); -// expect(result).toBeTruthy(); -// }); - -// /** -// * Check if the error in getJointTooltipElement() is correctly thrown -// */ -// it('should throw an error with an non existing operator', () => { -// expect(() => { -// service.getJointOperatorStatusTooltipElement( -// { -// operatorID: 'nonexistOperator', -// operatorType: 'nonexistOperatorType', -// operatorProperties: {}, -// inputPorts: [], -// outputPorts: [], -// showAdvanced: true -// }, -// mockPoint -// ); -// }).toThrowError(new RegExp(`doesn't exist`)); -// }); - -// /** -// * Check if showTooltip/hideTooltip works properly -// */ -// it('should reveal/hide tooltip and its content when showToolTip/hideTooltip is called', () => { -// // creating a JointJS graph with one operator and its tooltip -// const jointGraph = new joint.dia.Graph(); -// const jointPaperOptions: joint.dia.Paper.Options = {model: jointGraph}; -// const paper = new joint.dia.Paper(jointPaperOptions); - -// jointGraph.addCell([ -// service.getJointOperatorElement( -// mockScanPredicate, -// mockPoint -// ), -// service.getJointOperatorStatusTooltipElement( -// mockScanPredicate, -// mockPoint -// ) -// ]); -// // tooltip should not be shown when operator is just created -// // disply attr should be none -// const tooltipId = JointUIService.getOperatorStatusTooltipElementID(mockScanPredicate.operatorID); -// const graph_tooltip1 = jointGraph.getCell(tooltipId); -// expect(graph_tooltip1.attr('polygon')['display']).toEqual('none'); -// expect(graph_tooltip1.attr('#operatorCount')['display']).toEqual('none'); -// expect(graph_tooltip1.attr('#operatorSpeed')['display']).toEqual('none'); -// // showTooltip removes display == none attr to show tooltip -// service.showOperatorStatusToolTip(paper, tooltipId); -// expect(graph_tooltip1.attr('polygon')['display']).toBeUndefined(); -// expect(graph_tooltip1.attr('#operatorCount')['display']).toBeUndefined(); -// expect(graph_tooltip1.attr('#operatorSpeed')['display']).toBeUndefined(); -// // hideTooltip adds display == none attr to hide tooltip -// service.hideOperatorStatusToolTip(paper, tooltipId); -// expect(graph_tooltip1.attr('polygon')['display']).toEqual('none'); -// expect(graph_tooltip1.attr('#operatorCount')['display']).toEqual('none'); -// expect(graph_tooltip1.attr('#operatorSpeed')['display']).toEqual('none'); -// }); - -// /** -// * check if tooltip content can be updated properly -// */ -// it('should update the content in the tooltip when changeOperatorTooltipInfo is called', () => { -// // creating a JointJS graph with one operator and its tooltip -// const jointGraph = new joint.dia.Graph(); -// const jointPaperOptions: joint.dia.Paper.Options = {model: jointGraph}; -// const paper = new joint.dia.Paper(jointPaperOptions); - -// jointGraph.addCell([ -// service.getJointOperatorElement( -// mockScanPredicate, -// mockPoint -// ), -// service.getJointOperatorStatusTooltipElement( -// mockScanPredicate, -// mockPoint -// ) -// ]); -// const tooltipId = JointUIService.getOperatorStatusTooltipElementID(mockScanPredicate.operatorID); -// const graph_tooltip = jointGraph.getCell(tooltipId); -// // tooltip should not contain any content when created -// expect(graph_tooltip.attr('#operatorCount')['text']).toBeUndefined(); -// expect(graph_tooltip.attr('#operatorSpeed')['text']).toBeUndefined(); -// // updating it with mock statistics -// service.changeOperatorStatusTooltipInfo(paper, tooltipId, mockScanStatistic1); -// expect(graph_tooltip.attr('#operatorCount')['text']).toEqual('Output:' + mockScanStatistic1.outputCount + ' tuples'); -// expect(graph_tooltip.attr('#operatorSpeed')['text']).toEqual('Speed:' + mockScanStatistic1.speed + ' tuples/s'); -// // updating it with another mock statistics -// service.changeOperatorStatusTooltipInfo(paper, tooltipId, mockScanStatistic2); -// expect(graph_tooltip.attr('#operatorCount')['text']).toEqual('Output:' + mockScanStatistic2.outputCount + ' tuples'); -// expect(graph_tooltip.attr('#operatorSpeed')['text']).toEqual('Speed:' + mockScanStatistic2.speed + ' tuples/s'); -// }); - -// it('should change the operator state name and color when changeOperatorStates is called', () => { -// // creating a JointJS graph with one operator and its tooltip -// const jointGraph = new joint.dia.Graph(); -// const jointPaperOptions: joint.dia.Paper.Options = {model: jointGraph}; -// const paper = new joint.dia.Paper(jointPaperOptions); - -// jointGraph.addCell( -// service.getJointOperatorElement( -// mockScanPredicate, -// mockPoint -// )); - -// // operator state name and color should be changed correctly -// const graph_operator = jointGraph.getCell(mockScanPredicate.operatorID); -// expect(graph_operator.attr('#operatorStates')['text']).toEqual('Ready'); -// expect(graph_operator.attr('#operatorStates')['fill']).toEqual('green'); -// service.changeOperatorStates(paper, mockScanPredicate.operatorID, OperatorStates.Initializing); -// expect(graph_operator.attr('#operatorStates')['text']).toEqual('Initializing'); -// expect(graph_operator.attr('#operatorStates')['fill']).toEqual('orange'); -// service.changeOperatorStates(paper, mockScanPredicate.operatorID, OperatorStates.Running); -// expect(graph_operator.attr('#operatorStates')['text']).toEqual('Running'); -// expect(graph_operator.attr('#operatorStates')['fill']).toEqual('orange'); -// service.changeOperatorStates(paper, mockScanPredicate.operatorID, OperatorStates.Pausing); -// expect(graph_operator.attr('#operatorStates')['text']).toEqual('Pausing'); -// expect(graph_operator.attr('#operatorStates')['fill']).toEqual('red'); -// service.changeOperatorStates(paper, mockScanPredicate.operatorID, OperatorStates.Paused); -// expect(graph_operator.attr('#operatorStates')['text']).toEqual('Paused'); -// expect(graph_operator.attr('#operatorStates')['fill']).toEqual('red'); -// service.changeOperatorStates(paper, mockScanPredicate.operatorID, OperatorStates.Completed); -// expect(graph_operator.attr('#operatorStates')['text']).toEqual('Completed'); -// expect(graph_operator.attr('#operatorStates')['fill']).toEqual('green'); -// }); - -// /** -// * Check if the number of inPorts and outPorts created by getJointOperatorElement() -// * matches the port number specified by the operator metadata -// */ -// it('should create correct number of inPorts and outPorts based on operator metadata', () => { -// const element1 = service.getJointOperatorElement(mockScanPredicate, mockPoint); -// const element2 = service.getJointOperatorElement(mockSentimentPredicate, mockPoint); -// const element3 = service.getJointOperatorElement(mockResultPredicate, mockPoint); - -// const inPortCount1 = element1.getPorts().filter(port => port.group === 'in').length; -// const outPortCount1 = element1.getPorts().filter(port => port.group === 'out').length; -// const inPortCount2 = element2.getPorts().filter(port => port.group === 'in').length; -// const outPortCount2 = element2.getPorts().filter(port => port.group === 'out').length; -// const inPortCount3 = element3.getPorts().filter(port => port.group === 'in').length; -// const outPortCount3 = element3.getPorts().filter(port => port.group === 'out').length; - -// expect(inPortCount1).toEqual(0); -// expect(outPortCount1).toEqual(1); -// expect(inPortCount2).toEqual(1); -// expect(outPortCount2).toEqual(1); -// expect(inPortCount3).toEqual(1); -// expect(outPortCount3).toEqual(0); - -// }); - -// /** -// * Check if the custom attributes / svgs are correctly used by the JointJS graph -// */ -// it('should apply the custom SVG styling to the JointJS element', () => { - -// const graph = new joint.dia.Graph(); -// // operator and its tooltip element should be added together -// graph.addCell([ -// service.getJointOperatorElement( -// mockScanPredicate, -// mockPoint -// ), -// service.getJointOperatorStatusTooltipElement( -// mockScanPredicate, -// mockPoint -// ) -// ]); -// graph.addCell([ -// service.getJointOperatorElement( -// mockResultPredicate, -// { x: 500, y: 100 } -// ), -// service.getJointOperatorStatusTooltipElement( -// mockResultPredicate, -// { x: 500, y: 100 } -// ) -// ]); - -// const link = JointUIService.getJointLinkCell({ -// linkID: 'link-1', -// source: { operatorID: 'operator1', portID: 'out0' }, -// target: { operatorID: 'operator2', portID: 'in0' }, -// }); - -// graph.addCell(link); - -// const graph_operator1 = graph.getCell(mockScanPredicate.operatorID); -// const graph_operator2 = graph.getCell(mockResultPredicate.operatorID); -// const graph_link = graph.getLinks()[0]; -// const graph_tooltip1 = graph.getCell(JointUIService.getOperatorStatusTooltipElementID(mockScanPredicate.operatorID)); - -// // testing getCustomTooltipStyleAttrs() -// // style: {'pointer-events': 'none'} makes tooltip unselectable thus not draggable -// expect(graph_tooltip1.attr('polygon')).toEqual({ -// fill: '#FFFFFF', 'follow-scale': true, stroke: 'purple', 'stroke-width': '2', -// rx: '5px', ry: '5px', refPoints: '0,30 150,30 150,120 85,120 75,150 65,120 0,120', -// display: 'none', style: {'pointer-events': 'none'} -// }); -// expect(graph_tooltip1.attr('#operatorCount')).toEqual({ -// fill: '#595959', 'font-size': '12px', ref: 'polygon', -// 'y-alignment': 'middle', -// 'x-alignment': 'left', -// 'ref-x': .05, 'ref-y': .2, -// display: 'none', style: {'pointer-events': 'none'} -// }); -// expect(graph_tooltip1.attr('#operatorSpeed')).toEqual({ -// fill: '#595959', -// ref: 'polygon', -// 'x-alignment': 'left', -// 'font-size': '12px', -// 'ref-x': .05, 'ref-y': .5, -// display: 'none', style: {'pointer-events': 'none'} -// }); - -// // testing getCustomOperatorStyleAttrs() -// expect(graph_operator1.attr('#operatorStates')).toEqual({ -// text: 'Ready' , fill: 'green', 'font-size': '14px', 'visible' : false, -// 'ref-x': 0.5, 'ref-y': -10, ref: 'rect', 'y-alignment': 'middle', 'x-alignment': 'middle' -// }); -// expect(graph_operator1.attr('rect')).toEqual( -// { fill: '#FFFFFF', 'follow-scale': true, stroke: 'red', 'stroke-width': '2', -// rx: '5px', ry: '5px' } -// ); -// expect(graph_operator2.attr('rect')).toEqual( -// { fill: '#FFFFFF', 'follow-scale': true, stroke: 'red', 'stroke-width': '2', -// rx: '5px', ry: '5px' } -// ); -// expect(graph_operator1.attr('.delete-button')).toEqual( -// { -// x: 60, y: -20, cursor: 'pointer', -// fill: '#D8656A', event: 'element:delete' -// } -// ); -// expect(graph_operator2.attr('.delete-button')).toEqual( -// { -// x: 60, y: -20, cursor: 'pointer', -// fill: '#D8656A', event: 'element:delete' -// } -// ); - -// // testing getDefaultLinkElement() -// expect(graph_link.attr('.marker-source/d')).toEqual(sourceOperatorHandle); -// expect(graph_link.attr('.marker-target/d')).toEqual(targetOperatorHandle); -// expect(graph_link.attr('.tool-remove path/d')).toEqual(deleteButtonPath); -// }); -// }); - import { of } from "rxjs"; import * as joint from "jointjs"; -import { JointUIService, operatorNameClass } from "./joint-ui.service"; -import { OperatorPredicate } from "../../types/workflow-common.interface"; +import { JointUIService, operatorNameClass, operatorStateClass, operatorPortMetricsClass } from "./joint-ui.service"; +import { CommentBox, OperatorPredicate } from "../../types/workflow-common.interface"; +import { OperatorState } from "../../types/execute-workflow.interface"; +import { Coeditor } from "../../../common/type/user"; + +// Minimal mock of OperatorMetadataService — the constructor subscribes to +// getOperatorMetadata() but the schemas list isn't needed for the methods +// covered here. Tests that exercise `getJointOperatorElement` build their +// own metadata stub with real schemas inline. +const emptyMetadataStub = { + getOperatorMetadata: () => + of({ + operators: [], + groups: [], + }), +}; describe("JointUIService", () => { - // Pre-existing spec body is commented out. Placeholder keeps Vitest's - // discovery happy; rewriting the real tests against the new test - // runner is tracked in #4861. - it.todo("add unit tests for JointUIService"); - describe("truncateOperatorDisplayName", () => { // Deterministic measurer: 10px per character. With the 200-px budget, // 20 chars fit exactly; longer strings get truncated to a prefix plus "…". @@ -511,4 +210,521 @@ describe("JointUIService", () => { } }); }); + + // --------------------------------------------------------------------------- + // Static helpers — pure functions, easiest to test directly. + // --------------------------------------------------------------------------- + + describe("getOperatorFillColor (static)", () => { + it("returns the disabled fill for an isDisabled=true operator", () => { + expect(JointUIService.getOperatorFillColor({ isDisabled: true } as OperatorPredicate)).toBe("#E0E0E0"); + }); + it("returns the default white fill for an enabled operator", () => { + expect(JointUIService.getOperatorFillColor({} as OperatorPredicate)).toBe("#FFFFFF"); + expect(JointUIService.getOperatorFillColor({ isDisabled: false } as OperatorPredicate)).toBe("#FFFFFF"); + }); + }); + + describe("getOperatorCacheDisplayText (static)", () => { + it("returns empty string when cacheStatus is undefined", () => { + expect(JointUIService.getOperatorCacheDisplayText({ markedForReuse: true } as OperatorPredicate)).toBe(""); + }); + it("returns empty string when the operator is not marked for reuse", () => { + expect( + JointUIService.getOperatorCacheDisplayText({ markedForReuse: false } as OperatorPredicate, "cache valid") + ).toBe(""); + }); + it("returns the cache status text when both are set", () => { + expect( + JointUIService.getOperatorCacheDisplayText({ markedForReuse: true } as OperatorPredicate, "cache valid") + ).toBe("cache valid"); + }); + }); + + describe("getOperatorCacheIcon (static)", () => { + it("returns empty when the operator is not marked for reuse", () => { + expect(JointUIService.getOperatorCacheIcon({ markedForReuse: false } as OperatorPredicate, "cache valid")).toBe( + "" + ); + }); + it("returns the valid-cache icon when cacheStatus is 'cache valid'", () => { + expect(JointUIService.getOperatorCacheIcon({ markedForReuse: true } as OperatorPredicate, "cache valid")).toBe( + "assets/svg/operator-reuse-cache-valid.svg" + ); + }); + it("returns the invalid-cache icon for any other status (including undefined)", () => { + expect(JointUIService.getOperatorCacheIcon({ markedForReuse: true } as OperatorPredicate)).toBe( + "assets/svg/operator-reuse-cache-invalid.svg" + ); + expect(JointUIService.getOperatorCacheIcon({ markedForReuse: true } as OperatorPredicate, "cache invalid")).toBe( + "assets/svg/operator-reuse-cache-invalid.svg" + ); + }); + }); + + describe("getOperatorViewResultIcon (static)", () => { + it("returns the view-result asset when viewResult=true", () => { + expect(JointUIService.getOperatorViewResultIcon({ viewResult: true } as OperatorPredicate)).toBe( + "assets/svg/operator-view-result.svg" + ); + }); + it("returns empty otherwise", () => { + expect(JointUIService.getOperatorViewResultIcon({} as OperatorPredicate)).toBe(""); + expect(JointUIService.getOperatorViewResultIcon({ viewResult: false } as OperatorPredicate)).toBe(""); + }); + }); + + describe("getJointLinkCell (static)", () => { + it("builds a joint link cell carrying source/target/id from the OperatorLink", () => { + const link = JointUIService.getJointLinkCell({ + linkID: "link-1", + source: { operatorID: "op-A", portID: "out-0" }, + target: { operatorID: "op-B", portID: "in-0" }, + }); + expect(link.id).toBe("link-1"); + expect(link.get("source")).toEqual({ id: "op-A", port: "out-0" }); + expect(link.get("target")).toEqual({ id: "op-B", port: "in-0" }); + // z=0 keeps links rendered under operator elements (z=1 in + // getJointOperatorElement). + expect(link.get("z")).toBe(0); + }); + }); + + describe("getJointUserPointerName (static)", () => { + it("prefixes the coeditor clientId with 'pointer_'", () => { + expect(JointUIService.getJointUserPointerName({ clientId: "abc123" } as Coeditor)).toBe("pointer_abc123"); + }); + }); + + describe("getJointUserPointerCell (static)", () => { + it("builds a circle cell whose id matches getJointUserPointerName", () => { + const coeditor = { clientId: "42", name: "Ada", color: "#ff0000" } as Coeditor; + const cell = JointUIService.getJointUserPointerCell(coeditor, { x: 10, y: 20 }, "#abcdef"); + expect(cell.id).toBe(JointUIService.getJointUserPointerName(coeditor)); + // attr('body/fill') reflects the explicit color argument. + expect(cell.attr("body/fill")).toBe("#abcdef"); + expect(cell.attr("body/stroke")).toBe("#abcdef"); + }); + }); + + // --------------------------------------------------------------------------- + // Instance methods that operate on a joint Paper. Each test stubs the paper's + // getModelById to return a model with an `attr` spy; assertions look at what + // the SUT wrote on that model. + // --------------------------------------------------------------------------- + + function makePaperWithModel() { + const attrSpy = vi.fn(); + const portPropSpy = vi.fn(); + const getPortsSpy = vi.fn(() => [] as { id?: string; group?: string }[]); + const model = { attr: attrSpy, getPorts: getPortsSpy, portProp: portPropSpy }; + const paper = { getModelById: vi.fn(() => model) } as unknown as joint.dia.Paper; + return { paper, attrSpy, portPropSpy, getPortsSpy, model }; + } + + describe("changeOperatorColor", () => { + it("paints the body stroke neutral for a valid operator", () => { + const { paper, attrSpy } = makePaperWithModel(); + const service = new JointUIService(emptyMetadataStub as never); + service.changeOperatorColor(paper, "op-1", true); + expect(attrSpy).toHaveBeenCalledWith("rect.body/stroke", "#CFCFCF"); + }); + it("paints the body stroke red for an invalid operator", () => { + const { paper, attrSpy } = makePaperWithModel(); + const service = new JointUIService(emptyMetadataStub as never); + service.changeOperatorColor(paper, "op-1", false); + expect(attrSpy).toHaveBeenCalledWith("rect.body/stroke", "red"); + }); + }); + + describe("changeOperatorState", () => { + // For each state, the SUT writes a fill color to .${operatorStateClass}; + // we only assert on the color since the rest of the attr payload (port + // labels, worker count) is exercised through the existing port mocks. + const cases: Array<[OperatorState, string]> = [ + [OperatorState.Ready, "#a6bd37"], + [OperatorState.Completed, "green"], + [OperatorState.Paused, "magenta"], + [OperatorState.Pausing, "magenta"], + [OperatorState.Running, "orange"], + [OperatorState.Uninitialized, "gray"], + ]; + cases.forEach(([state, color]) => { + it(`writes fill='${color}' for state=${state}`, () => { + const { paper, attrSpy } = makePaperWithModel(); + const service = new JointUIService(emptyMetadataStub as never); + service.changeOperatorState(paper, "op-1", state); + // The attr payload is an object keyed by selectors; pluck the state class entry. + const [payload] = attrSpy.mock.calls[0]; + expect(payload[`.${operatorStateClass}`]).toEqual({ text: state.toString(), fill: color }); + expect(payload["rect.body"]).toEqual({ stroke: color }); + }); + }); + }); + + describe("foldOperatorDetails / unfoldOperatorDetails", () => { + it("hides operator state + metric texts and the action buttons when folded", () => { + const { paper, attrSpy } = makePaperWithModel(); + const service = new JointUIService(emptyMetadataStub as never); + service.foldOperatorDetails(paper, "op-1"); + const [payload] = attrSpy.mock.calls[0]; + expect(payload[`.${operatorStateClass}`].visibility).toBe("hidden"); + expect(payload[`.${operatorPortMetricsClass}`].visibility).toBe("hidden"); + expect(payload[".delete-button"].visibility).toBe("hidden"); + expect(payload[".chat-button"].visibility).toBe("hidden"); + }); + it("reveals the same surface when unfolded", () => { + const { paper, attrSpy } = makePaperWithModel(); + const service = new JointUIService(emptyMetadataStub as never); + service.unfoldOperatorDetails(paper, "op-1"); + const [payload] = attrSpy.mock.calls[0]; + expect(payload[`.${operatorStateClass}`].visibility).toBe("visible"); + expect(payload[`.${operatorPortMetricsClass}`].visibility).toBe("visible"); + expect(payload[".delete-button"].visibility).toBe("visible"); + expect(payload[".chat-button"].visibility).toBe("visible"); + }); + }); + + describe("showAgentActionLabel / hideAgentActionLabel", () => { + it("writes the action label with the agent name prefix when the model exists", () => { + const { paper, attrSpy } = makePaperWithModel(); + const service = new JointUIService(emptyMetadataStub as never); + service.showAgentActionLabel(paper, "op-1", "modified", "Aria"); + const [payload] = attrSpy.mock.calls[0]; + const entry = Object.values(payload)[0] as { text: string; visibility: string }; + expect(entry.text).toBe("Aria: modified"); + expect(entry.visibility).toBe("visible"); + }); + it("uses the default 'Agent' name when none is supplied", () => { + const { paper, attrSpy } = makePaperWithModel(); + const service = new JointUIService(emptyMetadataStub as never); + service.showAgentActionLabel(paper, "op-1", "viewed"); + const [payload] = attrSpy.mock.calls[0]; + const entry = Object.values(payload)[0] as { text: string }; + expect(entry.text).toBe("Agent: viewed"); + }); + it("no-ops when the model is missing", () => { + const paper = { getModelById: vi.fn(() => null) } as unknown as joint.dia.Paper; + const service = new JointUIService(emptyMetadataStub as never); + expect(() => service.showAgentActionLabel(paper, "missing-op", "added")).not.toThrow(); + }); + it("clears the label text and hides it on hideAgentActionLabel", () => { + const { paper, attrSpy } = makePaperWithModel(); + const service = new JointUIService(emptyMetadataStub as never); + service.hideAgentActionLabel(paper, "op-1"); + const [payload] = attrSpy.mock.calls[0]; + const entry = Object.values(payload)[0] as { text: string; visibility: string }; + expect(entry.text).toBe(""); + expect(entry.visibility).toBe("hidden"); + }); + it("hideAgentActionLabel is a no-op when the model is missing", () => { + const paper = { getModelById: vi.fn(() => null) } as unknown as joint.dia.Paper; + const service = new JointUIService(emptyMetadataStub as never); + expect(() => service.hideAgentActionLabel(paper, "missing-op")).not.toThrow(); + }); + }); + + describe("getCommentElement", () => { + it("builds a comment element with the supplied commentBoxID and position", () => { + const service = new JointUIService(emptyMetadataStub as never); + const cell = service.getCommentElement({ + commentBoxID: "cb-1", + commentBoxPosition: { x: 42, y: 99 }, + comments: [], + } as unknown as CommentBox); + expect(cell.id).toBe("cb-1"); + }); + it("falls back to (0,0) when commentBoxPosition is missing", () => { + const service = new JointUIService(emptyMetadataStub as never); + // Should not throw — the implementation guards both the basic + // shape position and the joint element construction. + const cell = service.getCommentElement({ + commentBoxID: "cb-no-pos", + comments: [], + } as unknown as CommentBox); + expect(cell.id).toBe("cb-no-pos"); + }); + }); + + describe("getJointOperatorElement", () => { + function buildMetadataWithSchemas(schemas: object[]) { + return { + getOperatorMetadata: () => + of({ + operators: schemas, + groups: [], + }), + }; + } + + const minimalSchema = (operatorType: string, friendlyName = "Friendly") => ({ + operatorType, + operatorVersion: "v1", + jsonSchema: {}, + additionalMetadata: { userFriendlyName: friendlyName }, + }); + + it("throws when the operator type isn't in the loaded schema list", () => { + const service = new JointUIService(emptyMetadataStub as never); + const operator = { + operatorID: "op-x", + operatorType: "DefinitelyNotARealType", + operatorProperties: {}, + inputPorts: [], + outputPorts: [], + showAdvanced: false, + } as unknown as OperatorPredicate; + expect(() => service.getJointOperatorElement(operator, { x: 0, y: 0 })).toThrow( + /operator type DefinitelyNotARealType doesn't exist/ + ); + }); + + it("returns an element carrying the predicate's operatorID and z-index 1", () => { + const schema = minimalSchema("TestOp", "Test Operator"); + const service = new JointUIService(buildMetadataWithSchemas([schema]) as never); + const predicate = { + operatorID: "my-op", + operatorType: "TestOp", + operatorVersion: "v1", + operatorProperties: {}, + inputPorts: [{ portID: "in-0" }], + outputPorts: [{ portID: "out-0" }], + showAdvanced: false, + isDisabled: false, + } as OperatorPredicate; + const element = service.getJointOperatorElement(predicate, { x: 100, y: 50 }); + expect(element.id).toBe("my-op"); + expect(element.get("z")).toBe(1); + // Both ports flow through addPort. + const ports = element.getPorts(); + expect(ports.map(p => p.id).sort()).toEqual(["in-0", "out-0"]); + }); + + it("emits add/remove port buttons in the markup when dynamicInputPorts and dynamicOutputPorts are true", () => { + const schema = minimalSchema("DynamicOp"); + const service = new JointUIService(buildMetadataWithSchemas([schema]) as never); + const predicate = { + operatorID: "dyn", + operatorType: "DynamicOp", + operatorVersion: "v1", + operatorProperties: {}, + inputPorts: [], + outputPorts: [], + showAdvanced: false, + isDisabled: false, + dynamicInputPorts: true, + dynamicOutputPorts: true, + } as unknown as OperatorPredicate; + const element = service.getJointOperatorElement(predicate, { x: 0, y: 0 }); + // The markup is stamped onto the element's attributes; both port-button + // classes only appear when their dynamic-ports flag is true. + const markup = (element as unknown as { attributes: { markup: string } }).attributes.markup; + expect(markup).toContain("add-input-port-button"); + expect(markup).toContain("add-output-port-button"); + expect(markup).toContain("remove-input-port-button"); + expect(markup).toContain("remove-output-port-button"); + }); + + it("renders customDisplayName in the operator name attr when supplied", () => { + const schema = minimalSchema("Named", "Friendly"); + const service = new JointUIService(buildMetadataWithSchemas([schema]) as never); + const predicate = { + operatorID: "named", + operatorType: "Named", + operatorVersion: "v1", + operatorProperties: {}, + inputPorts: [], + outputPorts: [], + showAdvanced: false, + isDisabled: false, + customDisplayName: "Custom-Display", + } as unknown as OperatorPredicate; + const element = service.getJointOperatorElement(predicate, { x: 0, y: 0 }); + // The display name lands in `.texera-operator-name/text` via + // getCustomOperatorStyleAttrs. truncateOperatorDisplayName is the no-op + // identity for short names so the value comes through verbatim. + expect(element.attr(`.${operatorNameClass}/text`)).toBe("Custom-Display"); + }); + }); + + // --------------------------------------------------------------------------- + // Port-iteration paths — the original tests stubbed getPorts() to return [], + // which skipped the forEach branches inside changeOperatorState and the + // entire body of changeOperatorStatistics. The tests below cover those. + // --------------------------------------------------------------------------- + + describe("changeOperatorState — port label re-coloring", () => { + it("re-paints every input and output port label to the state's fill color", () => { + const attrSpy = vi.fn(); + const portPropSpy = vi.fn(); + // changeOperatorState iterates getPorts() splitting by group; both + // groups must be present so the in/out filters each yield matches. + const getPortsSpy = vi.fn(() => [ + { id: "in-0", group: "in" }, + { id: "out-0", group: "out" }, + ]); + const model = { attr: attrSpy, getPorts: getPortsSpy, portProp: portPropSpy }; + const paper = { getModelById: vi.fn(() => model) } as unknown as joint.dia.Paper; + const service = new JointUIService(emptyMetadataStub as never); + + service.changeOperatorState(paper, "op-1", OperatorState.Running); + + // Running → orange. Both ports get the same color through portProp. + expect(portPropSpy).toHaveBeenCalledWith("in-0", "attrs/.port-label/fill", "orange"); + expect(portPropSpy).toHaveBeenCalledWith("out-0", "attrs/.port-label/fill", "orange"); + }); + }); + + describe("changeOperatorDisableStatus", () => { + it("paints the body fill with the disabled grey color for a disabled operator", () => { + const { paper, attrSpy } = makePaperWithModel(); + const service = new JointUIService(emptyMetadataStub as never); + service.changeOperatorDisableStatus(paper, { operatorID: "op-1", isDisabled: true } as OperatorPredicate); + expect(attrSpy).toHaveBeenCalledWith("rect.body/fill", "#E0E0E0"); + }); + it("paints the body fill with the default white color for an enabled operator", () => { + const { paper, attrSpy } = makePaperWithModel(); + const service = new JointUIService(emptyMetadataStub as never); + service.changeOperatorDisableStatus(paper, { operatorID: "op-1" } as OperatorPredicate); + expect(attrSpy).toHaveBeenCalledWith("rect.body/fill", "#FFFFFF"); + }); + }); + + describe("changeOperatorViewResultStatus", () => { + it("writes the view-result asset path when viewResult is true", () => { + const { paper, attrSpy } = makePaperWithModel(); + const service = new JointUIService(emptyMetadataStub as never); + service.changeOperatorViewResultStatus(paper, { + operatorID: "op-1", + viewResult: true, + } as unknown as OperatorPredicate); + expect(attrSpy).toHaveBeenCalledTimes(1); + const [selector, value] = attrSpy.mock.calls[0]; + expect(String(selector)).toContain("view-result"); + expect(value).toBe("assets/svg/operator-view-result.svg"); + }); + it("writes the empty asset path when viewResult is missing/false", () => { + const { paper, attrSpy } = makePaperWithModel(); + const service = new JointUIService(emptyMetadataStub as never); + service.changeOperatorViewResultStatus(paper, { operatorID: "op-1" } as OperatorPredicate); + expect(attrSpy.mock.calls[0][1]).toBe(""); + }); + }); + + describe("changeOperatorReuseCacheStatus", () => { + it("writes both the reuse-cache icon and the view-result icon when the cache is valid", () => { + const { paper, attrSpy } = makePaperWithModel(); + const service = new JointUIService(emptyMetadataStub as never); + service.changeOperatorReuseCacheStatus( + paper, + { operatorID: "op-1", markedForReuse: true } as unknown as OperatorPredicate, + "cache valid" + ); + // Two attr() calls — one for the cache icon, one for the view-result + // icon — both targeted at the operator's image attrs. + expect(attrSpy).toHaveBeenCalledTimes(2); + const allValues = attrSpy.mock.calls.map(c => c[1]).join(" | "); + expect(allValues).toContain("assets/svg/operator-reuse-cache-valid.svg"); + }); + it("writes the empty path for both icons when the operator isn't marked for reuse", () => { + const { paper, attrSpy } = makePaperWithModel(); + const service = new JointUIService(emptyMetadataStub as never); + service.changeOperatorReuseCacheStatus(paper, { + operatorID: "op-1", + markedForReuse: false, + } as unknown as OperatorPredicate); + expect(attrSpy).toHaveBeenCalledTimes(2); + expect(attrSpy.mock.calls.every(c => c[1] === "")).toBe(true); + }); + }); + + describe("changeOperatorStatistics", () => { + function makeStatsPaper(getPortsImpl: () => Array<{ id?: string; group?: string; attrs?: unknown }>) { + const attrSpy = vi.fn(); + const portPropSpy = vi.fn(); + const getPortsSpy = vi.fn(getPortsImpl); + const model = { attr: attrSpy, getPorts: getPortsSpy, portProp: portPropSpy }; + const paper = { getModelById: vi.fn(() => model) } as unknown as joint.dia.Paper; + return { paper, attrSpy, portPropSpy }; + } + + it("falls back to the Uninitialized state when statistics is undefined", () => { + const { paper, attrSpy } = makeStatsPaper(() => []); + const service = new JointUIService(emptyMetadataStub as never); + service.changeOperatorStatistics(paper, "op-1", undefined, false, false); + // changeOperatorState writes the state-class fill payload. + const [payload] = attrSpy.mock.calls[0]; + expect((payload as Record<string, { text: string }>)[`.${operatorStateClass}`].text).toBe( + OperatorState.Uninitialized.toString() + ); + }); + + it("writes per-port counts derived from inputPortMetrics and outputPortMetrics", () => { + const { paper, portPropSpy } = makeStatsPaper(() => [ + // Port IDs use the "<group>-<index>" convention; the SUT splits on "-" + // and uses the suffix to look up the metrics map. + { id: "in-0", group: "in", attrs: { ".port-label": { text: "data: 0" } } }, + { id: "out-1", group: "out", attrs: { ".port-label": { text: "result: 0" } } }, + ]); + const service = new JointUIService(emptyMetadataStub as never); + service.changeOperatorStatistics( + paper, + "op-1", + { + operatorState: OperatorState.Running, + aggregatedInputRowCount: 0, + aggregatedOutputRowCount: 0, + inputPortMetrics: { "0": 42 }, + outputPortMetrics: { "1": 7 }, + numWorkers: 3, + }, + false, + false + ); + expect(portPropSpy).toHaveBeenCalledWith("in-0", "attrs/.port-label/text", (42).toLocaleString()); + expect(portPropSpy).toHaveBeenCalledWith("out-1", "attrs/.port-label/text", (7).toLocaleString()); + }); + + it("writes the worker count label when statistics include numWorkers", () => { + const { paper, attrSpy } = makeStatsPaper(() => []); + const service = new JointUIService(emptyMetadataStub as never); + service.changeOperatorStatistics( + paper, + "op-1", + { + operatorState: OperatorState.Ready, + aggregatedInputRowCount: 0, + aggregatedOutputRowCount: 0, + inputPortMetrics: {}, + outputPortMetrics: {}, + numWorkers: 8, + }, + false, + false + ); + // attr() is called once with the workers selector and the formatted string. + const valuesWritten = attrSpy.mock.calls.map(c => c[1]); + expect(valuesWritten).toContain("#workers: 8"); + }); + + it("defaults the worker count to 1 when numWorkers is unspecified", () => { + const { paper, attrSpy } = makeStatsPaper(() => []); + const service = new JointUIService(emptyMetadataStub as never); + service.changeOperatorStatistics( + paper, + "op-1", + { + operatorState: OperatorState.Ready, + aggregatedInputRowCount: 0, + aggregatedOutputRowCount: 0, + inputPortMetrics: {}, + outputPortMetrics: {}, + }, + false, + false + ); + const valuesWritten = attrSpy.mock.calls.map(c => c[1]); + expect(valuesWritten).toContain("#workers: 1"); + }); + }); }); diff --git a/frontend/src/app/workspace/service/joint-ui/joint-ui.service.ts b/frontend/src/app/workspace/service/joint-ui/joint-ui.service.ts index d5d8f78c58..d069a270eb 100644 --- a/frontend/src/app/workspace/service/joint-ui/joint-ui.service.ts +++ b/frontend/src/app/workspace/service/joint-ui/joint-ui.service.ts @@ -396,18 +396,8 @@ export class JointUIService { if (portId != null) { const parts = portId.split("-"); const numericSuffix = parts.length > 1 ? parts[1] : portId; - const count: number = inputMetrics[numericSuffix] ?? 0; - const rawAttrs = (portDef.attrs as any) || {}; - const oldText: string = (rawAttrs[".port-label"] && rawAttrs[".port-label"].text) || ""; - let originalName = oldText.includes(":") ? oldText.split(":", 1)[0].trim() : oldText; - - if (!originalName) { - originalName = portId; - } - - const labelText = count.toLocaleString(); - element.portProp(portId, "attrs/.port-label/text", labelText); + element.portProp(portId, "attrs/.port-label/text", count.toLocaleString()); } }); @@ -416,19 +406,8 @@ export class JointUIService { if (portId != null) { const parts = portId.split("-"); const numericSuffix = parts.length > 1 ? parts[1] : portId; - const count: number = outputMetrics[numericSuffix] ?? 0; - const rawAttrs = (portDef.attrs as any) || {}; - const oldText: string = (rawAttrs[".port-label"] && rawAttrs[".port-label"].text) || ""; - let originalName = oldText.includes(":") ? oldText.split(":", 1)[0].trim() : oldText; - - if (!originalName) { - originalName = portId; - } - - const labelText = count.toLocaleString(); - - element.portProp(portId, "attrs/.port-label/text", labelText); + element.portProp(portId, "attrs/.port-label/text", count.toLocaleString()); } }); this.changeOperatorState(jointPaper, operatorID, statistics.operatorState); @@ -457,11 +436,6 @@ export class JointUIService { ".remove-input-port-button": { visibility: "visible" }, ".remove-output-port-button": { visibility: "visible" }, }); - - const element = jointPaper.getModelById(operatorID) as joint.shapes.devs.Model; - if (!element) { - return; - } } public changeOperatorState(jointPaper: joint.dia.Paper, operatorID: string, operatorState: OperatorState): void { @@ -696,40 +670,6 @@ export class JointUIService { }; } - /** - * This function create a custom svg style for the operator - * @returns the custom attributes of the tooltip. - */ - public static getCustomOperatorStatusTooltipStyleAttrs(): joint.shapes.devs.ModelSelectors { - return { - "element-node": { - style: { "pointer-events": "none" }, - }, - polygon: { - fill: "#FFFFFF", - "follow-scale": true, - stroke: "purple", - "stroke-width": "2", - rx: "5px", - ry: "5px", - refPoints: "0,30 150,30 150,120 85,120 75,150 65,120 0,120", - display: "none", - style: { "pointer-events": "none" }, - }, - "#operatorCount": { - fill: "#595959", - "font-size": "12px", - ref: "polygon", - "y-alignment": "middle", - "x-alignment": "left", - "ref-x": 0.05, - "ref-y": 0.2, - display: "none", - style: { "pointer-events": "none" }, - }, - }; - } - /** * This function creates a custom svg style for the operator. * This function also makes the delete button defined above to emit the delete event that will
