mengw15 commented on code in PR #7159:
URL: https://github.com/apache/texera/pull/7159#discussion_r3690494573
##########
agent-service/src/agent/workflow-state.spec.ts:
##########
@@ -174,3 +174,223 @@ describe("WorkflowState - getSubDAG", () => {
expect(subDag.operators.map(o => o.operatorID)).toEqual(["op2"]);
});
});
+
+describe("WorkflowState - toLogicalPlan", () => {
+ test("produces operators, port-indexed links, and an empty reuse list", ()
=> {
+ const state = new WorkflowState();
+ state.addOperator(makeOperator("op1"));
+ state.addOperator(makeOperator("op2"));
+ state.addLink(makeLink("l1", "op1", "op2"));
+
+ const plan = state.toLogicalPlan();
+
+ expect(plan.operators.map(o => o.operatorID)).toEqual(["op1", "op2"]);
+ // operatorProperties are spread onto each logical operator; type + ports
carry through
+ expect(plan.operators[0].operatorType).toBe("TestOp");
+ expect(plan.links).toEqual([
+ { fromOpId: "op1", fromPortId: { id: 0, internal: false }, toOpId:
"op2", toPortId: { id: 0, internal: false } },
+ ]);
+ expect(plan.opsToReuseResult).toEqual([]);
+ });
+
+ test("resolves the link port indices from the operators' port lists", () => {
+ const state = new WorkflowState();
+ state.addOperator(
+ makeOperator("src", {
+ outputPorts: [
+ { portID: "output-0", displayName: "Output 0" },
+ { portID: "output-1", displayName: "Output 1" },
+ ],
+ })
+ );
+ state.addOperator(
+ makeOperator("dst", {
+ inputPorts: [
+ { portID: "input-0", displayName: "Input 0" },
+ { portID: "input-1", displayName: "Input 1" },
+ ],
+ })
+ );
+ state.addLink({
+ linkID: "l1",
+ source: { operatorID: "src", portID: "output-1" },
+ target: { operatorID: "dst", portID: "input-1" },
+ });
+
+ expect(state.toLogicalPlan().links[0]).toEqual({
+ fromOpId: "src",
+ fromPortId: { id: 1, internal: false },
+ toOpId: "dst",
+ toPortId: { id: 1, internal: false },
+ });
+ });
+
+ test("the targetOperatorId argument is currently ignored — the whole-graph
plan is produced either way", () => {
+ // The parameter is accepted but unused in the current implementation;
assert the
+ // observed behavior so a future change that starts honoring it is caught
here.
+ const state = new WorkflowState();
+ state.addOperator(makeOperator("op1"));
+ state.addOperator(makeOperator("op2"));
+ state.addLink(makeLink("l1", "op1", "op2"));
+
+ expect(state.toLogicalPlan("op1")).toEqual(state.toLogicalPlan());
Review Comment:
Good catch. I've dropped both assertions that pinned the current behavior —
the `toLogicalPlan("op1") === toLogicalPlan()` equivalence and the "links left
untouched" check — so neither will block a future fix, and both methods stay
covered.
This PR only adds coverage for existing behavior. Honoring
`targetOperatorId` / pruning links to removed ports isn't exercised by any
caller today (`toLogicalPlan` is only ever called with no argument, and the
upstream sub-graph is built by `getSubDAG`), so that's better handled as a
separate change than folded into a coverage PR. Happy to file a follow-up issue
to track the two gaps.
--
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]
