You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@texera.apache.org by "Yicong-Huang (via GitHub)" <gi...@apache.org> on 2026/05/06 07:33:30 UTC

[PR] feat(frontend): wire up Vitest browser-mode infrastructure [texera]

Yicong-Huang opened a new pull request, #4971:
URL: https://github.com/apache/texera/pull/4971

   ### What changes were proposed in this PR?
   
   Land Vitest browser-mode infrastructure for the frontend so future jointjs / layout-dependent specs have a real-Chromium runner. Adds:
   
   - `vitest.browser.config.ts` (Playwright-driven, with the resolve aliases needed to keep `uuid@8.3.2` and `lib0/webcrypto` from picking their Node entrypoints in the browser bundle).
   - A separate `gui:test-browser` Architect target so the default `gui:test` jsdom path stays fast.
   - `yarn test:browser` / `yarn test:browser:ci` scripts and a CI step on `ubuntu-latest` (Playwright on macOS / Windows multiplies minute cost for no extra signal). Chromium binary is cached by `frontend/yarn.lock` hash.
   - `**/__screenshots__` to `frontend/.gitignore` so failure artifacts don't get committed or trip prettier-eslint.
   
   ### What's actually routed through the new target
   
   Just `drag-drop.service.spec.ts` for now — its 4 working tests are a smoke test that the browser pipeline launches and runs. The 2 `it.skip` tests in that file turned out to fail under real Chromium too, because of a test-order bug (`findClosestOperators` is called before any `addOperator`), not a jsdom layout limitation. The skip + comment have been updated to reflect that and stay tied to #4866.
   
   `workflow-editor.component.spec.ts` is still excluded from both targets. Beyond the jointjs paper geometry that motivated #4866, it transitively pulls in `download.service.ts`'s top-level `require("content-disposition")`, which esbuild can't rewrite for the browser bundle. Unblocking that is a separate cleanup, tracked in the comment of `tsconfig.spec.json`.
   
   ### Any related issues, documentation, discussions?
   
   Part of #4866 — wires up the path; doesn't close it.
   
   ### How was this PR tested?
   
   - `yarn test`: 269 pass, 11 skip, 2 todo (jsdom, unchanged).
   - `yarn test:browser`: 4 pass, 2 skip (Chromium via Playwright).
   - `yarn format:ci` clean.
   
   ### Was this PR authored or co-authored using generative AI tooling?
   
   Generated-by: Claude Opus 4.7 (1M context)


-- 
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: notifications-unsubscribe@texera.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] test(frontend): re-enable drag-drop tests; add Vitest browser-mode infra [texera]

Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang closed pull request #4971: test(frontend): re-enable drag-drop tests; add Vitest browser-mode infra
URL: https://github.com/apache/texera/pull/4971


-- 
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: notifications-unsubscribe@texera.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] test(frontend): re-enable two skipped drag-drop tests [texera]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #4971:
URL: https://github.com/apache/texera/pull/4971#discussion_r3193990661


##########
frontend/src/app/workspace/service/drag-drop/drag-drop.service.spec.ts:
##########
@@ -155,81 +159,89 @@ describe("DragDropService", () => {
     expect(inputOps).toEqual([]);
   });
 
-  // Same root cause as the skipped test above — link inference depends on
-  // findClosestOperators returning real geometry. Tracked in #4866.
-  it.skip(
-    "should update highlighting, add operator, and add links when an operator is dropped",
-    marbles(async () => {
-      const workflowActionService: WorkflowActionService = TestBed.inject(WorkflowActionService);
-      const workflowUtilService: WorkflowUtilService = TestBed.inject(WorkflowUtilService);
-      workflowActionService.getJointGraphWrapper();
-      const operatorType = "MultiInputOutput";
-      const operator = mockMultiInputOutputPredicate;
-      const input1 = workflowUtilService.getNewOperatorPredicate("ScanSource");
-      const input2 = workflowUtilService.getNewOperatorPredicate("ScanSource");
-      const input3 = workflowUtilService.getNewOperatorPredicate("ScanSource");
-      const output1 = workflowUtilService.getNewOperatorPredicate(VIEW_RESULT_OP_TYPE);
-      const output2 = workflowUtilService.getNewOperatorPredicate(VIEW_RESULT_OP_TYPE);
-      const output3 = workflowUtilService.getNewOperatorPredicate(VIEW_RESULT_OP_TYPE);
-      const heightSortedInputs: OperatorPredicate[] = [input1, input2, input3];
-      const heightSortedOutputs: OperatorPredicate[] = [output1, output2, output3];
-
-      // lists to be populated by observables/streams
-      const highlights: string[] = [];
-      const unhighlights: string[] = [];
-      const links: OperatorLink[] = [];
-      // expected end results of above lists
-      const expectedHighlights: OperatorPredicate[] = []; // expected empty
-      const expectedUnhighlights = [
+  it("should add the dropped operator with links to suggested neighbors and unhighlight prior suggestions", async () => {
+    const workflowActionService: WorkflowActionService = TestBed.inject(WorkflowActionService);
+    const workflowUtilService: WorkflowUtilService = TestBed.inject(WorkflowUtilService);
+    const input1 = workflowUtilService.getNewOperatorPredicate("ScanSource");
+    const input2 = workflowUtilService.getNewOperatorPredicate("ScanSource");
+    const input3 = workflowUtilService.getNewOperatorPredicate("ScanSource");
+    const output1 = workflowUtilService.getNewOperatorPredicate(VIEW_RESULT_OP_TYPE);
+    const output2 = workflowUtilService.getNewOperatorPredicate(VIEW_RESULT_OP_TYPE);
+    const output3 = workflowUtilService.getNewOperatorPredicate(VIEW_RESULT_OP_TYPE);
+
+    // Real main jointjs paper attached to a hidden DOM host so coordinate
+    // transforms in `dragStarted` / mousemove / `dragDropped` resolve without
+    // stubs. jsdom doesn't compute real layout, but the SVG polyfill returns
+    // identity transforms so `pageToLocalPoint(x, y)` ≈ (x, y) — fine for
+    // this test which doesn't assert on the dropped operator's position.

Review Comment:
   The comment about jsdom/SVG polyfills is internally inconsistent: it says `pageToLocalPoint(x, y)` is approximately `(x, y)`, but later the test relies on `pageToLocalPoint` always returning `(0, 0)`. Given the repo’s `jsdom-svg-polyfill` stubs `createSVGPoint().matrixTransform()` to return `{x:0,y:0}`, `pageToLocalPoint` will effectively collapse to `(0,0)`. Please update the earlier comment to match the actual behavior so future readers don’t misinterpret why the x=-100/+100 placement is necessary.
   



##########
frontend/src/app/workspace/service/drag-drop/drag-drop.service.spec.ts:
##########
@@ -155,81 +159,89 @@ describe("DragDropService", () => {
     expect(inputOps).toEqual([]);
   });
 
-  // Same root cause as the skipped test above — link inference depends on
-  // findClosestOperators returning real geometry. Tracked in #4866.
-  it.skip(
-    "should update highlighting, add operator, and add links when an operator is dropped",
-    marbles(async () => {
-      const workflowActionService: WorkflowActionService = TestBed.inject(WorkflowActionService);
-      const workflowUtilService: WorkflowUtilService = TestBed.inject(WorkflowUtilService);
-      workflowActionService.getJointGraphWrapper();
-      const operatorType = "MultiInputOutput";
-      const operator = mockMultiInputOutputPredicate;
-      const input1 = workflowUtilService.getNewOperatorPredicate("ScanSource");
-      const input2 = workflowUtilService.getNewOperatorPredicate("ScanSource");
-      const input3 = workflowUtilService.getNewOperatorPredicate("ScanSource");
-      const output1 = workflowUtilService.getNewOperatorPredicate(VIEW_RESULT_OP_TYPE);
-      const output2 = workflowUtilService.getNewOperatorPredicate(VIEW_RESULT_OP_TYPE);
-      const output3 = workflowUtilService.getNewOperatorPredicate(VIEW_RESULT_OP_TYPE);
-      const heightSortedInputs: OperatorPredicate[] = [input1, input2, input3];
-      const heightSortedOutputs: OperatorPredicate[] = [output1, output2, output3];
-
-      // lists to be populated by observables/streams
-      const highlights: string[] = [];
-      const unhighlights: string[] = [];
-      const links: OperatorLink[] = [];
-      // expected end results of above lists
-      const expectedHighlights: OperatorPredicate[] = []; // expected empty
-      const expectedUnhighlights = [
+  it("should add the dropped operator with links to suggested neighbors and unhighlight prior suggestions", async () => {
+    const workflowActionService: WorkflowActionService = TestBed.inject(WorkflowActionService);
+    const workflowUtilService: WorkflowUtilService = TestBed.inject(WorkflowUtilService);
+    const input1 = workflowUtilService.getNewOperatorPredicate("ScanSource");
+    const input2 = workflowUtilService.getNewOperatorPredicate("ScanSource");
+    const input3 = workflowUtilService.getNewOperatorPredicate("ScanSource");
+    const output1 = workflowUtilService.getNewOperatorPredicate(VIEW_RESULT_OP_TYPE);
+    const output2 = workflowUtilService.getNewOperatorPredicate(VIEW_RESULT_OP_TYPE);
+    const output3 = workflowUtilService.getNewOperatorPredicate(VIEW_RESULT_OP_TYPE);
+
+    // Real main jointjs paper attached to a hidden DOM host so coordinate
+    // transforms in `dragStarted` / mousemove / `dragDropped` resolve without
+    // stubs. jsdom doesn't compute real layout, but the SVG polyfill returns
+    // identity transforms so `pageToLocalPoint(x, y)` ≈ (x, y) — fine for
+    // this test which doesn't assert on the dropped operator's position.
+    const paperHost = document.createElement("div");
+    document.body.appendChild(paperHost);
+    workflowActionService.getJointGraphWrapper().attachMainJointPaper({ el: paperHost });
+
+    // `dragStarted` mounts a "ghost" jointjs paper in this DOM node.
+    const flyingOpHost = document.createElement("div");
+    flyingOpHost.id = "flyingOP";
+    document.body.appendChild(flyingOpHost);
+
+    // jsdom doesn't compute layout, so the paper's `pageToLocalPoint` always
+    // returns (0, 0) regardless of the dispatched mouse position. Place the
+    // input operators at negative x and outputs at positive x so the
+    // origin-at-(0,0) drop point classifies them correctly via
+    // `findClosestOperators` (which compares operator x against mouse x).
+    workflowActionService.addOperator(input1, { x: -100, y: 10 });
+    workflowActionService.addOperator(input2, { x: -100, y: 20 });
+    workflowActionService.addOperator(input3, { x: -100, y: 30 });
+    workflowActionService.addOperator(output1, { x: 100, y: 10 });
+    workflowActionService.addOperator(output2, { x: 100, y: 20 });
+    workflowActionService.addOperator(output3, { x: 100, y: 30 });
+
+    const unhighlights: string[] = [];
+    dragDropService.getOperatorSuggestionUnhighlightStream().subscribe(id => unhighlights.push(id));
+    const links: OperatorLink[] = [];
+    workflowActionService
+      .getTexeraGraph()
+      .getLinkAddStream()
+      .subscribe(link => links.push(link));
+
+    // dragStarted creates a fresh `op` of the given type and subscribes to
+    // window mousemove to populate suggestionInputs / suggestionOutputs.
+    dragDropService.dragStarted("MultiInputOutput");
+    const droppedOp = (dragDropService as any).op as OperatorPredicate;
+
+    // Drive the suggestion pipeline. Any mousemove will do — jsdom's
+    // `pageToLocalPoint` always resolves to (0, 0) since it has no layout,
+    // and we placed inputs/outputs around the origin to match.
+    window.dispatchEvent(new MouseEvent("mousemove", { clientX: 0, clientY: 0 }));
+    await new Promise(resolve => setTimeout(resolve, 0));
+
+    dragDropService.dragDropped({ x: 0, y: 0 });

Review Comment:
   `dragStarted()` installs window `mousemove` subscriptions that only stop processing after a `mouseup` sets `isOperatorDropped = true`. This test never dispatches a `mouseup`, so the subscriptions remain active after `dragDropped()` and can react to mouse events from other specs, potentially causing cross-test interference. After `dragDropped`, dispatch a synthetic `mouseup` (or otherwise terminate the drag recommendation pipeline) before cleaning up the DOM.
   



##########
frontend/src/app/workspace/service/drag-drop/drag-drop.service.spec.ts:
##########
@@ -155,81 +159,89 @@ describe("DragDropService", () => {
     expect(inputOps).toEqual([]);
   });
 
-  // Same root cause as the skipped test above — link inference depends on
-  // findClosestOperators returning real geometry. Tracked in #4866.
-  it.skip(
-    "should update highlighting, add operator, and add links when an operator is dropped",
-    marbles(async () => {
-      const workflowActionService: WorkflowActionService = TestBed.inject(WorkflowActionService);
-      const workflowUtilService: WorkflowUtilService = TestBed.inject(WorkflowUtilService);
-      workflowActionService.getJointGraphWrapper();
-      const operatorType = "MultiInputOutput";
-      const operator = mockMultiInputOutputPredicate;
-      const input1 = workflowUtilService.getNewOperatorPredicate("ScanSource");
-      const input2 = workflowUtilService.getNewOperatorPredicate("ScanSource");
-      const input3 = workflowUtilService.getNewOperatorPredicate("ScanSource");
-      const output1 = workflowUtilService.getNewOperatorPredicate(VIEW_RESULT_OP_TYPE);
-      const output2 = workflowUtilService.getNewOperatorPredicate(VIEW_RESULT_OP_TYPE);
-      const output3 = workflowUtilService.getNewOperatorPredicate(VIEW_RESULT_OP_TYPE);
-      const heightSortedInputs: OperatorPredicate[] = [input1, input2, input3];
-      const heightSortedOutputs: OperatorPredicate[] = [output1, output2, output3];
-
-      // lists to be populated by observables/streams
-      const highlights: string[] = [];
-      const unhighlights: string[] = [];
-      const links: OperatorLink[] = [];
-      // expected end results of above lists
-      const expectedHighlights: OperatorPredicate[] = []; // expected empty
-      const expectedUnhighlights = [
+  it("should add the dropped operator with links to suggested neighbors and unhighlight prior suggestions", async () => {
+    const workflowActionService: WorkflowActionService = TestBed.inject(WorkflowActionService);
+    const workflowUtilService: WorkflowUtilService = TestBed.inject(WorkflowUtilService);
+    const input1 = workflowUtilService.getNewOperatorPredicate("ScanSource");
+    const input2 = workflowUtilService.getNewOperatorPredicate("ScanSource");
+    const input3 = workflowUtilService.getNewOperatorPredicate("ScanSource");
+    const output1 = workflowUtilService.getNewOperatorPredicate(VIEW_RESULT_OP_TYPE);
+    const output2 = workflowUtilService.getNewOperatorPredicate(VIEW_RESULT_OP_TYPE);
+    const output3 = workflowUtilService.getNewOperatorPredicate(VIEW_RESULT_OP_TYPE);
+
+    // Real main jointjs paper attached to a hidden DOM host so coordinate
+    // transforms in `dragStarted` / mousemove / `dragDropped` resolve without
+    // stubs. jsdom doesn't compute real layout, but the SVG polyfill returns
+    // identity transforms so `pageToLocalPoint(x, y)` ≈ (x, y) — fine for
+    // this test which doesn't assert on the dropped operator's position.
+    const paperHost = document.createElement("div");
+    document.body.appendChild(paperHost);
+    workflowActionService.getJointGraphWrapper().attachMainJointPaper({ el: paperHost });
+
+    // `dragStarted` mounts a "ghost" jointjs paper in this DOM node.
+    const flyingOpHost = document.createElement("div");
+    flyingOpHost.id = "flyingOP";
+    document.body.appendChild(flyingOpHost);
+

Review Comment:
   This test appends `paperHost` and `flyingOpHost` to `document.body`, but cleanup happens only at the very end. If any assertion throws before lines 244-245, these DOM nodes (and the JointJS paper attached to them) will leak into later tests and can cause flakes. Consider wrapping the body in `try/finally` (or using `afterEach`) to guarantee DOM cleanup even on failure.



-- 
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: notifications-unsubscribe@texera.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] test(frontend): re-enable two skipped drag-drop tests [texera]

Posted by "aglinxinyuan (via GitHub)" <gi...@apache.org>.
aglinxinyuan commented on PR #4971:
URL: https://github.com/apache/texera/pull/4971#issuecomment-4412052910

   > The frontend test took 6 hours and failed. Please take a look: https://github.com/apache/texera/actions/runs/25425783736/job/74578713230
   
   Fixed in PR #4999


-- 
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: notifications-unsubscribe@texera.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(frontend): wire up Vitest browser-mode infrastructure [texera]

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #4971:
URL: https://github.com/apache/texera/pull/4971#issuecomment-4385977255

   ## [Codecov](https://app.codecov.io/gh/apache/texera/pull/4971?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :white_check_mark: All modified and coverable lines are covered by tests.
   :white_check_mark: Project coverage is 42.58%. Comparing base ([`933f7fe`](https://app.codecov.io/gh/apache/texera/commit/933f7fed772a84fd47726c5579c1b7624453a2d9?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`0404895`](https://app.codecov.io/gh/apache/texera/commit/040489541bc352f195b611b82ec619927147f989?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   :warning: Report is 1 commits behind head on main.
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@            Coverage Diff            @@
   ##               main    #4971   +/-   ##
   =========================================
     Coverage     42.58%   42.58%           
     Complexity     2184     2184           
   =========================================
     Files          1031     1031           
     Lines         38110    38110           
     Branches       4004     4004           
   =========================================
     Hits          16231    16231           
     Misses        20863    20863           
     Partials       1016     1016           
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/texera/pull/4971/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [agent-service](https://app.codecov.io/gh/apache/texera/pull/4971/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `33.72% <ø> (ø)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   </details>
   
   [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/texera/pull/4971?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </details>


-- 
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: notifications-unsubscribe@texera.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] test(frontend): re-enable two skipped drag-drop tests [texera]

Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang commented on code in PR #4971:
URL: https://github.com/apache/texera/pull/4971#discussion_r3194010810


##########
frontend/src/app/workspace/service/drag-drop/drag-drop.service.spec.ts:
##########
@@ -155,81 +159,89 @@ describe("DragDropService", () => {
     expect(inputOps).toEqual([]);
   });
 
-  // Same root cause as the skipped test above — link inference depends on
-  // findClosestOperators returning real geometry. Tracked in #4866.
-  it.skip(
-    "should update highlighting, add operator, and add links when an operator is dropped",
-    marbles(async () => {
-      const workflowActionService: WorkflowActionService = TestBed.inject(WorkflowActionService);
-      const workflowUtilService: WorkflowUtilService = TestBed.inject(WorkflowUtilService);
-      workflowActionService.getJointGraphWrapper();
-      const operatorType = "MultiInputOutput";
-      const operator = mockMultiInputOutputPredicate;
-      const input1 = workflowUtilService.getNewOperatorPredicate("ScanSource");
-      const input2 = workflowUtilService.getNewOperatorPredicate("ScanSource");
-      const input3 = workflowUtilService.getNewOperatorPredicate("ScanSource");
-      const output1 = workflowUtilService.getNewOperatorPredicate(VIEW_RESULT_OP_TYPE);
-      const output2 = workflowUtilService.getNewOperatorPredicate(VIEW_RESULT_OP_TYPE);
-      const output3 = workflowUtilService.getNewOperatorPredicate(VIEW_RESULT_OP_TYPE);
-      const heightSortedInputs: OperatorPredicate[] = [input1, input2, input3];
-      const heightSortedOutputs: OperatorPredicate[] = [output1, output2, output3];
-
-      // lists to be populated by observables/streams
-      const highlights: string[] = [];
-      const unhighlights: string[] = [];
-      const links: OperatorLink[] = [];
-      // expected end results of above lists
-      const expectedHighlights: OperatorPredicate[] = []; // expected empty
-      const expectedUnhighlights = [
+  it("should add the dropped operator with links to suggested neighbors and unhighlight prior suggestions", async () => {
+    const workflowActionService: WorkflowActionService = TestBed.inject(WorkflowActionService);
+    const workflowUtilService: WorkflowUtilService = TestBed.inject(WorkflowUtilService);
+    const input1 = workflowUtilService.getNewOperatorPredicate("ScanSource");
+    const input2 = workflowUtilService.getNewOperatorPredicate("ScanSource");
+    const input3 = workflowUtilService.getNewOperatorPredicate("ScanSource");
+    const output1 = workflowUtilService.getNewOperatorPredicate(VIEW_RESULT_OP_TYPE);
+    const output2 = workflowUtilService.getNewOperatorPredicate(VIEW_RESULT_OP_TYPE);
+    const output3 = workflowUtilService.getNewOperatorPredicate(VIEW_RESULT_OP_TYPE);
+
+    // Real main jointjs paper attached to a hidden DOM host so coordinate
+    // transforms in `dragStarted` / mousemove / `dragDropped` resolve without
+    // stubs. jsdom doesn't compute real layout, but the SVG polyfill returns
+    // identity transforms so `pageToLocalPoint(x, y)` ≈ (x, y) — fine for
+    // this test which doesn't assert on the dropped operator's position.

Review Comment:
   Good catch — the comment was internally inconsistent. Rewrote it to say what actually happens: the SVG polyfill returns identity matrices, so jointjs `pageToLocalPoint` collapses to (0, 0) regardless of input. That is the real reason the operators are placed at x=±100 around the origin.



##########
frontend/src/app/workspace/service/drag-drop/drag-drop.service.spec.ts:
##########
@@ -155,81 +159,89 @@ describe("DragDropService", () => {
     expect(inputOps).toEqual([]);
   });
 
-  // Same root cause as the skipped test above — link inference depends on
-  // findClosestOperators returning real geometry. Tracked in #4866.
-  it.skip(
-    "should update highlighting, add operator, and add links when an operator is dropped",
-    marbles(async () => {
-      const workflowActionService: WorkflowActionService = TestBed.inject(WorkflowActionService);
-      const workflowUtilService: WorkflowUtilService = TestBed.inject(WorkflowUtilService);
-      workflowActionService.getJointGraphWrapper();
-      const operatorType = "MultiInputOutput";
-      const operator = mockMultiInputOutputPredicate;
-      const input1 = workflowUtilService.getNewOperatorPredicate("ScanSource");
-      const input2 = workflowUtilService.getNewOperatorPredicate("ScanSource");
-      const input3 = workflowUtilService.getNewOperatorPredicate("ScanSource");
-      const output1 = workflowUtilService.getNewOperatorPredicate(VIEW_RESULT_OP_TYPE);
-      const output2 = workflowUtilService.getNewOperatorPredicate(VIEW_RESULT_OP_TYPE);
-      const output3 = workflowUtilService.getNewOperatorPredicate(VIEW_RESULT_OP_TYPE);
-      const heightSortedInputs: OperatorPredicate[] = [input1, input2, input3];
-      const heightSortedOutputs: OperatorPredicate[] = [output1, output2, output3];
-
-      // lists to be populated by observables/streams
-      const highlights: string[] = [];
-      const unhighlights: string[] = [];
-      const links: OperatorLink[] = [];
-      // expected end results of above lists
-      const expectedHighlights: OperatorPredicate[] = []; // expected empty
-      const expectedUnhighlights = [
+  it("should add the dropped operator with links to suggested neighbors and unhighlight prior suggestions", async () => {
+    const workflowActionService: WorkflowActionService = TestBed.inject(WorkflowActionService);
+    const workflowUtilService: WorkflowUtilService = TestBed.inject(WorkflowUtilService);
+    const input1 = workflowUtilService.getNewOperatorPredicate("ScanSource");
+    const input2 = workflowUtilService.getNewOperatorPredicate("ScanSource");
+    const input3 = workflowUtilService.getNewOperatorPredicate("ScanSource");
+    const output1 = workflowUtilService.getNewOperatorPredicate(VIEW_RESULT_OP_TYPE);
+    const output2 = workflowUtilService.getNewOperatorPredicate(VIEW_RESULT_OP_TYPE);
+    const output3 = workflowUtilService.getNewOperatorPredicate(VIEW_RESULT_OP_TYPE);
+
+    // Real main jointjs paper attached to a hidden DOM host so coordinate
+    // transforms in `dragStarted` / mousemove / `dragDropped` resolve without
+    // stubs. jsdom doesn't compute real layout, but the SVG polyfill returns
+    // identity transforms so `pageToLocalPoint(x, y)` ≈ (x, y) — fine for
+    // this test which doesn't assert on the dropped operator's position.
+    const paperHost = document.createElement("div");
+    document.body.appendChild(paperHost);
+    workflowActionService.getJointGraphWrapper().attachMainJointPaper({ el: paperHost });
+
+    // `dragStarted` mounts a "ghost" jointjs paper in this DOM node.
+    const flyingOpHost = document.createElement("div");
+    flyingOpHost.id = "flyingOP";
+    document.body.appendChild(flyingOpHost);
+

Review Comment:
   Wrapped the body in `try/finally` so `paperHost` and `flyingOpHost` are removed even if an assertion throws.



##########
frontend/src/app/workspace/service/drag-drop/drag-drop.service.spec.ts:
##########
@@ -155,81 +159,89 @@ describe("DragDropService", () => {
     expect(inputOps).toEqual([]);
   });
 
-  // Same root cause as the skipped test above — link inference depends on
-  // findClosestOperators returning real geometry. Tracked in #4866.
-  it.skip(
-    "should update highlighting, add operator, and add links when an operator is dropped",
-    marbles(async () => {
-      const workflowActionService: WorkflowActionService = TestBed.inject(WorkflowActionService);
-      const workflowUtilService: WorkflowUtilService = TestBed.inject(WorkflowUtilService);
-      workflowActionService.getJointGraphWrapper();
-      const operatorType = "MultiInputOutput";
-      const operator = mockMultiInputOutputPredicate;
-      const input1 = workflowUtilService.getNewOperatorPredicate("ScanSource");
-      const input2 = workflowUtilService.getNewOperatorPredicate("ScanSource");
-      const input3 = workflowUtilService.getNewOperatorPredicate("ScanSource");
-      const output1 = workflowUtilService.getNewOperatorPredicate(VIEW_RESULT_OP_TYPE);
-      const output2 = workflowUtilService.getNewOperatorPredicate(VIEW_RESULT_OP_TYPE);
-      const output3 = workflowUtilService.getNewOperatorPredicate(VIEW_RESULT_OP_TYPE);
-      const heightSortedInputs: OperatorPredicate[] = [input1, input2, input3];
-      const heightSortedOutputs: OperatorPredicate[] = [output1, output2, output3];
-
-      // lists to be populated by observables/streams
-      const highlights: string[] = [];
-      const unhighlights: string[] = [];
-      const links: OperatorLink[] = [];
-      // expected end results of above lists
-      const expectedHighlights: OperatorPredicate[] = []; // expected empty
-      const expectedUnhighlights = [
+  it("should add the dropped operator with links to suggested neighbors and unhighlight prior suggestions", async () => {
+    const workflowActionService: WorkflowActionService = TestBed.inject(WorkflowActionService);
+    const workflowUtilService: WorkflowUtilService = TestBed.inject(WorkflowUtilService);
+    const input1 = workflowUtilService.getNewOperatorPredicate("ScanSource");
+    const input2 = workflowUtilService.getNewOperatorPredicate("ScanSource");
+    const input3 = workflowUtilService.getNewOperatorPredicate("ScanSource");
+    const output1 = workflowUtilService.getNewOperatorPredicate(VIEW_RESULT_OP_TYPE);
+    const output2 = workflowUtilService.getNewOperatorPredicate(VIEW_RESULT_OP_TYPE);
+    const output3 = workflowUtilService.getNewOperatorPredicate(VIEW_RESULT_OP_TYPE);
+
+    // Real main jointjs paper attached to a hidden DOM host so coordinate
+    // transforms in `dragStarted` / mousemove / `dragDropped` resolve without
+    // stubs. jsdom doesn't compute real layout, but the SVG polyfill returns
+    // identity transforms so `pageToLocalPoint(x, y)` ≈ (x, y) — fine for
+    // this test which doesn't assert on the dropped operator's position.
+    const paperHost = document.createElement("div");
+    document.body.appendChild(paperHost);
+    workflowActionService.getJointGraphWrapper().attachMainJointPaper({ el: paperHost });
+
+    // `dragStarted` mounts a "ghost" jointjs paper in this DOM node.
+    const flyingOpHost = document.createElement("div");
+    flyingOpHost.id = "flyingOP";
+    document.body.appendChild(flyingOpHost);
+
+    // jsdom doesn't compute layout, so the paper's `pageToLocalPoint` always
+    // returns (0, 0) regardless of the dispatched mouse position. Place the
+    // input operators at negative x and outputs at positive x so the
+    // origin-at-(0,0) drop point classifies them correctly via
+    // `findClosestOperators` (which compares operator x against mouse x).
+    workflowActionService.addOperator(input1, { x: -100, y: 10 });
+    workflowActionService.addOperator(input2, { x: -100, y: 20 });
+    workflowActionService.addOperator(input3, { x: -100, y: 30 });
+    workflowActionService.addOperator(output1, { x: 100, y: 10 });
+    workflowActionService.addOperator(output2, { x: 100, y: 20 });
+    workflowActionService.addOperator(output3, { x: 100, y: 30 });
+
+    const unhighlights: string[] = [];
+    dragDropService.getOperatorSuggestionUnhighlightStream().subscribe(id => unhighlights.push(id));
+    const links: OperatorLink[] = [];
+    workflowActionService
+      .getTexeraGraph()
+      .getLinkAddStream()
+      .subscribe(link => links.push(link));
+
+    // dragStarted creates a fresh `op` of the given type and subscribes to
+    // window mousemove to populate suggestionInputs / suggestionOutputs.
+    dragDropService.dragStarted("MultiInputOutput");
+    const droppedOp = (dragDropService as any).op as OperatorPredicate;
+
+    // Drive the suggestion pipeline. Any mousemove will do — jsdom's
+    // `pageToLocalPoint` always resolves to (0, 0) since it has no layout,
+    // and we placed inputs/outputs around the origin to match.
+    window.dispatchEvent(new MouseEvent("mousemove", { clientX: 0, clientY: 0 }));
+    await new Promise(resolve => setTimeout(resolve, 0));
+
+    dragDropService.dragDropped({ x: 0, y: 0 });

Review Comment:
   Right — the `mouseup` observer in `dragStarted` only fires once and never gets unsubscribed otherwise. Added a `window.dispatchEvent(new MouseEvent("mouseup"))` after `dragDropped` to flip `isOperatorDropped` and let the `first()` operator complete the subscriptions.



-- 
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: notifications-unsubscribe@texera.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] test(frontend): re-enable two skipped drag-drop tests [texera]

Posted by "aglinxinyuan (via GitHub)" <gi...@apache.org>.
aglinxinyuan commented on PR #4971:
URL: https://github.com/apache/texera/pull/4971#issuecomment-4392867737

   The frontend test took 6 hours and failed. Please take a look:
   https://github.com/apache/texera/actions/runs/25425783736/job/74578713230


-- 
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: notifications-unsubscribe@texera.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] test(frontend): re-enable two skipped drag-drop tests [texera]

Posted by "aglinxinyuan (via GitHub)" <gi...@apache.org>.
aglinxinyuan merged PR #4971:
URL: https://github.com/apache/texera/pull/4971


-- 
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: notifications-unsubscribe@texera.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org