Skip to content
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

Fix/tests using called_once_with #2363

Merged
merged 9 commits into from
Nov 19, 2024
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
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
2 changes: 1 addition & 1 deletion tests/app/celery/test_process_pinpoint_receipts_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ def test_process_pinpoint_results_succeeded(sample_template, notify_db, notify_d
process_pinpoint_results(pinpoint_successful_callback(reference="ref"))

updated_notification = get_notification_by_id(notification.id)
assert mock_callback_task.not_called()
mock_callback_task.assert_not_called()
assert updated_notification.status == NOTIFICATION_SENT
assert updated_notification.provider_response is None

Expand Down
4 changes: 2 additions & 2 deletions tests/app/celery/test_process_sns_receipts_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,8 @@ def test_process_sns_results_does_not_process_other_providers(sample_template, m
)

process_sns_results(response=sns_success_callback(reference="ref1")) is None
assert mock_logger.called_once_with("")
assert not mock_dao.called
mock_logger.assert_called_once()
mock_dao.assert_not_called()


def test_process_sns_results_calls_service_callback(sample_template, notify_db_session, notify_db, mocker):
Expand Down
6 changes: 3 additions & 3 deletions tests/app/celery/test_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,14 +104,14 @@ def test_acknowledge_happy_path(self, mocker):
acknowledge_sms_normal_mock = mocker.patch("app.sms_normal.acknowledge", return_value=True)
acknowledge_sms_priority_mock = mocker.patch("app.sms_bulk.acknowledge", return_value=False)
acknowledge_receipt(SMS_TYPE, NORMAL, receipt)
assert acknowledge_sms_normal_mock.called_once_with(receipt)
assert acknowledge_sms_priority_mock.not_called()
acknowledge_sms_normal_mock.assert_called_once_with(receipt)
acknowledge_sms_priority_mock.assert_not_called()

def test_acknowledge_wrong_queue(self, mocker, notify_api):
receipt = uuid.uuid4()
acknowledge_sms_bulk_mock = mocker.patch("app.sms_bulk.acknowledge", return_value=True)
acknowledge_receipt(EMAIL_TYPE, NORMAL, receipt)
assert acknowledge_sms_bulk_mock.called_once_with(receipt)
acknowledge_sms_bulk_mock.assert_called_once_with(receipt)

def test_acknowledge_no_queue(self):
with pytest.raises(ValueError):
Expand Down
12 changes: 6 additions & 6 deletions tests/app/clients/test_freshdesk.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ def match_json(request):
with notify_api.app_context():
response = freshdesk.Freshdesk(ContactRequest(**contact_request)).send_ticket()
assert response == 201
assert email_freshdesk_ticket_mock.not_called()
email_freshdesk_ticket_mock.assert_not_called()

def test_send_ticket_go_live_request(self, email_freshdesk_ticket_mock, notify_api: Flask):
def match_json(request):
Expand Down Expand Up @@ -117,7 +117,7 @@ def match_json(request):
with notify_api.app_context():
response = freshdesk.Freshdesk(ContactRequest(**data)).send_ticket()
assert response == 201
assert email_freshdesk_ticket_mock.not_called()
email_freshdesk_ticket_mock.assert_not_called()

def test_send_ticket_branding_request(self, email_freshdesk_ticket_mock, notify_api: Flask):
def match_json(request):
Expand Down Expand Up @@ -179,7 +179,7 @@ def match_json(request):
with notify_api.app_context():
response = freshdesk.Freshdesk(ContactRequest(**data)).send_ticket()
assert response == 201
assert email_freshdesk_ticket_mock.not_called()
email_freshdesk_ticket_mock.assert_not_called()

def test_send_ticket_other_category(self, email_freshdesk_ticket_mock, notify_api: Flask):
def match_json(request):
Expand Down Expand Up @@ -227,7 +227,7 @@ def match_json(request):
with notify_api.app_context():
response = freshdesk.Freshdesk(ContactRequest(**data)).send_ticket()
assert response == 201
assert email_freshdesk_ticket_mock.not_called()
email_freshdesk_ticket_mock.assert_not_called()

def test_send_ticket_other(self, email_freshdesk_ticket_mock, notify_api: Flask):
def match_json(request):
Expand Down Expand Up @@ -258,7 +258,7 @@ def match_json(request):
with notify_api.app_context():
response = freshdesk.Freshdesk(ContactRequest(email_address="[email protected]")).send_ticket()
assert response == 201
assert email_freshdesk_ticket_mock.not_called()
email_freshdesk_ticket_mock.assert_not_called()

def test_send_ticket_user_profile(self, email_freshdesk_ticket_mock, notify_api: Flask):
def match_json(request):
Expand Down Expand Up @@ -294,7 +294,7 @@ def match_json(request):
)
).send_ticket()
assert response == 201
assert email_freshdesk_ticket_mock.not_called()
email_freshdesk_ticket_mock.assert_not_called()

def test_send_ticket_freshdesk_integration_disabled(self, mocker, email_freshdesk_ticket_mock, notify_api: Flask):
mocked_post = mocker.patch("requests.post")
Expand Down
4 changes: 2 additions & 2 deletions tests/app/template/test_rest.py
Original file line number Diff line number Diff line change
Expand Up @@ -1231,7 +1231,7 @@ def test_preview_letter_template_precompiled_pdf_file_type(notify_api, client, a
file_type="pdf",
)

assert mock_get_letter_pdf.called_once_with(notification)
mock_get_letter_pdf.assert_called_once_with(notification)
assert base64.b64decode(resp["content"]) == content


Expand Down Expand Up @@ -1334,7 +1334,7 @@ def test_preview_letter_template_precompiled_png_file_type_or_pdf_with_overlay(

with pytest.raises(ValueError):
mock_post.last_request.json()
assert mock_get_letter_pdf.called_once_with(notification)
mock_get_letter_pdf.assert_called_once_with(notification)
assert base64.b64decode(resp["content"]) == expected_returned_content


Expand Down
19 changes: 6 additions & 13 deletions tests/app/test_email_limit_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,28 +19,21 @@ def test_fetch_todays_requested_email_count(self, client, mocker, sample_service

assert actual_result == expected_result
if redis_value is None:
assert mocked_set.called_once_with(
mocked_set.assert_called_once_with(
cache_key,
db_value,
ex=7200,
)
else:
mocked_set.assert_not_called()

@pytest.mark.parametrize("redis_value, db_value, increment_by", [(None, 5, 5), ("3", 5, 3)])
def test_increment_todays_requested_email_count(self, mocker, sample_service, redis_value, db_value, increment_by):
def test_increment_todays_requested_email_count(self, client, mocker, sample_service, redis_value, db_value, increment_by):
cache_key = email_daily_count_cache_key(sample_service.id)
mocker.patch("app.redis_store.get", lambda x: redis_value if x == cache_key else None)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The mocked_set variable is being used in the test but it is not defined. You should add back the line mocked_set = mocker.patch("app.redis_store.set") to ensure the test works correctly.

mocked_set = mocker.patch("app.redis_store.set")
mocked_incrby = mocker.patch("app.redis_store.incrby")
mocker.patch("app.email_limit_utils.fetch_todays_email_count", return_value=db_value)

increment_todays_email_count(sample_service.id, increment_by)

assert mocked_incrby.called_once_with(cache_key, increment_by)
if redis_value is None:
assert mocked_set.called_once_with(
cache_key,
db_value,
)
else:
mocked_set.assert_not_called()
Copy link
Collaborator Author

@sastels sastels Nov 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're mocking the code that involves the set (fetch_todays_email_count()) so the set isn't actually called. That's ok since this code is tested above, so it's cleaner to mock it out in this test (we just can't test the set)

with set_config(client.application, "REDIS_ENABLED", True):
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we don't set REDIS_ENABLED then increment_todays_email_count() immediately returns None, and it turns out that REDIS_ENABLED is false in CI by default.

increment_todays_email_count(sample_service.id, increment_by)
mocked_incrby.assert_called_once_with(cache_key, increment_by)
19 changes: 6 additions & 13 deletions tests/app/test_sms_fragment_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,29 +21,22 @@ def test_fetch_todays_requested_sms_count(client, mocker, sample_service, redis_

assert actual_result == expected_result
if redis_value is None:
assert mocked_set.called_once_with(
mocked_set.assert_called_once_with(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The indentation of the ex=7200 argument should be aligned with the previous arguments for better readability.

cache_key,
db_value,
ex=7200,
)
else:
mocked_set.assert_not_called()


@pytest.mark.parametrize("redis_value,db_value,increment_by", [(None, 5, 5), ("3", 5, 3)])
def test_increment_todays_requested_sms_count(mocker, sample_service, redis_value, db_value, increment_by):
def test_increment_todays_requested_sms_count(mocker, client, sample_service, redis_value, db_value, increment_by):
cache_key = sms_daily_count_cache_key(sample_service.id)
mocker.patch("app.redis_store.get", lambda x: redis_value if x == cache_key else None)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The mocked_set variable is removed but still referenced in the test. Either remove the references to mocked_set or reintroduce the variable.

mocked_set = mocker.patch("app.redis_store.set")
mocked_incrby = mocker.patch("app.redis_store.incrby")
mocker.patch("app.sms_fragment_utils.fetch_todays_requested_sms_count", return_value=db_value)

increment_todays_requested_sms_count(sample_service.id, increment_by)

assert mocked_incrby.called_once_with(cache_key, increment_by)
if redis_value is None:
assert mocked_set.called_once_with(
cache_key,
db_value,
)
else:
mocked_set.assert_not_called()
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comment as above.

with set_config(client.application, "REDIS_ENABLED", True):
increment_todays_requested_sms_count(sample_service.id, increment_by)
mocked_incrby.assert_called_once_with(cache_key, increment_by)
Loading