Skip to content

tests: keep the shared gcsfs client alive across the PID-change unit test - #725

Merged
amarcozzi merged 1 commit into
mainfrom
724-documents-unit-loop
Oct 15, 2026
Merged

amarcozzi merged 1 commit into
mainfrom
724-documents-unit-loop

Conversation

@amarcozzi

Copy link
Copy Markdown
Contributor

Closes #724

Summary

test_rebuilds_client_after_pid_change patches os.getpid, so fsspec clears its instance cache and drops the only strong reference to the process-wide gcsfs client. That client was already used by the blobs integration tests and holds an aiohttp session on a closed loop. When GC later frees it, gcsfs's finalizer closes the session on whatever loop is running, and an unrelated async test fails with "attached to a different loop" (on main: test_documents_unit::test_owner_validation_success).

A keep_gcsfs_client_alive fixture on that test saves blobs._gcsfs_client, its PID, fsspec's cache and fsspec's recorded PID, and restores them on teardown, so the parent client is never orphaned. Test-only change; production never patches the PID or clears the cache.

Testing

  • tests/db/test_blobs_unit.py and tests/db/test_documents_unit.py: 57 passed locally.
  • A local probe showed the parent client still alive after GC with the fixture, and freed without it.
  • Not reproduced end to end: the failing order needs the real GCS and Firestore integration tests, and API tests run only on pushes to main/dev. Confirmed by the post-merge run.

…test (#724)

fsspec clears its instance cache on a new PID, orphaning the client the
blobs integration tests used; its finalizer later closed the session on
another test's loop.
@amarcozzi amarcozzi self-assigned this Oct 15, 2026
@amarcozzi
amarcozzi merged commit 5556958 into main Oct 15, 2026
1 check passed
@amarcozzi
amarcozzi deleted the 724-documents-unit-loop branch October 15, 2026 15:32
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.

CI: test_documents_unit fails with 'attached to a different loop' after the PID-change blobs test

1 participant