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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
