Skip to content

fix(library): close sqlite and PDF temp handles so tests pass on Windows - #145

Open
hudsonwa wants to merge 1 commit into
LLMQuant:masterfrom
hudsonwa:fix/library-windows-test-file-locking
Open

fix(library): close sqlite and PDF temp handles so tests pass on Windows#145
hudsonwa wants to merge 1 commit into
LLMQuant:masterfrom
hudsonwa:fix/library-windows-test-file-locking

Conversation

@hudsonwa

Copy link
Copy Markdown

Summary

A fresh Windows checkout fails scripts/verify.sh at the pytest step (18 failures) because several tests and one production helper assumed POSIX semantics where an open file handle does not block another process from opening that path. On Windows, an open handle prevents deletion/opening, so:

  • The sqlite-backed library tests used with sqlite3.connect(path) as db:, which commits on exit but does not close the connection. tempfile.TemporaryDirectory.cleanup() in tearDown therefore failed with PermissionError [WinError 32] (17 of the 18 failures, in tests/library/).
  • _write_artifacts in quantmind/preprocess/format/pdf.py kept a NamedTemporaryFile handle open while handing source.name to the native liteparse parser, which then could not open the path (PDF error: file not found) — the 18th failure.

This PR fixes both root causes:

  • Close the sqlite connection after each with sqlite3.connect(...) as db: helper block in the four library test files (db.close() immediately after the block). This preserves the block's commit-on-exit semantics while releasing the file handle so the temp dir can be cleaned up.
  • For the PDF artifact helper, use delete=False, close the temp file handle before passing the path to the native parser, and unlink the file afterward in a finally.

These are test-infra / Windows-portability fixes only; no production behavior changes.

Related Issue

Closes #143

Verification

Performed on a Windows 11 host (Python 3.12.10) with the .[dev] environment, on a fresh shallow clone:

  • Before (clean checkout, unmodified): test_pdf.py::test_golden_preserves_pages_blocks_coordinates_and_artifacts fails with PDF error: file not found; tests/library/test_local.py, test_structure.py, test_paper.py fail with PermissionError [WinError 32] (17 failures).
  • After this patch:
    • pytest tests/library/test_local.py tests/library/test_structure.py tests/library/test_paper.py tests/library/test_example_bundle.py tests/preprocess/format/test_pdf.py -> 40 passed
    • Full suite: 425 passed, 1 failed where the single failure is the pre-existing, unrelated DiscoverPRNewswireTests::test_cache_bust_retries_listing_404 timing flake (microsecond cache-bust timestamp collision in quantmind/preprocess/pr_newswire.py); it fails identically on an unmodified master checkout and is not touched by this change.
    • ruff check on the changed files -> "All checks passed!"; ruff format --check -> all changed files formatted.

scripts/verify.sh could not be run end-to-end on the Windows box because its shell script is checked out with CRLF line endings (fails under Git Bash on Windows with $'\r': command not found); its individual steps were run and verified as green as noted above.

Checklist

  • The title uses English Conventional Commit format: type(scope): summary.
  • The related issue or design discussion is linked when applicable.
  • bash scripts/verify.sh passes.
  • Every applicable live-network component smoke test passes, or this PR states why none applies.
  • Public behavior has focused tests, an example, and documentation where applicable.
  • The PR is complete, small, and contains no unrelated changes.

tests/library/* and the PDF golden test assumed POSIX semantics where an open
file handle does not block another process from opening that path. On Windows:

- sqlite test helpers used \with sqlite3.connect(path) as db:\, which commits
  on exit but does not close the connection, so temporary_directory.cleanup()
  in tearDown failed with PermissionError [WinError 32].
- _write_artifacts kept a NamedTemporaryFile handle open while handing
  source.name to the native liteparse parser, which then could not open it
  (PDF error: file not found).

Fix: close the sqlite connection after each helper block, and for the PDF
artifact use delete=False + close the handle before parsing, then unlink.
Prevents a fresh Windows checkout from failing scripts/verify.sh pytest step.
Closes LLMQuant#143.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: fresh Windows checkout fails scripts/verify.sh pytest step (sqlite tempdir cleanup race + PDF golden test file-locking)

1 participant