aglinxinyuan commented on code in PR #6352:
URL: https://github.com/apache/texera/pull/6352#discussion_r3565355053
##########
frontend/src/app/workspace/service/workflow-graph/model/workflow-action.service.spec.ts:
##########
@@ -277,4 +279,305 @@ describe("WorkflowActionService", () => {
// expect(sentimentOpPos).toEqual(mockPoint);
// expect(resultOpPos).toEqual(mockPoint);
});
+
+ it("should reload a workflow, repopulating the graph and clearing the
undo/redo stacks", () => {
+ const settings: WorkflowSettings = {
+ dataTransferBatchSize: 250,
+ executionMode: ExecutionMode.MATERIALIZED,
+ };
+ const workflow: Workflow = {
+ ...DEFAULT_WORKFLOW,
+ name: "Reloaded WF",
+ content: {
+ operators: [mockScanPredicate, mockResultPredicate],
+ operatorPositions: {
+ [mockScanPredicate.operatorID]: mockPoint,
+ [mockResultPredicate.operatorID]: mockPoint,
+ },
+ links: [mockScanResultLink],
+ commentBoxes: [{ ...mockCommentBox, commentBoxID: "commentBox-1" }],
+ settings,
+ },
+ };
+ const clearUndoSpy = vi.spyOn(undoRedo, "clearUndoStack");
+ const clearRedoSpy = vi.spyOn(undoRedo, "clearRedoStack");
+ const restoreSpy = vi.spyOn(service.getJointGraphWrapper(),
"restoreDefaultZoomAndOffset");
+
+ service.reloadWorkflow(workflow);
+
+ expect(texeraGraph.hasOperator(mockScanPredicate.operatorID)).toBeTruthy();
+
expect(texeraGraph.hasOperator(mockResultPredicate.operatorID)).toBeTruthy();
+ expect(texeraGraph.hasLinkWithID(mockScanResultLink.linkID)).toBeTruthy();
+ expect(texeraGraph.hasCommentBox("commentBox-1")).toBeTruthy();
+ expect(service.getWorkflowMetadata().name).toEqual("Reloaded WF");
+ expect(service.getWorkflowSettings().dataTransferBatchSize).toEqual(250);
+ expect(clearUndoSpy).toHaveBeenCalled();
+ expect(clearRedoSpy).toHaveBeenCalled();
+ expect(restoreSpy).toHaveBeenCalled();
+ });
+
+ it("should throw when a reloaded operator is missing its position", () => {
+ const workflow: Workflow = {
+ ...DEFAULT_WORKFLOW,
+ content: {
+ operators: [mockScanPredicate],
+ operatorPositions: {},
+ links: [],
+ commentBoxes: [],
+ settings: { dataTransferBatchSize: 100, executionMode:
ExecutionMode.PIPELINED },
+ },
+ };
+ expect(() => service.reloadWorkflow(workflow)).toThrowError(
+ new RegExp(`position error: ${mockScanPredicate.operatorID}`)
+ );
+ });
+
+ it("should empty the graph and skip viewport restore when reloading
undefined", () => {
+ service.addOperator(mockScanPredicate, mockPoint);
+ const restoreSpy = vi.spyOn(service.getJointGraphWrapper(),
"restoreDefaultZoomAndOffset");
+
+ service.reloadWorkflow(undefined, false, false);
+
+ expect(texeraGraph.getAllOperators().length).toEqual(0);
+ expect(restoreSpy).not.toHaveBeenCalled();
+ });
+
+ it("should clear the workflow back to its defaults", () => {
+ service.addOperator(mockScanPredicate, mockPoint);
+ service.setWorkflowName("Something");
+ service.setWorkflowDataTransferBatchSize(999);
+ service.setHighlightingEnabled(true);
+
+ service.clearWorkflow();
+
+ expect(service.getWorkflowMetadata()).toBe(DEFAULT_WORKFLOW);
Review Comment:
Changed to `toEqual(DEFAULT_WORKFLOW)` — a value comparison, so it no longer
depends on the metadata being the exact same object reference (3f322ca).
##########
frontend/src/app/workspace/service/workflow-graph/model/workflow-action.service.spec.ts:
##########
@@ -277,4 +279,305 @@ describe("WorkflowActionService", () => {
// expect(sentimentOpPos).toEqual(mockPoint);
// expect(resultOpPos).toEqual(mockPoint);
});
+
+ it("should reload a workflow, repopulating the graph and clearing the
undo/redo stacks", () => {
+ const settings: WorkflowSettings = {
+ dataTransferBatchSize: 250,
+ executionMode: ExecutionMode.MATERIALIZED,
+ };
+ const workflow: Workflow = {
+ ...DEFAULT_WORKFLOW,
+ name: "Reloaded WF",
+ content: {
+ operators: [mockScanPredicate, mockResultPredicate],
+ operatorPositions: {
+ [mockScanPredicate.operatorID]: mockPoint,
+ [mockResultPredicate.operatorID]: mockPoint,
+ },
+ links: [mockScanResultLink],
+ commentBoxes: [{ ...mockCommentBox, commentBoxID: "commentBox-1" }],
+ settings,
+ },
+ };
+ const clearUndoSpy = vi.spyOn(undoRedo, "clearUndoStack");
+ const clearRedoSpy = vi.spyOn(undoRedo, "clearRedoStack");
+ const restoreSpy = vi.spyOn(service.getJointGraphWrapper(),
"restoreDefaultZoomAndOffset");
+
+ service.reloadWorkflow(workflow);
+
+ expect(texeraGraph.hasOperator(mockScanPredicate.operatorID)).toBeTruthy();
+
expect(texeraGraph.hasOperator(mockResultPredicate.operatorID)).toBeTruthy();
+ expect(texeraGraph.hasLinkWithID(mockScanResultLink.linkID)).toBeTruthy();
+ expect(texeraGraph.hasCommentBox("commentBox-1")).toBeTruthy();
+ expect(service.getWorkflowMetadata().name).toEqual("Reloaded WF");
+ expect(service.getWorkflowSettings().dataTransferBatchSize).toEqual(250);
+ expect(clearUndoSpy).toHaveBeenCalled();
+ expect(clearRedoSpy).toHaveBeenCalled();
+ expect(restoreSpy).toHaveBeenCalled();
+ });
+
+ it("should throw when a reloaded operator is missing its position", () => {
+ const workflow: Workflow = {
+ ...DEFAULT_WORKFLOW,
+ content: {
+ operators: [mockScanPredicate],
+ operatorPositions: {},
+ links: [],
+ commentBoxes: [],
+ settings: { dataTransferBatchSize: 100, executionMode:
ExecutionMode.PIPELINED },
+ },
+ };
+ expect(() => service.reloadWorkflow(workflow)).toThrowError(
+ new RegExp(`position error: ${mockScanPredicate.operatorID}`)
+ );
+ });
+
+ it("should empty the graph and skip viewport restore when reloading
undefined", () => {
+ service.addOperator(mockScanPredicate, mockPoint);
+ const restoreSpy = vi.spyOn(service.getJointGraphWrapper(),
"restoreDefaultZoomAndOffset");
+
+ service.reloadWorkflow(undefined, false, false);
+
+ expect(texeraGraph.getAllOperators().length).toEqual(0);
+ expect(restoreSpy).not.toHaveBeenCalled();
+ });
+
+ it("should clear the workflow back to its defaults", () => {
+ service.addOperator(mockScanPredicate, mockPoint);
+ service.setWorkflowName("Something");
+ service.setWorkflowDataTransferBatchSize(999);
+ service.setHighlightingEnabled(true);
+
+ service.clearWorkflow();
+
+ expect(service.getWorkflowMetadata()).toBe(DEFAULT_WORKFLOW);
+ expect(service.getWorkflowSettings().dataTransferBatchSize).toEqual(100);
+
expect(service.getWorkflowSettings().executionMode).toEqual(ExecutionMode.PIPELINED);
+ expect(texeraGraph.getAllOperators().length).toEqual(0);
+ expect(service.getHighlightingEnabled()).toBeFalsy();
+ });
+
+ it("should emit on workflowChanged when resetting as a new workflow", () => {
+ let changed = false;
+ service.workflowChanged().subscribe(() => (changed = true));
+
+ service.resetAsNewWorkflow();
+
+ expect(changed).toBeTruthy();
+ });
+
+ it("should only emit metadata changes when the reference changes", () => {
+ const current = service.getWorkflowMetadata();
+ let emitCount = 0;
+ service.workflowMetaDataChanged().subscribe(() => (emitCount += 1));
+
+ // identical reference -> no emission
+ service.setWorkflowMetadata(current);
+ expect(emitCount).toEqual(0);
+
+ // new metadata -> emission
+ service.setWorkflowMetadata({ ...DEFAULT_WORKFLOW, name: "renamed" });
+ expect(emitCount).toEqual(1);
+ expect(service.getWorkflowMetadata().name).toEqual("renamed");
+ });
+
+ it("should set the workflow name, defaulting blank names", () => {
+ service.setWorkflowName(" ");
+ expect(service.getWorkflowMetadata().name).toEqual(DEFAULT_WORKFLOW_NAME);
+
+ service.setWorkflowName("My Workflow");
+ expect(service.getWorkflowMetadata().name).toEqual("My Workflow");
+ });
+
+ it("should manage workflow settings and publish state", () => {
+ expect(service.getWorkflowSettings().dataTransferBatchSize).toEqual(100);
+
expect(service.getWorkflowSettings().executionMode).toEqual(ExecutionMode.PIPELINED);
+
+ // non-positive batch sizes are ignored, positive ones are applied
+ service.setWorkflowDataTransferBatchSize(0);
+ expect(service.getWorkflowSettings().dataTransferBatchSize).toEqual(100);
+ service.setWorkflowDataTransferBatchSize(400);
+ expect(service.getWorkflowSettings().dataTransferBatchSize).toEqual(400);
+
+ service.updateExecutionMode(ExecutionMode.MATERIALIZED);
+
expect(service.getWorkflowSettings().executionMode).toEqual(ExecutionMode.MATERIALIZED);
+
+ service.setWorkflowIsPublished(1);
+ expect(service.getWorkflowMetadata().isPublished).toEqual(1);
+
+ const settings: WorkflowSettings = { dataTransferBatchSize: 42,
executionMode: ExecutionMode.PIPELINED };
+ service.setWorkflowSettings(settings);
+ expect(service.getWorkflowSettings()).toEqual(settings);
+ });
+
+ it("should toggle the shared-editing connection through temp workflow", ()
=> {
+ const wsProvider = texeraGraph.sharedModel.wsProvider;
+ const disconnectSpy = vi.spyOn(wsProvider,
"disconnect").mockImplementation(() => {});
+ const connectSpy = vi.spyOn(wsProvider, "connect").mockImplementation(()
=> {});
Review Comment:
Now forces `wsProvider.shouldConnect = true` before `setTempWorkflow`, so
the `disconnect()` assertion is deterministic regardless of the provider's
default state (3f322ca).
##########
frontend/src/app/workspace/service/workflow-graph/model/workflow-action.service.spec.ts:
##########
@@ -277,4 +279,305 @@ describe("WorkflowActionService", () => {
// expect(sentimentOpPos).toEqual(mockPoint);
// expect(resultOpPos).toEqual(mockPoint);
});
+
+ it("should reload a workflow, repopulating the graph and clearing the
undo/redo stacks", () => {
+ const settings: WorkflowSettings = {
+ dataTransferBatchSize: 250,
+ executionMode: ExecutionMode.MATERIALIZED,
+ };
+ const workflow: Workflow = {
+ ...DEFAULT_WORKFLOW,
+ name: "Reloaded WF",
+ content: {
+ operators: [mockScanPredicate, mockResultPredicate],
+ operatorPositions: {
+ [mockScanPredicate.operatorID]: mockPoint,
+ [mockResultPredicate.operatorID]: mockPoint,
+ },
+ links: [mockScanResultLink],
+ commentBoxes: [{ ...mockCommentBox, commentBoxID: "commentBox-1" }],
+ settings,
+ },
+ };
+ const clearUndoSpy = vi.spyOn(undoRedo, "clearUndoStack");
+ const clearRedoSpy = vi.spyOn(undoRedo, "clearRedoStack");
+ const restoreSpy = vi.spyOn(service.getJointGraphWrapper(),
"restoreDefaultZoomAndOffset");
+
+ service.reloadWorkflow(workflow);
+
+ expect(texeraGraph.hasOperator(mockScanPredicate.operatorID)).toBeTruthy();
+
expect(texeraGraph.hasOperator(mockResultPredicate.operatorID)).toBeTruthy();
+ expect(texeraGraph.hasLinkWithID(mockScanResultLink.linkID)).toBeTruthy();
+ expect(texeraGraph.hasCommentBox("commentBox-1")).toBeTruthy();
+ expect(service.getWorkflowMetadata().name).toEqual("Reloaded WF");
+ expect(service.getWorkflowSettings().dataTransferBatchSize).toEqual(250);
+ expect(clearUndoSpy).toHaveBeenCalled();
+ expect(clearRedoSpy).toHaveBeenCalled();
+ expect(restoreSpy).toHaveBeenCalled();
+ });
+
+ it("should throw when a reloaded operator is missing its position", () => {
+ const workflow: Workflow = {
+ ...DEFAULT_WORKFLOW,
+ content: {
+ operators: [mockScanPredicate],
+ operatorPositions: {},
+ links: [],
+ commentBoxes: [],
+ settings: { dataTransferBatchSize: 100, executionMode:
ExecutionMode.PIPELINED },
+ },
+ };
+ expect(() => service.reloadWorkflow(workflow)).toThrowError(
+ new RegExp(`position error: ${mockScanPredicate.operatorID}`)
+ );
+ });
+
+ it("should empty the graph and skip viewport restore when reloading
undefined", () => {
+ service.addOperator(mockScanPredicate, mockPoint);
+ const restoreSpy = vi.spyOn(service.getJointGraphWrapper(),
"restoreDefaultZoomAndOffset");
+
+ service.reloadWorkflow(undefined, false, false);
+
+ expect(texeraGraph.getAllOperators().length).toEqual(0);
+ expect(restoreSpy).not.toHaveBeenCalled();
+ });
+
+ it("should clear the workflow back to its defaults", () => {
+ service.addOperator(mockScanPredicate, mockPoint);
+ service.setWorkflowName("Something");
+ service.setWorkflowDataTransferBatchSize(999);
+ service.setHighlightingEnabled(true);
+
+ service.clearWorkflow();
+
+ expect(service.getWorkflowMetadata()).toBe(DEFAULT_WORKFLOW);
+ expect(service.getWorkflowSettings().dataTransferBatchSize).toEqual(100);
+
expect(service.getWorkflowSettings().executionMode).toEqual(ExecutionMode.PIPELINED);
+ expect(texeraGraph.getAllOperators().length).toEqual(0);
+ expect(service.getHighlightingEnabled()).toBeFalsy();
+ });
+
+ it("should emit on workflowChanged when resetting as a new workflow", () => {
+ let changed = false;
+ service.workflowChanged().subscribe(() => (changed = true));
+
+ service.resetAsNewWorkflow();
+
+ expect(changed).toBeTruthy();
+ });
+
+ it("should only emit metadata changes when the reference changes", () => {
+ const current = service.getWorkflowMetadata();
+ let emitCount = 0;
+ service.workflowMetaDataChanged().subscribe(() => (emitCount += 1));
+
+ // identical reference -> no emission
+ service.setWorkflowMetadata(current);
+ expect(emitCount).toEqual(0);
+
+ // new metadata -> emission
+ service.setWorkflowMetadata({ ...DEFAULT_WORKFLOW, name: "renamed" });
+ expect(emitCount).toEqual(1);
+ expect(service.getWorkflowMetadata().name).toEqual("renamed");
+ });
+
+ it("should set the workflow name, defaulting blank names", () => {
+ service.setWorkflowName(" ");
+ expect(service.getWorkflowMetadata().name).toEqual(DEFAULT_WORKFLOW_NAME);
+
+ service.setWorkflowName("My Workflow");
+ expect(service.getWorkflowMetadata().name).toEqual("My Workflow");
+ });
+
+ it("should manage workflow settings and publish state", () => {
+ expect(service.getWorkflowSettings().dataTransferBatchSize).toEqual(100);
+
expect(service.getWorkflowSettings().executionMode).toEqual(ExecutionMode.PIPELINED);
+
+ // non-positive batch sizes are ignored, positive ones are applied
+ service.setWorkflowDataTransferBatchSize(0);
+ expect(service.getWorkflowSettings().dataTransferBatchSize).toEqual(100);
+ service.setWorkflowDataTransferBatchSize(400);
+ expect(service.getWorkflowSettings().dataTransferBatchSize).toEqual(400);
+
+ service.updateExecutionMode(ExecutionMode.MATERIALIZED);
+
expect(service.getWorkflowSettings().executionMode).toEqual(ExecutionMode.MATERIALIZED);
+
+ service.setWorkflowIsPublished(1);
+ expect(service.getWorkflowMetadata().isPublished).toEqual(1);
+
+ const settings: WorkflowSettings = { dataTransferBatchSize: 42,
executionMode: ExecutionMode.PIPELINED };
+ service.setWorkflowSettings(settings);
+ expect(service.getWorkflowSettings()).toEqual(settings);
+ });
+
+ it("should toggle the shared-editing connection through temp workflow", ()
=> {
+ const wsProvider = texeraGraph.sharedModel.wsProvider;
+ const disconnectSpy = vi.spyOn(wsProvider,
"disconnect").mockImplementation(() => {});
+ const connectSpy = vi.spyOn(wsProvider, "connect").mockImplementation(()
=> {});
+ const tempWorkflow: Workflow = {
+ ...DEFAULT_WORKFLOW,
+ content: {
+ operators: [],
+ operatorPositions: {},
+ links: [],
+ commentBoxes: [],
+ settings: { dataTransferBatchSize: 100, executionMode:
ExecutionMode.PIPELINED },
+ },
+ };
+
+ service.setTempWorkflow(tempWorkflow);
+ expect(disconnectSpy).toHaveBeenCalled();
+ expect(service.getTempWorkflow()).toBe(tempWorkflow);
+
+ service.resetTempWorkflow();
+ expect(connectSpy).toHaveBeenCalled();
+ expect(service.getTempWorkflow()).toBeUndefined();
+ });
+
+ it("should add a dynamic input port to both the texera and joint graphs", ()
=> {
+ const dynamicOp: OperatorPredicate = {
+ ...mockMultiInputOutputPredicate,
+ dynamicInputPorts: true,
+ dynamicOutputPorts: true,
+ };
+ service.addOperator(dynamicOp, mockPoint);
+ const element = jointGraph.getCell(dynamicOp.operatorID) as
joint.dia.Element;
+ const beforeInputs =
texeraGraph.getOperator(dynamicOp.operatorID).inputPorts.length;
+ const beforePorts = element.getPorts().length;
+
+ service.addPort(dynamicOp.operatorID, true);
+
+
expect(texeraGraph.getOperator(dynamicOp.operatorID).inputPorts.length).toEqual(beforeInputs
+ 1);
+ expect(element.getPorts().length).toEqual(beforePorts + 1);
+ });
+
+ it("should honor disallowMultiInputs when adding an input port", () => {
+ const dynamicOp: OperatorPredicate = { ...mockMultiInputOutputPredicate,
dynamicInputPorts: true };
+ service.addOperator(dynamicOp, mockPoint);
+
+ service.addPort(dynamicOp.operatorID, true, true);
+
+ const inputPorts =
texeraGraph.getOperator(dynamicOp.operatorID).inputPorts;
+ expect(inputPorts[inputPorts.length - 1].disallowMultiInputs).toBe(true);
+ });
+
+ it("should remove the last dynamic port from both the texera and joint
graphs", () => {
+ const dynamicOp: OperatorPredicate = { ...mockMultiInputOutputPredicate,
dynamicInputPorts: true };
+ service.addOperator(dynamicOp, mockPoint);
+ const element = jointGraph.getCell(dynamicOp.operatorID) as
joint.dia.Element;
+ const beforeInputs =
texeraGraph.getOperator(dynamicOp.operatorID).inputPorts.length;
+ const beforePorts = element.getPorts().length;
+
+ service.removePort(dynamicOp.operatorID, true);
+
+
expect(texeraGraph.getOperator(dynamicOp.operatorID).inputPorts.length).toEqual(beforeInputs
- 1);
+ expect(element.getPorts().length).toEqual(beforePorts - 1);
+ });
Review Comment:
Reworked to add a dynamic input port first and then remove it, asserting the
counts round-trip back to baseline — so it actually exercises the dynamic-port
removal path rather than deleting a pre-existing port (3f322ca).
--
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]
