aglinxinyuan opened a new issue, #7603: URL: https://github.com/apache/texera/issues/7603
### Task Summary `collectOperatorResult` is the half of `SyncExecutionResource` that decides what an external caller actually receives from a synchronous run: how many rows come back, **which rows are dropped** when a result exceeds the character budget, and how individual cells are shortened. It is roughly 180 lines (523–708) and none of it is covered. The spec's own scaladoc currently disclaims the region as unreachable. **That claim is wrong**, and it is worth correcting rather than working around: `ExecutionResultServiceSpec` has been creating real Iceberg-backed documents in amber's test scope for some time, using `DocumentFactory.createDocument` against the ambient postgres catalog — no helper, no new dependency, no build change. The same pattern reaches this engine directly. The seam, in four steps: 1. `VFSURIFactory.resultURI(VFSURIFactory.createPortBaseURI(...))` builds the URI shape `getResultUriByLogicalPortId` decodes. 2. `DocumentFactory.createDocument(uri, schema)` plus `document.writer(...)` writes a real Iceberg table. 3. An insert into `OPERATOR_PORT_EXECUTIONS` makes the URI findable; `MockTexeraDB` already backs that table. 4. `collectOperatorResult` takes the `ExecutionIdentity` as a parameter and resolves the rest from the DB and storage, so it needs no engine. **Do not** add `WorkflowCore % "test->test"` to reach `LocalHadoopIcebergCatalog`. It was tested and it breaks the build: it reorders amber's test classpath so `jsr311-api` (JAX-RS 1.1) precedes `javax.ws.rs-api` 2.1, `scalac` then resolves `Response` without `getHeaderString`/`getMediaType`, and 13 compile errors appear in `HuggingFaceModelResourceSpec` and `ResultExportServiceSpec`. It is also unsafe on its own terms — that helper calls `IcebergCatalogInstance.replaceInstance`, a JVM-wide singleton, and amber has no `Test/fork`, so it would pull the catalog out from under two other suites mid-run. Landmines for whoever writes this: - **The "front half exactly full" branch is not reachable with a round character limit.** Any convenient number lands in the neighbouring sliding-window branch instead, and the test then silently covers code it does not name. Compute the per-tuple size in the test and derive the limit from it. - **Do not reuse the executeWorkflowSync fixture's computing-unit id.** `initExecutionService` treats the newest execution of that pair as "the previous run" and cleans up its registered storage. - **Give every operator id a repo-unique prefix.** Iceberg table identity derives from the URI path and amber runs all suites in one unforked JVM. - **Assert on row counts and indices, not `isDefined`.** The catch-all at the end of the method also returns a well-formed tuple, so a weak assertion passes on a fixture that never opened a document. Also found: `processedCount` inside the method is written in all three walk paths and never read — a safe deletion. ### 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]
