aglinxinyuan opened a new issue, #7867:
URL: https://github.com/apache/texera/issues/7867
### Task Summary
`amber/src/main/scala/org/apache/texera/amber/engine/architecture/worker/DPThread.scala`
is at 74.7% by Codecov's metric — 3 missed and 19 partial of 87 lines, with 26
of 76 branch arms uncovered. `DPThreadSpec` has 8 tests and none of them
exercise the thread's lifecycle edges: stopping before start, parking when
idle, or a pause arriving mid-batch.
The tests are cheap — the existing fixtures need no DB, Iceberg, Python or
actor system — but the measurement is full of traps, and most of the apparent
gap is not real:
1. **The entire payload-dispatch block (lines 204-223) is not counted at
all.** `logManager.withFaultTolerant { msgOpt match { … } }` compiles into
`$anonfun$runDPThreadMainLogic$2`, and `javap -v` shows `ACC_SYNTHETIC`, so
JaCoCo's `SyntheticFilter` drops it — the tracked line list jumps straight from
203 to 224. Any test aimed at ECM/DCM dispatch here moves Codecov by exactly
zero. Same for line 201's filter lambda.
2. **Four lines are permanently partial because of scala-logging.** Lines
84, 102, 110 and 112 each expand to `if (logger.underlying.isXEnabled) …`, so
at any fixed log level exactly one arm is dead. CI pins
`TEXERA_SERVICE_LOG_LEVEL=WARN`. Closing them would mean mutating logback
levels from a test, which is a pure count-raising trick.
3. **Line 88 can never be fully covered.** `if
(dp.stateManager.getCurrentState == UNINITIALIZED)` disassembles to `ifnonnull`
/ `ifnull` / `ifeq` = 6 arms with 4 missed, and since the state object is never
null the null-equals-null leg is unreachable dead code. Even covering the
reachable arm leaves 3 arms missed, so the line stays "missed" forever.
4. **Other structurally dead arms**: the lazy-val double-checked-lock arm,
`$outer == null` guards, the `isDefinedAt` else-arm (dead because the partial
function's last case is `case err: Throwable`, hence total), the `MatchError`
arm of a sealed trait with three final case classes, and the `MatchError` arms
of exhaustive `Option` matches.
5. **Line 91's false arm is only reachable by assigning the public `var
dpThread` from outside**, since `stop()` nulls neither field. That is an
artificial test of an unreachable state.
6. **A stop-during-busy-spin test is a race.** It depends on the interrupt
landing while the thread is in a non-interruptible spin, so the loop exits via
the stop flag rather than via `InterruptedException` from `internalQueue.take`.
Give it a large margin and do not shrink the tuple count. Conversely, tests
that prove data is *not* flowing are one-sided — a slow runner makes them more
reliable, not less.
7. Watch for unsynchronized reads of plain `var`s written by the DP thread
as test gates; they work by luck. And do not locate the thread by name —
several tests never stop theirs, so multiple live threads share the name.
85.1% is the realistic ceiling here.
### 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]
