Skip to content

tests: stop room tests hanging on macOS - #6857

Open
longcw wants to merge 2 commits into
mainfrom
longc/bound-room-test-fd-limit
Open

tests: stop room tests hanging on macOS#6857
longcw wants to merge 2 commits into
mainfrom
longc/bound-room-test-fd-limit

Conversation

@longcw

@longcw longcw commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Problem

tests/test_room.py is marked concurrent, so its nine tests open every room at the same time. When the file descriptor soft limit is large, the native layer on macOS never completes Room.connect and pytest --unit hangs forever. Nothing bounds Room.connect, and per-item timeouts do not arm inside a concurrent group, so the run stops with no test name.

Fix

pytest_configure now caps the file descriptor soft limit at 8192 on macOS, and only ever lowers it, so a machine already below that value is unaffected. Room.connect now runs under asyncio.wait_for, so a wedged connection fails in 15 seconds with a test name. Linux is untouched, so CI behavior does not change.

The 8192 value is measured on one macOS machine: 8192 passes on three trials, 12288 and above deadlock, and 1024 and below fail on descriptor exhaustion.

test_room.py is marked concurrent, so its nine tests open every room at
once. On macOS the native layer never completes Room.connect when the
file descriptor soft limit is large, and the whole --unit run hangs.

Cap the soft limit at 8192 in pytest_configure, lowering it only, and
wrap Room.connect in asyncio.wait_for so a wedged connection fails with
a test name attached.
@longcw
longcw requested a review from a team as a code owner August 14, 2026 13:24
devin-ai-integration[bot]

This comment was marked as resolved.

The timeout raised before the try block, so the room was never
disconnected and its FFI handle, event tasks and socket stayed alive for
the rest of the shared-loop concurrent group.

Move the connect inside the try whose finally disconnects, and reuse
that helper in _test_wait_disconnect instead of repeating the connect.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

View 1 additional finding in Devin Review.

Open in Devin Review

Comment thread tests/conftest.py
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.

1 participant