Copilot commented on code in PR #6370:
URL: https://github.com/apache/texera/pull/6370#discussion_r3566356591
##########
frontend/src/app/workspace/service/operator-menu/operator-menu.service.spec.ts:
##########
@@ -32,8 +32,11 @@ import {
mockPoint,
mockResultPredicate,
mockScanPredicate,
+ mockScanSentimentLink,
mockSentimentPredicate,
} from "../workflow-graph/model/mock-workflow-data";
+import { NotificationService } from
"../../../common/service/notification/notification.service";
+import { ExecuteWorkflowService } from
"../execute-workflow/execute-workflow.service";
Review Comment:
This spec currently constructs the real
`ExecuteWorkflowService`/`NotificationService` via `OperatorMenuService` DI,
which pulls in a lot of unrelated dependencies (NgZorro notification services,
websocket heartbeat timers, etc.). Most frontend specs stub these with
`useValue` to keep unit tests isolated (e.g.
`frontend/src/app/workspace/component/workspace.component.spec.ts:168-193`).
Consider stubbing them here too so the new tests only exercise
`OperatorMenuService` behavior.
##########
frontend/src/app/workspace/service/operator-menu/operator-menu.service.spec.ts:
##########
@@ -184,4 +191,257 @@ describe("OperatorMenuService", () => {
expect(service.isToViewResult).toBe(false);
});
});
+
+ describe("operator action callbacks", () => {
+ beforeEach(() => {
+ workflowActionService.addOperator(mockScanPredicate, mockPoint);
+
workflowActionService.getJointGraphWrapper().highlightOperators(mockScanPredicate.operatorID);
+ });
+
+ it("disables then re-enables the highlighted operators based on
isDisableOperator", () => {
+ const texeraGraph = workflowActionService.getTexeraGraph();
+ // fresh non-disabled operator → the button is in "disable" mode.
+ expect(service.isDisableOperator).toBe(true);
+
+ service.disableHighlightedOperators();
+
expect(texeraGraph.isOperatorDisabled(mockScanPredicate.operatorID)).toBe(true);
+ // recompute flips the toggle so the next click re-enables.
+ expect(service.isDisableOperator).toBe(false);
+
+ service.disableHighlightedOperators();
+
expect(texeraGraph.isOperatorDisabled(mockScanPredicate.operatorID)).toBe(false);
+ });
+
+ it("sets then unsets view-result on the highlighted non-sink operators",
() => {
+ const texeraGraph = workflowActionService.getTexeraGraph();
+ expect(service.isToViewResult).toBe(true);
+
+ service.viewResultHighlightedOperators();
+
expect(texeraGraph.isViewingResult(mockScanPredicate.operatorID)).toBe(true);
+ expect(service.isToViewResult).toBe(false);
+
+ service.viewResultHighlightedOperators();
+
expect(texeraGraph.isViewingResult(mockScanPredicate.operatorID)).toBe(false);
+ });
+
+ it("marks then unmarks the highlighted non-sink operators for reuse", ()
=> {
+ const texeraGraph = workflowActionService.getTexeraGraph();
+ expect(service.isMarkForReuse).toBe(true);
+
+ service.reuseResultHighlightedOperator();
+
expect(texeraGraph.isMarkedForReuseResult(mockScanPredicate.operatorID)).toBe(true);
+ expect(service.isMarkForReuse).toBe(false);
+
+ service.reuseResultHighlightedOperator();
+
expect(texeraGraph.isMarkedForReuseResult(mockScanPredicate.operatorID)).toBe(false);
+ });
+ });
+
+ describe("executeUpToOperator", () => {
+ it("errors and does not execute when zero operators are highlighted", ()
=> {
+ const notificationService = TestBed.inject(NotificationService);
+ const executeWorkflowService = TestBed.inject(ExecuteWorkflowService);
+ const errorSpy = vi.spyOn(notificationService,
"error").mockImplementation(() => {});
+ const executeSpy = vi.spyOn(executeWorkflowService,
"executeWorkflow").mockImplementation(() => undefined);
+
+ service.executeUpToOperator();
+
+ expect(errorSpy).toHaveBeenCalledWith("Can only execute to exactly one
target operator.");
+ expect(executeSpy).not.toHaveBeenCalled();
+ });
+
+ it("errors and does not execute when more than one operator is
highlighted", () => {
+ const notificationService = TestBed.inject(NotificationService);
+ const executeWorkflowService = TestBed.inject(ExecuteWorkflowService);
+ const errorSpy = vi.spyOn(notificationService,
"error").mockImplementation(() => {});
+ const executeSpy = vi.spyOn(executeWorkflowService,
"executeWorkflow").mockImplementation(() => undefined);
+ workflowActionService.addOperatorsAndLinks(
+ [
+ { op: mockScanPredicate, pos: mockPoint },
+ { op: mockSentimentPredicate, pos: mockPoint },
+ ],
+ []
+ );
+ const wrapper = workflowActionService.getJointGraphWrapper();
+
wrapper.unhighlightOperators(...wrapper.getCurrentHighlightedOperatorIDs());
+ wrapper.setMultiSelectMode(true);
+ wrapper.highlightOperators(mockScanPredicate.operatorID,
mockSentimentPredicate.operatorID);
+
+ service.executeUpToOperator();
+
+ expect(errorSpy).toHaveBeenCalledWith("Can only execute to exactly one
target operator.");
+ expect(executeSpy).not.toHaveBeenCalled();
+ });
+
+ it("executes the workflow up to the single highlighted operator", () => {
+ const executeWorkflowService = TestBed.inject(ExecuteWorkflowService);
+ const executeSpy = vi.spyOn(executeWorkflowService,
"executeWorkflow").mockImplementation(() => undefined);
+ workflowActionService.addOperator(mockScanPredicate, mockPoint);
+
workflowActionService.getJointGraphWrapper().highlightOperators(mockScanPredicate.operatorID);
+
+ service.executeUpToOperator();
+
+ expect(executeSpy).toHaveBeenCalledWith("",
mockScanPredicate.operatorID);
+ });
+ });
+
+ describe("saveHighlightedElements", () => {
+ let originalClipboard: PropertyDescriptor | undefined;
+ let writeText: ReturnType<typeof vi.fn>;
+
+ beforeEach(() => {
+ originalClipboard = Object.getOwnPropertyDescriptor(navigator,
"clipboard");
+ writeText = vi.fn().mockResolvedValue(undefined);
+ Object.defineProperty(navigator, "clipboard", { value: { writeText },
configurable: true });
+ });
+
+ afterEach(() => {
+ if (originalClipboard) {
+ Object.defineProperty(navigator, "clipboard", originalClipboard);
+ } else {
+ delete (navigator as any).clipboard;
+ }
+ });
+
+ it("serializes the highlighted operators, links, and comment boxes to the
clipboard", () => {
+ // give the comment box a distinct ID: operators and comment boxes share
the joint-graph
+ // cell namespace, and mockCommentBox reuses operator ID "1".
+ const commentBox = { ...mockCommentBox, commentBoxID: "comment-box-a" };
+ workflowActionService.addOperatorsAndLinks(
+ [
+ { op: mockScanPredicate, pos: mockPoint },
+ { op: mockSentimentPredicate, pos: mockPoint },
+ ],
+ [mockScanSentimentLink]
+ );
+ workflowActionService.addCommentBox(commentBox);
+ const wrapper = workflowActionService.getJointGraphWrapper();
+ // start from a clean highlight state, then multi-select every element
we want copied.
+
wrapper.unhighlightOperators(...wrapper.getCurrentHighlightedOperatorIDs());
+ wrapper.setMultiSelectMode(true);
+ wrapper.highlightOperators(mockScanPredicate.operatorID,
mockSentimentPredicate.operatorID);
+ wrapper.highlightLinks(mockScanSentimentLink.linkID);
+ wrapper.highlightCommentBoxes(commentBox.commentBoxID);
+
+ service.saveHighlightedElements();
+
+ expect(writeText).toHaveBeenCalledTimes(1);
+ const serialized = JSON.parse(writeText.mock.calls[0][0]);
+ expect(serialized.operators.map((op: any) =>
op.operatorID).sort()).toEqual(["1", "2"]);
+ expect(Object.keys(serialized.operatorPositions).sort()).toEqual(["1",
"2"]);
Review Comment:
These assertions hard-code operator IDs as "1"/"2", making the test brittle
if mock IDs ever change. Prefer asserting against
`mockScanPredicate.operatorID` / `mockSentimentPredicate.operatorID` so the
test remains coupled to the fixtures rather than specific literal values.
##########
frontend/src/app/workspace/service/operator-menu/operator-menu.service.spec.ts:
##########
@@ -184,4 +191,257 @@ describe("OperatorMenuService", () => {
expect(service.isToViewResult).toBe(false);
});
});
+
+ describe("operator action callbacks", () => {
+ beforeEach(() => {
+ workflowActionService.addOperator(mockScanPredicate, mockPoint);
+
workflowActionService.getJointGraphWrapper().highlightOperators(mockScanPredicate.operatorID);
+ });
+
+ it("disables then re-enables the highlighted operators based on
isDisableOperator", () => {
+ const texeraGraph = workflowActionService.getTexeraGraph();
+ // fresh non-disabled operator → the button is in "disable" mode.
+ expect(service.isDisableOperator).toBe(true);
+
+ service.disableHighlightedOperators();
+
expect(texeraGraph.isOperatorDisabled(mockScanPredicate.operatorID)).toBe(true);
+ // recompute flips the toggle so the next click re-enables.
+ expect(service.isDisableOperator).toBe(false);
+
+ service.disableHighlightedOperators();
+
expect(texeraGraph.isOperatorDisabled(mockScanPredicate.operatorID)).toBe(false);
+ });
+
+ it("sets then unsets view-result on the highlighted non-sink operators",
() => {
+ const texeraGraph = workflowActionService.getTexeraGraph();
+ expect(service.isToViewResult).toBe(true);
+
+ service.viewResultHighlightedOperators();
+
expect(texeraGraph.isViewingResult(mockScanPredicate.operatorID)).toBe(true);
+ expect(service.isToViewResult).toBe(false);
+
+ service.viewResultHighlightedOperators();
+
expect(texeraGraph.isViewingResult(mockScanPredicate.operatorID)).toBe(false);
+ });
+
+ it("marks then unmarks the highlighted non-sink operators for reuse", ()
=> {
+ const texeraGraph = workflowActionService.getTexeraGraph();
+ expect(service.isMarkForReuse).toBe(true);
+
+ service.reuseResultHighlightedOperator();
+
expect(texeraGraph.isMarkedForReuseResult(mockScanPredicate.operatorID)).toBe(true);
+ expect(service.isMarkForReuse).toBe(false);
+
+ service.reuseResultHighlightedOperator();
+
expect(texeraGraph.isMarkedForReuseResult(mockScanPredicate.operatorID)).toBe(false);
+ });
+ });
+
+ describe("executeUpToOperator", () => {
+ it("errors and does not execute when zero operators are highlighted", ()
=> {
+ const notificationService = TestBed.inject(NotificationService);
+ const executeWorkflowService = TestBed.inject(ExecuteWorkflowService);
+ const errorSpy = vi.spyOn(notificationService,
"error").mockImplementation(() => {});
+ const executeSpy = vi.spyOn(executeWorkflowService,
"executeWorkflow").mockImplementation(() => undefined);
+
+ service.executeUpToOperator();
+
+ expect(errorSpy).toHaveBeenCalledWith("Can only execute to exactly one
target operator.");
+ expect(executeSpy).not.toHaveBeenCalled();
+ });
+
+ it("errors and does not execute when more than one operator is
highlighted", () => {
+ const notificationService = TestBed.inject(NotificationService);
+ const executeWorkflowService = TestBed.inject(ExecuteWorkflowService);
+ const errorSpy = vi.spyOn(notificationService,
"error").mockImplementation(() => {});
+ const executeSpy = vi.spyOn(executeWorkflowService,
"executeWorkflow").mockImplementation(() => undefined);
+ workflowActionService.addOperatorsAndLinks(
+ [
+ { op: mockScanPredicate, pos: mockPoint },
+ { op: mockSentimentPredicate, pos: mockPoint },
+ ],
+ []
+ );
+ const wrapper = workflowActionService.getJointGraphWrapper();
+
wrapper.unhighlightOperators(...wrapper.getCurrentHighlightedOperatorIDs());
+ wrapper.setMultiSelectMode(true);
+ wrapper.highlightOperators(mockScanPredicate.operatorID,
mockSentimentPredicate.operatorID);
+
+ service.executeUpToOperator();
+
+ expect(errorSpy).toHaveBeenCalledWith("Can only execute to exactly one
target operator.");
+ expect(executeSpy).not.toHaveBeenCalled();
+ });
+
+ it("executes the workflow up to the single highlighted operator", () => {
+ const executeWorkflowService = TestBed.inject(ExecuteWorkflowService);
+ const executeSpy = vi.spyOn(executeWorkflowService,
"executeWorkflow").mockImplementation(() => undefined);
+ workflowActionService.addOperator(mockScanPredicate, mockPoint);
+
workflowActionService.getJointGraphWrapper().highlightOperators(mockScanPredicate.operatorID);
+
+ service.executeUpToOperator();
+
+ expect(executeSpy).toHaveBeenCalledWith("",
mockScanPredicate.operatorID);
+ });
+ });
+
+ describe("saveHighlightedElements", () => {
+ let originalClipboard: PropertyDescriptor | undefined;
+ let writeText: ReturnType<typeof vi.fn>;
+
+ beforeEach(() => {
+ originalClipboard = Object.getOwnPropertyDescriptor(navigator,
"clipboard");
+ writeText = vi.fn().mockResolvedValue(undefined);
+ Object.defineProperty(navigator, "clipboard", { value: { writeText },
configurable: true });
+ });
+
+ afterEach(() => {
+ if (originalClipboard) {
+ Object.defineProperty(navigator, "clipboard", originalClipboard);
+ } else {
+ delete (navigator as any).clipboard;
+ }
+ });
+
+ it("serializes the highlighted operators, links, and comment boxes to the
clipboard", () => {
+ // give the comment box a distinct ID: operators and comment boxes share
the joint-graph
+ // cell namespace, and mockCommentBox reuses operator ID "1".
+ const commentBox = { ...mockCommentBox, commentBoxID: "comment-box-a" };
+ workflowActionService.addOperatorsAndLinks(
+ [
+ { op: mockScanPredicate, pos: mockPoint },
+ { op: mockSentimentPredicate, pos: mockPoint },
+ ],
+ [mockScanSentimentLink]
+ );
+ workflowActionService.addCommentBox(commentBox);
+ const wrapper = workflowActionService.getJointGraphWrapper();
+ // start from a clean highlight state, then multi-select every element
we want copied.
+
wrapper.unhighlightOperators(...wrapper.getCurrentHighlightedOperatorIDs());
+ wrapper.setMultiSelectMode(true);
+ wrapper.highlightOperators(mockScanPredicate.operatorID,
mockSentimentPredicate.operatorID);
+ wrapper.highlightLinks(mockScanSentimentLink.linkID);
+ wrapper.highlightCommentBoxes(commentBox.commentBoxID);
+
+ service.saveHighlightedElements();
+
+ expect(writeText).toHaveBeenCalledTimes(1);
+ const serialized = JSON.parse(writeText.mock.calls[0][0]);
+ expect(serialized.operators.map((op: any) =>
op.operatorID).sort()).toEqual(["1", "2"]);
+ expect(Object.keys(serialized.operatorPositions).sort()).toEqual(["1",
"2"]);
+ // the serialized position mirrors what the graph reports for that
operator.
+
expect(serialized.operatorPositions[mockScanPredicate.operatorID]).toEqual(
+ wrapper.getElementPosition(mockScanPredicate.operatorID)
+ );
+ expect(serialized.links.map((link: any) =>
link.linkID)).toEqual([mockScanSentimentLink.linkID]);
+ expect(serialized.commentBoxes.map((box: any) =>
box.commentBoxID)).toEqual([commentBox.commentBoxID]);
+ });
+
+ it("notifies the user when writing to the clipboard is rejected", async ()
=> {
+ writeText.mockRejectedValue(new Error("no permission"));
+ const notificationService = TestBed.inject(NotificationService);
+ const errorSpy = vi.spyOn(notificationService,
"error").mockImplementation(() => {});
+ workflowActionService.addOperator(mockScanPredicate, mockPoint);
+
workflowActionService.getJointGraphWrapper().highlightOperators(mockScanPredicate.operatorID);
+
+ service.saveHighlightedElements();
+ await flushAsync();
+
+ expect(errorSpy).toHaveBeenCalledWith("Copy failed. You don't have the
permission to write to the clipboard.");
+ });
+ });
+
+ describe("performPasteOperation", () => {
+ let originalClipboard: PropertyDescriptor | undefined;
+ let readText: ReturnType<typeof vi.fn>;
+
+ beforeEach(() => {
+ originalClipboard = Object.getOwnPropertyDescriptor(navigator,
"clipboard");
+ readText = vi.fn();
+ Object.defineProperty(navigator, "clipboard", { value: { readText },
configurable: true });
+ });
+
+ afterEach(() => {
+ if (originalClipboard) {
+ Object.defineProperty(navigator, "clipboard", originalClipboard);
+ } else {
+ delete (navigator as any).clipboard;
+ }
+ });
+
+ it("pastes operators, links, and comment boxes with fresh IDs and
non-overlapping positions", async () => {
+ // both operators share the same clipboard position, so pasting the
second one must be
+ // shifted by the getNonOverlappingPosition loop to avoid landing on the
first.
+ const clipboard = {
+ operators: [mockScanPredicate, mockSentimentPredicate],
+ operatorPositions: {
+ [mockScanPredicate.operatorID]: { x: 100, y: 100 },
+ [mockSentimentPredicate.operatorID]: { x: 100, y: 100 },
+ },
+ links: [mockScanSentimentLink],
+ commentBoxes: [mockCommentBox],
+ };
+ readText.mockResolvedValue(JSON.stringify(clipboard));
+ const texeraGraph = workflowActionService.getTexeraGraph();
+
+ service.performPasteOperation();
+ await flushAsync();
+
+ const pastedOperators = texeraGraph.getAllOperators();
+ expect(pastedOperators.length).toBe(2);
+ // fresh IDs are assigned; none of the originals are reused.
+ expect(pastedOperators.map(op =>
op.operatorID)).not.toContain(mockScanPredicate.operatorID);
+ expect(pastedOperators.map(op =>
op.operatorID)).not.toContain(mockSentimentPredicate.operatorID);
+ expect(texeraGraph.getAllLinks().length).toBe(1);
+ expect(texeraGraph.getAllCommentBoxes().length).toBe(1);
+ // the pasted comment box is a distinct copy, not the original.
+
expect(texeraGraph.getAllCommentBoxes()[0].commentBoxID).not.toBe(mockCommentBox.commentBoxID);
Review Comment:
The test title/comment says it verifies non-overlapping paste positions, but
the assertions only check counts/IDs. Consider adding a position assertion (via
`workflowActionService.getJointGraphWrapper().getElementPosition(...)`) so the
test actually validates the non-overlap behavior it describes.
--
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]
