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-6829-d2ab9668a759d732e41144966a6f85c12ec91630 in repository https://gitbox.apache.org/repos/asf/texera.git
commit 87415b56dbcc8a5b2320ddb0a98ff92c600d95b7 Author: Meng Wang <[email protected]> AuthorDate: Thu Jul 23 17:11:13 2026 -0700 test(frontend): cover WorkflowEditorComponent connection validation, clipboard, and delete paths (#6829) ### What changes were proposed in this PR? Extends the existing `WorkflowEditorComponent` spec with the file's non-rendering slice — connection validation, the clipboard handlers, and the delete / select-all paths the keyboard tests never reach. The paper-event and pointer handlers stay out of scope (they need real `dia.Paper` geometry / browser mode). 12 new tests take the component from 68.6% to 74.0% statement coverage (+31 lines) and bring every targeted method to 100%: - `validateJointOperatorConnection` — rejects a link drawn out of an input port and one dropped onto an output port; delegates an output→input pair to the operator-level check (proved by re-running once the link exists). - `validateOperatorConnection` guards — self-connection, a missing port on either end, and an endpoint that is not an operator. - `deleteElements` — highlighted links and comment boxes are deleted, not just operators. - `handleElementSelectAll` — ⌘/Ctrl+A also highlights links and comment boxes. - `handleElementCopy` / `handleElementCut` / `handleElementPaste` — copy caches the selection, cut caches then deletes it, paste delegates to the operator menu. - `handleDisableOperator` — repaints the operator on disable and on re-enable. No production code was changed. ### Any related issues, documentation, discussions? Closes #6827. ### How was this PR tested? `ng test --watch=false --include src/app/workspace/component/workflow-editor/workflow-editor.component.spec.ts` — 57 passed (45 existing + 12 new), run 3× for determinism. The failure path was verified by breaking an assertion (red, non-zero exit), and eslint/prettier are clean. Coverage was measured with `--coverage` before and after to confirm each targeted method reached 100%. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 4.8 [1M context]) --- .../workflow-editor.component.spec.ts | 264 +++++++++++++++++++++ 1 file changed, 264 insertions(+) diff --git a/frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.spec.ts b/frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.spec.ts index 88332d82c5..d721f10a8d 100644 --- a/frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.spec.ts +++ b/frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.spec.ts @@ -58,6 +58,7 @@ import { ContextMenuComponent } from "./context-menu/context-menu/context-menu.c import { ComputingUnitStatusService } from "../../../common/service/computing-unit/computing-unit-status/computing-unit-status.service"; import { MockComputingUnitStatusService } from "../../../common/service/computing-unit/computing-unit-status/mock-computing-unit-status.service"; import { commonTestProviders } from "../../../common/testing/test-utils"; +import { OperatorMenuService } from "../../service/operator-menu/operator-menu.service"; describe("WorkflowEditorComponent", () => { /** @@ -1249,5 +1250,268 @@ describe("WorkflowEditorComponent", () => { expect(element.attr(`.${operatorAgentActionProgressClass}/visibility`)).toEqual("hidden"); }); }); + + /** + * The editor's non-rendering logic: connection validation, the clipboard + * handlers, and the delete / select-all paths that the keyboard tests above + * never reach (links and comment boxes). None of these need pointer geometry, + * so they run under jsdom. + */ + describe("connection validation, clipboard, and delete/select-all", () => { + /** A port magnet as JointJS hands it to validateConnection. */ + function magnet(attributes: Record<string, string>): SVGElement { + const element = document.createElementNS("http://www.w3.org/2000/svg";, "rect"); + Object.entries(attributes).forEach(([name, value]) => element.setAttribute(name, value)); + return element as unknown as SVGElement; + } + + function validateJointConnection( + sourceOperatorID: string, + sourceMagnet: SVGElement | undefined, + targetOperatorID: string, + targetMagnet: SVGElement | undefined + ): boolean { + const cellView = (id: string) => ({ model: { id } }) as unknown as joint.dia.CellView; + return component["validateJointOperatorConnection"]( + cellView(sourceOperatorID), + sourceMagnet, + cellView(targetOperatorID), + targetMagnet, + "target" as joint.dia.LinkEnd, + {} as joint.dia.LinkView + ); + } + + /** These handlers only run while the body has focus (i.e. no input is being edited). */ + function dispatchOnBody(event: Event): void { + (document.activeElement as HTMLElement)?.blur(); + document.dispatchEvent(event); + fixture.detectChanges(); + } + + describe("validateJointOperatorConnection", () => { + beforeEach(() => { + workflowActionService.addOperator(mockScanPredicate, mockPoint); + workflowActionService.addOperator(mockSentimentPredicate, mockPoint); + }); + + it("rejects a link drawn out of an input port", () => { + expect( + validateJointConnection( + mockScanPredicate.operatorID, + magnet({ "port-group": "in", port: "input-0" }), + mockSentimentPredicate.operatorID, + magnet({ "port-group": "in", port: "input-0" }) + ) + ).toBe(false); + }); + + it("rejects a link dropped onto an output port", () => { + expect( + validateJointConnection( + mockScanPredicate.operatorID, + magnet({ "port-group": "out", port: "output-0" }), + mockSentimentPredicate.operatorID, + magnet({ "port-group": "out", port: "output-0" }) + ) + ).toBe(false); + }); + + it("delegates an output-to-input pair to the operator-level validation", () => { + const outMagnet = () => magnet({ "port-group": "out", port: "output-0" }); + const inMagnet = () => magnet({ "port-group": "in", port: "input-0" }); + + expect( + validateJointConnection( + mockScanPredicate.operatorID, + outMagnet(), + mockSentimentPredicate.operatorID, + inMagnet() + ) + ).toBe(true); + + // Once the link exists the same pair is rejected — proving the call is + // really delegated to validateOperatorConnection rather than hardcoded. + workflowActionService.addLink(mockScanSentimentLink); + expect( + validateJointConnection( + mockScanPredicate.operatorID, + outMagnet(), + mockSentimentPredicate.operatorID, + inMagnet() + ) + ).toBe(false); + }); + }); + + describe("validateOperatorConnection guards", () => { + beforeEach(() => { + workflowActionService.addOperator(mockScanPredicate, mockPoint); + workflowActionService.addOperator(mockSentimentPredicate, mockPoint); + }); + + it("rejects a connection from an operator to itself", () => { + expect( + component["validateOperatorConnection"]( + mockScanPredicate.operatorID, + "output-0", + mockScanPredicate.operatorID, + "input-0" + ) + ).toBe(false); + }); + + it("rejects a connection that is missing a port on either end", () => { + expect( + component["validateOperatorConnection"]( + mockScanPredicate.operatorID, + undefined, + mockSentimentPredicate.operatorID, + "input-0" + ) + ).toBe(false); + expect( + component["validateOperatorConnection"]( + mockScanPredicate.operatorID, + "output-0", + mockSentimentPredicate.operatorID, + null + ) + ).toBe(false); + }); + + it("rejects a connection whose endpoint is not an operator", () => { + expect( + component["validateOperatorConnection"]( + "not-an-operator", + "output-0", + mockSentimentPredicate.operatorID, + "input-0" + ) + ).toBe(false); + expect( + component["validateOperatorConnection"]( + mockScanPredicate.operatorID, + "output-0", + "not-an-operator", + "input-0" + ) + ).toBe(false); + }); + }); + + describe("delete", () => { + it("deletes highlighted links and comment boxes, not just operators", () => { + const texeraGraph = workflowActionService.getTexeraGraph(); + const jointGraphWrapper = workflowActionService.getJointGraphWrapper(); + workflowActionService.addOperatorsAndLinks( + [ + { op: mockScanPredicate, pos: mockPoint }, + { op: mockResultPredicate, pos: mockPoint }, + ], + [mockScanResultLink] + ); + workflowActionService.addCommentBox(mockCommentBox); + // multi-select keeps both selections alive: highlighting with it off + // unhighlights everything else first, which would silently drop the link. + jointGraphWrapper.setMultiSelectMode(true); + jointGraphWrapper.unhighlightOperators(...jointGraphWrapper.getCurrentHighlightedOperatorIDs()); + jointGraphWrapper.highlightLinks(mockScanResultLink.linkID); + jointGraphWrapper.highlightCommentBoxes(mockCommentBox.commentBoxID); + + // guard the setup so the assertions below cannot pass vacuously + expect(texeraGraph.hasLinkWithID(mockScanResultLink.linkID)).toBe(true); + expect(jointGraphWrapper.getCurrentHighlightedOperatorIDs()).toEqual([]); + expect(jointGraphWrapper.getCurrentHighlightedLinkIDs()).toContain(mockScanResultLink.linkID); + expect(jointGraphWrapper.getCurrentHighlightedCommentBoxIDs()).toContain(mockCommentBox.commentBoxID); + + dispatchOnBody(new KeyboardEvent("keydown", { key: "Delete" })); + + expect(texeraGraph.hasLinkWithID(mockScanResultLink.linkID)).toBe(false); + expect(texeraGraph.hasCommentBox(mockCommentBox.commentBoxID)).toBe(false); + // the operators were not highlighted, so they survive + expect(texeraGraph.hasOperator(mockScanPredicate.operatorID)).toBe(true); + }); + }); + + describe("select all", () => { + it("highlights links and comment boxes as well as operators", () => { + const jointGraphWrapper = workflowActionService.getJointGraphWrapper(); + workflowActionService.addOperatorsAndLinks( + [ + { op: mockScanPredicate, pos: mockPoint }, + { op: mockResultPredicate, pos: mockPoint }, + ], + [mockScanResultLink] + ); + workflowActionService.addCommentBox(mockCommentBox); + + dispatchOnBody(new KeyboardEvent("keydown", { key: "a", metaKey: true })); + + expect(jointGraphWrapper.getCurrentHighlightedOperatorIDs()).toContain(mockScanPredicate.operatorID); + expect(jointGraphWrapper.getCurrentHighlightedLinkIDs()).toContain(mockScanResultLink.linkID); + expect(jointGraphWrapper.getCurrentHighlightedCommentBoxIDs()).toContain(mockCommentBox.commentBoxID); + }); + }); + + describe("disabled-operator stream", () => { + it("repaints an operator when it is disabled and again when it is re-enabled", () => { + workflowActionService.addOperator(mockScanPredicate, mockPoint); + const changeSpy = vi.spyOn(jointUIService, "changeOperatorDisableStatus"); + try { + workflowActionService.disableOperators([mockScanPredicate.operatorID]); + expect(changeSpy).toHaveBeenCalledTimes(1); + + workflowActionService.enableOperators([mockScanPredicate.operatorID]); + expect(changeSpy).toHaveBeenCalledTimes(2); + } finally { + changeSpy.mockRestore(); + } + }); + }); + + describe("clipboard", () => { + let operatorMenu: OperatorMenuService; + + beforeEach(() => { + operatorMenu = TestBed.inject(OperatorMenuService); + workflowActionService.addOperator(mockScanPredicate, mockPoint); + // the copy/cut handlers read the menu's latest highlighted-element snapshot + workflowActionService.getJointGraphWrapper().highlightOperators(mockScanPredicate.operatorID); + }); + + it("caches the highlighted elements on copy", () => { + const saveSpy = vi.spyOn(operatorMenu, "saveHighlightedElements").mockImplementation(() => {}); + try { + dispatchOnBody(new Event("copy")); + expect(saveSpy).toHaveBeenCalledTimes(1); + expect(workflowActionService.getTexeraGraph().hasOperator(mockScanPredicate.operatorID)).toBe(true); + } finally { + saveSpy.mockRestore(); + } + }); + + it("caches and then deletes the highlighted elements on cut", () => { + const saveSpy = vi.spyOn(operatorMenu, "saveHighlightedElements").mockImplementation(() => {}); + try { + dispatchOnBody(new Event("cut")); + expect(saveSpy).toHaveBeenCalledTimes(1); + expect(workflowActionService.getTexeraGraph().hasOperator(mockScanPredicate.operatorID)).toBe(false); + } finally { + saveSpy.mockRestore(); + } + }); + + it("pastes the cached elements on paste", () => { + const pasteSpy = vi.spyOn(operatorMenu, "performPasteOperation").mockImplementation(() => {}); + try { + dispatchOnBody(new Event("paste")); + expect(pasteSpy).toHaveBeenCalledTimes(1); + } finally { + pasteSpy.mockRestore(); + } + }); + }); + }); }); });
