From 4bb73593b81fa87c4207eda99d749ad79fff2b79 Mon Sep 17 00:00:00 2001 From: William Bergamin Date: Fri, 22 Nov 2024 15:09:31 -0500 Subject: [PATCH 1/7] tests: add tests that aim to ensure consistent behavior around url formats --- tests/helpers.py | 18 +++++ .../web/test_web_client_url_format.py | 64 +++++++++++++++++ .../web/test_web_legacy_client_url_format.py | 68 ++++++++++++++++++ .../web/test_web_client_url_format.py | 71 +++++++++++++++++++ 4 files changed, 221 insertions(+) create mode 100644 tests/slack_sdk/web/test_web_client_url_format.py create mode 100644 tests/slack_sdk/web/test_web_legacy_client_url_format.py create mode 100644 tests/slack_sdk_async/web/test_web_client_url_format.py diff --git a/tests/helpers.py b/tests/helpers.py index 598d74f1c..bcae8a81a 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -1,5 +1,7 @@ import asyncio import os +import sys +from types import ModuleType def async_test(coro): @@ -43,3 +45,19 @@ def get_mock_server_mode() -> str: def is_ci_unstable_test_skip_enabled() -> bool: return os.environ.get("CI_UNSTABLE_TESTS_SKIP_ENABLED") == "1" + + +def reload_module(root_module: ModuleType): + package_name = root_module.__name__ + loaded_package_modules = { + key: value for key, value in sys.modules.items() if key.startswith(package_name) and isinstance(value, ModuleType) + } + + for key in loaded_package_modules: + del sys.modules[key] + + for key in loaded_package_modules: + new_module = __import__(key) + old_module = loaded_package_modules[key] + old_module.__dict__.clear() + old_module.__dict__.update(new_module.__dict__) diff --git a/tests/slack_sdk/web/test_web_client_url_format.py b/tests/slack_sdk/web/test_web_client_url_format.py new file mode 100644 index 000000000..353aeb7c1 --- /dev/null +++ b/tests/slack_sdk/web/test_web_client_url_format.py @@ -0,0 +1,64 @@ +from unittest import TestCase +from unittest.mock import patch +from urllib import request +from urllib.request import Request, urlopen + +import slack_sdk.web +from tests.helpers import reload_module +from tests.slack_sdk.web.mock_web_api_server import ( + setup_mock_web_api_server, + cleanup_mock_web_api_server, +) + + +def build_spy_urlopen(test: TestCase): + def spy_urlopen(*args, **kwargs): + test.urlopen_spy_args = args + test.urlopen_spy_kwargs = kwargs + return urlopen(*args, **kwargs) + + return spy_urlopen + + +class TestWebClientUrlFormat(TestCase): + def setUp(self): + setup_mock_web_api_server(self) + with patch.object(request, "urlopen") as mock_urlopen: + mock_urlopen.side_effect = build_spy_urlopen(self) + reload_module(slack_sdk.web) + self.client = slack_sdk.web.WebClient(token="xoxb-api_test", base_url="http://localhost:8888") + self.client_base_url_slash = slack_sdk.web.WebClient(token="xoxb-api_test", base_url="http://localhost:8888/") + + def tearDown(self): + cleanup_mock_web_api_server(self) + self.urlopen_spy_args = None + self.urlopen_spy_kwargs = None + + @classmethod + def tearDownClass(cls): + reload_module(slack_sdk.web) + + def test_base_url_without_slash_api_method_without_slash(self): + self.client.api_call("api.test") + self.assertIsInstance(self.urlopen_spy_args[0], Request) + self.assertEqual(self.urlopen_spy_args[0].full_url, "http://localhost:8888/api.test") + + def test_base_url_without_slash_api_method_with_slash(self): + self.client.api_call("/api.test") + self.assertIsInstance(self.urlopen_spy_args[0], Request) + self.assertEqual(self.urlopen_spy_args[0].full_url, "http://localhost:8888/api.test") + + def test_base_url_with_slash_api_method_without_slash(self): + self.client_base_url_slash.api_call("api.test") + self.assertIsInstance(self.urlopen_spy_args[0], Request) + self.assertEqual(self.urlopen_spy_args[0].full_url, "http://localhost:8888/api.test") + + def test_base_url_with_slash_api_method_with_slash(self): + self.client_base_url_slash.api_call("/api.test") + self.assertIsInstance(self.urlopen_spy_args[0], Request) + self.assertEqual(self.urlopen_spy_args[0].full_url, "http://localhost:8888/api.test") + + def test_base_url_without_slash_api_method_with_slash_and_trailing_slash(self): + self.client.api_call("/api.test/") + self.assertIsInstance(self.urlopen_spy_args[0], Request) + self.assertEqual(self.urlopen_spy_args[0].full_url, "http://localhost:8888/api.test/") diff --git a/tests/slack_sdk/web/test_web_legacy_client_url_format.py b/tests/slack_sdk/web/test_web_legacy_client_url_format.py new file mode 100644 index 000000000..ec5e0c4bb --- /dev/null +++ b/tests/slack_sdk/web/test_web_legacy_client_url_format.py @@ -0,0 +1,68 @@ +from unittest import TestCase +from unittest.mock import patch +from urllib import request +from urllib.request import Request, urlopen + +import slack_sdk.web.legacy_base_client +from tests.helpers import reload_module +from tests.slack_sdk.web.mock_web_api_server import ( + setup_mock_web_api_server, + cleanup_mock_web_api_server, +) + + +def build_spy_urlopen(test: TestCase): + def spy_urlopen(*args, **kwargs): + test.urlopen_spy_args = args + test.urlopen_spy_kwargs = kwargs + return urlopen(*args, **kwargs) + + return spy_urlopen + + +class TestLegacyWebClientUrlFormat(TestCase): + def setUp(self): + setup_mock_web_api_server(self) + with patch.object(request, "urlopen") as mock_urlopen: + mock_urlopen.side_effect = build_spy_urlopen(self) + reload_module(slack_sdk.web.legacy_base_client) + self.client = slack_sdk.web.legacy_base_client.LegacyBaseClient( + token="xoxb-api_test", base_url="http://localhost:8888" + ) + self.client_base_url_slash = slack_sdk.web.legacy_base_client.LegacyBaseClient( + token="xoxb-api_test", base_url="http://localhost:8888/" + ) + + def tearDown(self): + cleanup_mock_web_api_server(self) + self.urlopen_spy_args = None + self.urlopen_spy_kwargs = None + + @classmethod + def tearDownClass(cls): + reload_module(slack_sdk.web.legacy_base_client) + + def test_base_url_without_slash_api_method_without_slash(self): + self.client.api_call("api.test") + self.assertIsInstance(self.urlopen_spy_args[0], Request) + self.assertEqual(self.urlopen_spy_args[0].full_url, "http://localhost:8888/api.test") + + def test_base_url_without_slash_api_method_with_slash(self): + self.client.api_call("/api.test") + self.assertIsInstance(self.urlopen_spy_args[0], Request) + self.assertEqual(self.urlopen_spy_args[0].full_url, "http://localhost:8888/api.test") + + def test_base_url_with_slash_api_method_without_slash(self): + self.client_base_url_slash.api_call("api.test") + self.assertIsInstance(self.urlopen_spy_args[0], Request) + self.assertEqual(self.urlopen_spy_args[0].full_url, "http://localhost:8888/api.test") + + def test_base_url_with_slash_api_method_with_slash(self): + self.client_base_url_slash.api_call("/api.test") + self.assertIsInstance(self.urlopen_spy_args[0], Request) + self.assertEqual(self.urlopen_spy_args[0].full_url, "http://localhost:8888/api.test") + + def test_base_url_without_slash_api_method_with_slash_and_trailing_slash(self): + self.client.api_call("/api.test/") + self.assertIsInstance(self.urlopen_spy_args[0], Request) + self.assertEqual(self.urlopen_spy_args[0].full_url, "http://localhost:8888/api.test/") diff --git a/tests/slack_sdk_async/web/test_web_client_url_format.py b/tests/slack_sdk_async/web/test_web_client_url_format.py new file mode 100644 index 000000000..583584abc --- /dev/null +++ b/tests/slack_sdk_async/web/test_web_client_url_format.py @@ -0,0 +1,71 @@ +import unittest +from unittest.mock import Mock +from aiohttp import ClientSession + +from slack_sdk.web.async_client import AsyncWebClient +from tests.slack_sdk_async.helpers import async_test +from tests.slack_sdk.web.mock_web_api_server import ( + setup_mock_web_api_server, + cleanup_mock_web_api_server, +) + + +class TestWebClientUrlFormat(unittest.TestCase): + def setUp(self): + setup_mock_web_api_server(self) + + self.session = ClientSession() + + def spy_session_request(*args, **kwargs): + self.session_request_spy_args = args + self.session_request_spy_kwargs = kwargs + return self.session.request(*args, **kwargs) + + self.spy_session = ClientSession() + self.spy_session.request = Mock(side_effect=spy_session_request) + self.client = AsyncWebClient(token="xoxb-api_test", base_url="http://localhost:8888", session=self.spy_session) + self.client_base_url_slash = AsyncWebClient( + token="xoxb-api_test", base_url="http://localhost:8888/", session=self.spy_session + ) + + def tearDown(self): + cleanup_mock_web_api_server(self) + + async def close_sessions(self): + await self.session.close() + await self.spy_session.close() + + @async_test + async def test_base_url_without_slash_api_method_without_slash(self): + await self.client.api_call("api.test") + await self.close_sessions() + self.assertIsInstance(self.session_request_spy_args[1], str) + self.assertEqual(self.session_request_spy_args[1], "http://localhost:8888/api.test") + + @async_test + async def test_base_url_without_slash_api_method_with_slash(self): + await self.client.api_call("/api.test") + await self.close_sessions() + self.assertIsInstance(self.session_request_spy_args[1], str) + self.assertEqual(self.session_request_spy_args[1], "http://localhost:8888/api.test") + + @async_test + async def test_base_url_with_slash_api_method_without_slash(self): + await self.client_base_url_slash.api_call("api.test") + await self.close_sessions() + self.assertIsInstance(self.session_request_spy_args[1], str) + self.assertEqual(self.session_request_spy_args[1], "http://localhost:8888/api.test") + + @async_test + async def test_base_url_with_slash_api_method_with_slash(self): + await self.client_base_url_slash.api_call("/api.test") + await self.close_sessions() + self.assertIsInstance(self.session_request_spy_args[1], str) + self.assertEqual(self.session_request_spy_args[1], "http://localhost:8888/api.test") + + @async_test + async def test_base_url_without_slash_api_method_with_slash_and_trailing_slash(self): + await self.client.api_call("/api.test/") + await self.close_sessions() + self.assertIsInstance(self.session_request_spy_args[1], str) + self.assertEqual(self.session_request_spy_args[1], "http://localhost:8888/api.test/") From 29b4170d55ee4e5b870e7ac90eb315c4bede5203 Mon Sep 17 00:00:00 2001 From: William Bergamin Date: Fri, 22 Nov 2024 15:14:09 -0500 Subject: [PATCH 2/7] Fix naming --- tests/slack_sdk_async/web/test_web_client_url_format.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/slack_sdk_async/web/test_web_client_url_format.py b/tests/slack_sdk_async/web/test_web_client_url_format.py index 583584abc..240a08919 100644 --- a/tests/slack_sdk_async/web/test_web_client_url_format.py +++ b/tests/slack_sdk_async/web/test_web_client_url_format.py @@ -10,7 +10,7 @@ ) -class TestWebClientUrlFormat(unittest.TestCase): +class TestAsyncWebClientUrlFormat(unittest.TestCase): def setUp(self): setup_mock_web_api_server(self) From ca551a1111ce59921fae98e9039fcd35aa30a73a Mon Sep 17 00:00:00 2001 From: William Bergamin Date: Tue, 26 Nov 2024 09:53:55 -0500 Subject: [PATCH 3/7] make the tests work --- tests/helpers.py | 28 +++--- tests/slack_sdk/web/mock_web_api_server.py | 89 ++++++++++++++----- .../web/test_legacy_web_client_url_format.py | 38 ++++++++ .../web/test_web_client_url_format.py | 56 ++++-------- .../web/test_web_legacy_client_url_format.py | 68 -------------- .../token_rotation/test_token_rotator.py | 8 +- .../web/test_async_web_client.py | 8 +- .../web/test_async_web_client_http_retry.py | 8 +- .../web/test_web_client_coverage.py | 8 +- .../web/test_web_client_issue_829.py | 8 +- .../web/test_web_client_issue_891.py | 8 +- ...test_web_client_issue_921_custom_logger.py | 8 +- .../web/test_web_client_url_format.py | 62 ++++--------- 13 files changed, 181 insertions(+), 216 deletions(-) create mode 100644 tests/slack_sdk/web/test_legacy_web_client_url_format.py delete mode 100644 tests/slack_sdk/web/test_web_legacy_client_url_format.py diff --git a/tests/helpers.py b/tests/helpers.py index bcae8a81a..e769c10c9 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -1,7 +1,9 @@ import asyncio import os +from queue import Queue import sys from types import ModuleType +from typing import Optional, Union def async_test(coro): @@ -47,17 +49,19 @@ def is_ci_unstable_test_skip_enabled() -> bool: return os.environ.get("CI_UNSTABLE_TESTS_SKIP_ENABLED") == "1" -def reload_module(root_module: ModuleType): - package_name = root_module.__name__ - loaded_package_modules = { - key: value for key, value in sys.modules.items() if key.startswith(package_name) and isinstance(value, ModuleType) - } +class ReceivedRequests: + def __init__(self, queue: Union[Queue, asyncio.Queue]): + self.queue = queue + self.received_requests: dict = {} - for key in loaded_package_modules: - del sys.modules[key] + def get(self, key: str, default: Optional[int] = None) -> Optional[int]: + while not self.queue.empty(): + path = self.queue.get() + self.received_requests[path] = self.received_requests.get(path, 0) + 1 + return self.received_requests.get(key, default) - for key in loaded_package_modules: - new_module = __import__(key) - old_module = loaded_package_modules[key] - old_module.__dict__.clear() - old_module.__dict__.update(new_module.__dict__) + async def get_async(self, key: str, default: Optional[int] = None) -> Optional[int]: + while not self.queue.empty(): + path = await self.queue.get() + self.received_requests[path] = self.received_requests.get(path, 0) + 1 + return self.received_requests.get(key, default) diff --git a/tests/slack_sdk/web/mock_web_api_server.py b/tests/slack_sdk/web/mock_web_api_server.py index c7dbf6a54..2e11b1b9f 100644 --- a/tests/slack_sdk/web/mock_web_api_server.py +++ b/tests/slack_sdk/web/mock_web_api_server.py @@ -8,12 +8,13 @@ from http import HTTPStatus from http.server import HTTPServer, SimpleHTTPRequestHandler from multiprocessing.context import Process -from typing import Type +from queue import Queue +from typing import Type, Union from unittest import TestCase -from urllib.parse import urlparse, parse_qs +from urllib.parse import parse_qs, urlparse from urllib.request import Request, urlopen -from tests.helpers import get_mock_server_mode +from tests.helpers import ReceivedRequests, get_mock_server_mode class MockHandler(SimpleHTTPRequestHandler): @@ -78,6 +79,8 @@ def set_common_headers(self): def _handle(self): try: + # put_nowait is common between Queue & asyncio.Queue, it does not need to be awaited + self.server.queue.put_nowait(self.path) if self.path == "/received_requests.json": self.send_response(200) self.set_common_headers() @@ -313,24 +316,35 @@ def stop(self): class MockServerThread(threading.Thread): - def __init__(self, test: TestCase, handler: Type[SimpleHTTPRequestHandler] = MockHandler): + 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() + 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() @@ -338,7 +352,8 @@ def stop(self): def setup_mock_web_api_server(test: TestCase): if get_mock_server_mode() == "threading": test.server_started = threading.Event() - test.thread = MockServerThread(test) + test.received_requests = ReceivedRequests(Queue()) + test.thread = MockServerThread(test.received_requests.queue, test) test.thread.start() test.server_started.wait() else: @@ -389,37 +404,65 @@ def cleanup_mock_web_api_server(test: TestCase): test.process = None -def assert_auth_test_count(test: TestCase, expected_count: int): - time.sleep(0.1) - retry_count = 0 +def assert_received_request_count(test: TestCase, path: str, min_count: int, timeout: float = 1): + start_time = time.time() error = None - while retry_count < 3: + while time.time() - start_time < timeout: try: - test.mock_received_requests["/auth.test"] == expected_count - break + received_count = test.received_requests.get(path, 0) + assert ( + received_count == min_count + ), f"Expected {min_count} '{path}' {'requests' if min_count > 1 else 'request'}, but got {received_count}!" + return except Exception as e: error = e - retry_count += 1 - # waiting for mock_received_requests updates - time.sleep(0.1) + # waiting for some requests to be received + time.sleep(0.05) if error is not None: raise error -async def assert_auth_test_count_async(test: TestCase, expected_count: int): - await asyncio.sleep(0.1) - retry_count = 0 +def assert_auth_test_count(test: TestCase, expected_count: int): + assert_received_request_count(test, "/auth.test", expected_count, 0.5) + + +######### +# async # +######### + + +def setup_mock_web_api_server_async(test: TestCase): + test.server_started = threading.Event() + test.received_requests = ReceivedRequests(asyncio.Queue()) + test.thread = MockServerThread(test.received_requests.queue, test) + test.thread.start() + test.server_started.wait() + + +def cleanup_mock_web_api_server_async(test: TestCase): + test.thread.stop_unsafe() + test.thread = None + + +async def assert_received_request_count_async(test: TestCase, path: str, min_count: int, timeout: float = 1): + start_time = time.time() error = None - while retry_count < 3: + while time.time() - start_time < timeout: try: - test.mock_received_requests["/auth.test"] == expected_count - break + received_count = await test.received_requests.get_async(path, 0) + assert ( + received_count == min_count + ), f"Expected {min_count} '{path}' {'requests' if min_count > 1 else 'request'}, but got {received_count}!" + return except Exception as e: error = e - retry_count += 1 # waiting for mock_received_requests updates - await asyncio.sleep(0.1) + await asyncio.sleep(0.05) if error is not None: raise error + + +async def assert_auth_test_count_async(test: TestCase, expected_count: int): + await assert_received_request_count_async(test, "/auth.test", expected_count, 0.5) diff --git a/tests/slack_sdk/web/test_legacy_web_client_url_format.py b/tests/slack_sdk/web/test_legacy_web_client_url_format.py new file mode 100644 index 000000000..127b1272e --- /dev/null +++ b/tests/slack_sdk/web/test_legacy_web_client_url_format.py @@ -0,0 +1,38 @@ +from unittest import TestCase + +from slack_sdk.web.legacy_client import LegacyWebClient +from tests.slack_sdk.web.mock_web_api_server import ( + assert_received_request_count, + cleanup_mock_web_api_server, + setup_mock_web_api_server, +) + + +class TestLegacyWebClientUrlFormat(TestCase): + def setUp(self): + setup_mock_web_api_server(self) + self.client = LegacyWebClient(token="xoxb-api_test", base_url="http://localhost:8888") + self.client_base_url_slash = LegacyWebClient(token="xoxb-api_test", base_url="http://localhost:8888/") + + def tearDown(self): + cleanup_mock_web_api_server(self) + + def test_base_url_without_slash_api_method_without_slash(self): + self.client.api_call("chat.postMessage") + assert_received_request_count(self, "/chat.postMessage", 1) + + def test_base_url_without_slash_api_method_with_slash(self): + self.client.api_call("/chat.postMessage") + assert_received_request_count(self, "/chat.postMessage", 1) + + def test_base_url_with_slash_api_method_without_slash(self): + self.client_base_url_slash.api_call("chat.postMessage") + assert_received_request_count(self, "/chat.postMessage", 1) + + def test_base_url_with_slash_api_method_with_slash(self): + self.client_base_url_slash.api_call("/chat.postMessage") + assert_received_request_count(self, "/chat.postMessage", 1) + + def test_base_url_without_slash_api_method_with_slash_and_trailing_slash(self): + self.client.api_call("/chat.postMessage/") + assert_received_request_count(self, "/chat.postMessage/", 1) diff --git a/tests/slack_sdk/web/test_web_client_url_format.py b/tests/slack_sdk/web/test_web_client_url_format.py index 353aeb7c1..eae9d1806 100644 --- a/tests/slack_sdk/web/test_web_client_url_format.py +++ b/tests/slack_sdk/web/test_web_client_url_format.py @@ -1,64 +1,38 @@ from unittest import TestCase -from unittest.mock import patch -from urllib import request -from urllib.request import Request, urlopen -import slack_sdk.web -from tests.helpers import reload_module +from slack_sdk.web import WebClient from tests.slack_sdk.web.mock_web_api_server import ( - setup_mock_web_api_server, + assert_received_request_count, cleanup_mock_web_api_server, + setup_mock_web_api_server, ) -def build_spy_urlopen(test: TestCase): - def spy_urlopen(*args, **kwargs): - test.urlopen_spy_args = args - test.urlopen_spy_kwargs = kwargs - return urlopen(*args, **kwargs) - - return spy_urlopen - - class TestWebClientUrlFormat(TestCase): def setUp(self): setup_mock_web_api_server(self) - with patch.object(request, "urlopen") as mock_urlopen: - mock_urlopen.side_effect = build_spy_urlopen(self) - reload_module(slack_sdk.web) - self.client = slack_sdk.web.WebClient(token="xoxb-api_test", base_url="http://localhost:8888") - self.client_base_url_slash = slack_sdk.web.WebClient(token="xoxb-api_test", base_url="http://localhost:8888/") + self.client = WebClient(token="xoxb-api_test", base_url="http://localhost:8888") + self.client_base_url_slash = WebClient(token="xoxb-api_test", base_url="http://localhost:8888/") def tearDown(self): cleanup_mock_web_api_server(self) - self.urlopen_spy_args = None - self.urlopen_spy_kwargs = None - - @classmethod - def tearDownClass(cls): - reload_module(slack_sdk.web) def test_base_url_without_slash_api_method_without_slash(self): - self.client.api_call("api.test") - self.assertIsInstance(self.urlopen_spy_args[0], Request) - self.assertEqual(self.urlopen_spy_args[0].full_url, "http://localhost:8888/api.test") + self.client.api_call("chat.postMessage") + assert_received_request_count(self, "/chat.postMessage", 1) def test_base_url_without_slash_api_method_with_slash(self): - self.client.api_call("/api.test") - self.assertIsInstance(self.urlopen_spy_args[0], Request) - self.assertEqual(self.urlopen_spy_args[0].full_url, "http://localhost:8888/api.test") + self.client.api_call("/chat.postMessage") + assert_received_request_count(self, "/chat.postMessage", 1) def test_base_url_with_slash_api_method_without_slash(self): - self.client_base_url_slash.api_call("api.test") - self.assertIsInstance(self.urlopen_spy_args[0], Request) - self.assertEqual(self.urlopen_spy_args[0].full_url, "http://localhost:8888/api.test") + self.client_base_url_slash.api_call("chat.postMessage") + assert_received_request_count(self, "/chat.postMessage", 1) def test_base_url_with_slash_api_method_with_slash(self): - self.client_base_url_slash.api_call("/api.test") - self.assertIsInstance(self.urlopen_spy_args[0], Request) - self.assertEqual(self.urlopen_spy_args[0].full_url, "http://localhost:8888/api.test") + self.client_base_url_slash.api_call("/chat.postMessage") + assert_received_request_count(self, "/chat.postMessage", 1) def test_base_url_without_slash_api_method_with_slash_and_trailing_slash(self): - self.client.api_call("/api.test/") - self.assertIsInstance(self.urlopen_spy_args[0], Request) - self.assertEqual(self.urlopen_spy_args[0].full_url, "http://localhost:8888/api.test/") + self.client.api_call("/chat.postMessage/") + assert_received_request_count(self, "/chat.postMessage/", 1) diff --git a/tests/slack_sdk/web/test_web_legacy_client_url_format.py b/tests/slack_sdk/web/test_web_legacy_client_url_format.py deleted file mode 100644 index ec5e0c4bb..000000000 --- a/tests/slack_sdk/web/test_web_legacy_client_url_format.py +++ /dev/null @@ -1,68 +0,0 @@ -from unittest import TestCase -from unittest.mock import patch -from urllib import request -from urllib.request import Request, urlopen - -import slack_sdk.web.legacy_base_client -from tests.helpers import reload_module -from tests.slack_sdk.web.mock_web_api_server import ( - setup_mock_web_api_server, - cleanup_mock_web_api_server, -) - - -def build_spy_urlopen(test: TestCase): - def spy_urlopen(*args, **kwargs): - test.urlopen_spy_args = args - test.urlopen_spy_kwargs = kwargs - return urlopen(*args, **kwargs) - - return spy_urlopen - - -class TestLegacyWebClientUrlFormat(TestCase): - def setUp(self): - setup_mock_web_api_server(self) - with patch.object(request, "urlopen") as mock_urlopen: - mock_urlopen.side_effect = build_spy_urlopen(self) - reload_module(slack_sdk.web.legacy_base_client) - self.client = slack_sdk.web.legacy_base_client.LegacyBaseClient( - token="xoxb-api_test", base_url="http://localhost:8888" - ) - self.client_base_url_slash = slack_sdk.web.legacy_base_client.LegacyBaseClient( - token="xoxb-api_test", base_url="http://localhost:8888/" - ) - - def tearDown(self): - cleanup_mock_web_api_server(self) - self.urlopen_spy_args = None - self.urlopen_spy_kwargs = None - - @classmethod - def tearDownClass(cls): - reload_module(slack_sdk.web.legacy_base_client) - - def test_base_url_without_slash_api_method_without_slash(self): - self.client.api_call("api.test") - self.assertIsInstance(self.urlopen_spy_args[0], Request) - self.assertEqual(self.urlopen_spy_args[0].full_url, "http://localhost:8888/api.test") - - def test_base_url_without_slash_api_method_with_slash(self): - self.client.api_call("/api.test") - self.assertIsInstance(self.urlopen_spy_args[0], Request) - self.assertEqual(self.urlopen_spy_args[0].full_url, "http://localhost:8888/api.test") - - def test_base_url_with_slash_api_method_without_slash(self): - self.client_base_url_slash.api_call("api.test") - self.assertIsInstance(self.urlopen_spy_args[0], Request) - self.assertEqual(self.urlopen_spy_args[0].full_url, "http://localhost:8888/api.test") - - def test_base_url_with_slash_api_method_with_slash(self): - self.client_base_url_slash.api_call("/api.test") - self.assertIsInstance(self.urlopen_spy_args[0], Request) - self.assertEqual(self.urlopen_spy_args[0].full_url, "http://localhost:8888/api.test") - - def test_base_url_without_slash_api_method_with_slash_and_trailing_slash(self): - self.client.api_call("/api.test/") - self.assertIsInstance(self.urlopen_spy_args[0], Request) - self.assertEqual(self.urlopen_spy_args[0].full_url, "http://localhost:8888/api.test/") diff --git a/tests/slack_sdk_async/oauth/token_rotation/test_token_rotator.py b/tests/slack_sdk_async/oauth/token_rotation/test_token_rotator.py index f379de925..572e5e90d 100644 --- a/tests/slack_sdk_async/oauth/token_rotation/test_token_rotator.py +++ b/tests/slack_sdk_async/oauth/token_rotation/test_token_rotator.py @@ -6,14 +6,14 @@ from slack_sdk.web.async_client import AsyncWebClient from tests.helpers import async_test from tests.slack_sdk.web.mock_web_api_server import ( - setup_mock_web_api_server, - cleanup_mock_web_api_server, + setup_mock_web_api_server_async, + cleanup_mock_web_api_server_async, ) class TestTokenRotator(unittest.TestCase): def setUp(self): - setup_mock_web_api_server(self) + setup_mock_web_api_server_async(self) self.token_rotator = AsyncTokenRotator( client=AsyncWebClient(base_url="http://localhost:8888", token=None), client_id="111.222", @@ -21,7 +21,7 @@ def setUp(self): ) def tearDown(self): - cleanup_mock_web_api_server(self) + cleanup_mock_web_api_server_async(self) @async_test async def test_refresh(self): diff --git a/tests/slack_sdk_async/web/test_async_web_client.py b/tests/slack_sdk_async/web/test_async_web_client.py index b3a5287a0..48b08a24e 100644 --- a/tests/slack_sdk_async/web/test_async_web_client.py +++ b/tests/slack_sdk_async/web/test_async_web_client.py @@ -6,21 +6,21 @@ from slack_sdk.web.async_client import AsyncWebClient from tests.slack_sdk_async.helpers import async_test from tests.slack_sdk.web.mock_web_api_server import ( - setup_mock_web_api_server, - cleanup_mock_web_api_server, + setup_mock_web_api_server_async, + cleanup_mock_web_api_server_async, ) class TestAsyncWebClient(unittest.TestCase): def setUp(self): - setup_mock_web_api_server(self) + setup_mock_web_api_server_async(self) self.client = AsyncWebClient( token="xoxp-1234", base_url="http://localhost:8888", ) def tearDown(self): - cleanup_mock_web_api_server(self) + cleanup_mock_web_api_server_async(self) pattern_for_language = re.compile("python/(\\S+)", re.IGNORECASE) pattern_for_package_identifier = re.compile("slackclient/(\\S+)") diff --git a/tests/slack_sdk_async/web/test_async_web_client_http_retry.py b/tests/slack_sdk_async/web/test_async_web_client_http_retry.py index 8a352e0d5..e7f955778 100644 --- a/tests/slack_sdk_async/web/test_async_web_client_http_retry.py +++ b/tests/slack_sdk_async/web/test_async_web_client_http_retry.py @@ -5,8 +5,8 @@ from slack_sdk.web.async_client import AsyncWebClient from tests.slack_sdk_async.helpers import async_test from tests.slack_sdk.web.mock_web_api_server import ( - setup_mock_web_api_server, - cleanup_mock_web_api_server, + setup_mock_web_api_server_async, + cleanup_mock_web_api_server_async, ) from ..fatal_error_retry_handler import FatalErrorRetryHandler from ..my_retry_handler import MyRetryHandler @@ -14,10 +14,10 @@ class TestAsyncWebClient_HttpRetries(unittest.TestCase): def setUp(self): - setup_mock_web_api_server(self) + setup_mock_web_api_server_async(self) def tearDown(self): - cleanup_mock_web_api_server(self) + cleanup_mock_web_api_server_async(self) @async_test async def test_remote_disconnected(self): diff --git a/tests/slack_sdk_async/web/test_web_client_coverage.py b/tests/slack_sdk_async/web/test_web_client_coverage.py index 885acdb0d..e8651ccf6 100644 --- a/tests/slack_sdk_async/web/test_web_client_coverage.py +++ b/tests/slack_sdk_async/web/test_web_client_coverage.py @@ -8,8 +8,8 @@ from slack_sdk.web.async_client import AsyncWebClient from slack_sdk.web.legacy_client import LegacyWebClient from tests.slack_sdk.web.mock_web_api_server import ( - setup_mock_web_api_server, - cleanup_mock_web_api_server, + setup_mock_web_api_server_async, + cleanup_mock_web_api_server_async, ) from tests.slack_sdk_async.helpers import async_test @@ -25,7 +25,7 @@ class TestWebClientCoverage(unittest.TestCase): os.environ.setdefault("SLACKCLIENT_SKIP_DEPRECATION", "1") def setUp(self): - setup_mock_web_api_server(self) + setup_mock_web_api_server_async(self) self.client = WebClient(token="xoxb-coverage", base_url="http://localhost:8888") self.legacy_client = LegacyWebClient(token="xoxb-coverage", base_url="http://localhost:8888") self.async_client = AsyncWebClient(token="xoxb-coverage", base_url="http://localhost:8888") @@ -74,7 +74,7 @@ def setUp(self): self.api_methods_to_call.append(api_method) def tearDown(self): - cleanup_mock_web_api_server(self) + cleanup_mock_web_api_server_async(self) async def run_method(self, method_name, method, async_method): # Run the api calls with required arguments diff --git a/tests/slack_sdk_async/web/test_web_client_issue_829.py b/tests/slack_sdk_async/web/test_web_client_issue_829.py index 3fe077723..86a0f4fb9 100644 --- a/tests/slack_sdk_async/web/test_web_client_issue_829.py +++ b/tests/slack_sdk_async/web/test_web_client_issue_829.py @@ -4,17 +4,17 @@ from slack_sdk.web.async_client import AsyncWebClient from tests.helpers import async_test from tests.slack_sdk.web.mock_web_api_server import ( - setup_mock_web_api_server, - cleanup_mock_web_api_server, + setup_mock_web_api_server_async, + cleanup_mock_web_api_server_async, ) class TestWebClient_Issue_829(unittest.TestCase): def setUp(self): - setup_mock_web_api_server(self) + setup_mock_web_api_server_async(self) def tearDown(self): - cleanup_mock_web_api_server(self) + cleanup_mock_web_api_server_async(self) @async_test async def test_html_response_body_issue_829_async(self): diff --git a/tests/slack_sdk_async/web/test_web_client_issue_891.py b/tests/slack_sdk_async/web/test_web_client_issue_891.py index 55e0cdb12..9dca5b379 100644 --- a/tests/slack_sdk_async/web/test_web_client_issue_891.py +++ b/tests/slack_sdk_async/web/test_web_client_issue_891.py @@ -3,17 +3,17 @@ from slack_sdk.web.async_client import AsyncWebClient from tests.helpers import async_test from tests.slack_sdk.web.mock_web_api_server import ( - setup_mock_web_api_server, - cleanup_mock_web_api_server, + setup_mock_web_api_server_async, + cleanup_mock_web_api_server_async, ) class TestWebClient_Issue_829(unittest.TestCase): def setUp(self): - setup_mock_web_api_server(self) + setup_mock_web_api_server_async(self) def tearDown(self): - cleanup_mock_web_api_server(self) + cleanup_mock_web_api_server_async(self) @async_test async def test_missing_text_warning_chat_postMessage(self): diff --git a/tests/slack_sdk_async/web/test_web_client_issue_921_custom_logger.py b/tests/slack_sdk_async/web/test_web_client_issue_921_custom_logger.py index 4b69f6bfc..e24c656f2 100644 --- a/tests/slack_sdk_async/web/test_web_client_issue_921_custom_logger.py +++ b/tests/slack_sdk_async/web/test_web_client_issue_921_custom_logger.py @@ -4,17 +4,17 @@ from slack_sdk.web.async_client import AsyncWebClient from tests.helpers import async_test from tests.slack_sdk.web.mock_web_api_server import ( - setup_mock_web_api_server, - cleanup_mock_web_api_server, + setup_mock_web_api_server_async, + cleanup_mock_web_api_server_async, ) class TestWebClient_Issue_921_CustomLogger(unittest.TestCase): def setUp(self): - setup_mock_web_api_server(self) + setup_mock_web_api_server_async(self) def tearDown(self): - cleanup_mock_web_api_server(self) + cleanup_mock_web_api_server_async(self) @async_test async def test_if_it_uses_custom_logger(self): diff --git a/tests/slack_sdk_async/web/test_web_client_url_format.py b/tests/slack_sdk_async/web/test_web_client_url_format.py index 240a08919..337ae5904 100644 --- a/tests/slack_sdk_async/web/test_web_client_url_format.py +++ b/tests/slack_sdk_async/web/test_web_client_url_format.py @@ -1,71 +1,45 @@ import unittest -from unittest.mock import Mock -from aiohttp import ClientSession + from slack_sdk.web.async_client import AsyncWebClient from tests.slack_sdk_async.helpers import async_test from tests.slack_sdk.web.mock_web_api_server import ( - setup_mock_web_api_server, - cleanup_mock_web_api_server, + assert_received_request_count_async, + cleanup_mock_web_api_server_async, + setup_mock_web_api_server_async, ) class TestAsyncWebClientUrlFormat(unittest.TestCase): def setUp(self): - setup_mock_web_api_server(self) - - self.session = ClientSession() - - def spy_session_request(*args, **kwargs): - self.session_request_spy_args = args - self.session_request_spy_kwargs = kwargs - return self.session.request(*args, **kwargs) - - self.spy_session = ClientSession() - self.spy_session.request = Mock(side_effect=spy_session_request) - self.client = AsyncWebClient(token="xoxb-api_test", base_url="http://localhost:8888", session=self.spy_session) - self.client_base_url_slash = AsyncWebClient( - token="xoxb-api_test", base_url="http://localhost:8888/", session=self.spy_session - ) + setup_mock_web_api_server_async(self) + self.client = AsyncWebClient(token="xoxb-api_test", base_url="http://localhost:8888") + self.client_base_url_slash = AsyncWebClient(token="xoxb-api_test", base_url="http://localhost:8888/") def tearDown(self): - cleanup_mock_web_api_server(self) - - async def close_sessions(self): - await self.session.close() - await self.spy_session.close() + cleanup_mock_web_api_server_async(self) @async_test async def test_base_url_without_slash_api_method_without_slash(self): - await self.client.api_call("api.test") - await self.close_sessions() - self.assertIsInstance(self.session_request_spy_args[1], str) - self.assertEqual(self.session_request_spy_args[1], "http://localhost:8888/api.test") + await self.client.api_call("chat.postMessage") + await assert_received_request_count_async(self, "/chat.postMessage", 1) @async_test async def test_base_url_without_slash_api_method_with_slash(self): - await self.client.api_call("/api.test") - await self.close_sessions() - self.assertIsInstance(self.session_request_spy_args[1], str) - self.assertEqual(self.session_request_spy_args[1], "http://localhost:8888/api.test") + await self.client.api_call("/chat.postMessage") + await assert_received_request_count_async(self, "/chat.postMessage", 1) @async_test async def test_base_url_with_slash_api_method_without_slash(self): - await self.client_base_url_slash.api_call("api.test") - await self.close_sessions() - self.assertIsInstance(self.session_request_spy_args[1], str) - self.assertEqual(self.session_request_spy_args[1], "http://localhost:8888/api.test") + await self.client_base_url_slash.api_call("chat.postMessage") + await assert_received_request_count_async(self, "/chat.postMessage", 1) @async_test async def test_base_url_with_slash_api_method_with_slash(self): - await self.client_base_url_slash.api_call("/api.test") - await self.close_sessions() - self.assertIsInstance(self.session_request_spy_args[1], str) - self.assertEqual(self.session_request_spy_args[1], "http://localhost:8888/api.test") + await self.client_base_url_slash.api_call("/chat.postMessage") + await assert_received_request_count_async(self, "/chat.postMessage", 1) @async_test async def test_base_url_without_slash_api_method_with_slash_and_trailing_slash(self): - await self.client.api_call("/api.test/") - await self.close_sessions() - self.assertIsInstance(self.session_request_spy_args[1], str) - self.assertEqual(self.session_request_spy_args[1], "http://localhost:8888/api.test/") + await self.client.api_call("/chat.postMessage/") + await assert_received_request_count_async(self, "/chat.postMessage/", 1) From d9d012fddfa8e6f499e5065cddd2b6398a0c7375 Mon Sep 17 00:00:00 2001 From: William Bergamin Date: Tue, 26 Nov 2024 10:02:29 -0500 Subject: [PATCH 4/7] Clean up imports --- tests/helpers.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/helpers.py b/tests/helpers.py index e769c10c9..922bfe30c 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -1,8 +1,6 @@ import asyncio import os from queue import Queue -import sys -from types import ModuleType from typing import Optional, Union From 31f81c02015dbc0481e8536db1b14e0a66a5a383 Mon Sep 17 00:00:00 2001 From: William Bergamin Date: Wed, 27 Nov 2024 09:56:53 -0500 Subject: [PATCH 5/7] Improve based on rebase --- .../slack_sdk/web/test_legacy_web_client_url_format.py | 9 +++------ tests/slack_sdk/web/test_web_client_url_format.py | 9 +++------ .../slack_sdk_async/web/test_web_client_url_format.py | 10 +++++----- 3 files changed, 11 insertions(+), 17 deletions(-) diff --git a/tests/slack_sdk/web/test_legacy_web_client_url_format.py b/tests/slack_sdk/web/test_legacy_web_client_url_format.py index 127b1272e..0ba2f5ae2 100644 --- a/tests/slack_sdk/web/test_legacy_web_client_url_format.py +++ b/tests/slack_sdk/web/test_legacy_web_client_url_format.py @@ -1,16 +1,13 @@ from unittest import TestCase from slack_sdk.web.legacy_client import LegacyWebClient -from tests.slack_sdk.web.mock_web_api_server import ( - assert_received_request_count, - cleanup_mock_web_api_server, - setup_mock_web_api_server, -) +from tests.slack_sdk.web.mock_web_api_handler import MockHandler +from tests.mock_web_api_server import setup_mock_web_api_server, cleanup_mock_web_api_server, assert_received_request_count class TestLegacyWebClientUrlFormat(TestCase): def setUp(self): - setup_mock_web_api_server(self) + setup_mock_web_api_server(self, MockHandler) self.client = LegacyWebClient(token="xoxb-api_test", base_url="http://localhost:8888") self.client_base_url_slash = LegacyWebClient(token="xoxb-api_test", base_url="http://localhost:8888/") diff --git a/tests/slack_sdk/web/test_web_client_url_format.py b/tests/slack_sdk/web/test_web_client_url_format.py index eae9d1806..9ef4f9329 100644 --- a/tests/slack_sdk/web/test_web_client_url_format.py +++ b/tests/slack_sdk/web/test_web_client_url_format.py @@ -1,16 +1,13 @@ from unittest import TestCase from slack_sdk.web import WebClient -from tests.slack_sdk.web.mock_web_api_server import ( - assert_received_request_count, - cleanup_mock_web_api_server, - setup_mock_web_api_server, -) +from tests.slack_sdk.web.mock_web_api_handler import MockHandler +from tests.mock_web_api_server import setup_mock_web_api_server, cleanup_mock_web_api_server, assert_received_request_count class TestWebClientUrlFormat(TestCase): def setUp(self): - setup_mock_web_api_server(self) + setup_mock_web_api_server(self, MockHandler) self.client = WebClient(token="xoxb-api_test", base_url="http://localhost:8888") self.client_base_url_slash = WebClient(token="xoxb-api_test", base_url="http://localhost:8888/") diff --git a/tests/slack_sdk_async/web/test_web_client_url_format.py b/tests/slack_sdk_async/web/test_web_client_url_format.py index 337ae5904..3f25dac0a 100644 --- a/tests/slack_sdk_async/web/test_web_client_url_format.py +++ b/tests/slack_sdk_async/web/test_web_client_url_format.py @@ -1,18 +1,18 @@ import unittest - from slack_sdk.web.async_client import AsyncWebClient from tests.slack_sdk_async.helpers import async_test -from tests.slack_sdk.web.mock_web_api_server import ( - assert_received_request_count_async, - cleanup_mock_web_api_server_async, +from tests.slack_sdk.web.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, + assert_received_request_count_async, ) class TestAsyncWebClientUrlFormat(unittest.TestCase): def setUp(self): - setup_mock_web_api_server_async(self) + setup_mock_web_api_server_async(self, MockHandler) self.client = AsyncWebClient(token="xoxb-api_test", base_url="http://localhost:8888") self.client_base_url_slash = AsyncWebClient(token="xoxb-api_test", base_url="http://localhost:8888/") From 985cba1aa6d2eceba8d27d79911a373fef153dc6 Mon Sep 17 00:00:00 2001 From: William Bergamin Date: Wed, 27 Nov 2024 09:58:41 -0500 Subject: [PATCH 6/7] Clean up PR --- tests/helpers.py | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/tests/helpers.py b/tests/helpers.py index 4988988a9..8b08b7d36 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -1,7 +1,5 @@ import asyncio import os -from queue import Queue -from typing import Optional, Union def async_test(coro): @@ -30,21 +28,3 @@ def restore_os_env(old_env: dict) -> None: def is_ci_unstable_test_skip_enabled() -> bool: return os.environ.get("CI_UNSTABLE_TESTS_SKIP_ENABLED") == "1" - - -class ReceivedRequests: - def __init__(self, queue: Union[Queue, asyncio.Queue]): - self.queue = queue - self.received_requests: dict = {} - - def get(self, key: str, default: Optional[int] = None) -> Optional[int]: - while not self.queue.empty(): - path = self.queue.get() - self.received_requests[path] = self.received_requests.get(path, 0) + 1 - return self.received_requests.get(key, default) - - async def get_async(self, key: str, default: Optional[int] = None) -> Optional[int]: - while not self.queue.empty(): - path = await self.queue.get() - self.received_requests[path] = self.received_requests.get(path, 0) + 1 - return self.received_requests.get(key, default) From 67dc5be481518e5156dfa8eac9b03549ede9500d Mon Sep 17 00:00:00 2001 From: William Bergamin Date: Wed, 27 Nov 2024 10:01:35 -0500 Subject: [PATCH 7/7] clean up PR --- tests/slack_sdk/web/mock_web_api_handler.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/slack_sdk/web/mock_web_api_handler.py b/tests/slack_sdk/web/mock_web_api_handler.py index 0916e7643..12a487e05 100644 --- a/tests/slack_sdk/web/mock_web_api_handler.py +++ b/tests/slack_sdk/web/mock_web_api_handler.py @@ -76,6 +76,7 @@ def _handle(self): try: # put_nowait is common between Queue & asyncio.Queue, it does not need to be awaited self.server.queue.put_nowait(self.path) + if self.path in {"/oauth.access", "/oauth.v2.access"}: self.send_response(200) self.set_common_headers()