aglinxinyuan opened a new issue, #7691: URL: https://github.com/apache/texera/issues/7691
### Task Summary `amber/src/main/scala/org/apache/texera/web/service/ExecutionReconfigurationService.scala` sits at **51.4% of 35 lines** behind a 10-test spec, and the reason is structural rather than a gap in effort: the existing spec's `RecordingService` passes `client = null` / `workflow = null` and **overrides all three protected seams**. The uncovered residue is precisely what those overrides skip -- `modifyOperatorLogic`, the `dispatch` body, the worker-completion callback, and the completion diff handler. The consequence worth stating plainly: **any test that keeps the `dispatch` override leaves line 110 uncovered no matter how much it asserts.** Covering it needs a real empty-plan `AmberClient` with a mocked `coordinatorInterface`, the pattern already proven at `ExecutionRuntimeServiceSpec.scala:71-95`. Everything is reachable in-module with no DB, no Iceberg, no cluster and no network. `Workflow` is a plain case class, and `LogicalPlan.getOperator` is a `Map` lookup. Four traps here each produce a test that passes while pinning nothing: 1. **The diff-handler lambda does not run on `updateState` alone.** `StateStore.diffSubject` is a cold RxJava chain -- something must first subscribe to `getWebsocketEventObservable`. Worse, `.filter(states => states.get(0) != states.get(1))` silently drops any update equal to the previous state. A test that forgets either gets **zero handler invocations and zero failures**. 2. **Line 142's `workflow.physicalPlan` dereference is unpinnable.** `getPhysicalOpByWorkerId` looks the operator up *by the name parsed out of the worker id*, so the emitted logical op id is forced equal to the worker id's operator segment. Any assertion on it passes identically against a plan-free implementation. Cover it; do not claim to pin it. 3. **The failure arm needs a spec-local `LogicalOp`.** No production descriptor ever returns `Failure` -- all six `runtimeReconfiguration` overrides return `Success`, and the base at `LogicalOp.scala:484` *throws* instead, so an unsupported operator propagates out rather than reaching line 70. 4. **One line pins an impossible state.** Line 147 (guard true, diff empty) is only reachable by *shrinking* `completedReconfigurations`, which the service's own API can never do. A spec-local `LogicalOp` subtype is safe here: `OperatorMetadataGenerator` resolves subtypes via Jackson's `@JsonSubTypes` annotation, not a ClassGraph scan, so an unregistered test-only subclass is invisible to it. ### Task Type - [ ] Refactor / Cleanup - [ ] DevOps / Deployment / CI - [x] Testing / QA - [ ] Documentation - [ ] Performance - [ ] Other -- 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]
