Skip to content

Fix #611 - stop propagating user exceptions to the connection management layer #665

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
seratch wants to merge 12 commits into from
Closed

Fix #611 - stop propagating user exceptions to the connection management layer #665

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
a7a679c
Modify unit tests not to depend on asyncio
seratch Apr 27, 2020
aa0b45f
Change marks for fixed integration tests
seratch Apr 27, 2020
e92442d
Change marks for fixed integration tests
seratch Apr 27, 2020
fb21a25
Revise WebClient/RTMClient internals for run_async=False
seratch Apr 27, 2020
5cde3e2
Fix #650 Deprecation warnings to channels/groups/mpim/im API method c…
seratch Apr 28, 2020
6b562d4
Fix #560 Allow boolean kwargs
seratch Apr 28, 2020
59fd21d
Apply formatter and organize imports
seratch Apr 28, 2020
1ba2378
Add an integration test verifying #378 is no longer an issue
seratch Apr 28, 2020
74a5a62
Skip depreaction warnings in unit tests and fix an integration test
seratch Apr 29, 2020
163a997
Add a test case for #480 and some fixes
seratch Apr 29, 2020
5f813f2
Fix anti-patterns detected by DeepSource
seratch Apr 30, 2020
48c6799
Fix #611 - stop propagating user exceptions to the connection managem…
seratch Apr 30, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions integration_tests/rtm/test_issue_530.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,7 @@
import logging
import unittest

import pytest

from integration_tests.helpers import async_test, is_not_specified
from integration_tests.helpers import async_test
from slack import RTMClient


Expand All @@ -22,27 +20,29 @@ def tearDown(self):
# Reset the decorators by @RTMClient.run_on
RTMClient._callbacks = collections.defaultdict(list)

@pytest.mark.skipif(condition=is_not_specified(), reason="still unfixed")
def test_issue_530(self):
try:
rtm_client = RTMClient(token="I am not a token", run_async=False, loop=asyncio.new_event_loop())
rtm_client.start()
self.fail("Raising an error here was expected")
except Exception as e:
self.assertEqual(str(e), "The server responded with: {'ok': False, 'error': 'invalid_auth'}")
self.assertEqual(
"The request to the Slack API failed.\n"
"The server responded with: {'ok': False, 'error': 'invalid_auth'}", str(e))
finally:
if not rtm_client._stopped:
rtm_client.stop()

@pytest.mark.skipif(condition=is_not_specified(), reason="still unfixed")
@async_test
async def test_issue_530_async(self):
try:
rtm_client = RTMClient(token="I am not a token", run_async=True)
await rtm_client.start()
self.fail("Raising an error here was expected")
except Exception as e:
self.assertEqual(str(e), "The server responded with: {'ok': False, 'error': 'invalid_auth'}")
self.assertEqual(
"The request to the Slack API failed.\n"
"The server responded with: {'ok': False, 'error': 'invalid_auth'}", str(e))
finally:
if not rtm_client._stopped:
rtm_client.stop()
Expand Down
12 changes: 3 additions & 9 deletions integration_tests/rtm/test_issue_569.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,16 +50,10 @@ def tearDown(self):
if hasattr(self, "rtm_client") and not self.rtm_client._stopped:
self.rtm_client.stop()

@pytest.mark.skipif(condition=is_not_specified(), reason="still unfixed")
@pytest.mark.skipif(condition=is_not_specified(), reason="To avoid rate_limited errors")
def test_cpu_usage(self):
self.rtm_client = RTMClient(
token=self.bot_token,
run_async=False,
loop=asyncio.new_event_loop())
self.web_client = WebClient(
token=self.bot_token,
run_async=False,
loop=asyncio.new_event_loop())
self.rtm_client = RTMClient(token=self.bot_token, run_async=False, loop=asyncio.new_event_loop())
self.web_client = WebClient(token=self.bot_token, run_async=False)

self.call_count = 0
TestRTMClient.cpu_usage = 0
Expand Down
5 changes: 2 additions & 3 deletions integration_tests/rtm/test_issue_605.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import asyncio
import collections
import collections
import logging
import os
Expand Down Expand Up @@ -31,7 +31,7 @@ def tearDown(self):
# Reset the decorators by @RTMClient.run_on
RTMClient._callbacks = collections.defaultdict(list)

@pytest.mark.skipif(condition=is_not_specified(), reason="still unfixed")
@pytest.mark.skipif(condition=is_not_specified(), reason="To avoid rate_limited errors")
def test_issue_605(self):
self.text = "This message was sent to verify issue #605"
self.called = False
Expand All @@ -56,7 +56,6 @@ def connect():
self.web_client = WebClient(
token=self.bot_token,
run_async=False,
loop=asyncio.new_event_loop(), # TODO: this doesn't work without this
)
new_message = self.web_client.chat_postMessage(channel=self.channel_id, text=self.text)
self.assertFalse("error" in new_message)
Expand Down
12 changes: 5 additions & 7 deletions integration_tests/rtm/test_issue_631.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,15 @@ def tearDown(self):
if hasattr(self, "rtm_client") and not self.rtm_client._stopped:
self.rtm_client.stop()

@pytest.mark.skipif(condition=is_not_specified(), reason="this is just for reference")
@pytest.mark.skipif(condition=is_not_specified(), reason="to avoid rate_limited errors")
def test_issue_631_sharing_event_loop(self):
self.success = None
self.text = "This message was sent to verify issue #631"

self.rtm_client = RTMClient(
token=self.bot_token,
run_async=False, # even though run_async=False, handlers for RTM events can be a coroutine
loop=asyncio.new_event_loop(), # TODO: remove this
run_async=False,
loop=asyncio.new_event_loop(), # TODO: this doesn't work without this
)

# @RTMClient.run_on(event="message")
Expand Down Expand Up @@ -72,8 +72,7 @@ def test_issue_631_sharing_event_loop(self):

# Solution (1) for #631
@RTMClient.run_on(event="message")
# even though run_async=False, handlers for RTM events can be a coroutine
async def send_reply(**payload):
def send_reply(**payload):
self.logger.debug(payload)
data = payload['data']
web_client = payload['web_client']
Expand All @@ -82,7 +81,7 @@ async def send_reply(**payload):
if "text" in data and self.text in data["text"]:
channel_id = data['channel']
thread_ts = data['ts']
self.success = await web_client.chat_postMessage(
self.success = web_client.chat_postMessage(
channel=channel_id,
text="Thanks!",
thread_ts=thread_ts
Expand All @@ -106,7 +105,6 @@ def connect():
self.web_client = WebClient(
token=self.bot_token,
run_async=False,
loop=asyncio.new_event_loop(), # TODO: this doesn't work without this
)
new_message = self.web_client.chat_postMessage(channel=self.channel_id, text=self.text)
self.assertFalse("error" in new_message)
Expand Down
2 changes: 1 addition & 1 deletion integration_tests/samples/issues/issue_497.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
)


# This doesn't work
# Fixed in 2.6.0: This doesn't work
@app.route("/sync/singleton", methods=["GET"])
def singleton():
try:
Expand Down
34 changes: 34 additions & 0 deletions integration_tests/web/test_issue_378.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import logging
import os
import unittest

from integration_tests.env_variable_names import SLACK_SDK_TEST_USER_TOKEN
from integration_tests.helpers import async_test
from slack import WebClient


class TestWebClient(unittest.TestCase):
"""Runs integration tests with real Slack API

https://github.com/slackapi/python-slackclient/issues/378
"""

def setUp(self):
self.logger = logging.getLogger(__name__)
self.user_token = os.environ[SLACK_SDK_TEST_USER_TOKEN]
self.sync_client: WebClient = WebClient(token=self.user_token, run_async=False)
self.async_client: WebClient = WebClient(token=self.user_token, run_async=True)

def tearDown(self):
pass

def test_issue_378(self):
client = self.sync_client
response = client.users_setPhoto(image="tests/data/slack_logo_new.png")
self.assertIsNotNone(response)

@async_test
async def test_issue_378_async(self):
client = self.async_client
response = await client.users_setPhoto(image="tests/data/slack_logo_new.png")
self.assertIsNotNone(response)
66 changes: 66 additions & 0 deletions integration_tests/web/test_issue_480.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
import logging
import multiprocessing
import os
import threading
import unittest

from integration_tests.env_variable_names import SLACK_SDK_TEST_USER_TOKEN
from integration_tests.helpers import async_test
from slack import WebClient


class TestWebClient(unittest.TestCase):
"""Runs integration tests with real Slack API

https://github.com/slackapi/python-slackclient/issues/480
"""

def setUp(self):
self.logger = logging.getLogger(__name__)
self.user_token = os.environ[SLACK_SDK_TEST_USER_TOKEN]
self.sync_client: WebClient = WebClient(token=self.user_token, run_async=False)
self.async_client: WebClient = WebClient(token=self.user_token, run_async=True)

def tearDown(self):
pass

def test_issue_480_processes(self):
client = self.sync_client
before = len(multiprocessing.active_children())
for idx in range(10):
response = client.api_test()
self.assertIsNotNone(response)
after = len(multiprocessing.active_children())
self.assertEqual(0, after - before)

@async_test
async def test_issue_480_processes_async(self):
client = self.async_client
before = len(multiprocessing.active_children())
for idx in range(10):
response = await client.api_test()
self.assertIsNotNone(response)
after = len(multiprocessing.active_children())
self.assertEqual(0, after - before)

# fails with Python 3.6
def test_issue_480_threads(self):
client = self.sync_client
before = threading.active_count()
for idx in range(10):
response = client.api_test()
self.assertIsNotNone(response)
after = threading.active_count()
self.assertEqual(0, after - before)

# fails with Python 3.6
@async_test
async def test_issue_480_threads_async(self):
client = self.async_client
before = threading.active_count()
for idx in range(10):
response = await client.api_test()
self.assertIsNotNone(response)
after = threading.active_count()
self.assertEqual(0, after - before)

37 changes: 11 additions & 26 deletions integration_tests/web/test_issue_560.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,8 @@
import os
import unittest

import pytest

from integration_tests.env_variable_names import SLACK_SDK_TEST_BOT_TOKEN
from integration_tests.helpers import async_test, is_not_specified
from integration_tests.helpers import async_test
from slack import WebClient


Expand All @@ -19,7 +17,7 @@ class TestWebClient(unittest.TestCase):
def setUp(self):
self.logger = logging.getLogger(__name__)
self.bot_token = os.environ[SLACK_SDK_TEST_BOT_TOKEN]
self.sync_client: WebClient = WebClient(token=self.bot_token, run_async=False, loop=asyncio.new_event_loop())
self.sync_client: WebClient = WebClient(token=self.bot_token, run_async=False)
self.async_client: WebClient = WebClient(token=self.bot_token, run_async=True)

def tearDown(self):
Expand All @@ -33,35 +31,22 @@ def test_issue_560_success(self):
response = client.conversations_list(exclude_archived="true")
self.assertIsNotNone(response)

@pytest.mark.skipif(condition=is_not_specified(), reason="still unfixed")
@async_test
async def test_issue_560_success_async(self):
client = self.sync_client
response = client.conversations_list(exclude_archived=1)
self.assertIsNotNone(response)

response = client.conversations_list(exclude_archived="true")
self.assertIsNotNone(response)

def test_issue_560_failure(self):
client = self.sync_client
response = client.conversations_list(exclude_archived=True)
self.assertIsNotNone(response)

@pytest.mark.skipif(condition=is_not_specified(), reason="still unfixed")
@async_test
async def test_issue_560_failure_async(self):
client = self.async_client
response = await client.conversations_list(exclude_archived=True)
self.assertIsNotNone(response)

# _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
#
# v = True
#
# @staticmethod
# def _query_var(v):
# if isinstance(v, str):
# return v
# if type(v) is int: # no subclasses like bool
# return str(v)
# > raise TypeError(
# "Invalid variable type: value "
# "should be str or int, got {!r} "
# "of type {}".format(v, type(v))
# )
# E TypeError: Invalid variable type: value should be str or int, got True of type <class 'bool'>
#
# path-to-python/site-packages/yarl/__init__.py:824: TypeError
# -------------------------------------------------------------------------------------------------- Captured log call --------------------------------------------------------------------------------------------------
4 changes: 2 additions & 2 deletions integration_tests/web/test_issue_594.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def test_issue_594(self):

message = client.chat_postEphemeral(
channel=self.channel_id,
user="U03E94MK0",
user=self.user_id,
blocks=[
{
"type": "section",
Expand Down Expand Up @@ -83,7 +83,7 @@ def test_no_preview_image(self):

message = client.chat_postEphemeral(
channel=self.channel_id,
user="U03E94MK0",
user=self.user_id,
blocks=[
{
"type": "section",
Expand Down
Loading