lance-format / lance-format/lance
bug: the scratch dir leak tests can pass without checking anything
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 7.1k
- Forks
- 852
- Avg merge
- 3d 18h
- Merged PRs (30d)
- 272
Description
Problem
Two tests exist to catch a leaked scratch directory: test_hnsw_pq_scratch_dir_is_not_leaked for the legacy IVF_HNSW partition staging dir, and test_shuffle_scratch_dir_is_not_leaked (added in #8993) for the shuffle scratch dir. Both work the same way: re-run the build in a child process with the child's temp dir pointed at an isolated directory the parent owns, then scan that directory for survivors.
Both are unreliable in ways that produce a green result rather than a failure.
They check nothing on Windows. The redirect sets only TMPDIR. std::env::temp_dir() reads TMPDIR on POSIX, but on Windows it reads TMP, then TEMP, then USERPROFILE. So on a Windows runner the child stages into the real temp directory, the parent scans a directory that holds nothing but the child's dataset, and the assertion passes without having looked at anything. A reintroduced leak stays green there, and the failure mode is invisible: a leak and a fix produce the same result.
The legacy test can go green permanently after a rename. Its child is selected by a hardcoded test-path string, and the parent's only gate is the child's exit code. libtest exits 0 when --exact matches nothing, so renaming the child test, moving it, or renaming LANCE_HNSW_LEAK_TEST_ROOT on one side makes the filter match nothing, the child exit 0 without doing anything, and the parent scan an untouched directory. test_shuffle_scratch_dir_is_not_leaked closed this by deriving the filter from std::thread::current().name() and asserting the child's dataset directory exists; the legacy test has neither guard.
What should change
Redirect all three variables, and have the child prove the redirect took effect rather than trusting it, so a host where the environment cannot be redirected fails loudly instead of reporting a clean scan. Give the legacy parent the same proof-of-work check the newer test has.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the Rust tests named test_hnsw_pq_scratch_dir_is_not_leaked and test_shuffle_scratch_dir_is_not_leaked, then run both leak tests on POSIX and Windows where possible. Check how the child process receives its temporary-directory environment and how each parent verifies child work. Done means all three variables are redirected, the child proves the redirect took effect, and the legacy test rejects a non-matching filter.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- testing-qa
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 75/100