mengw15 opened a new issue, #6898:
URL: https://github.com/apache/texera/issues/6898

   ### Task Summary
   
   `ResultExportService` 
(`amber/src/main/scala/org/apache/texera/web/service/ResultExportService.scala`)
 is at ~6% — 160 of 170 tracked lines are unhit. The existing 
`ResultExportServiceSpec` only covers `parseOperators` and 
`validateExportRequest`. The five `streamDocumentAs*` writers look 
storage-bound but are not: each takes a `VirtualDocument[Tuple]` parameter, and 
`VirtualDocument` is an ordinary abstract class whose only abstract members are 
`getURI` and `clear()` — every accessor defaults to `throw new 
NotImplementedError`. A small in-spec fake overriding `getCount` / `get()` / 
`getRange` / `asInputStream()` makes the whole export-encoding layer testable 
against a `ByteArrayOutputStream`, with no Iceberg, MinIO or DB.
   
   ### Behavior to add
   
   Extend 
`amber/src/test/scala/org/apache/texera/web/service/ResultExportServiceSpec.scala`.
 The service constructs cheaply — the spec already does `new 
ResultExportService(WorkflowIdentity(1L), computingUnitId = 0)`. Reach the 
private writers with `PrivateMethodTester` (the idiom 
`WorkflowExecutionsResourceSpec` already uses). For the fake document, mirror 
`MinimalDoc` in 
`common/workflow-core/src/test/scala/org/apache/texera/amber/core/storage/model/VirtualDocumentSpec.scala`
 — override only what each test needs.
   
   **`streamDocumentAsCSV`** (private; `doc`, `outputStream`, `maybeHeaders: 
Option[List[String]]`)
   - `getCount == 0` → returns without writing a byte.
   - `maybeHeaders = None` → header row is inferred from 
`firstRow.getSchema.getAttributeNames`, and that first row is also written to 
the body.
   - `maybeHeaders = Some(hdrs)` → `hdrs` is written as the header and the 
iterator's first row is **not** consumed, so it still appears in the body. This 
asymmetry between the two branches is the off-by-one worth pinning.
   - A document larger than `Constants.CHUNK_SIZE` (10) exercises the buffered 
chunk loop and the per-chunk `flush()`.
   
   **`streamCellData`** (private; `out`, `request`, `operatorDocument`)
   - `rowIndex >= getCount` → `WebApplicationException` with message `Invalid 
rowIndex (5). Total rows: 2`.
   - `columnIndex >= selectedRow.getFields.length` → `WebApplicationException` 
with the `Invalid columnIndex` message.
   - Happy path writes exactly the UTF-8 bytes of the selected cell.
   
   **`convertFieldToBytes`** (private) — three branches: an `Array[Byte]` 
passes through unchanged; a `String` encodes as UTF-8 (use a multi-byte 
character to pin the charset); anything else goes through `toString`.
   
   **`streamDocumentAsHTML`** (private) — writes 
`results.head.getField(0).toString` as UTF-8. An empty document hits an 
unguarded `.head` and throws `NoSuchElementException`; assert that rather than 
leaving it undocumented.
   
   **`streamDocumentAsParquetZip`** (private) — with a fake whose 
`asInputStream()` returns a `ByteArrayInputStream`, assert the bytes are copied 
through verbatim and the source stream is closed.
   
   **`streamDocumentAsArrow`** (private) — runs entirely in-process on a 
`RootAllocator`; assert the Arrow stream round-trips the tuples back. Include 
only if it keeps the PR reviewable; it is the largest single block.
   
   **`NonClosingOutputStream.close`** — asserts the wrapped stream is *not* 
closed.
   
   ### Out of scope
   
   `getOperatorDocument`, `saveToDatasets`, `saveStreamToDataset` and the 
`exportSingleOperatorToDataset` happy path need `DocumentFactory.openDocument` 
and a live file-service HTTP endpoint — leave them uncovered. Note 
`fileServiceUploadOneFileToDatasetEndpoint` is a `lazy val` defaulting to 
`http://localhost:9092/...`, so any test that reaches `saveToDatasets` will 
attempt a real socket connection.
   
   `generateFileName` embeds `LocalDateTime.now()` in the name. If it is 
covered at all, assert only the stable parts — the 
`${workflowName}-op$operatorId-v$latestVersion-` prefix, the `parquet` → `.zip` 
extension mapping, and that `StringUtils.replaceEach` strips `/` and `\` from 
the workflow name. Never assert the full filename.
   
   ### 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]

Reply via email to