Copilot commented on code in PR #6689:
URL: https://github.com/apache/texera/pull/6689#discussion_r3626778950
##########
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:
Import ExecutionMode from the workflow types so the test data can use a real
enum value (and avoid `as any`) instead of the non-existent "BATCH" mode.
##########
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 as above: "BATCH" is not a valid `ExecutionMode` value. Use
`ExecutionMode.PIPELINED` (or MATERIALIZED) to keep the fixture aligned with
the type definition.
##########
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:
`ExecutionMode` only defines PIPELINED/MATERIALIZED; "BATCH" isn't a valid
mode (and isn't referenced elsewhere). Using an invalid value here makes the
test data unrealistic and can hide type regressions.
--
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]
