eugenegujing opened a new pull request, #7816: URL: https://github.com/apache/texera/pull/7816
### What changes were proposed in this PR? This PR extends `amber/src/test/scala/org/apache/texera/web/service/ResultExportServiceSpec.scala` (40 → 60 tests) to cover the three `ResultExportService` methods that previously had zero test references anywhere in the repository — `exportOperatorsAsZip`, `exportSingleOperatorToDataset` and `getOperatorDocument` — together with the edge cases of the export paths that reach them. - `exportOperatorsAsZip`: throws `WebApplicationException` when the workflow never ran (where the sibling `exportOperatorResultAsStream` returns `(null, None)` for the same condition); names the archive `<workflowName>-<yyyy-MM-dd_HH-mm-ss>.zip`; dispatches each entry through the per-format writer on that operator's own `outputType`, with an unrecognised type falling back to CSV; routes every entry through `NonClosingOutputStream` so one entry's writer closing its stream cannot close the shared `ZipOutputStream` (asserted by a later entry surviving an earlier writer's close — the wrapper was already unit-tested in isolation, but nothing checked that the ZIP path uses it); and substitutes a `<id>-empty.txt` placeholder instead of aborting the archive, covered for both halves of the guard — an operator with no stored result URI, and an operator whose stored result table exists but holds zero rows. - `exportSingleOperatorToDataset` (driven through `exportToDataset`): uploads each operator's rows to the file service in that operator's own format with the exact bodies asserted against a stub upload server, turns a thrown per-operator failure into an error entry instead of aborting the batch, reports a stored-but-empty result as having nothing to export, and pins the partial-success policy as characterization — when some operators export and some fail, the response is `success` carrying only the success lines. - `getOperatorDocument`: returns null when the operator stored no result URI, opens the stored result document with its count and contents read back, and refuses a result stored in a per-user warehouse while the feature is off (`WarehouseUnavailableException` via the `WarehouseReadGuard` from #6930). - Edge cases along these paths: every row is written on both sides of the CSV chunk-buffer boundary (`CHUNK_SIZE` 10, exercised one-under, exactly-one and one-over); CSV values containing a comma, a double quote, a newline and non-ASCII text round-trip through the streamed export; `streamCellData` accepts the maximum row and column index and rejects the out-of-range ones with their exact reasons, while negative indexes are pinned as characterization because the `>=` guards let them through to an internal reader error and an array-index error; a result stored under an older execution is not exported once a newer execution exists, and the newer execution's rows win when both stored results; and requesting the same operator twice aborts the archive mid-stream with a duplicate-entry `ZipException`, pinned as characterization since the client has already received its 200 by then. Result-bearing tests store real Iceberg result tables using the same arrangement `ExecutionResultServiceSpec` already uses in amber unit tests, which made the spec's old "opening a result document needs a live Iceberg catalog, out of reach here" banner stale; the banner and class scaladoc are updated accordingly. No production code is changed. ### Any related issues, documentation, discussions? Closes #7814 ### How was this PR tested? All changes are new or extended unit tests in `ResultExportServiceSpec`; run them with `STORAGE_ICEBERG_CATALOG_TYPE=postgres sbt "WorkflowExecutionService/testOnly org.apache.texera.web.service.ResultExportServiceSpec"` — 60 succeeded, 0 failed, and the suite was verified idempotent by running twice back-to-back against the same catalog database. Key assertions were mutation-checked by hand-editing production code and confirming that exactly the intended tests fail: replacing the `NonClosingOutputStream` wrapper with the raw stream, making the wrapper's `close()` close the underlying stream, skipping the empty-operator placeholder branch, dropping the zero-row half of both placeholder guards, forcing the ZIP dispatch to CSV, dropping the timestamp from the archive name, returning `(null, None)` instead of throwing when there is no execution, breaking the result-document lookup, removing the `WarehouseReadGuard` check, appending the dropped error lines to a partial-success respo nse, cutting the CSV chunk loop to a single chunk, and resolving the oldest execution instead of the latest. Every mutation was reverted byte-exact afterwards, and `scalafmtCheck` passes. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Fable 5, Claude Opus 5) -- 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]
