aglinxinyuan opened a new issue, #7846:
URL: https://github.com/apache/texera/issues/7846
### Task Summary
`amber/.../web/resource/pythonvirtualenvironment/PveWebsocketResource.scala`
has no spec at all — 9 tracked lines at 0%, one call site, and no branches
JaCoCo can see. Alongside it, two guards in `PveManager.scala` are unreached on
CI: the missing-interpreter early returns in `installUserPackages` (around line
452) and `deletePackages` (around line 524).
The reason this pair is safe to take, and the thing to verify before
starting: **both guards return before the `systemPackageNames` read.**
`installUserPackages` returns at 454, `deletePackages` at 527, while the reads
are at 468 and 533. So neither needs a fake process runner, and neither forces
the memoized `private lazy val systemPackages` — which matters because
`PveManager.runProcess` is a JVM-global `var` and resolving that `lazy val`
once poisons every suite that follows in the same JVM.
Traps worth knowing:
1. **A plain `jacoco` run on this module emits an all-zero report, not a
partial one.** sbt-jacoco instruments offline and runs tests unforked, so its
`saveRuntimeData` step never runs when the test task fails — and
`PveResourceSpec` has 6 pre-existing Windows failures. The runbook that works:
run `jacoco` (it fails, but the counters now live in the sbt JVM), then via
`onFailure` append a second `Tests.Filter(_ => false)` and run `jacoco` again;
the test step succeeds with 0 tests and dumps everything the JVM accumulated.
Also `rm -rf` the jacoco dir between measurements and use one fresh sbt JVM per
measurement, since unforked counters accumulate in-JVM.
2. **`-z` name filters are a trap for attribution.** Filtering by a phrase
that only one spec's test names contain silently excludes the other spec
entirely, which can make one suite look like it owns lines another suite
already covers. Attribute with suite-name filters, one spec at a time.
3. **Nothing inside the pump loop is measurable.** Every conditional in this
file — the `action match`, the `while (!done && session.isOpen)` guard, the
`__DONE__` comparison — sits inside a `Future { … }` lambda, and JaCoCo's
`SyntheticFilter` drops those wholesale. The class element contains exactly
`onOpen` and `<init>`, with no `$anonfun$onOpen$*` at all, and there is no
BRANCH counter element for the file. Those conditionals can still be
*asserted*; they just cannot be *counted*, so don't plan coverage around them.
4. **The loop's exit is easy to leave unpinned.** A fixture that stubs
`isOpen` as always-true and `close()` as a counter cannot observe either
termination condition, so setting the done flag to `false` — which parks the
pump in `queue.take()` forever on a queue nothing will refill, leaking a thread
per connection — passes. Counting guard evaluations is enough to catch it: the
guard reads the session exactly n times for an n-line sentinel-terminated
stream, because the (n+1)th pass short-circuits on the flag.
5. **Local coverage of `PveManager` on Windows is the mirror image of
CI's.** The interpreter is never found here, so `PveResourceSpec`'s
pre-existing failures already take the guards' true arms incidentally; on Linux
those are exactly the missed arms. Expect zero local movement on that file and
say so rather than reporting a gain you cannot measure.
### 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]
