From 1e55a8cbbe01e3f20ea6be6d4ab7180690bebb37 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 28 Aug 2020 13:26:14 +0100 Subject: [PATCH] Remove uses of .assert_not_called MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I prefer to avoid `assert_not_called`, because if I make a typo like this, the tests will still pass: ``` app.invite_api_client.create_invite.asset_not_called() ``` It’s harder to have a false positive with the statement written this way: ``` assert app.invite_api_client.create_invite.called is False ``` --- tests/app/main/views/test_api_integration.py | 2 +- tests/app/main/views/test_jobs.py | 2 +- tests/app/main/views/test_letter_branding.py | 8 +++--- tests/app/main/views/test_platform_admin.py | 2 +- tests/app/main/views/test_returned_letters.py | 2 +- tests/app/main/views/test_service_settings.py | 28 +++++++++---------- tests/app/main/views/test_two_factor.py | 6 ++-- tests/app/s3_client/test_s3_logo_client.py | 8 +++--- 8 files changed, 29 insertions(+), 29 deletions(-) diff --git a/tests/app/main/views/test_api_integration.py b/tests/app/main/views/test_api_integration.py index 1204dc42b..f6f7a6344 100644 --- a/tests/app/main/views/test_api_integration.py +++ b/tests/app/main/views/test_api_integration.py @@ -301,7 +301,7 @@ def test_cant_create_normal_api_key_in_trial_mode( }, _expected_status=400, ) - mock_post.assert_not_called() + assert mock_post.called is False def test_should_show_confirm_revoke_api_key( diff --git a/tests/app/main/views/test_jobs.py b/tests/app/main/views/test_jobs.py index 996d77dcc..3fdfbf985 100644 --- a/tests/app/main/views/test_jobs.py +++ b/tests/app/main/views/test_jobs.py @@ -642,7 +642,7 @@ def test_dont_cancel_letter_job_when_to_early_to_cancel( job_id=str(job_id), _expected_status=200, ) - mock_cancel.assert_not_called() + assert mock_cancel.called is False flash_message = normalize_spaces(page.find('div', class_='banner-dangerous').text) assert 'We are still processing these letters, please try again in a minute.' in flash_message diff --git a/tests/app/main/views/test_letter_branding.py b/tests/app/main/views/test_letter_branding.py index a1b756f96..dd8e11fd8 100644 --- a/tests/app/main/views/test_letter_branding.py +++ b/tests/app/main/views/test_letter_branding.py @@ -92,7 +92,7 @@ def test_update_letter_branding_with_new_valid_file( assert page.select_one('#name').attrs.get('value') == 'HM Government' assert mock_s3_upload.called - mock_delete_temp_files.assert_not_called() + assert mock_delete_temp_files.called is False def test_update_letter_branding_when_uploading_invalid_file( @@ -163,7 +163,7 @@ def test_update_letter_branding_with_original_file_and_new_details( assert response.status_code == 200 assert page.find('h1').text == 'Letter branding' - mock_upload_logos.assert_not_called() + assert mock_upload_logos.called is False mock_client_update.assert_called_once_with( branding_id=fake_uuid, @@ -343,7 +343,7 @@ def test_create_letter_branding_when_uploading_valid_file( assert page.select_one('#logo-img > img').attrs['src'].endswith(expected_temp_filename) assert mock_s3_upload.called - mock_delete_temp_files.assert_not_called() + assert mock_delete_temp_files.called is False def test_create_letter_branding_fails_validation_when_uploading_SVG_with_embedded_image( @@ -372,7 +372,7 @@ def test_create_letter_branding_fails_validation_when_uploading_SVG_with_embedde assert page.findAll('div', {'id': 'logo-img'}) == [] - mock_s3_upload.assert_not_called() + assert mock_s3_upload.called is False def test_create_letter_branding_when_uploading_invalid_file(platform_admin_client): diff --git a/tests/app/main/views/test_platform_admin.py b/tests/app/main/views/test_platform_admin.py index 1f9581ba8..811eadc0c 100644 --- a/tests/app/main/views/test_platform_admin.py +++ b/tests/app/main/views/test_platform_admin.py @@ -854,7 +854,7 @@ def test_get_notifications_sent_by_service_validates_form(mocker, client_request for error in errors: assert 'Not a valid date value' in error.text - mock_get_stats_from_api.assert_not_called() + assert mock_get_stats_from_api.called is False def test_usage_for_all_services_when_no_results_for_date(client_request, platform_admin_user, mocker): diff --git a/tests/app/main/views/test_returned_letters.py b/tests/app/main/views/test_returned_letters.py index c8aadafc5..b04981adf 100644 --- a/tests/app/main/views/test_returned_letters.py +++ b/tests/app/main/views/test_returned_letters.py @@ -183,4 +183,4 @@ def test_returned_letters_reports_returns_404_for_bad_date( service_id=SERVICE_ONE_ID, reported_at='19-12-2019', _expected_status=404) - mock.assert_not_called() + assert mock.called is False diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 0643abec0..7ec87bbce 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -789,8 +789,8 @@ def test_service_name_change_confirm_handles_expired_session( service_id=SERVICE_ONE_ID, _follow_redirects=True ) - mock_verify_password.assert_not_called() - mock_update_service.assert_not_called() + assert mock_verify_password.called is False + assert mock_update_service.called is False assert page.find('div', 'banner-dangerous').text.strip() == "The change you made was not saved. Please try again." @@ -2378,14 +2378,14 @@ def test_service_verify_reply_to_address( mock_update_reply_to_email_address.assert_called_once_with( SERVICE_ONE_ID, "123", email_address=notification["to"], is_default=is_default ) - mock_add_reply_to_email_address.assert_not_called() + assert mock_add_reply_to_email_address.called is False else: mock_add_reply_to_email_address.assert_called_once_with( SERVICE_ONE_ID, email_address=notification["to"], is_default=is_default ) - mock_update_reply_to_email_address.assert_not_called() + assert mock_update_reply_to_email_address.called is False else: - mock_add_reply_to_email_address.assert_not_called() + assert mock_add_reply_to_email_address.called is False if status == "permanent-failure": assert page.find('input', type='email').attrs["value"] == notification["to"] @@ -2414,7 +2414,7 @@ def test_add_reply_to_email_address_fails_if_notification_not_delivered_in_45_se ) expected_banner = page.find_all('div', class_='banner-dangerous')[0] assert 'There’s a problem with your reply-to address' in expected_banner.text.strip() - mock_add_reply_to_email_address.assert_not_called() + assert mock_add_reply_to_email_address.called is False @pytest.mark.parametrize('letter_contact_blocks, data, api_default_args', [ @@ -2620,7 +2620,7 @@ def test_edit_reply_to_email_address_goes_straight_to_update_if_address_not_chan email_address="test@example.com", is_default=api_default_args ) - mock_verify.assert_not_called() + assert mock_verify.called is False @pytest.mark.parametrize('url', [ @@ -2662,7 +2662,7 @@ def test_add_edit_reply_to_email_address_goes_straight_to_update_if_address_not_ assert page.find('h1').text == "Reply-to email addresses" assert error_message in page.find('div', class_='banner-dangerous').text - mock_update_reply_to_email_address.assert_not_called() + assert mock_update_reply_to_email_address.called is False @pytest.mark.parametrize('reply_to_address, expected_link_text, partial_href', [ @@ -4482,8 +4482,8 @@ def test_show_branding_request_page_when_no_branding_is_set( '.branding_request', service_id=SERVICE_ONE_ID, branding_type=branding_type ) - mock_get_email_branding.assert_not_called() - mock_get_letter_branding_by_id.assert_not_called() + assert mock_get_email_branding.called is False + assert mock_get_letter_branding_by_id.called is False if expected_options: assert [ @@ -4555,8 +4555,8 @@ def test_show_branding_request_page_when_no_branding_is_set_but_organisation_exi '.branding_request', service_id=SERVICE_ONE_ID, branding_type=branding_type ) - mock_get_email_branding.assert_not_called() - mock_get_letter_branding_by_id.assert_not_called() + assert mock_get_email_branding.called is False + assert mock_get_letter_branding_by_id.called is False assert [ ( @@ -4599,8 +4599,8 @@ def test_show_branding_request_page_when_no_branding_is_set_but_organisation_exi '.branding_request', service_id=SERVICE_ONE_ID, branding_type=branding_type ) - mock_get_email_branding.assert_not_called() - mock_get_letter_branding_by_id.assert_not_called() + assert mock_get_email_branding.called is False + assert mock_get_letter_branding_by_id.called is False assert [ ( diff --git a/tests/app/main/views/test_two_factor.py b/tests/app/main/views/test_two_factor.py index 70c480242..2c2c9dd2e 100644 --- a/tests/app/main/views/test_two_factor.py +++ b/tests/app/main/views/test_two_factor.py @@ -331,7 +331,7 @@ def test_two_factor_email_link_has_expired( page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') assert page.h1.text.strip() == 'The link has expired' - mock_send_verify_code.assert_not_called + assert mock_send_verify_code.called is False def test_two_factor_email_link_is_invalid( @@ -367,7 +367,7 @@ def test_two_factor_email_link_is_already_used( assert response.status_code == 200 assert page.h1.text.strip() == 'The link has expired' - mock_send_verify_code.assert_not_called + assert mock_send_verify_code.called is False def test_two_factor_email_link_when_user_is_locked_out( @@ -387,7 +387,7 @@ def test_two_factor_email_link_when_user_is_locked_out( assert response.status_code == 200 assert page.h1.text.strip() == 'The link has expired' - mock_send_verify_code.assert_not_called + assert mock_send_verify_code.called is False def test_two_factor_email_link_used_when_user_already_logged_in( diff --git a/tests/app/s3_client/test_s3_logo_client.py b/tests/app/s3_client/test_s3_logo_client.py index a36d73c97..8b896f089 100644 --- a/tests/app/s3_client/test_s3_logo_client.py +++ b/tests/app/s3_client/test_s3_logo_client.py @@ -95,8 +95,8 @@ def test_persist_logo_returns_if_not_temp(client, mocker, fake_uuid): mocked_get_s3_object = mocker.patch('app.s3_client.s3_logo_client.get_s3_object') mocked_delete_s3_object = mocker.patch('app.s3_client.s3_logo_client.delete_s3_object') - mocked_get_s3_object.assert_not_called() - mocked_delete_s3_object.assert_not_called() + assert mocked_get_s3_object.called is False + assert mocked_delete_s3_object.called is False def test_permanent_email_logo_name_removes_TEMP_TAG_from_filename(upload_filename, fake_uuid): @@ -163,7 +163,7 @@ def test_does_not_delete_non_temp_email_file(client, mocker): with pytest.raises(ValueError) as error: delete_email_temp_file(filename) - mocked_delete_s3_object.assert_not_called + assert mocked_delete_s3_object.called is False assert str(error.value) == 'Not a temp file: {}'.format(filename) @@ -183,5 +183,5 @@ def test_does_not_delete_non_temp_letter_file(mocker, fake_uuid): with pytest.raises(ValueError) as error: delete_letter_temp_file(svg_filename) - mocked_delete_s3_object.assert_not_called + assert mocked_delete_s3_object.called is False assert str(error.value) == 'Not a temp file: {}'.format(svg_filename)