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

   ### Task Summary
   
   `amber/src/main/scala/org/apache/texera/web/service/WorkflowService.scala` 
sits at **79.2% of 96 lines**, and `clearExecutionResources` -- the method that 
wipes the previous run's storage registry before a new execution starts -- is 
never entered by any test. It is unreachable today only by accident: 
`SyncExecutionResourceSpec:145-151` documents parking its fixtures under a 
*second* computing unit specifically to avoid this path, so the recipe is 
proven and merely needs inverting.
   
   **The structural fact that should shape any plan here:** JaCoCo's 
`SyntheticFilter` drops methods that are `ACC_SYNTHETIC` and not 
`lambda$`-prefixed, and Scala 2.13 compiles closures to synthetic `$anonfun$` 
methods. Every lambda *body* in this file is therefore absent from the 96 
tracked lines -- the lifecycle cleanup callback, the constructor's completion 
diff-handler, the errorHandler, both cleanup loop bodies, and the replay 
`foreach`. That is roughly 30 lines where a test moves the coverage number by 
**exactly zero**. Anyone targeting them is wasting effort.
   
   Traps worth knowing, each of which produces a green test that pins nothing:
   
   1. **Reusing one number for every id.** If `wid == uid == cuid`, a 
transposed `WID.eq(cuid).and(CUID.eq(wid))` produces byte-identical SQL. Every 
id domain needs its own literal.
   2. **A single previous execution cannot pin "latest".** With one row under 
the unit, `Some(executions.max)` -> `.min` is undetectable. Seed an older one 
and assert it survives.
   3. **A single workflow cannot pin the `WID` leg.** `workflow_computing_unit` 
carries `uid` and no `wid`, so one unit legitimately runs many workflows -- 
dropping the `WID` leg needs a second workflow on the same unit to be caught.
   4. **Relying on `SERIAL` ids hides an off-by-one.** With no execution below 
the one under test, `.eq` -> `.le` on the delete survives. Set explicit eids 
and place one below.
   5. **Line 406 makes a real network call.** 
`LargeBinaryManager.deleteByExecution` reaches `S3StorageClient`, whose client 
is a `private lazy val` in a Scala `object` with no seam (the injectable 
overload is `private[util]`). With `SERIAL` ids the first execution gets eid 1, 
which is `LargeBinaryManager`'s `DEFAULT_EXECUTION_ID` sentinel -- so a dev box 
running `bin/local-dev.sh up` really does recursively delete `objects/1/` in 
MinIO. Explicit non-1 eids avoid that.
   6. **Do not encode a non-terminal previous execution as clearable.** 
Guarding the clean-up on the previous execution being terminal is a defensible 
production fix; a fixture that persists a running execution and asserts it is 
wiped would block it.
   
   The fault-tolerance block is out of reach: it is gated on 
`ApplicationConfig.faultToleranceLogRootFolder`, a `val` in a Scala `object` 
read at object-init time, with no parameter or override -- and 
`ApplicationConfigSpec:90` asserts it is `None`.
   
   ### 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