Skip to content

Commit

Permalink
Replace Flask-Sockets with aiohttp for testing (#1445)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Jamim authored Dec 24, 2023
1 parent 56f7127 commit ebdfe45
Show file tree
Hide file tree
Showing 9 changed files with 111 additions and 98 deletions.
6 changes: 5 additions & 1 deletion .github/workflows/ci-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand All @@ -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
Expand Down
6 changes: 1 addition & 5 deletions requirements/testing.txt
Original file line number Diff line number Diff line change
@@ -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
Expand Down
7 changes: 2 additions & 5 deletions scripts/run_validation.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
40 changes: 21 additions & 19 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = []
Expand All @@ -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"
Expand Down Expand Up @@ -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(
Expand All @@ -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}")]
Expand Down
85 changes: 50 additions & 35 deletions tests/slack_sdk/socket_mode/mock_socket_mode_server.py
Original file line number Diff line number Diff line change
@@ -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":""}""",
Expand All @@ -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
19 changes: 9 additions & 10 deletions tests/slack_sdk/socket_mode/test_interactions_builtin.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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 = []
Expand Down Expand Up @@ -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}")

Expand All @@ -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,
Expand All @@ -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)
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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():
Expand All @@ -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,
Expand All @@ -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)
19 changes: 10 additions & 9 deletions tests/slack_sdk_async/socket_mode/test_interactions_aiohttp.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand All @@ -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):
Expand All @@ -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)
Loading

0 comments on commit ebdfe45

Please sign in to comment.