-
Notifications
You must be signed in to change notification settings - Fork 19
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.
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
Add mocking guidance #205
Add mocking guidance #205
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @gisellerosetta - it's an excellent minimal introduction to why and how to mock. I spied a couple of typos and had a couple of suggestions to increase the stringency, particularly with the example that checked the exception was raised. It's very easy to write mocked code that doesn't behave the way you expect it to - the additional verification assertions should safeguard against that.
Finally, a bonus would be to show how to serve a mock fixture that could be shared across several tests, dynamically returning the expected values. That would introduce even more complexity so I'll leave that at your discretion below. Many thanks for spending the time on adding this guidance into the duck book - I'm sure our gov colleagues will find it very useful.
# logical extension based on the original code, is how to serve a fixture that could
# meet various conditions in an ifelse flow, eg
@pytest.fixture(scope="function")
def mock_response():
def _create_mock_response(url):
"""factory function allows flexible return values when evaluated"""
mock_response = mock.Mock()
if url == "http://example.com/good-request":
mock_response.text = "Successful"
mock_response.status_code = 200
elif url == "http://example.com/bad-request":
mock_response.status_code = 400
return mock_response
return _create_mock_response
@mock.patch("requests.get")
def test_get_response_all_conditions(mock_requests_get, mock_response):
good_url = "http://example.com/good-request"
mock_requests_get.return_value = mock_response(good_url)
actual = get_response(good_url)
assert actual == "Successful"
mock_requests_get.assert_called_once_with(good_url)
# Reset the mock for the next test case
mock_requests_get.reset_mock()
# Test failure case
bad_url = "http://example.com/bad-request"
mock_requests_get.return_value = mock_response(bad_url)
with pytest.raises(requests.HTTPError, match="Unsuccessful request"):
get_response(bad_url)
mock_requests_get.assert_called_once_with(bad_url)
Thanks for these comments Rich! I really appreciate the way you've delivered this feedback and the points you've raised. I've aimed to address all points - the code is first presented as a basic example, and then I've copied it again with the new assert statements and also with the matching on error messages. I've implemented your fixtures example too as a valuable example of how to do this in a more succinct way. Thanks again for your contributions. |
Reviewing the changes since I last viewed, I agree that the way the test cases are built upon are an effective compromise between simplicity and stringency. Thanks for taking the time to put it all together - I look forward to being able to point junior python devs to this chapter as a source of support for mocking. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks fab, thanks both!
Hey @sarahcollyer have you noticed that these changes are not being deployed to the live site? |
@r-leyshon yep, we haven't done a deployment yet. The bulk of the work on the testing chapter should finish in 3 weeks (if not sooner), so we will be deploying the changes to the live site then :) |
ah ok, thanks for the heads up Sarah. |
This is a draft pull request while feedback is gathered from different places. This was a new topic to me so want to be triple sure it's technically correct!
@r-leyshon has kindly volunteered to take a look the changes, as has an internal colleague. Once any requested changes are made, I plan to ask a RAP colleague for a team review. :)