fix(tests): unblock Windows pytest cleanup of components fixture (#206)#754
Merged
fix(tests): unblock Windows pytest cleanup of components fixture (#206)#754
Conversation
#206) On Windows, pytest's tmp_path roots are reused across sessions, and any file handle held past ``close_components()`` blocks ``rmtree`` on the next session, leaving stale dirs behind that trip ``FileExistsError [WinError 183]`` on the following ``mkdir()`` (reported by @powerzist during #162). Two layered changes: - ``OnnxEmbedder.close()`` and ``FastEmbedReranker.close()`` now call ``gc.collect()`` after dropping ``self._model``, forcing the underlying ORT ``InferenceSession`` (mmap on the model file + thread-local arenas) to release before pytest reuses ``tmp_path``. Best-effort handle hygiene — helps but isn't fully reliable under Windows file-locking timing. - The three ``components``-style fixtures and the two ``test_cli_ingest`` Codex tests now ``mkdir(exist_ok=True)`` (with ``shutil.rmtree`` first where the dir gets populated with .md files). Each site links to #206 so the workaround stays visible if the underlying close paths regress. ``Storage.close()`` already runs ``PRAGMA wal_checkpoint(TRUNCATE)`` and closes the read-pool — no change needed there. Linux suite green (4012 passed). Windows confirmation by @powerzist on the PR, since CI doesn't run a Windows matrix. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Owner
Author
|
@powerzist — could you give this a try in your Windows environment when you have a chance? You filed the original report in #206, and since the repo doesn't run a Windows CI matrix yet (#634), your reproduction is the only way to confirm the |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
OnnxEmbedder.close()andFastEmbedReranker.close()nowgc.collect()after droppingself._model, forcing the underlying ORTInferenceSession(mmap on the model file + thread-local arenas) to release promptly. Best-effort handle hygiene.components-style fixtures (conftest.py) and twotest_cli_ingestCodex tests nowmkdir(exist_ok=True)(withshutil.rmtree(..., ignore_errors=True)first where.mdcontent gets written), so reusedtmp_pathroots from a prior failed-cleanup session no longer tripFileExistsError [WinError 183]. Each site has an inline#206reference.Storage.close()already runsPRAGMA wal_checkpoint(TRUNCATE)and closes the read-pool — no change needed there.Closes #206. Original report by @powerzist during #162.
Why both source-level and site-targeted
The issue's acceptance bullets allow either approach:
exist_ok=Trueneeded at sites.Neither is fully reliable on its own under Windows file-locking semantics (handles get released when the previous Python process dies, but Windows takes a moment; AV/indexing services can also briefly hold scans). The site-targeted change is the load-bearing one — without it, any future regression in a
.close()method silently re-introduces the failure. The source-level change makes it less likely the leftover ever happens in the first place. Defense in depth, with comments at every site so neither layer becomes invisible.Test plan
uv run ruff check packages/memtomem/src packages/memtomem/tests— cleanuv run ruff format --check packages/memtomem/src packages/memtomem/tests— cleanuv run pytest -m "not ollama" packages/memtomem/tests— 4012 passed, 7 skipped, 46 deselected🤖 Generated with Claude Code