From 6b1be51c20b08e643441b562ab9e117c5903f604 Mon Sep 17 00:00:00 2001 From: William Bergamin Date: Wed, 22 Jul 2026 13:07:20 -0400 Subject: [PATCH 1/2] test: fix intermittent CI hang in the unit test matrix 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 --- pyproject.toml | 2 + requirements/testing.txt | 4 ++ tests/mock_web_api_server/__init__.py | 6 ++- .../mock_web_api_server/mock_server_thread.py | 10 ++--- tests/rtm/mock_web_api_server.py | 17 +++++--- .../socket_mode/test_interactions_builtin.py | 4 +- tests/slack_sdk/web/mock_web_api_handler.py | 41 +------------------ .../test_interactions_websockets.py | 2 + 8 files changed, 30 insertions(+), 56 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 712f19340..8a74a0d93 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -70,6 +70,8 @@ filterwarnings = [ "ignore:slack.* package is deprecated. Please use slack_sdk.* package instead.*:UserWarning", ] asyncio_mode = "auto" +timeout = 180 +timeout_method = "thread" [tool.mypy] diff --git a/requirements/testing.txt b/requirements/testing.txt index c7d5cb680..c024e4169 100644 --- a/requirements/testing.txt +++ b/requirements/testing.txt @@ -15,6 +15,10 @@ pytest>=9.1.1,<10; python_version >= "3.10" # Note: for async. pytest-asyncio<2 +# pytest-timeout +# Note: per-test timeout so a hung test fails fast with a thread dump instead of stalling CI. +pytest-timeout>=2.2,<3 + # pytest-cov # Note: pytest-cov 7.1+ requires Python >=3.9; cap older interpreters below it. pytest-cov>=4,<7.1.0; python_version < "3.9" diff --git a/tests/mock_web_api_server/__init__.py b/tests/mock_web_api_server/__init__.py index afb5761c4..d62b25704 100644 --- a/tests/mock_web_api_server/__init__.py +++ b/tests/mock_web_api_server/__init__.py @@ -15,7 +15,8 @@ def setup_mock_web_api_server(test: TestCase, handler: Type[SimpleHTTPRequestHan test.received_requests = ReceivedRequests(Queue()) test.thread = MockServerThread(queue=test.received_requests.queue, test=test, handler=handler, port=port) test.thread.start() - test.server_started.wait() + if not test.server_started.wait(timeout=5): + raise RuntimeError(f"Mock web API server failed to start on port {port} within 5s (port already in use?)") def cleanup_mock_web_api_server(test: TestCase): @@ -56,7 +57,8 @@ def setup_mock_web_api_server_async(test: TestCase, handler: Type[SimpleHTTPRequ test.received_requests = ReceivedRequests(asyncio.Queue()) test.thread = MockServerThread(queue=test.received_requests.queue, test=test, handler=handler, port=port) test.thread.start() - test.server_started.wait() + if not test.server_started.wait(timeout=5): + raise RuntimeError(f"Mock web API server failed to start on port {port} within 5s (port already in use?)") def cleanup_mock_web_api_server_async(test: TestCase): diff --git a/tests/mock_web_api_server/mock_server_thread.py b/tests/mock_web_api_server/mock_server_thread.py index 0888cc4ea..10b0dc071 100644 --- a/tests/mock_web_api_server/mock_server_thread.py +++ b/tests/mock_web_api_server/mock_server_thread.py @@ -1,6 +1,6 @@ from asyncio import Queue import asyncio -from http.server import HTTPServer, SimpleHTTPRequestHandler +from http.server import SimpleHTTPRequestHandler, ThreadingHTTPServer import threading from typing import Type, Union from unittest import TestCase @@ -10,14 +10,14 @@ class MockServerThread(threading.Thread): def __init__( self, queue: Union[Queue, asyncio.Queue], test: TestCase, handler: Type[SimpleHTTPRequestHandler], port: int = 8888 ): - threading.Thread.__init__(self) + threading.Thread.__init__(self, daemon=True) self.handler = handler self.test = test self.queue = queue self.port = port def run(self): - self.server = HTTPServer(("localhost", self.port), self.handler) + self.server = ThreadingHTTPServer(("localhost", self.port), self.handler) self.server.queue = self.queue self.test.server_url = f"http://localhost:{str(self.port)}" self.test.host, self.test.port = self.server.socket.getsockname() @@ -33,9 +33,9 @@ def stop(self): with self.server.queue.mutex: del self.server.queue self.server.shutdown() - self.join() + self.join(timeout=5) def stop_unsafe(self): del self.server.queue self.server.shutdown() - self.join() + self.join(timeout=5) diff --git a/tests/rtm/mock_web_api_server.py b/tests/rtm/mock_web_api_server.py index 9d03ac837..eae8b300b 100644 --- a/tests/rtm/mock_web_api_server.py +++ b/tests/rtm/mock_web_api_server.py @@ -2,7 +2,7 @@ import logging import threading from http import HTTPStatus -from http.server import HTTPServer, SimpleHTTPRequestHandler +from http.server import SimpleHTTPRequestHandler, ThreadingHTTPServer from typing import Type from unittest import TestCase @@ -63,14 +63,16 @@ def do_POST(self): class MockServerThread(threading.Thread): + port = 8888 + def __init__(self, test: TestCase, handler: Type[SimpleHTTPRequestHandler] = MockHandler): - threading.Thread.__init__(self) + threading.Thread.__init__(self, daemon=True) self.handler = handler self.test = test def run(self): - self.server = HTTPServer(("localhost", 8888), self.handler) - self.test.server_url = "http://localhost:8888" + self.server = ThreadingHTTPServer(("localhost", self.port), self.handler) + self.test.server_url = f"http://localhost:{self.port}" self.test.host, self.test.port = self.server.socket.getsockname() self.test.server_started.set() # threading.Event() @@ -82,14 +84,17 @@ def run(self): def stop(self): self.server.shutdown() - self.join() + self.join(timeout=5) def setup_mock_web_api_server(test: TestCase): test.server_started = threading.Event() test.thread = MockServerThread(test) test.thread.start() - test.server_started.wait() + if not test.server_started.wait(timeout=5): + raise RuntimeError( + f"Mock web API server failed to start on port {test.thread.port} within 5s (port already in use?)" + ) def cleanup_mock_web_api_server(test: TestCase): diff --git a/tests/slack_sdk/socket_mode/test_interactions_builtin.py b/tests/slack_sdk/socket_mode/test_interactions_builtin.py index 4ff576fcf..3f53e4cd1 100644 --- a/tests/slack_sdk/socket_mode/test_interactions_builtin.py +++ b/tests/slack_sdk/socket_mode/test_interactions_builtin.py @@ -112,14 +112,12 @@ def socket_mode_request_handler(client: BaseSocketModeClient, request: SocketMod self.assertEqual(len(socket_mode_envelopes), len(received_socket_mode_requests)) finally: - pass - # client.close() + client.close() self.logger.info(f"Passed with buffer size: {buffer_size}") finally: # Restore the default value sys.setrecursionlimit(default_recursion_limit) - client.close() self.logger.info(f"Passed with buffer size: {buffer_size_list}") diff --git a/tests/slack_sdk/web/mock_web_api_handler.py b/tests/slack_sdk/web/mock_web_api_handler.py index 12a487e05..5416f4d31 100644 --- a/tests/slack_sdk/web/mock_web_api_handler.py +++ b/tests/slack_sdk/web/mock_web_api_handler.py @@ -1,14 +1,9 @@ -import asyncio import json import logging -from queue import Queue import re -import threading import time from http import HTTPStatus -from http.server import HTTPServer, SimpleHTTPRequestHandler -from typing import Type, Union -from unittest import TestCase +from http.server import SimpleHTTPRequestHandler from urllib.parse import urlparse, parse_qs @@ -258,37 +253,3 @@ def do_GET(self): def do_POST(self): self._handle() - - -class MockServerThread(threading.Thread): - def __init__( - self, queue: Union[Queue, asyncio.Queue], test: TestCase, handler: Type[SimpleHTTPRequestHandler] = MockHandler - ): - threading.Thread.__init__(self) - self.handler = handler - self.test = test - self.queue = queue - - def run(self): - self.server = HTTPServer(("localhost", 8888), self.handler) - self.server.queue = self.queue - self.test.server_url = "http://localhost:8888" - self.test.host, self.test.port = self.server.socket.getsockname() - self.test.server_started.set() # threading.Event() - - self.test = None - try: - self.server.serve_forever(0.05) - finally: - self.server.server_close() - - def stop(self): - with self.server.queue.mutex: - del self.server.queue - self.server.shutdown() - self.join() - - def stop_unsafe(self): - del self.server.queue - self.server.shutdown() - self.join() diff --git a/tests/slack_sdk_async/socket_mode/test_interactions_websockets.py b/tests/slack_sdk_async/socket_mode/test_interactions_websockets.py index 5b408dbcf..ca7237566 100644 --- a/tests/slack_sdk_async/socket_mode/test_interactions_websockets.py +++ b/tests/slack_sdk_async/socket_mode/test_interactions_websockets.py @@ -17,6 +17,7 @@ start_socket_mode_server, socket_mode_envelopes, socket_mode_hello_message, + stop_socket_mode_server, ) from tests.slack_sdk.socket_mode.mock_web_api_handler import MockHandler from tests.mock_web_api_server import setup_mock_web_api_server_async, cleanup_mock_web_api_server_async @@ -36,6 +37,7 @@ def setUp(self): def tearDown(self): cleanup_mock_web_api_server_async(self) + stop_socket_mode_server(self) @async_test async def test_interactions(self): From 57346bec28f04c35fda6ef90fbe660f069a73c32 Mon Sep 17 00:00:00 2001 From: William Bergamin Date: Thu, 23 Jul 2026 10:59:23 -0400 Subject: [PATCH 2/2] test: harden mock server teardown from code-review findings 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 --- tests/mock_web_api_server/__init__.py | 10 ++++-- .../mock_web_api_server/mock_server_thread.py | 31 ++++++++++-------- tests/rtm/mock_web_api_server.py | 32 +++---------------- .../test_interactions_websockets.py | 6 ++-- 4 files changed, 33 insertions(+), 46 deletions(-) diff --git a/tests/mock_web_api_server/__init__.py b/tests/mock_web_api_server/__init__.py index d62b25704..0c582ae40 100644 --- a/tests/mock_web_api_server/__init__.py +++ b/tests/mock_web_api_server/__init__.py @@ -16,7 +16,9 @@ def setup_mock_web_api_server(test: TestCase, handler: Type[SimpleHTTPRequestHan test.thread = MockServerThread(queue=test.received_requests.queue, test=test, handler=handler, port=port) test.thread.start() if not test.server_started.wait(timeout=5): - raise RuntimeError(f"Mock web API server failed to start on port {port} within 5s (port already in use?)") + raise RuntimeError( + f"Mock web API server failed to start on port {test.thread.port} within 5s (port already in use?)" + ) def cleanup_mock_web_api_server(test: TestCase): @@ -58,11 +60,13 @@ def setup_mock_web_api_server_async(test: TestCase, handler: Type[SimpleHTTPRequ test.thread = MockServerThread(queue=test.received_requests.queue, test=test, handler=handler, port=port) test.thread.start() if not test.server_started.wait(timeout=5): - raise RuntimeError(f"Mock web API server failed to start on port {port} within 5s (port already in use?)") + raise RuntimeError( + f"Mock web API server failed to start on port {test.thread.port} within 5s (port already in use?)" + ) def cleanup_mock_web_api_server_async(test: TestCase): - test.thread.stop_unsafe() + test.thread.stop() test.thread = None diff --git a/tests/mock_web_api_server/mock_server_thread.py b/tests/mock_web_api_server/mock_server_thread.py index 10b0dc071..6022bb14b 100644 --- a/tests/mock_web_api_server/mock_server_thread.py +++ b/tests/mock_web_api_server/mock_server_thread.py @@ -1,14 +1,21 @@ -from asyncio import Queue import asyncio -from http.server import SimpleHTTPRequestHandler, ThreadingHTTPServer +import logging import threading -from typing import Type, Union +from http.server import SimpleHTTPRequestHandler, ThreadingHTTPServer +from queue import Queue +from typing import Optional, Type, Union from unittest import TestCase +logger = logging.getLogger(__name__) + class MockServerThread(threading.Thread): def __init__( - self, queue: Union[Queue, asyncio.Queue], test: TestCase, handler: Type[SimpleHTTPRequestHandler], port: int = 8888 + self, + test: TestCase, + handler: Type[SimpleHTTPRequestHandler], + queue: Optional[Union[Queue, asyncio.Queue]] = None, + port: int = 8888, ): threading.Thread.__init__(self, daemon=True) self.handler = handler @@ -18,8 +25,9 @@ def __init__( def run(self): self.server = ThreadingHTTPServer(("localhost", self.port), self.handler) - self.server.queue = self.queue - self.test.server_url = f"http://localhost:{str(self.port)}" + if self.queue is not None: + self.server.queue = self.queue + self.test.server_url = f"http://localhost:{self.port}" self.test.host, self.test.port = self.server.socket.getsockname() self.test.server_started.set() # threading.Event() @@ -30,12 +38,9 @@ def run(self): self.server.server_close() def stop(self): - with self.server.queue.mutex: - del self.server.queue - self.server.shutdown() - self.join(timeout=5) - - def stop_unsafe(self): - del self.server.queue self.server.shutdown() self.join(timeout=5) + if self.is_alive(): + logger.warning(f"Mock web API server thread on port {self.port} did not stop within 5s") + if getattr(self.server, "queue", None) is not None: + del self.server.queue diff --git a/tests/rtm/mock_web_api_server.py b/tests/rtm/mock_web_api_server.py index eae8b300b..2cb279887 100644 --- a/tests/rtm/mock_web_api_server.py +++ b/tests/rtm/mock_web_api_server.py @@ -2,10 +2,11 @@ import logging import threading from http import HTTPStatus -from http.server import SimpleHTTPRequestHandler, ThreadingHTTPServer -from typing import Type +from http.server import SimpleHTTPRequestHandler from unittest import TestCase +from tests.mock_web_api_server.mock_server_thread import MockServerThread + class MockHandler(SimpleHTTPRequestHandler): protocol_version = "HTTP/1.1" @@ -62,34 +63,9 @@ def do_POST(self): self._handle() -class MockServerThread(threading.Thread): - port = 8888 - - def __init__(self, test: TestCase, handler: Type[SimpleHTTPRequestHandler] = MockHandler): - threading.Thread.__init__(self, daemon=True) - self.handler = handler - self.test = test - - def run(self): - self.server = ThreadingHTTPServer(("localhost", self.port), self.handler) - self.test.server_url = f"http://localhost:{self.port}" - self.test.host, self.test.port = self.server.socket.getsockname() - self.test.server_started.set() # threading.Event() - - self.test = None - try: - self.server.serve_forever(0.05) - finally: - self.server.server_close() - - def stop(self): - self.server.shutdown() - self.join(timeout=5) - - def setup_mock_web_api_server(test: TestCase): test.server_started = threading.Event() - test.thread = MockServerThread(test) + test.thread = MockServerThread(test=test, handler=MockHandler) test.thread.start() if not test.server_started.wait(timeout=5): raise RuntimeError( diff --git a/tests/slack_sdk_async/socket_mode/test_interactions_websockets.py b/tests/slack_sdk_async/socket_mode/test_interactions_websockets.py index ca7237566..4209195e8 100644 --- a/tests/slack_sdk_async/socket_mode/test_interactions_websockets.py +++ b/tests/slack_sdk_async/socket_mode/test_interactions_websockets.py @@ -36,8 +36,10 @@ def setUp(self): start_socket_mode_server(self, 3001) def tearDown(self): - cleanup_mock_web_api_server_async(self) - stop_socket_mode_server(self) + try: + cleanup_mock_web_api_server_async(self) + finally: + stop_socket_mode_server(self) @async_test async def test_interactions(self):