Ma77Ball commented on code in PR #6689:
URL: https://github.com/apache/texera/pull/6689#discussion_r3627120770
##########
frontend/src/app/workspace/service/workflow-graph/util/workflow-util.service.spec.ts:
##########
@@ -22,8 +22,10 @@ import { OperatorMetadataService } from
"./../../operator-metadata/operator-meta
import { inject, TestBed } from "@angular/core/testing";
import { WorkflowUtilService } from "./workflow-util.service";
-import { mockScanSourceSchema } from
"../../operator-metadata/mock-operator-metadata.data";
+import { mockMultiInputOutputSchema, mockScanSourceSchema } from
"../../operator-metadata/mock-operator-metadata.data";
import { commonTestProviders } from "../../../../common/testing/test-utils";
+import { OperatorPredicate } from "../../../types/workflow-common.interface";
+import { Workflow, WorkflowContent } from "../../../../common/type/workflow";
Review Comment:
Good catch. `ExecutionMode` only defines `PIPELINED` and `MATERIALIZED`, so
`"BATCH"` was invalid. I imported `ExecutionMode` from `common/type/workflow`
and set both fixtures to `ExecutionMode.PIPELINED`, dropping the `as any` casts.
##########
frontend/src/app/workspace/service/workflow-graph/util/workflow-util.service.spec.ts:
##########
@@ -101,4 +103,128 @@ describe("WorkflowUtilService", () => {
// assert all IDs are distinct
expect(idSet.size).toEqual(repeat);
});
+
+ it("should rebuild ports and version from the schema when the operator has
no ports", () => {
+ const op: OperatorPredicate = {
+ operatorID: "op-1",
+ operatorType: mockMultiInputOutputSchema.operatorType,
+ operatorVersion: "stale-version",
+ operatorProperties: {},
+ inputPorts: [],
+ outputPorts: [],
+ showAdvanced: false,
+ isDisabled: false,
+ };
+
+ const updated = workflowUtilService.updateOperatorVersion(op);
+
+ // version is refreshed from the schema
+
expect(updated.operatorVersion).toEqual(mockMultiInputOutputSchema.operatorVersion);
+ // ports are regenerated from the schema's port metadata
+ expect(updated.inputPorts.map(port => port.portID)).toEqual(["input-0",
"input-1", "input-2"]);
+ expect(updated.outputPorts.map(port => port.portID)).toEqual(["output-0",
"output-1", "output-2"]);
+ });
+
+ it("should keep the operator's existing ports when they are already
present", () => {
+ const existingInputPorts = [{ portID: "input-existing", displayName:
"keep-me" }];
+ const existingOutputPorts = [{ portID: "output-existing", displayName:
"keep-me-too" }];
+ const op: OperatorPredicate = {
+ operatorID: "op-2",
+ operatorType: mockMultiInputOutputSchema.operatorType,
+ operatorVersion: "stale-version",
+ operatorProperties: {},
+ inputPorts: existingInputPorts,
+ outputPorts: existingOutputPorts,
+ showAdvanced: false,
+ isDisabled: false,
+ };
+
+ const updated = workflowUtilService.updateOperatorVersion(op);
+
+ // existing ports are preserved untouched, only the version is refreshed
+ expect(updated.inputPorts).toEqual(existingInputPorts);
+ expect(updated.outputPorts).toEqual(existingOutputPorts);
+
expect(updated.operatorVersion).toEqual(mockMultiInputOutputSchema.operatorVersion);
+ });
+
+ it("should throw an error when updating the version of an operator with an
unknown type", () => {
+ const op: OperatorPredicate = {
+ operatorID: "op-3",
+ operatorType: "non-exist-operator-type",
+ operatorVersion: "v1",
+ operatorProperties: {},
+ inputPorts: [],
+ outputPorts: [],
+ showAdvanced: false,
+ isDisabled: false,
+ };
+
+ expect(() =>
workflowUtilService.updateOperatorVersion(op)).toThrowError(new RegExp("doesn't
exist"));
+ });
+
+ it("should parse the workflow content string into an object", () => {
+ const content: WorkflowContent = {
+ operators: [],
+ operatorPositions: {},
+ links: [],
+ commentBoxes: [],
+ settings: { dataTransferBatchSize: 400, executionMode: "BATCH" as any },
Review Comment:
Fixed as part of the same change: this fixture now uses
`ExecutionMode.PIPELINED` instead of the invalid `"BATCH"`.
##########
frontend/src/app/workspace/service/workflow-graph/util/workflow-util.service.spec.ts:
##########
@@ -101,4 +103,128 @@ describe("WorkflowUtilService", () => {
// assert all IDs are distinct
expect(idSet.size).toEqual(repeat);
});
+
+ it("should rebuild ports and version from the schema when the operator has
no ports", () => {
+ const op: OperatorPredicate = {
+ operatorID: "op-1",
+ operatorType: mockMultiInputOutputSchema.operatorType,
+ operatorVersion: "stale-version",
+ operatorProperties: {},
+ inputPorts: [],
+ outputPorts: [],
+ showAdvanced: false,
+ isDisabled: false,
+ };
+
+ const updated = workflowUtilService.updateOperatorVersion(op);
+
+ // version is refreshed from the schema
+
expect(updated.operatorVersion).toEqual(mockMultiInputOutputSchema.operatorVersion);
+ // ports are regenerated from the schema's port metadata
+ expect(updated.inputPorts.map(port => port.portID)).toEqual(["input-0",
"input-1", "input-2"]);
+ expect(updated.outputPorts.map(port => port.portID)).toEqual(["output-0",
"output-1", "output-2"]);
+ });
+
+ it("should keep the operator's existing ports when they are already
present", () => {
+ const existingInputPorts = [{ portID: "input-existing", displayName:
"keep-me" }];
+ const existingOutputPorts = [{ portID: "output-existing", displayName:
"keep-me-too" }];
+ const op: OperatorPredicate = {
+ operatorID: "op-2",
+ operatorType: mockMultiInputOutputSchema.operatorType,
+ operatorVersion: "stale-version",
+ operatorProperties: {},
+ inputPorts: existingInputPorts,
+ outputPorts: existingOutputPorts,
+ showAdvanced: false,
+ isDisabled: false,
+ };
+
+ const updated = workflowUtilService.updateOperatorVersion(op);
+
+ // existing ports are preserved untouched, only the version is refreshed
+ expect(updated.inputPorts).toEqual(existingInputPorts);
+ expect(updated.outputPorts).toEqual(existingOutputPorts);
+
expect(updated.operatorVersion).toEqual(mockMultiInputOutputSchema.operatorVersion);
+ });
+
+ it("should throw an error when updating the version of an operator with an
unknown type", () => {
+ const op: OperatorPredicate = {
+ operatorID: "op-3",
+ operatorType: "non-exist-operator-type",
+ operatorVersion: "v1",
+ operatorProperties: {},
+ inputPorts: [],
+ outputPorts: [],
+ showAdvanced: false,
+ isDisabled: false,
+ };
+
+ expect(() =>
workflowUtilService.updateOperatorVersion(op)).toThrowError(new RegExp("doesn't
exist"));
+ });
+
+ it("should parse the workflow content string into an object", () => {
+ const content: WorkflowContent = {
+ operators: [],
+ operatorPositions: {},
+ links: [],
+ commentBoxes: [],
+ settings: { dataTransferBatchSize: 400, executionMode: "BATCH" as any },
+ };
+ const workflow = {
+ name: "test-workflow",
+ description: undefined,
+ wid: 1,
+ creationTime: undefined,
+ lastModifiedTime: undefined,
+ isPublished: 0,
+ readonly: false,
+ content: JSON.stringify(content) as any,
+ } as unknown as Workflow;
+
+ const parsed = WorkflowUtilService.parseWorkflowInfo(workflow);
+
+ // the string content is replaced with the parsed object
+ expect(typeof parsed.content).toEqual("object");
+ expect(parsed.content).toEqual(content);
+ });
+
+ it("should leave workflow content untouched when it is already an object",
() => {
+ const content: WorkflowContent = {
+ operators: [],
+ operatorPositions: {},
+ links: [],
+ commentBoxes: [],
+ settings: { dataTransferBatchSize: 400, executionMode: "BATCH" as any },
Review Comment:
Same fix here too: replaced `"BATCH"` with `ExecutionMode.PIPELINED` so the
fixture matches the type definition.
--
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]
