mengw15 opened a new pull request, #7540:
URL: https://github.com/apache/texera/pull/7540
### What changes were proposed in this PR?
Covers the remaining unit-testable pure logic in two of the three classes the
issue lists. No production code was changed.
**`WorkflowVersionResource`** (+7 tests) — the version-importance helpers,
all
private and exercised through `PrivateMethodTester`:
- `isSnapshotImportant` — a patch whose ops are all `replace` is
unimportant, any
other op makes it important, and an empty patch is unimportant.
- `isVersionImportant` — a patch touching only `/operatorPositions/` is
unimportant; anything else is important.
- `isWithinTimeLimit` — holds inside the aggregate window, not outside it.
- `encodeVersionImportance` — the latest version is always important, a
version
inside the aggregate window is folded in as unimportant, and one outside
it is
judged on its content.
> The issue calls this gap `jsonTreeIterator`; that is a local variable
inside
> `isSnapshotImportant` / `isVersionImportant` rather than a method, so the
tests
> target those two methods (plus the rest of the family).
**`WorkflowEmailNotifier`** (+2 tests)
- `createEmailMessage` — the assembled message addresses the recipient and
carries the same subject the dedicated builder produces, plus the workflow
name, id and state in its content.
- `sendStatusEmail` — the invalid-recipient arm returns before dispatching,
so
the test never reaches `GmailResource.sendEmail`.
After this change (measured with `sbt WorkflowExecutionService/jacoco`):
`WorkflowVersionResource` 116/121 lines (95%), `WorkflowEmailNotifier` 40/43
(93%). The lines that remain are not unit-reachable:
- `updateLatestVersion` — defined but has no callers anywhere in the repo.
- `sendStatusEmail`'s dispatch arm — it calls `GmailResource.sendEmail`,
i.e. it
would send real mail.
- `cloneVersion`'s catch block — needs a database-layer failure to trigger.
- `isSnapshotInRangeUnimportant`'s `lowerBound == UpperBound` early return is
already asserted by an existing test, but jacoco still reports it unhit:
the
parameters are `java.lang.Integer`, so `==` compares references rather than
values. Worth a look separately — this PR does not change production code.
**`ResultExportService`** — no tests added. Its in-scope pure logic is
already
covered by the existing spec: `parseOperators` (valid / empty / malformed),
`validateExportRequest` (both arms), `convertFieldToBytes` (all three cases),
`generateFileName` (parquet→zip, path-separator stripping), `streamCellData`
(all three guards), and the `errorMessages` accumulation the issue mentions
(`exportToDataset` "collect one message per operator" / "turn a thrown
per-operator failure into an error entry"). The `download.dat` default the
issue
asks for is unreachable: when `exportOperatorResultAsStream` returns no file
name it also returns a null stream, and the preceding line throws. The
remaining
uncovered methods (`exportOperatorsAsZip`, `exportSingleOperatorToDataset`,
`getOperatorDocument`) all go through `DocumentFactory`/storage, which the
issue
places out of scope.
### Any related issues, documentation, discussions?
Closes #7537
### How was this PR tested?
Unit tests, run locally against embedded Postgres (`MockTexeraDB`). All
pass, and
the failure path was verified by breaking an assertion to confirm the suite
goes
red:
```
sbt "WorkflowExecutionService/testOnly *WorkflowVersionResourceSpec
*WorkflowEmailNotifierSpec"
# Tests: succeeded 37, failed 0
sbt "WorkflowExecutionService/Test/scalafmtCheck" # clean
sbt "WorkflowExecutionService/Test/scalafix --check" # clean
```
### Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 4.8 [1M context])
--
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]
