File-handle leak aborts the operator suite, plus an unseeded spec and a vacuous assertion
- Dominant language
- Scala
- Stars
- 314
- Forks
- 187
- Avg merge
- 1d 21h
- Merged PRs (30d)
- 214
Description
### Describe the bug
Three defects in test code. The first blocks coverage measurement outright on Windows.
**1. `FileScanSourceOpExecSpec` leaks a file handle, aborting the suite and killing the whole module's coverage run.**
In `"create LargeBinary when reading file with LARGE_BINARY type"`, `createTuplesFromFile` throws mid-`map` with `IllegalStateException: LargeBinaryManager.create() requires a base URI, but none was set on the current thread`. That abandons the `AutoClosingIterator` before exhaustion, so its close-on-exhaustion hook never fires and the handle stays open. `afterAll` then fails with `test_large_binary.txt: The process cannot access the file because it is being used by another process`, which **aborts the suite**.
The consequence is bigger than one suite: `WorkflowOperator/jacoco` exits 1 and **emits no report directory at all**, so the whole module cannot be measured. POSIX `unlink` hides this on Linux CI, which is why it has gone unnoticed.
(The `.zip` fixture is stranded too — it only *looks* fine because `.gitignore:10` is `*.zip`. `afterAll` never reaches it, because the `.txt` delete throws first.)
**2. `IntervalOpExecSpec` is unseeded-random.** It imports `scala.util.Random.{nextInt, nextLong}` and uses them at four sites — input ordering twice, a 1k-row dataset, and the interval constant. The coverage footprint of `IntervalJoinOpExec` therefore drifts between runs of identical source: CI has reported 2 missed + 14 partial where a local run gave 0 missed + 13 partial.
**3. `PveResourceSpec` has a vacuous path-traversal assertion.** `getPythonBin(testCuid, "..") shouldBe None` claims to pin the guard at `PveManager.scala:91` (`if (!resolved.startsWith(root)) return None`), but `".."` **matches** the name regex at line 88 (`^[A-Za-z0-9._-]+$` admits dots) and `//../pve` normalizes to `/pve` — still under root. The `None` actually comes from the `Files.exists` check on line 92.
Proof: deleting line 91 entirely leaves that test passing, with results identical to baseline. Line 91 is in fact **unreachable by construction** — the regex forbids `/`, and `cuid` is an `Int` — so it is defensive code, not untested code.
### How to reproduce?
Run `sbt "WorkflowOperator/jacoco"` on Windows for (1) — the suite aborts and no report is produced. For (2), run `IntervalOpExecSpec` twice and compare the per-line jacoco counters for `IntervalJoinOpExec`. For (3), delete `PveManager.scala:91` and observe that the traversal test still passes.
### Version/Branch
main
### Task Type
- [ ] Refactor / Cleanup
- [ ] DevOps / Deployment / CI
- [x] Testing / QA
- [ ] Documentation
- [ ] Performance
- [ ] Other
Contributor guide
Assessment
This issue has not been assessed yet.