test: fix intermittent CI hang in the unit test matrix by WilliamBergamin · Pull Request #1921 · slackapi/python-slack-sdk

WilliamBergamin · 2026-07-23T15:17:20Z

Summary

Fixes an intermittent hang in the Unit tests CI matrix that occasionally stalled the job until the workflow-level timeout.

Root cause: the tests' mock web API server ran on a single-threaded HTTPServer with a non-daemon thread, and teardown did del self.server.queue before server.shutdown(). An in-flight request handler (whose first action is queue.put_nowait(...)) could race the teardown, and a non-daemon thread that never joined could wedge the whole run.

Changes (test infra only):

Switch the mock server to ThreadingHTTPServer and run it on a daemon=True thread so a stuck worker can never block interpreter exit.
Reorder teardown to shutdown() → join(timeout=5) → del queue, closing the delete-before-shutdown race (server_close() in the run loop's finally joins workers, so no handler touches the queue after the join).
Unify stop()/stop_unsafe() into a single stop() that works for both the sync queue.Queue and async asyncio.Queue paths (the mutex is no longer needed), and make the queue optional so RTM can reuse the class.
Add startup guards: server_started.wait(timeout=5) now raises a clear RuntimeError (e.g. "port already in use") instead of blocking forever.
Log a warning when a server thread outlives the 5s join, so a re-emerging leak is visible instead of silent.
De-duplicate: delete the near-identical MockServerThread copies in tests/rtm/ and tests/slack_sdk/web/; both now import the canonical implementation.
Wrap the async socket-mode websockets tearDown in try/finally so the socket-mode server is always stopped even if mock-server cleanup raises.
Restore client.close() in test_interactions_builtin.py (was commented out).
Add pytest-timeout (timeout=180, timeout_method="thread") so a future hang fails fast with a thread dump instead of stalling CI. The thread method is required here because signal cannot interrupt hangs on non-main threads (asyncio/socket-mode).

Testing

Reviewers can verify with:

./scripts/run_tests.sh tests/slack_sdk/web/
./scripts/run_tests.sh tests/slack_sdk_async/web/
./scripts/run_tests.sh tests/rtm/
./scripts/run_tests.sh tests/slack_sdk/rtm_v2/
./scripts/run_tests.sh tests/slack_sdk_async/socket_mode/test_interactions_websockets.py

Verified locally: full suite 988 passed, 5 skipped (4m17s) with the per-test timeout armed (no hang), black --check clean, and the async socket-mode test stable across 5 consecutive runs.

Requirements

I've read and understood the Contributing Guidelines and have done my best effort to follow them.
I've read and agree to the Code of Conduct.
I've run python3 -m venv .venv && source .venv/bin/activate && ./scripts/run_validation.sh after making the changes.

The "Unit tests (3.x)" matrix job intermittently hung on "Run tests" until its 15-minute timeout on a random Python version, passing on retry. Root cause was in the test infrastructure, not the SDK: - No per-test timeout, so a hung test gave a blind 15-min job timeout with no indication of which test stuck. - The shared mock web API server used a single-threaded HTTPServer on a non-daemon thread with unbounded join()/wait(). With HTTP/1.1 keep-alive it could leak a thread holding port 8888, hanging the next test's setUp. - Two socket-mode interaction tests leaked servers/clients: test_interactions_websockets omitted stop_socket_mode_server in tearDown and shared port 3001 with the aiohttp test; test_interactions_builtin had its per-iteration client.close() commented out, leaking thread pools. Fixes (test/CI only, no shipped SDK change): - Add pytest-timeout with a thread-method timeout so a hang fails fast with a full thread dump instead of stalling the job. - Make the mock web API server threads daemon + ThreadingHTTPServer, and bound join()/server_started.wait() so a failed bind fails loudly. Apply the same to the live duplicate in tests/rtm and drop the dead MockServerThread in tests/slack_sdk/web/mock_web_api_handler.py. - Fix the two leaking socket-mode tests: add the missing teardown, move off the shared port, and re-enable per-iteration client.close(). Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

Follow-up to the CI-hang fix, addressing review findings: - Fix a shutdown race: stop() deleted self.server.queue before server.shutdown(), so under ThreadingHTTPServer an in-flight handler thread could hit AttributeError. Reorder to shutdown() -> join() -> del, after which no handler threads remain. - Unify stop()/stop_unsafe() into a single stop(). The split only existed because asyncio.Queue lacks .mutex; with the safe ordering it is unnecessary. Make the queue optional and drop redundant str(). - Warn when the server thread outlives join(timeout=5) instead of silently abandoning it, so a re-emerging leak is visible. - Dedupe tests/rtm/mock_web_api_server.py to reuse the canonical MockServerThread instead of a near-identical copy. - Wrap the async websockets tearDown in try/finally so the socket-mode server is always stopped even if mock-server cleanup raises. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

codecov · 2026-07-23T15:49:17Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.14%. Comparing base (f41fd83) to head (a2ed641).
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1921      +/-   ##
==========================================
- Coverage   84.17%   84.14%   -0.03%     
==========================================
  Files         118      118              
  Lines       13423    13423              
==========================================
- Hits        11299    11295       -4     
- Misses       2124     2128       +4

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

WilliamBergamin · 2026-07-23T16:24:40Z

        self._handle()


-class MockServerThread(threading.Thread):


Keeps implementation DRY

WilliamBergamin · 2026-07-23T16:25:03Z

    test.thread.start()
-    test.server_started.wait()
+    if not test.server_started.wait(timeout=5):
+        raise RuntimeError(


Fails with noise

WilliamBergamin · 2026-07-23T16:25:21Z

        self._handle()
-
-
-class MockServerThread(threading.Thread):


Keeps implementations DRY

WilliamBergamin · 2026-07-23T16:26:32Z

+        try:
+            cleanup_mock_web_api_server_async(self)
+        finally:
+            stop_socket_mode_server(self)


We were not always stopping the socket mode server I think this was leading to test hanging sometimes

WilliamBergamin · 2026-07-23T16:27:38Z

-        self.server = HTTPServer(("localhost", self.port), self.handler)
-        self.server.queue = self.queue
-        self.test.server_url = f"http://localhost:{str(self.port)}"
+        self.server = ThreadingHTTPServer(("localhost", self.port), self.handler)


The ThreadingHTTPServer is only available in python 3.7+, this is why we were using HTTPServer

hello-ashleyintech

Great work, changes make sense and LGTM! 🙏 thank you

WilliamBergamin and others added 3 commits July 22, 2026 13:07

Merge branch 'main' into fix-flaky-ci-test-hang

a2ed641

WilliamBergamin marked this pull request as ready for review July 23, 2026 15:34

WilliamBergamin requested a review from a team as a code owner July 23, 2026 15:34

WilliamBergamin self-assigned this Jul 23, 2026

WilliamBergamin added the tests M-T: Testing work only label Jul 23, 2026

WilliamBergamin requested review from srtaalej, vegeris and zimeg July 23, 2026 16:23

WilliamBergamin commented Jul 23, 2026

View reviewed changes

hello-ashleyintech approved these changes Jul 28, 2026

View reviewed changes

WilliamBergamin merged commit e60ab67 into main Jul 28, 2026
18 checks passed

WilliamBergamin deleted the fix-flaky-ci-test-hang branch July 28, 2026 17:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

test: fix intermittent CI hang in the unit test matrix - #1921

test: fix intermittent CI hang in the unit test matrix#1921
WilliamBergamin merged 3 commits into
mainfrom
fix-flaky-ci-test-hang

WilliamBergamin commented Jul 23, 2026

Uh oh!

codecov Bot commented Jul 23, 2026

Uh oh!

WilliamBergamin Jul 23, 2026

Uh oh!

WilliamBergamin Jul 23, 2026

Uh oh!

WilliamBergamin Jul 23, 2026

Uh oh!

WilliamBergamin Jul 23, 2026

Uh oh!

WilliamBergamin Jul 23, 2026

Uh oh!

hello-ashleyintech left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

WilliamBergamin commented Jul 23, 2026

Summary

Testing

Category

Requirements

Uh oh!

codecov Bot commented Jul 23, 2026

Codecov Report

Uh oh!

WilliamBergamin Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

WilliamBergamin Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

WilliamBergamin Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

WilliamBergamin Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

WilliamBergamin Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

hello-ashleyintech left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants