From ebdfe4537535d3057b67d58218e8128017c8e34b Mon Sep 17 00:00:00 2001 From: Aliaksei Urbanski Date: Sun, 24 Dec 2023 06:28:08 +0300 Subject: [PATCH] Replace Flask-Sockets with aiohttp for testing (#1445) Flask-Sockets is deprecated and it won't receive any fixes in the future, so it seems reasonable to replace it with some up-to-date library. aiohttp has been chosen since it was already presented as an optional requirement. These changes also: * move format checking for tests from run_validation.sh to setup.py * remove duplicated dependency installations from setup.py * add a codegen step on CI * force colors on CI --- .github/workflows/ci-build.yml | 6 +- requirements/testing.txt | 6 +- scripts/run_validation.sh | 7 +- setup.py | 40 ++++----- .../socket_mode/mock_socket_mode_server.py | 85 +++++++++++-------- .../socket_mode/test_interactions_builtin.py | 19 ++--- .../test_interactions_websocket_client.py | 13 +-- .../socket_mode/test_interactions_aiohttp.py | 19 +++-- .../test_interactions_websockets.py | 14 ++- 9 files changed, 111 insertions(+), 98 deletions(-) diff --git a/.github/workflows/ci-build.yml b/.github/workflows/ci-build.yml index bb88d11fb..371523012 100644 --- a/.github/workflows/ci-build.yml +++ b/.github/workflows/ci-build.yml @@ -26,6 +26,7 @@ jobs: env: PYTHON_SLACK_SDK_MOCK_SERVER_MODE: 'threading' CI_LARGE_SOCKET_MODE_PAYLOAD_TESTING_DISABLED: '1' + FORCE_COLOR: '1' steps: - uses: actions/checkout@v3 - name: Set up Python ${{ matrix.python-version }} @@ -35,9 +36,12 @@ jobs: cache: pip - name: Install dependencies run: | - pip install -U pip wheel + pip install -U pip setuptools wheel pip install -r requirements/testing.txt pip install -r requirements/optional.txt + - name: Run codegen + run: | + python setup.py codegen - name: Run validation (black/flake8/pytest) run: | python setup.py validate diff --git a/requirements/testing.txt b/requirements/testing.txt index 06555a553..0a4d7ae9f 100644 --- a/requirements/testing.txt +++ b/requirements/testing.txt @@ -1,11 +1,7 @@ # pip install -r requirements/testing.txt +aiohttp<4 # used for a WebSocket server mock pytest>=7.0.1,<8 pytest-asyncio<1 # for async -Flask-Sockets>=0.2,<1 -Flask>=1,<2 # TODO: Flask-Sockets is not yet compatible with Flask 2.x -Werkzeug<2 # TODO: Flask-Sockets is not yet compatible with Flask 2.x -itsdangerous==1.1.0 # TODO: Flask-Sockets is not yet compatible with Flask 2.x -Jinja2==3.0.3 # https://github.com/pallets/flask/issues/4494 pytest-cov>=2,<3 # while flake8 5.x have issues with Python 3.12, flake8 6.x requires Python >= 3.8.1, # so 5.x should be kept in order to stay compatible with Python 3.6/3.7 diff --git a/scripts/run_validation.sh b/scripts/run_validation.sh index b09e3f097..9248e7509 100755 --- a/scripts/run_validation.sh +++ b/scripts/run_validation.sh @@ -5,13 +5,10 @@ set -e script_dir=`dirname $0` cd ${script_dir}/.. -pip install -U pip + +pip install -U pip setuptools wheel pip install -r requirements/testing.txt \ -r requirements/optional.txt -black --check tests/ integration_tests/ -# TODO: resolve linting errors for tests -# flake8 tests/ integration_tests/ - python setup.py codegen python setup.py validate diff --git a/setup.py b/setup.py index 6be1405b9..fbc963e24 100644 --- a/setup.py +++ b/setup.py @@ -11,11 +11,6 @@ here = os.path.abspath(os.path.dirname(__file__)) -codegen_dependencies = [ - # Don't change this version without running CI builds; - # The latest version may not be available for older Python runtime - "black==22.10.0", -] class BaseCommand(Command): user_options = [] @@ -41,11 +36,6 @@ def _run(self, s, command): class CodegenCommand(BaseCommand): def run(self): - self._run( - "Installing required dependencies ...", - [sys.executable, "-m", "pip", "install"] + codegen_dependencies, - ) - header = ( "# !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!\n" "#\n" @@ -149,16 +139,28 @@ def initialize_options(self): self.test_target = "" def run(self): - self._run( - "Installing test dependencies ...", - [sys.executable, "-m", "pip", "install", "-r", "requirements/testing.txt"], - ) + def run_black(target, target_name=None): + self._run( + f"Running black for {target_name or target} ...", + [sys.executable, "-m", "black", "--check", f"{here}/{target}"], + ) - self._run("Running black for legacy packages ...", [sys.executable, "-m", "black", f"{here}/slack"]) - self._run("Running black for slack_sdk package ...", [sys.executable, "-m", "black", f"{here}/slack_sdk"]) + run_black("slack", "legacy packages") + run_black("slack_sdk", "slack_sdk package") + run_black("tests") + run_black("integration_tests") - self._run("Running flake8 for legacy packages ...", [sys.executable, "-m", "flake8", f"{here}/slack"]) - self._run("Running flake8 for slack_sdk package ...", [sys.executable, "-m", "flake8", f"{here}/slack_sdk"]) + def run_flake8(target, target_name=None): + self._run( + f"Running flake8 for {target_name or target} ...", + [sys.executable, "-m", "flake8", f"{here}/{target}"], + ) + + run_flake8("slack", "legacy packages") + run_flake8("slack_sdk", "slack_sdk package") + # TODO: resolve linting errors for tests + # run_flake8("tests") + # run_flake8("integration_tests") target = self.test_target.replace("tests/", "", 1) self._run( @@ -175,7 +177,7 @@ def run(self): class UnitTestsCommand(BaseCommand): - """Support setup.py validate.""" + """Support setup.py unit_tests.""" description = "Run unit tests (pytest)." user_options = [("test-target=", "i", "tests/{test-target}")] diff --git a/tests/slack_sdk/socket_mode/mock_socket_mode_server.py b/tests/slack_sdk/socket_mode/mock_socket_mode_server.py index 34c8843ac..7dba25e53 100644 --- a/tests/slack_sdk/socket_mode/mock_socket_mode_server.py +++ b/tests/slack_sdk/socket_mode/mock_socket_mode_server.py @@ -1,6 +1,10 @@ +import asyncio import logging import os +from aiohttp import WSMsgType, web + + socket_mode_envelopes = [ """{"envelope_id":"1d3c79ab-0ffb-41f3-a080-d19e85f53649","payload":{"token":"verification-token","team_id":"T111","team_domain":"xxx","channel_id":"C111","channel_name":"random","user_id":"U111","user_name":"testxyz","command":"/hello-socket-mode","text":"","api_app_id":"A111","response_url":"https://hooks.slack.com/commands/T111/111/xxx","trigger_id":"111.222.xxx"},"type":"slash_commands","accepts_response_payload":true}""", """{"envelope_id":"cda4159a-72a5-4744-aba3-4d66eb52682b","payload":{"token":"verification-token","team_id":"T111","api_app_id":"A111","event":{"client_msg_id":"f0582a78-72db-4feb-b2f3-1e47d66365c8","type":"app_mention","text":"<@U111>","user":"U222","ts":"1610241741.000200","team":"T111","blocks":[{"type":"rich_text","block_id":"Sesm","elements":[{"type":"rich_text_section","elements":[{"type":"user","user_id":"U111"}]}]}],"channel":"C111","event_ts":"1610241741.000200"},"type":"event_callback","event_id":"Ev111","event_time":1610241741,"authorizations":[{"enterprise_id":null,"team_id":"T111","user_id":"U222","is_bot":true,"is_enterprise_install":false}],"is_ext_shared_channel":false,"event_context":"1-app_mention-T111-C111"},"type":"events_api","accepts_response_payload":false,"retry_attempt":0,"retry_reason":""}""", @@ -20,50 +24,61 @@ socket_mode_hello_message = """{"type":"hello","num_connections":2,"debug_info":{"host":"applink-111-xxx","build_number":10,"approximate_connection_time":18060},"connection_info":{"app_id":"A111"}}""" -from flask import Flask -from flask_sockets import Sockets - def start_socket_mode_server(self, port: int): - def _start_socket_mode_server(): - logger = logging.getLogger(__name__) - app: Flask = Flask(__name__) - sockets: Sockets = Sockets(app) + logger = logging.getLogger(__name__) + state = {} + + def reset_server_state(): + state.update( + hello_sent=False, + envelopes_to_consume=list(socket_mode_envelopes), + ) + + self.reset_server_state = reset_server_state + + async def link(request): + ws = web.WebSocketResponse() + await ws.prepare(request) + + async for msg in ws: + if msg.type != WSMsgType.TEXT: + continue + + message = msg.data + logger.debug(f"Server received a message: {message}") - state = { - "hello_sent": False, - "envelopes_to_consume": list(socket_mode_envelopes), - } + if not state["hello_sent"]: + state["hello_sent"] = True + await ws.send_str(socket_mode_hello_message) - @sockets.route("/link") - def link(ws): - while not ws.closed: - message = ws.read_message() - if message is not None: - if not state["hello_sent"]: - ws.send(socket_mode_hello_message) - state["hello_sent"] = True + if state["envelopes_to_consume"]: + e = state["envelopes_to_consume"].pop(0) + logger.debug(f"Send an envelope: {e}") + await ws.send_str(e) - if len(state.get("envelopes_to_consume")) > 0: - e = state.get("envelopes_to_consume").pop(0) - logger.debug(f"Send an envelope: {e}") - ws.send(e) + await ws.send_str(message) - logger.debug(f"Server received a message: {message}") - ws.send(message) + return ws - from gevent import pywsgi - from geventwebsocket.handler import WebSocketHandler + app = web.Application() + app.add_routes([web.get("/link", link)]) + runner = web.AppRunner(app) - server = pywsgi.WSGIServer(("", port), app, handler_class=WebSocketHandler) - self.server = server + def run_server(): + reset_server_state() - def reset_sever_state(): - state["hello_sent"] = False - state["envelopes_to_consume"] = list(socket_mode_envelopes) + self.loop = loop = asyncio.new_event_loop() + asyncio.set_event_loop(loop) + loop.run_until_complete(runner.setup()) + site = web.TCPSite(runner, "127.0.0.1", port, reuse_port=True) + loop.run_until_complete(site.start()) - self.reset_sever_state = reset_sever_state + # run until it's stopped from the main thread + loop.run_forever() - server.serve_forever(stop_timeout=1) + loop.run_until_complete(runner.cleanup()) + loop.run_until_complete(asyncio.sleep(1)) + loop.close() - return _start_socket_mode_server + return run_server diff --git a/tests/slack_sdk/socket_mode/test_interactions_builtin.py b/tests/slack_sdk/socket_mode/test_interactions_builtin.py index 9d1e80f0a..7e3c588c1 100644 --- a/tests/slack_sdk/socket_mode/test_interactions_builtin.py +++ b/tests/slack_sdk/socket_mode/test_interactions_builtin.py @@ -4,6 +4,8 @@ from random import randint from threading import Thread +import pytest + from slack_sdk.errors import SlackClientConfigurationError, SlackClientNotConnectedError from slack_sdk.socket_mode.request import SocketModeRequest @@ -66,7 +68,7 @@ def test_interactions(self): try: buffer_size_list = [1024, 9000, 35, 49] + list([randint(16, 128) for _ in range(10)]) for buffer_size in buffer_size_list: - self.reset_sever_state() + self.reset_server_state() received_messages = [] received_socket_mode_requests = [] @@ -127,8 +129,8 @@ def socket_mode_request_handler(client: BaseSocketModeClient, request: SocketMod # Restore the default value sys.setrecursionlimit(default_recursion_limit) client.close() - self.server.stop() - self.server.close() + self.loop.stop() + t.join(timeout=5) self.logger.info(f"Passed with buffer size: {buffer_size_list}") @@ -141,7 +143,7 @@ def test_send_message_while_disconnection(self): time.sleep(2) # wait for the server try: - self.reset_sever_state() + self.reset_server_state() client = SocketModeClient( app_token="xapp-A111-222-xyz", web_client=self.web_client, @@ -155,16 +157,13 @@ def test_send_message_while_disconnection(self): client.disconnect() time.sleep(1) # wait for the connection - try: + with pytest.raises(SlackClientNotConnectedError): client.send_message("foo") - self.fail("SlackClientNotConnectedError is expected here") - except SlackClientNotConnectedError as _: - pass client.connect() time.sleep(1) # wait for the connection client.send_message("foo") finally: client.close() - self.server.stop() - self.server.close() + self.loop.stop() + t.join(timeout=5) diff --git a/tests/slack_sdk/socket_mode/test_interactions_websocket_client.py b/tests/slack_sdk/socket_mode/test_interactions_websocket_client.py index 728406e7f..463bd7ea9 100644 --- a/tests/slack_sdk/socket_mode/test_interactions_websocket_client.py +++ b/tests/slack_sdk/socket_mode/test_interactions_websocket_client.py @@ -83,7 +83,9 @@ def socket_mode_request_handler(client: BaseSocketModeClient, request: SocketMod expected.sort() count = 0 - while count < 10 and len(received_messages) < len(expected): + while count < 10 and ( + len(received_messages) < len(expected) or len(received_socket_mode_requests) < len(socket_mode_envelopes) + ): time.sleep(0.2) count += 0.2 @@ -93,8 +95,8 @@ def socket_mode_request_handler(client: BaseSocketModeClient, request: SocketMod self.assertEqual(len(socket_mode_envelopes), len(received_socket_mode_requests)) finally: client.close() - self.server.stop() - self.server.close() + self.loop.stop() + t.join(timeout=5) def test_send_message_while_disconnection(self): if is_ci_unstable_test_skip_enabled(): @@ -105,7 +107,6 @@ def test_send_message_while_disconnection(self): time.sleep(2) # wait for the server try: - self.reset_sever_state() client = SocketModeClient( app_token="xapp-A111-222-xyz", web_client=self.web_client, @@ -131,5 +132,5 @@ def test_send_message_while_disconnection(self): client.send_message("foo") finally: client.close() - self.server.stop() - self.server.close() + self.loop.stop() + t.join(timeout=5) diff --git a/tests/slack_sdk_async/socket_mode/test_interactions_aiohttp.py b/tests/slack_sdk_async/socket_mode/test_interactions_aiohttp.py index 9903ce488..74f30d042 100644 --- a/tests/slack_sdk_async/socket_mode/test_interactions_aiohttp.py +++ b/tests/slack_sdk_async/socket_mode/test_interactions_aiohttp.py @@ -5,7 +5,9 @@ from random import randint from threading import Thread +import pytest from aiohttp import WSMessage + from slack_sdk.socket_mode.request import SocketModeRequest from slack_sdk.socket_mode.async_client import AsyncBaseSocketModeClient @@ -87,7 +89,9 @@ async def socket_mode_listener( expected.sort() count = 0 - while count < 10 and len(received_messages) < len(expected): + while count < 10 and ( + len(received_messages) < len(expected) or len(received_socket_mode_requests) < len(socket_mode_envelopes) + ): await asyncio.sleep(0.2) count += 0.2 @@ -97,8 +101,8 @@ async def socket_mode_listener( self.assertEqual(len(socket_mode_envelopes), len(received_socket_mode_requests)) finally: await client.close() - self.server.stop() - self.server.close() + self.loop.stop() + t.join(timeout=5) @async_test async def test_send_message_while_disconnection(self): @@ -124,16 +128,13 @@ async def test_send_message_while_disconnection(self): await client.disconnect() await asyncio.sleep(1) # wait for the message receiver - try: + with pytest.raises(ConnectionError): await client.send_message("foo") - self.fail("ConnectionError is expected here") - except ConnectionError as _: - pass await client.connect() await asyncio.sleep(1) # wait for the message receiver await client.send_message("foo") finally: await client.close() - self.server.stop() - self.server.close() + self.loop.stop() + t.join(timeout=5) 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 b1b7c1652..7f165af5c 100644 --- a/tests/slack_sdk_async/socket_mode/test_interactions_websockets.py +++ b/tests/slack_sdk_async/socket_mode/test_interactions_websockets.py @@ -6,6 +6,7 @@ from threading import Thread from typing import Optional +import pytest from websockets.exceptions import WebSocketException from slack_sdk.socket_mode.request import SocketModeRequest @@ -102,8 +103,8 @@ async def socket_mode_listener( self.assertEqual(len(socket_mode_envelopes), len(received_socket_mode_requests)) finally: await client.close() - self.server.stop() - self.server.close() + self.loop.stop() + t.join(timeout=5) @async_test async def test_send_message_while_disconnection(self): @@ -129,16 +130,13 @@ async def test_send_message_while_disconnection(self): await client.disconnect() await asyncio.sleep(1) # wait for the message receiver - try: + with pytest.raises(WebSocketException): await client.send_message("foo") - self.fail("WebSocketException is expected here") - except WebSocketException as _: - pass await client.connect() await asyncio.sleep(1) # wait for the message receiver await client.send_message("foo") finally: await client.close() - self.server.stop() - self.server.close() + self.loop.stop() + t.join(timeout=5)