refactor: Inline succeed_with_message().

This two-line function wasn't really carrying its
weight, and it just made it harder to refactor the
overall codepath.

Eliminating the function forces us to mock at a slightly
deeper level, which is probably a good thing for what
the test intends to do.  The deeper mock still verifies that
we're sending the message (good) without digging into
all the details of how we send it (good).

Note that we will still keep around the similarly named
`fail_with_message` helper, which is a lot more useful.
(The succeed/fail scenarios aren't really symmetric here.
For success, there are fewer codepaths that do more complex
things, whereas we have lots and lots of failure codepaths
that all do the same simple thing of replying with a canned
message.)
This commit is contained in:
Steve Howell
2018-10-09 14:25:18 +00:00
committed by Tim Abbott
parent fa505a1af1
commit f2dd218331
2 changed files with 5 additions and 8 deletions

View File

@@ -160,10 +160,6 @@ def send_response_message(bot_id: str, message: Dict[str, Any], response_message
realm=realm,
)
def succeed_with_message(event: Dict[str, Any], success_message: str) -> None:
success_message = "Success! " + success_message
send_response_message(event['user_profile_id'], event['message'], success_message)
def fail_with_message(event: Dict[str, Any], failure_message: str) -> None:
failure_message = "Failure! " + failure_message
send_response_message(event['user_profile_id'], event['message'], failure_message)
@@ -239,7 +235,8 @@ def process_success_response(event: Dict[str, Any],
if success_message is None:
return
succeed_with_message(event, success_message)
success_message = "Success! " + success_message
send_response_message(event['user_profile_id'], event['message'], success_message)
def do_rest_call(rest_operation: Dict[str, Any],
request_data: Optional[Dict[str, Any]],

View File

@@ -62,12 +62,12 @@ class DoRestCallTests(ZulipTestCase):
self.bot_user = self.example_user('outgoing_webhook_bot')
logging.disable(logging.WARNING)
@mock.patch('zerver.lib.outgoing_webhook.succeed_with_message')
def test_successful_request(self, mock_succeed_with_message: mock.Mock) -> None:
@mock.patch('zerver.lib.outgoing_webhook.send_response_message')
def test_successful_request(self, mock_send: mock.Mock) -> None:
response = ResponseMock(200)
with mock.patch('requests.request', return_value=response):
do_rest_call(self.rest_operation, None, self.mock_event, service_handler, None)
self.assertTrue(mock_succeed_with_message.called)
self.assertTrue(mock_send.called)
def test_retry_request(self: mock.Mock) -> None:
response = ResponseMock(500)