aglinxinyuan opened a new pull request, #7556:
URL: https://github.com/apache/texera/pull/7556
### What changes were proposed in this PR?
`ExecutionResultService` sat at **22.9% of 122 lines**. Its 18 existing
tests cover the JSON conversion helpers and the `WebOutputMode` round-trips and
stop at the class's own behaviour, so nothing exercised the paths a user
actually hits: paging through a stored result, searching and slicing its
columns, and the polling loop that pushes updates to the frontend while an
execution runs.
Adds 22 tests to the existing spec, taking the file to **99.2% of lines**
(121/122).
The seam is `attachToExecution`'s `client` parameter: `AmberClient` is
non-final with an overridable `registerCallback`, so a test subclass captures
the registrations and fires them directly over a bare `ActorSystem` — the
pattern `ExecutionConsoleServiceSpec` already uses. A fresh
`ExecutionStateStore` that never sees RUNNING is what keeps `AmberRuntime` out
of it.
Covered: page origin and range end, case-insensitive column search, column
offset and limit, the warehouse read guard, all three output modes and the
internal-port filter, the dirty-page computation, snapshot versus delta reads,
table statistics, the terminal-state transition that cancels polling and runs
one final update, and the fatal-error path.
### Verification
32 mutations applied one at a time, each reverted with the production diff
confirmed empty before the next. All 32 red. Three were then re-run
independently after the fact — the page origin off-by-one page, the dirty-page
count flooring instead of ceiling, and the delta reading from the new tuple
count instead of the old — all three red again.
Two details worth stating rather than glossing:
- **The page end bound rests on one assertion.** Only the exact-id-list test
pins it; the "clamp the last page" test cannot, because with 7 rows both
`[6,9)` and `[6,10)` yield the same single row. The redundancy one would assume
is not there, so that test is load-bearing on its own.
- **The warehouse-guard test uses `a[WarehouseUnavailableException] should
be thrownBy`**, which is a loose form — any collaborator throwing that type
satisfies it, and `DocumentFactory.openDocument` is a plausible second thrower
since it also resolves warehouses. Dropping the guard was checked directly and
fails exactly that one test, so the line is genuinely load-bearing for it.
### Deliberately not included
One line remains uncovered, and it should be **deleted rather than tested**:
the `case _ => throw new RuntimeException("update mode combination not
supported: ...")` in `convertWebResultUpdate`. `webOutputMode` is built
immediately above from a total match over `OutputMode`, so it is provably one
of `PaginationMode` / `SetSnapshotMode` / `SetDeltaMode` and all three are
matched by the preceding cases. No test can kill a mutation there.
No production file is touched.
### Any related issues, documentation, discussions?
Closes #7555
### How was this PR tested?
```
STORAGE_ICEBERG_CATALOG_TYPE=postgres sbt "WorkflowExecutionService/testOnly
org.apache.texera.web.service.ExecutionResultServiceSpec"
```
```
[info] Total number of tests run: 40
[info] Tests: succeeded 40, failed 0, canceled 0, ignored 0, pending 0
```
22 new on top of the existing 18. Coverage measured with sbt-jacoco filtered
to this spec — note that a plain `testOnly` reports 0% for this module, since
the destfile javaOption only comes from the `jacoco` task. `Test/scalafmtCheck`
and `Test/scalafix --check` both pass.
### 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]
