mengw15 opened a new pull request, #7964:
URL: https://github.com/apache/texera/pull/7964
### What changes were proposed in this PR?
Extends `workflow-execution-tools.spec.ts` over the "every enabled operator"
branch
of `buildLogicalPlan`, the second of the two gaps #7959 lists. Measured
locally with
`bun test --coverage`:
| `workflow-execution-tools.ts` | Before | After |
| --- | --- | --- |
| lines | 95.16 % (uncovered 169-178, 221-226, 229-233) | **97.70 %
(uncovered 169-178)** |
| functions | 85.71 % | **90.48 %** |
The branch is reachable, but not the way the issue assumed.
`buildLogicalPlan`'s only
call site passes `[operatorId]` — always a one-element array — so
`useSubDAG` is always
true and the sub-DAG path is chosen by `targetOperatorId` being *truthy*.
What actually
falls through to the full-workflow branch is an **empty** operator id, which
the tool's
`z.string()` schema accepts. The test drives that path through the public
`executeOperatorAndFormat` and asserts the request the backend receives:
- every operator is sent, including `sink` and `orphan` — neither is
upstream of `mid`,
so a sub-DAG walk would have dropped them; that is what distinguishes the
two branches;
- both links resolve to the right port ordinals on both ends;
- `operatorProperties` are flattened onto the wire operator alongside its
ports;
- `opsToViewResult` comes back empty, since the empty id matches no operator;
- the same state with a real id (`"mid"`) is asserted to send only `mid` and
`src`, so the
test fails if the sub-DAG path is taken instead.
Two things the test records rather than asserts as designed:
- **`getAllEnabledOperators()` does not filter on `isDisabled`** —
`workflow-state.ts:122`
is `return this.getAllOperators()`. The issue asked for a disabled
operator to be
excluded here; it is not, so a disabled operator is sent to the engine on
this path.
`getSubDAG` does honour `isDisabled`, so the two paths disagree.
- An empty `operatorId` silently turns "execute this operator's upstream
slice" into
"execute the whole workflow", and then reports that the operator has no
result.
Both are noted for follow-up; this PR only characterises the current
behaviour and
changes no production code.
### Any related issues, documentation, discussions?
Part of #7959. The other half of that issue, `formatWorkflowValidationErrors`
(169-178), has no call site anywhere in the repository and has had none
since it was
introduced in #4540 — `executeOperatorAndFormat` inlines its own,
differently indented
formatter instead. It is module-private, so covering it would mean exporting
dead code
and freezing an output format nothing uses. Removing it is proposed
separately.
### How was this PR tested?
`bun test src/agent/tools/workflow-execution-tools.spec.ts` — 24 pass (23
before, 1 new),
repeated 3× for stability; the whole agent-service suite stays green (300
pass across 19
files). `bun run typecheck` and `bun run format:check` clean. Failure path
verified by
changing one port ordinal in the new test's link assertion: 1 fail / 23
pass, non-zero
exit, then restored to green.
Determinism: the workflow state is built from plain in-memory fixtures,
`fetch` is a spy
that resolves a canned response, and the assertions read the captured
request body rather
than comparing whole-plan JSON.
### Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 4.8 [1M context])
--
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]
