Xiao-zhen-Liu commented on code in PR #5146:
URL: https://github.com/apache/texera/pull/5146#discussion_r3320350405


##########
frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.spec.ts:
##########
@@ -232,99 +233,107 @@ describe("WorkflowEditorComponent", () => {
       fixture.detectChanges();
     });
 
-    it("should try to highlight the operator when user mouse clicks on an 
operator", () => {
-      const jointGraphWrapper = workflowActionService.getJointGraphWrapper();
-      // install a spy on the highlight operator function and pass the call 
through
-      vi.spyOn(jointGraphWrapper, "highlightOperators");
-      workflowActionService.addOperator(mockScanPredicate, mockPoint);
-
-      // unhighlight the operator in case it's automatically highlighted
-      jointGraphWrapper.unhighlightOperators(mockScanPredicate.operatorID);
-
-      // find the joint Cell View object of the operator element
-      const jointCellView = 
component.paper.findViewByModel(mockScanPredicate.operatorID);
-      jointCellView.$el.trigger("mousedown");
-
-      fixture.detectChanges();
-
-      // assert the function is called once
-      // expect(highlightOperatorFunctionSpy.calls.count()).toEqual(1);
-      // assert the highlighted operator is correct
-      
expect(jointGraphWrapper.getCurrentHighlightedOperatorIDs()).toEqual([mockScanPredicate.operatorID]);
-    });
-
-    it("should highlight the commentBox when user clicks on a commentBox", () 
=> {
-      const jointGraphWrapper = workflowActionService.getJointGraphWrapper();
-      vi.spyOn(jointGraphWrapper, "highlightCommentBoxes");
-      workflowActionService.addCommentBox(mockCommentBox);
-      jointGraphWrapper.unhighlightCommentBoxes(mockCommentBox.commentBoxID);
-      const jointCellView = 
component.paper.findViewByModel(mockCommentBox.commentBoxID);
-      jointCellView.$el.trigger("mousedown");
-      fixture.detectChanges();
-      
expect(jointGraphWrapper.getCurrentHighlightedCommentBoxIDs()).toEqual([mockCommentBox.commentBoxID]);
-    });
-
-    it("should open commentBox as NzModal when user double clicks on a 
commentBox", () => {
-      const modalRef: NzModalRef = nzModalService.create({
-        nzTitle: "CommentBox",
-        nzContent: NzModalCommentBoxComponent,
-        nzData: { commentBox: createYTypeFromObject(mockCommentBox) },
-        nzAutofocus: null,
-        nzFooter: [
-          {
-            label: "OK",
-            onClick: () => {
-              modalRef.destroy();
-            },
-            type: "primary",
-          },
-        ],
-      });
-      vi.spyOn(nzModalService, "create").mockReturnValue(modalRef);
-      const jointGraphWrapper = workflowActionService.getJointGraphWrapper();
-      workflowActionService.addCommentBox(mockCommentBox);
-      jointGraphWrapper.highlightCommentBoxes(mockCommentBox.commentBoxID);
-      const jointCellView = 
component.paper.findViewByModel(mockCommentBox.commentBoxID);
-      jointCellView.$el.trigger("dblclick");
-      expect(nzModalService.create).toHaveBeenCalled();
-      fixture.detectChanges();
-      modalRef.destroy();
-    });
-
-    it("should unhighlight all highlighted operators when user mouse clicks on 
the blank space", () => {
-      const jointGraphWrapper = workflowActionService.getJointGraphWrapper();
-
-      // add and highlight two operators
-      workflowActionService.addOperatorsAndLinks(
-        [
-          { op: mockScanPredicate, pos: mockPoint },
-          { op: mockResultPredicate, pos: mockPoint },
-        ],
-        []
-      );
-      jointGraphWrapper.highlightOperators(mockScanPredicate.operatorID, 
mockResultPredicate.operatorID);
-
-      // assert that both operators are highlighted
-      
expect(jointGraphWrapper.getCurrentHighlightedOperatorIDs()).toContain(mockScanPredicate.operatorID);
-      
expect(jointGraphWrapper.getCurrentHighlightedOperatorIDs()).toContain(mockResultPredicate.operatorID);
-
-      // find a blank area on the JointJS paper
-      const blankPoint = { x: mockPoint.x + 100, y: mockPoint.y + 100 };
-      expect(component.paper.findViewsFromPoint(blankPoint)).toEqual([]);
-
-      // trigger a click on the blank area using JointJS paper's jQuery element
-      const point = component.paper.localToClientPoint(blankPoint);
-      const event = createJQueryEvent("mousedown", {
-        clientX: point.x,
-        clientY: point.y,
-      });
-      component.paper.$el.trigger(event);
-
-      fixture.detectChanges();
-
-      // assert that all operators are unhighlighted
-      expect(jointGraphWrapper.getCurrentHighlightedOperatorIDs()).toEqual([]);
-    });
+    // TODO(#3614): the following four mouse/click-event tests rely on JointJS

Review Comment:
   These 6 tests are the browser-only coverage for this file, and the 
`test-browser` target (`angular.json:105`) runs *only* this spec. Since they're 
commented out *here*, they no longer run under `ng run gui:test-browser` either 
— the PR description's claim that they "continue to pass" there isn't accurate; 
commenting them out drops them from every target.
   
   Prefer `it.skip(...)`/`xit(...)` so they still appear as explicitly skipped 
(discoverable, not silently lost), or better, a runtime guard that skips under 
jsdom but executes under the real-Chrome `test-browser` runner — that keeps the 
coverage the browser target exists to provide. ~150 lines of commented-out test 
code will rot quickly and a follow-up PR to "revive" them rarely happens.



-- 
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]

Reply via email to