aglinxinyuan opened a new pull request, #7240: URL: https://github.com/apache/texera/pull/7240
### What changes were proposed in this PR? `ResultExportService` sat at 42% (97 of 170 lines missed). The existing `ResultExportServiceSpec` already covered the pure helpers and every encoding writer — roughly the covered 73 lines — so this extends that file to the request-level layer that was untested. 16 new tests against `MockTexeraDB`'s embedded Postgres plus a local stand-in for the file service: | Area | What is pinned | |---|---| | `exportToLocal` | both failure guards | | `exportToDataset` | per-operator error collection | | `generateFileName` | workflow version, parquet→zip mapping, path-separator stripping | | upload | target URL, URL-encoding, signed bearer token, 500 mapping | Three of these deserve a note, because each was originally written in a way that looked right and proved nothing: **The per-operator failure test now uses two operators.** The try/catch lives *inside* the `foreach`, so the property worth testing is "the loop keeps going after one throws" — invisible with a single operator. Both operators now produce an `Error exporting operator …` line; hoist the catch out of the loop and only the first survives. **The two `exportToLocal` guards raise the same message**, so a single test could not distinguish them. They are now named for the guard each actually reaches and told apart by fixture: one seeds no execution (only `execIdOpt.isEmpty` can fire), the other seeds one (so reaching the throw proves the null-document guard fired). The original single test was named for the second case but exercised the first. **The stub upload server fails loudly rather than cancelling.** It binds the real file-service port, which the production endpoint resolves at class-load time and a test cannot redirect. With `cancel`/`assume`, anyone running the local stack would silently lose the three strongest tests here while CI stayed green. It now `fail`s with an actionable message, and catches `Exception` rather than `IOException` since a port-less endpoint override makes `new InetSocketAddress(-1)` throw `IllegalArgumentException`. Out of scope, and stated in the file: amber's test-scope dependencies are only `DAO % "test->test"` and `Auth % "test->test"`, so `workflow-core`'s test helpers — including the local Iceberg catalog — are not on amber's test classpath. Paths needing a real Iceberg document are left alone. No production file is touched. ### Any related issues, documentation, discussions? Closes #7237 ### How was this PR tested? 16 new tests, run as the whole spec so the pre-existing cases are proven intact — 40 tests, Java 17: ``` sbt "WorkflowExecutionService/testOnly org.apache.texera.web.service.ResultExportServiceSpec" ``` ``` [info] Suites: completed 1, aborted 0 [info] Tests: succeeded 40, failed 0, canceled 0, ignored 0, pending 0 [info] All tests passed. ``` The two HTTP tests really do bind the port and exchange bytes rather than being skipped. `Test/scalafmtCheck` and `Test/scalafix --check` both `[success]`. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (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]
