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-7951-b0dd3ffc5a2356b9d2c62626d5e6e8a407c60f74 in repository https://gitbox.apache.org/repos/asf/texera.git
commit 34eb44b21f3dd91340cb63310bb2e9dd31727d92 Author: Xinyuan Lin <[email protected]> AuthorDate: Wed Aug 26 04:13:58 2026 +0000 test(frontend): run the edge-intersection search against a real paper (#7951) ### What changes were proposed in this PR? `findIntersectedLink`'s loop body had never executed: every existing call site either has no links, no attached paper, or mocks the method out. It decides whether dropping an operator onto an edge splices it into that edge. Adds 6 tests driving it against a real joint paper with a rendered link: the edge found when the drop lands on it, nothing found when the drop is far away or the canvas has no links, and the three defensive `continue`s each yielding no edge rather than throwing. jsdom implements no SVG path geometry, so `getTotalLength`/`getPointAtLength` are installed on the rendered `.connection` path, exactly as the existing `doesOperatorIntersectPath` tests already do. Everything the loop itself does — model lookup, view lookup, path lookup, the early continues — stays real. **Verified by mutation**, all reverted (production diff empty): | Mutation | Result | |---|---| | missing view no longer skipped | red | | missing drawn path no longer skipped | red | | intersection result inverted | red | | always reports an intersection | red | | never reports an intersection | red | | wrong path selector | red | | missing **model** no longer skipped | **survived — see below** | ### A redundant guard Removing `if (!jointLink) continue` changes nothing observable: `paper.findViewByModel(undefined)` returns `undefined`, so the very next guard (`if (!linkView) continue`) catches the same case and the function still returns `null`. The first guard is defensive but subsumed by the second. No test can distinguish the two implementations, so this is reported rather than papered over with a contrived assertion. Removing it would be a small, separate cleanup. No production file is touched. ### Measured on current main This work was built on 2026-08-09 and sat unpushed. It has been rebased onto current main and **re-measured rather than trusted**. Coverage of `drag-drop.service.ts`, via `ng test --coverage --coverage-reporters=lcovonly` with the same spec filter on both sides: | Metric | Before | After | |---|---|---| | Lines hit | 186/193 | **191/193** | | Fully covered (Codecov's metric) | 179 | **185** | | Missed branch arms | 11 | **6** | So **+6 fully-covered lines and +5 branch arms.** Tests 18 → 24; the branch was written against 17, and main has since gained one. Worth noting the cases still hold after #7512 removed two pieces of unreachable code in this same file — the guards these tests pin are not among what it deleted. ### Any related issues, documentation, discussions? Closes #7950 ### How was this PR tested? ``` npx ng test --watch=false --include="**/drag-drop.service.spec.ts" ``` ``` Test Files 1 passed (1) Tests 24 passed (24) ``` jsdom prints `HTMLCanvasElement.prototype.getContext` warnings throughout. Those are jointjs rendering into jsdom and appear identically on the untouched baseline. `yarn format:ci` passes. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 5) --------- Signed-off-by: Xinyuan Lin <[email protected]> Co-authored-by: Copilot Autofix powered by AI <[email protected]> --- .../service/drag-drop/drag-drop.service.spec.ts | 109 +++++++++++++++++++++ 1 file changed, 109 insertions(+) diff --git a/frontend/src/app/workspace/service/drag-drop/drag-drop.service.spec.ts b/frontend/src/app/workspace/service/drag-drop/drag-drop.service.spec.ts index f76b041556..dcc3ef172e 100644 --- a/frontend/src/app/workspace/service/drag-drop/drag-drop.service.spec.ts +++ b/frontend/src/app/workspace/service/drag-drop/drag-drop.service.spec.ts @@ -605,4 +605,113 @@ describe("DragDropService", () => { expect(outputOps.map(o => o.operatorID)).toEqual([sinkNear.operatorID]); }); }); + /** + * Previously, our tests never drove `findIntersectedLink` far enough to evaluate a rendered + * link’s `.connection` path geometry (they ran with no links/paper or hit early `continue`s). + * This suite attaches a real JointJS paper and link so the intersection logic runs end-to-end. + */ + describe("findIntersectedLink (against a real paper)", () => { + /** + * Attaches a paper, wires scan -> result as a horizontal edge through y = 0, and hands the + * body a probe that asks which link a drop at the given point would land on. + */ + function withGraph(body: (probe: (x: number, y: number) => OperatorLink | null, link: OperatorLink) => void): void { + const workflowActionService: WorkflowActionService = TestBed.inject(WorkflowActionService); + const workflowUtilService: WorkflowUtilService = TestBed.inject(WorkflowUtilService); + const paperHost = document.createElement("div"); + document.body.appendChild(paperHost); + try { + workflowActionService.getJointGraphWrapper().attachMainJointPaper({ el: paperHost }); + + const scan = workflowUtilService.getNewOperatorPredicate("ScanSource"); + const result = workflowUtilService.getNewOperatorPredicate(VIEW_RESULT_OP_TYPE); + workflowActionService.addOperator(scan, { x: -200, y: 0 }); + workflowActionService.addOperator(result, { x: 200, y: 0 }); + const link: OperatorLink = { + linkID: workflowUtilService.getLinkRandomUUID(), + source: { operatorID: scan.operatorID, portID: scan.outputPorts[0].portID }, + target: { operatorID: result.operatorID, portID: result.inputPorts[0].portID }, + }; + workflowActionService.addLink(link); + + // jsdom implements no SVG path geometry, so install it on the rendered .connection path — + // the same stubbing the doesOperatorIntersectPath tests above already do. Everything the + // loop under test actually does (model lookup, view lookup, .connection lookup, early + // continues) stays real; only the browser geometry is supplied. + const paper = workflowActionService.getJointGraphWrapper().getMainJointPaper()!; + const view = paper.findViewByModel(paper.getModelById(link.linkID) as any) as any; + const pathElement = view.el.querySelector(".connection"); + Object.assign(pathElement, { + getTotalLength: () => 400, + getPointAtLength: (len: number) => ({ x: -200 + len, y: 0 }), + }); + + body((x, y) => (dragDropService as any).findIntersectedLink({ x, y }), link); + } finally { + paperHost.remove(); + } + } + + it("finds the edge an operator is dropped on top of", () => { + withGraph((probe, link) => { + expect(probe(0, 0)?.linkID).toBe(link.linkID); + }); + }); + + it("finds nothing when the drop is nowhere near the edge", () => { + // Far below the y = 0 run of the link; splicing here would rewire a graph the user never + // pointed at. + withGraph(probe => { + expect(probe(0, 5000)).toBeNull(); + }); + }); + + it("finds nothing when the canvas has no links at all", () => { + const workflowActionService: WorkflowActionService = TestBed.inject(WorkflowActionService); + const paperHost = document.createElement("div"); + document.body.appendChild(paperHost); + try { + workflowActionService.getJointGraphWrapper().attachMainJointPaper({ el: paperHost }); + + expect((dragDropService as any).findIntersectedLink({ x: 0, y: 0 })).toBeNull(); + } finally { + paperHost.remove(); + } + }); + + it("skips a link the paper has no model for instead of throwing", () => { + // The workflow graph and the joint paper can disagree transiently; the loop is written to + // continue past a link the paper does not know about rather than fail the whole drop. + withGraph(probe => { + const workflowActionService: WorkflowActionService = TestBed.inject(WorkflowActionService); + const paper = workflowActionService.getJointGraphWrapper().getMainJointPaper()!; + vi.spyOn(paper, "getModelById").mockReturnValue(undefined as any); + + expect(probe(0, 0)).toBeNull(); + }); + }); + + it("skips a link whose view has no drawn path instead of throwing", () => { + // A link view mid-render has no .connection child yet. Without the guard the geometry call + // receives null and the whole drop throws instead of simply finding no edge. + withGraph((probe, link) => { + const workflowActionService: WorkflowActionService = TestBed.inject(WorkflowActionService); + const paper = workflowActionService.getJointGraphWrapper().getMainJointPaper()!; + const view = paper.findViewByModel(paper.getModelById(link.linkID) as any) as any; + vi.spyOn(view.el, "querySelector").mockReturnValue(null); + + expect(probe(0, 0)).toBeNull(); + }); + }); + + it("skips a link the paper has no view for instead of throwing", () => { + withGraph(probe => { + const workflowActionService: WorkflowActionService = TestBed.inject(WorkflowActionService); + const paper = workflowActionService.getJointGraphWrapper().getMainJointPaper()!; + vi.spyOn(paper, "findViewByModel").mockReturnValue(undefined as any); + + expect(probe(0, 0)).toBeNull(); + }); + }); + }); });
