aglinxinyuan opened a new pull request, #7729:
URL: https://github.com/apache/texera/pull/7729
### What changes were proposed in this PR?
Two worker-side files had no spec. Adds one each, 14 tests total.
Measured by running the full amber unit suite under the CI environment
(`STORAGE_ICEBERG_CATALOG_TYPE=postgres`, `TEXERA_SERVICE_LOG_LEVEL=WARN`)
before and after, so these are directly comparable to the reported figures —
the baseline reproduced all of them exactly:
| File | Before | After |
|---|---|---|
| `InputPortMaterializationReaderThread.scala` | 52/63 (82.5%), 7 missed + 4
partial | **62/63 (98.4%)**, 0 missed |
| `DataProcessorRPCHandlerInitializer.scala` | 5/15 (33.3%), 8 missed + 2
partial | **8/15 (53.3%)** |
Instruction coverage: reader thread 85.1% -> **100.0%**; initializer 81.0%
-> 89.1%. Run on their own, the two specs take the reader thread to 63/63 lines
and 343/343 instructions.
**This is +13 lines, and I would rather state that plainly than dress it
up.** What earns the PR is the pinning rather than the count: the reader
thread's replay ordering, batching, partition filtering and failure wrapping
were all unverified, and eight distinct mutants now die on them.
### A third target was assessed and rejected
`ClientActor.scala` (65.2%) is **saturated, not undertested**, and roughly
190 lines of test bought exactly 1 line, 1 branch and 3 instructions — so that
work was dropped rather than shipped. Its 7 remaining misses are the companion
object's own `<init>`, four `case class` declaration lines carrying 112 of the
file's 176 branches inside scalac-generated
`equals`/`hashCode`/`productElement` (all with zero covered branches), and two
structural lines. The cause is that the e2e specs are not tagged
`@IntegrationTest`, so they run in the coverage job and already drive a real
Coordinator through a real `ClientActor`.
### Verification
12 mutations, **11 killed, 1 recorded equivalent**, each applied one at a
time with the production file diffed after every revert and the failing test
read by name.
| Mutation | Killed by |
|---|---|
| exchange `workerIdx` and `workerCount` | builds the named class from the
descriptor, worker index and worker count |
| exchange `OpExecWithCode`'s code and language fields | sends the code to
the Java compiler, surfacing its diagnostics |
| install an executor only when none is installed | replaces an executor
that was already installed |
| empty init info returns `null` instead of throwing | rejects an empty
`OpExecInitInfo` instead of installing a null executor |
| exchange START and END channel alignments | brackets the replay and marks
itself finished |
| exchange channel sender and receiver | sends every message from a
URI-derived sender, numbered from zero |
| `buffer.size >= batchSize` -> `>` | cuts a full batch as soon as the
buffer reaches the transfer batch size |
| drop the receiver check in the partitioner filter | drops tuples routed to
another worker; splits a hash-shuffled link |
| drop the cause when wrapping a storage failure | wraps a storage failure
in a `RuntimeException` that keeps the cause |
| `isFinished.set(true)` -> `false` | two tests |
| shift every replayed loop counter by one | replays states ahead of tuples,
carrying each row's loop envelope |
**The equivalent mutant:** flipping the trailing-flush guard survives. `if
(buffer.nonEmpty) flush()` is immediately followed by
`emitECM(METHOD_END_CHANNEL, ...)`, whose first statement is `flush()`, and
`flush()` itself returns early on an empty buffer. Both legs enqueue the same
`DataFrame`, in the same position, with the same sequence number. No test can
distinguish them — the guard is dead weight.
**One process failure worth recording**, because it is the kind that
silently corrupts a mutation table: the partition-filter mutation was first
spelled `.nonEmpty`, which collided with a nearby `buffer.nonEmpty`. The
revert's exactly-once assertion correctly refused to fire and left the mutant
applied, contaminating the four runs after it — their tables showed the earlier
mutation's tests failing alongside the real kill. The `git diff` guard caught
it; the file was restored, the mutation respelled as `.exists(_ => true)`, and
those runs redone. The table above is the clean run.
### Deliberately not included
- `newExecFromJavaCode` is uncoverable from a unit test:
`JavaRuntimeCompilation.compileCode` passes null options to
`compiler.getTask(...)`, so system javac uses `java.class.path`, which under
sbt (`fork := false`) does not contain workflow-core. A UDF implementing
`OperatorExecutor` cannot compile, so the method never returns normally.
- `noOperation` is dead on the Scala path — `NoOperation` is produced and
consumed only by the Python worker. The other three `???` members have live
coordinator-side call sites but resolve to `???` on a Scala worker; testing
them would cement "unimplemented" as a contract.
- Remaining partials are the unreachable kinds: a `lazy val` bitmap and the
`MatchError` fall-through of a scalapb sealed oneof.
A possible defect is reported rather than pinned:
`InputManager.startInputPortReaderThreads` wraps a start failure as `new
RuntimeException(s"Error starting input port materialization reader thread:
${e.getMessage}")`, **dropping the cause** — unlike the reader thread itself,
which passes `e`. That discards the stack trace of the real failure.
No production file is touched.
### Any related issues, documentation, discussions?
Closes #7728
### How was this PR tested?
```
STORAGE_ICEBERG_CATALOG_TYPE=postgres sbt "WorkflowExecutionService/testOnly
org.apache.texera.amber.engine.architecture.worker.DataProcessorRPCHandlerInitializerSpec
org.apache.texera.amber.engine.architecture.worker.managers.InputPortMaterializationReaderThreadSpec"
```
```
[info] Total number of tests run: 14
[info] Tests: succeeded 14, failed 0, canceled 0, ignored 0, pending 0
```
The full amber unit suite is green alongside them (1930 passed).
`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]
