Skip to content

Commit

Permalink
If new msg text is None, fallback to empty str (scrapinghub#144)
Browse files Browse the repository at this point in the history
* Defend against get_plugins() being called with None

* Add unit test for scrapinghub#138

* gitignore: .vscode

* msg.get() in MessageDispatcher._dispatch_msg_handler()
  • Loading branch information
tehranian authored and lins05 committed May 28, 2017
1 parent f6c8d4f commit c0dbea6
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 1 deletion.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,4 @@ slackbot_test_settings.py
/dist
/*.egg-info
.cache
/.vscode/
2 changes: 1 addition & 1 deletion slackbot/dispatcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def dispatch_msg(self, msg):

def _dispatch_msg_handler(self, category, msg):
responded = False
for func, args in self._plugins.get_plugins(category, msg['text']):
for func, args in self._plugins.get_plugins(category, msg.get('text', None)):
if func:
responded = True
try:
Expand Down
2 changes: 2 additions & 0 deletions slackbot/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ def _load_plugins(self, plugin):

def get_plugins(self, category, text):
has_matching_plugin = False
if text is None:
text = ''
for matcher in self.commands[category]:
m = matcher.search(text)
if m:
Expand Down
11 changes: 11 additions & 0 deletions tests/unit/test_dispatcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -200,3 +200,14 @@ def test_dispatch_default_msg_plugin(dispatcher, monkeypatch):
dispatcher.dispatch_msg(
['respond_to', {'text': 'default_okay', 'channel': FAKE_CHANNEL}])
assert dispatcher._client.rtm_messages == [(FAKE_CHANNEL, 'default_okay')]


def test_none_text(dispatcher):
# Test for #138: If new msg text is None, fallback to empty str
msg = {
'text': None,
'channel': 'C99999'
}
# Should not raise a TypeError
msg = dispatcher.filter_text(msg)
assert msg is None
10 changes: 10 additions & 0 deletions tests/unit/test_manager.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import re
import sys
import os

Expand All @@ -15,3 +16,12 @@ def test_import_plugin_single_module():
assert 'fake_plugin_module' not in sys.modules
PluginsManager()._load_plugins('fake_plugin_module')
assert 'fake_plugin_module' in sys.modules


def test_get_plugins_none_text():
p = PluginsManager()
p.commands['respond_to'][re.compile(r'^dummy regexp$')] = lambda x: x
# Calling get_plugins() with `text == None`
for func, args in p.get_plugins('respond_to', None):
assert func is None
assert args is None

0 comments on commit c0dbea6

Please sign in to comment.