From 9e8930f6defd8be4226dd2cea6bad375532c46f5 Mon Sep 17 00:00:00 2001 From: Steve Howell Date: Thu, 23 Aug 2018 12:51:17 +0000 Subject: [PATCH] tests: Test get_widget_data() helper. We also remove some unreachable code. Calling split() always returns at least one token, even if it's just the empty string. This is tested directly on this commit, plus messages with empty content get rejected pretty early in the execution path. --- zerver/lib/widget.py | 3 +-- zerver/tests/test_widgets.py | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/zerver/lib/widget.py b/zerver/lib/widget.py index 193a96b63d..bab6414f4b 100644 --- a/zerver/lib/widget.py +++ b/zerver/lib/widget.py @@ -10,9 +10,8 @@ from zerver.models import SubMessage def get_widget_data(content: str) -> Tuple[Optional[str], Optional[str]]: valid_widget_types = ['tictactoe', 'poll', 'todo'] tokens = content.split(' ') - if not tokens: - return None, None + # tokens[0] will always exist if tokens[0].startswith('/'): widget_type = tokens[0][1:] if widget_type in valid_widget_types: diff --git a/zerver/tests/test_widgets.py b/zerver/tests/test_widgets.py index b9e81e6729..79b2f905da 100644 --- a/zerver/tests/test_widgets.py +++ b/zerver/tests/test_widgets.py @@ -6,6 +6,8 @@ from zerver.models import SubMessage from zerver.lib.test_classes import ZulipTestCase +from zerver.lib.widget import get_widget_data + from zerver.lib.validator import check_widget_content class WidgetContentTestCase(ZulipTestCase): @@ -81,6 +83,26 @@ class WidgetContentTestCase(ZulipTestCase): result = self.api_post(sender_email, "/api/v1/messages", payload) self.assert_json_error_contains(result, 'Widgets: widget_type is not in widget_content') + def test_get_widget_data_for_non_widget_messages(self) -> None: + # This is a pretty important test, despite testing the + # "negative" case. We never want widgets to interfere + # with normal messages. + + test_messages = [ + '', + ' ', + 'this is an ordinary message', + '/bogus_command', + '/me shrugs', + 'use /poll', + ] + + for message in test_messages: + self.assertEqual(get_widget_data(content=message), (None, None)) + + # Add a positive check for context + self.assertEqual(get_widget_data(content='/tictactoe'), ('tictactoe', None)) + def test_tictactoe(self) -> None: # The tictactoe widget is mostly useful as a code sample, # and it also helps us get test coverage that could apply