Merge pull request #1570 from GSA/notify-admin-1041

use moto to mock s3

.ds.baseline

@@ -675,7 +675,7 @@
         "filename": "tests/conftest.py",
         "hashed_secret": "f8377c90fcfd699f0ddbdcb30c2c9183d2d933ea",
         "is_verified": false,
-        "line_number": 3289,
+        "line_number": 3266,
         "is_secret": false
       }
     ],
@@ -710,5 +710,5 @@
       }
     ]
   },
-  "generated_at": "2024-05-20T16:03:05Z"
+  "generated_at": "2024-05-29T21:18:03Z"
 }

app/s3_client/__init__.py

@@ -1,3 +1,5 @@
+import os
+
 import botocore
 from boto3 import Session
 from botocore.config import Config
@@ -29,6 +31,17 @@ def get_s3_object(
     )
     s3 = session.resource("s3", config=AWS_CLIENT_CONFIG)
     obj = s3.Object(bucket_name, filename)
+    # This 'proves' that use of moto in the relevant tests in test_send.py
+    # mocks everything related to S3.  What you will see in the logs is:
+    # Exception: CREATED AT <MagicMock name='resource().Bucket().creation_date' id='4665562448'>
+    #
+    # raise Exception(f"CREATED AT {_s3.Bucket(bucket_name).creation_date}")
+    if os.getenv("NOTIFY_ENVIRONMENT") == "test":
+        teststr = str(s3.Bucket(bucket_name).creation_date).lower()
+        if "magicmock" not in teststr:
+            raise Exception(
+                "Test is not mocked, use @mock_aws or the relevant mocker.patch to avoid accessing S3"
+            )
     return obj
 
 

app/s3_client/s3_csv_client.py

@@ -28,6 +28,7 @@ def get_csv_upload(service_id, upload_id):
 
 
 def s3upload(service_id, filedata):
+
     upload_id = str(uuid.uuid4())
     bucket_name, file_location, access_key, secret_key, region = get_csv_location(
         service_id, upload_id

notifications_utils/s3.py

@@ -38,6 +38,17 @@ def s3upload(
         region_name=region,
     )
     _s3 = session.resource("s3", config=AWS_CLIENT_CONFIG)
+    # This 'proves' that use of moto in the relevant tests in test_send.py
+    # mocks everything related to S3.  What you will see in the logs is:
+    # Exception: CREATED AT <MagicMock name='resource().Bucket().creation_date' id='4665562448'>
+    #
+    # raise Exception(f"CREATED AT {_s3.Bucket(bucket_name).creation_date}")
+    if os.getenv("NOTIFY_ENVIRONMENT") == "test":
+        teststr = str(_s3.Bucket(bucket_name).creation_date).lower()
+        if "magicmock" not in teststr:
+            raise Exception(
+                "Test is not mocked, use @mock_aws or the relevant mocker.patch to avoid accessing S3"
+            )
 
     key = _s3.Object(bucket_name, file_location)
 
@@ -82,6 +93,17 @@ def s3download(
         )
         s3 = session.resource("s3", config=AWS_CLIENT_CONFIG)
         key = s3.Object(bucket_name, filename)
+        # This 'proves' that use of moto in the relevant tests in test_send.py
+        # mocks everything related to S3.  What you will see in the logs is:
+        # Exception: CREATED AT <MagicMock name='resource().Bucket().creation_date' id='4665562448'>
+        #
+        # raise Exception(f"CREATED AT {_s3.Bucket(bucket_name).creation_date}")
+        if os.getenv("NOTIFY_ENVIRONMENT") == "test":
+            teststr = str(s3.Bucket(bucket_name).creation_date).lower()
+            if "magicmock" not in teststr:
+                raise Exception(
+                    "Test is not mocked, use @mock_aws or the relevant mocker.patch to avoid accessing S3"
+                )
         return key.get()["Body"]
     except botocore.exceptions.ClientError as error:
         raise S3ObjectNotFound(error.response, error.operation_name)

poetry.lock

@@ -1535,6 +1535,50 @@ files = [
     {file = "mistune-0.8.4.tar.gz", hash = "sha256:59a3429db53c50b5c6bcc8a07f8848cb00d7dc8bdb431a4ab41920d201d4756e"},
 ]
 
+[[package]]
+name = "moto"
+version = "5.0.8"
+description = ""
+optional = false
+python-versions = ">=3.8"
+files = [
+    {file = "moto-5.0.8-py2.py3-none-any.whl", hash = "sha256:7d1035e366434bfa9fcc0621f07d5aa724b6846408071d540137a0554c46f214"},
+    {file = "moto-5.0.8.tar.gz", hash = "sha256:517fb808dc718bcbdda54c6ffeaca0adc34cf6e10821bfb01216ce420a31765c"},
+]
+
+[package.dependencies]
+boto3 = ">=1.9.201"
+botocore = ">=1.14.0"
+cryptography = ">=3.3.1"
+Jinja2 = ">=2.10.1"
+python-dateutil = ">=2.1,<3.0.0"
+requests = ">=2.5"
+responses = ">=0.15.0"
+werkzeug = ">=0.5,<2.2.0 || >2.2.0,<2.2.1 || >2.2.1"
+xmltodict = "*"
+
+[package.extras]
+all = ["PyYAML (>=5.1)", "antlr4-python3-runtime", "aws-xray-sdk (>=0.93,!=0.96)", "cfn-lint (>=0.40.0)", "docker (>=3.0.0)", "graphql-core", "joserfc (>=0.9.0)", "jsondiff (>=1.1.2)", "jsonpath-ng", "multipart", "openapi-spec-validator (>=0.5.0)", "py-partiql-parser (==0.5.5)", "pyparsing (>=3.0.7)", "setuptools"]
+apigateway = ["PyYAML (>=5.1)", "joserfc (>=0.9.0)", "openapi-spec-validator (>=0.5.0)"]
+apigatewayv2 = ["PyYAML (>=5.1)", "openapi-spec-validator (>=0.5.0)"]
+appsync = ["graphql-core"]
+awslambda = ["docker (>=3.0.0)"]
+batch = ["docker (>=3.0.0)"]
+cloudformation = ["PyYAML (>=5.1)", "aws-xray-sdk (>=0.93,!=0.96)", "cfn-lint (>=0.40.0)", "docker (>=3.0.0)", "graphql-core", "joserfc (>=0.9.0)", "jsondiff (>=1.1.2)", "openapi-spec-validator (>=0.5.0)", "py-partiql-parser (==0.5.5)", "pyparsing (>=3.0.7)", "setuptools"]
+cognitoidp = ["joserfc (>=0.9.0)"]
+dynamodb = ["docker (>=3.0.0)", "py-partiql-parser (==0.5.5)"]
+dynamodbstreams = ["docker (>=3.0.0)", "py-partiql-parser (==0.5.5)"]
+glue = ["pyparsing (>=3.0.7)"]
+iotdata = ["jsondiff (>=1.1.2)"]
+proxy = ["PyYAML (>=5.1)", "antlr4-python3-runtime", "aws-xray-sdk (>=0.93,!=0.96)", "cfn-lint (>=0.40.0)", "docker (>=2.5.1)", "graphql-core", "joserfc (>=0.9.0)", "jsondiff (>=1.1.2)", "jsonpath-ng", "multipart", "openapi-spec-validator (>=0.5.0)", "py-partiql-parser (==0.5.5)", "pyparsing (>=3.0.7)", "setuptools"]
+resourcegroupstaggingapi = ["PyYAML (>=5.1)", "cfn-lint (>=0.40.0)", "docker (>=3.0.0)", "graphql-core", "joserfc (>=0.9.0)", "jsondiff (>=1.1.2)", "openapi-spec-validator (>=0.5.0)", "py-partiql-parser (==0.5.5)", "pyparsing (>=3.0.7)"]
+s3 = ["PyYAML (>=5.1)", "py-partiql-parser (==0.5.5)"]
+s3crc32c = ["PyYAML (>=5.1)", "crc32c", "py-partiql-parser (==0.5.5)"]
+server = ["PyYAML (>=5.1)", "antlr4-python3-runtime", "aws-xray-sdk (>=0.93,!=0.96)", "cfn-lint (>=0.40.0)", "docker (>=3.0.0)", "flask (!=2.2.0,!=2.2.1)", "flask-cors", "graphql-core", "joserfc (>=0.9.0)", "jsondiff (>=1.1.2)", "jsonpath-ng", "openapi-spec-validator (>=0.5.0)", "py-partiql-parser (==0.5.5)", "pyparsing (>=3.0.7)", "setuptools"]
+ssm = ["PyYAML (>=5.1)"]
+stepfunctions = ["antlr4-python3-runtime", "jsonpath-ng"]
+xray = ["aws-xray-sdk (>=0.93,!=0.96)", "setuptools"]
+
 [[package]]
 name = "msgpack"
 version = "1.0.8"
@@ -2598,6 +2642,25 @@ requests = ">=2.22,<3"
 [package.extras]
 fixture = ["fixtures"]
 
+[[package]]
+name = "responses"
+version = "0.25.0"
+description = "A utility library for mocking out the `requests` Python library."
+optional = false
+python-versions = ">=3.8"
+files = [
+    {file = "responses-0.25.0-py3-none-any.whl", hash = "sha256:2f0b9c2b6437db4b528619a77e5d565e4ec2a9532162ac1a131a83529db7be1a"},
+    {file = "responses-0.25.0.tar.gz", hash = "sha256:01ae6a02b4f34e39bffceb0fc6786b67a25eae919c6368d05eabc8d9576c2a66"},
+]
+
+[package.dependencies]
+pyyaml = "*"
+requests = ">=2.30.0,<3.0"
+urllib3 = ">=1.25.10,<3.0"
+
+[package.extras]
+tests = ["coverage (>=6.0.0)", "flake8", "mypy", "pytest (>=7.0.0)", "pytest-asyncio", "pytest-cov", "pytest-httpserver", "tomli", "tomli-w", "types-PyYAML", "types-requests"]
+
 [[package]]
 name = "rich"
 version = "13.7.1"
@@ -2925,7 +2988,18 @@ files = [
     {file = "xlwt-1.3.0.tar.gz", hash = "sha256:c59912717a9b28f1a3c2a98fd60741014b06b043936dcecbc113eaaada156c88"},
 ]
 
+[[package]]
+name = "xmltodict"
+version = "0.13.0"
+description = "Makes working with XML feel like you are working with JSON"
+optional = false
+python-versions = ">=3.4"
+files = [
+    {file = "xmltodict-0.13.0-py2.py3-none-any.whl", hash = "sha256:aa89e8fd76320154a40d19a0df04a4695fb9dc5ba977cbb68ab3e4eb225e7852"},
+    {file = "xmltodict-0.13.0.tar.gz", hash = "sha256:341595a488e3e01a85a9d8911d8912fd922ede5fecc4dce437eb4b6c8d037e56"},
+]
+
 [metadata]
 lock-version = "2.0"
 python-versions = "^3.12.2"
-content-hash = "dbfc0e66e41f6da3845785157b95ef70193bc66c91b53fe0afb0eac0a1ea5490"
+content-hash = "ebecc7a1869605e491b4cb168a621e9e8b64a69bca74189857782c51c9fd7b9c"

pyproject.toml

@@ -82,6 +82,7 @@ flake8-print = "^5.0.0"
 flake8-pytest-style = "^1.7.2"
 isort = "^5.13.2"
 jinja2-cli = {version = "==0.8.2", extras = ["yaml"]}
+moto = "*"
 pip-audit = "*"
 pre-commit = "^3.7.1"
 pytest = "^8.2.1"

tests/app/main/views/test_send.py

@@ -6,6 +6,7 @@ from io import BytesIO
 from itertools import repeat
 from os import path
 from random import randbytes
+from unittest.mock import ANY
 from uuid import uuid4
 from zipfile import BadZipFile
 
@@ -17,7 +18,11 @@ from xlrd.xldate import XLDateAmbiguous, XLDateError, XLDateNegative, XLDateTooL
 
 from notifications_utils.recipients import RecipientCSV
 from notifications_utils.template import SMSPreviewTemplate
-from tests import validate_route_permission, validate_route_permission_with_client
+from tests import (
+    sample_uuid,
+    validate_route_permission,
+    validate_route_permission_with_client,
+)
 from tests.conftest import (
     SERVICE_ONE_ID,
     create_active_caseworking_user,
@@ -434,10 +439,15 @@ def test_upload_files_in_different_formats(
     service_one,
     mocker,
     mock_get_service_template,
-    mock_s3_set_metadata,
-    mock_s3_upload,
     fake_uuid,
 ):
+
+    mock_s3_set_metadata = mocker.patch(
+        "app.main.views.send.set_metadata_on_csv_upload"
+    )
+
+    mock_s3_upload = mocker.patch("app.main.views.send.s3upload")
+
     with open(filename, "rb") as uploaded:
         page = client_request.post(
             "main.send_messages",
@@ -456,7 +466,7 @@ def test_upload_files_in_different_formats(
             "202 205 8823,Still Not Pete,Crimson,Pear"
         )
         mock_s3_set_metadata.assert_called_once_with(
-            SERVICE_ONE_ID, fake_uuid, original_file_name=filename
+            SERVICE_ONE_ID, ANY, original_file_name=filename
         )
     else:
         assert not mock_s3_upload.called
@@ -470,13 +480,16 @@ def test_send_messages_sanitises_and_truncates_file_name_for_metadata(
     service_one,
     mocker,
     mock_get_service_template_with_placeholders,
-    mock_s3_set_metadata,
-    mock_s3_get_metadata,
-    mock_s3_upload,
-    mock_s3_download,
     mock_get_job_doesnt_exist,
     fake_uuid,
 ):
+
+    mock_s3_set_metadata = mocker.patch(
+        "app.main.views.send.set_metadata_on_csv_upload"
+    )
+
+    mocker.patch("app.main.views.send.s3upload")
+
     filename = f"😁{'a' * 2000}.csv"
 
     client_request.post(
@@ -577,15 +590,20 @@ def test_upload_csv_file_with_errors_shows_check_page_with_errors(
     service_one,
     mocker,
     mock_get_service_template_with_placeholders,
-    mock_s3_set_metadata,
-    mock_s3_get_metadata,
-    mock_s3_upload,
     mock_get_users_by_service,
     mock_get_service_statistics,
     mock_get_job_doesnt_exist,
     mock_get_jobs,
     fake_uuid,
 ):
+
+    mocker.patch("app.main.views.send.set_metadata_on_csv_upload")
+
+    mocker.patch(
+        "app.main.views.send.get_csv_metadata",
+        return_value={"original_file_name": "example.csv"},
+    )
+    mocker.patch("app.main.views.send.s3upload", return_value=sample_uuid())
     mocker.patch(
         "app.main.views.send.s3download",
         return_value="""
@@ -624,15 +642,21 @@ def test_upload_csv_file_with_empty_message_shows_check_page_with_errors(
     service_one,
     mocker,
     mock_get_empty_service_template_with_optional_placeholder,
-    mock_s3_set_metadata,
-    mock_s3_get_metadata,
-    mock_s3_upload,
     mock_get_users_by_service,
     mock_get_service_statistics,
     mock_get_job_doesnt_exist,
     mock_get_jobs,
     fake_uuid,
 ):
+
+    mocker.patch("app.main.views.send.set_metadata_on_csv_upload")
+
+    mocker.patch(
+        "app.main.views.send.get_csv_metadata",
+        return_value={"original_file_name": "example.csv"},
+    )
+    mocker.patch("app.main.views.send.s3upload", return_value=sample_uuid())
+
     mocker.patch(
         "app.main.views.send.s3download",
         return_value="""
@@ -676,15 +700,20 @@ def test_upload_csv_file_with_very_long_placeholder_shows_check_page_with_errors
     service_one,
     mocker,
     mock_get_service_template_with_placeholders,
-    mock_s3_set_metadata,
-    mock_s3_get_metadata,
-    mock_s3_upload,
     mock_get_users_by_service,
     mock_get_service_statistics,
     mock_get_job_doesnt_exist,
     mock_get_jobs,
     fake_uuid,
 ):
+
+    mocker.patch("app.main.views.send.set_metadata_on_csv_upload")
+
+    mocker.patch(
+        "app.main.views.send.get_csv_metadata",
+        return_value={"original_file_name": "example.csv"},
+    )
+    mocker.patch("app.main.views.send.s3upload", return_value=sample_uuid())
     big_placeholder = " ".join(["not ok"] * 402)
     mocker.patch(
         "app.main.views.send.s3download",
@@ -811,9 +840,6 @@ def test_upload_csv_file_with_missing_columns_shows_error(
     client_request,
     mocker,
     mock_get_service_template_with_placeholders,
-    mock_s3_set_metadata,
-    mock_s3_get_metadata,
-    mock_s3_upload,
     mock_get_users_by_service,
     mock_get_service_statistics,
     mock_get_job_doesnt_exist,
@@ -823,6 +849,15 @@ def test_upload_csv_file_with_missing_columns_shows_error(
     file_contents,
     expected_error,
 ):
+
+    mocker.patch("app.main.views.send.set_metadata_on_csv_upload")
+
+    mocker.patch(
+        "app.main.views.send.get_csv_metadata",
+        return_value={"original_file_name": "example.csv"},
+    )
+    mocker.patch("app.main.views.send.s3upload", return_value=sample_uuid())
+
     mocker.patch("app.main.views.send.s3download", return_value=file_contents)
 
     page = client_request.post(
@@ -885,10 +920,13 @@ def test_upload_csv_size_too_big(
 def test_upload_valid_csv_redirects_to_check_page(
     client_request,
     mock_get_service_template_with_placeholders,
-    mock_s3_upload,
-    mock_s3_set_metadata,
     fake_uuid,
+    mocker,
 ):
+
+    mocker.patch("app.main.views.send.set_metadata_on_csv_upload")
+
+    mocker.patch("app.main.views.send.s3upload", return_value=sample_uuid())
     client_request.post(
         "main.send_messages",
         service_id=SERVICE_ONE_ID,
@@ -937,13 +975,16 @@ def test_upload_valid_csv_shows_preview_and_table(
     mock_get_service_statistics,
     mock_get_job_doesnt_exist,
     mock_get_jobs,
-    mock_s3_get_metadata,
-    mock_s3_set_metadata,
     fake_uuid,
     extra_args,
     expected_link_in_first_row,
     expected_message,
 ):
+
+    mocker.patch(
+        "app.main.views.send.get_csv_metadata",
+        return_value={"original_file_name": "example.csv"},
+    )
     with client_request.session_transaction() as session:
         session["file_uploads"] = {fake_uuid: {"template_id": fake_uuid}}
 
@@ -1031,8 +1072,12 @@ def test_show_all_columns_if_there_are_duplicate_recipient_columns(
     mock_get_job_doesnt_exist,
     mock_get_jobs,
     fake_uuid,
-    mock_s3_get_metadata,
 ):
+
+    mocker.patch(
+        "app.main.views.send.get_csv_metadata",
+        return_value={"original_file_name": "example.csv"},
+    )
     with client_request.session_transaction() as session:
         session["file_uploads"] = {fake_uuid: {"template_id": fake_uuid}}
 
@@ -1080,12 +1125,17 @@ def test_404_for_previewing_a_row_out_of_range(
     mock_get_service_statistics,
     mock_get_job_doesnt_exist,
     mock_get_jobs,
-    mock_s3_get_metadata,
-    mock_s3_set_metadata,
     fake_uuid,
     row_index,
     expected_status,
 ):
+
+    mocker.patch("app.main.views.send.set_metadata_on_csv_upload")
+
+    mocker.patch(
+        "app.main.views.send.get_csv_metadata",
+        return_value={"original_file_name": "example.csv"},
+    )
     with client_request.session_transaction() as session:
         session["file_uploads"] = {fake_uuid: {"template_id": fake_uuid}}
 
@@ -1523,7 +1573,6 @@ def test_send_one_off_redirects_to_start_if_you_skip_steps(
     client_request,
     service_one,
     fake_uuid,
-    mock_s3_upload,
     mock_get_users_by_service,
     mock_get_service_statistics,
     mock_has_no_jobs,
@@ -1559,7 +1608,6 @@ def test_send_one_off_redirects_to_start_if_index_out_of_bounds_and_some_placeho
     service_one,
     fake_uuid,
     mock_get_service_email_template,
-    mock_s3_download,
     mock_get_users_by_service,
     mock_get_service_statistics,
     mock_has_no_jobs,
@@ -1628,7 +1676,6 @@ def test_send_one_off_email_to_self_without_placeholders_redirects_to_check_page
     mocker,
     service_one,
     mock_get_service_email_template_without_placeholders,
-    mock_s3_upload,
     mock_get_users_by_service,
     mock_get_service_statistics,
     mock_has_no_jobs,
@@ -1835,13 +1882,20 @@ def test_upload_csvfile_with_valid_phone_shows_all_numbers(
     mock_get_live_service,
     mock_get_job_doesnt_exist,
     mock_get_jobs,
-    mock_s3_get_metadata,
-    mock_s3_set_metadata,
     service_one,
     fake_uuid,
-    mock_s3_upload,
     mocker,
 ):
+
+    mock_s3_set_metadata = mocker.patch(
+        "app.main.views.send.set_metadata_on_csv_upload"
+    )
+
+    mocker.patch(
+        "app.main.views.send.get_csv_metadata",
+        return_value={"original_file_name": "example.csv"},
+    )
+    mocker.patch("app.main.views.send.s3upload", return_value=sample_uuid())
     mocker.patch(
         "app.main.views.send.s3download",
         return_value="\n".join(
@@ -1895,9 +1949,6 @@ def test_upload_csvfile_with_international_validates(
     api_user_active,
     client_request,
     mock_get_service_template,
-    mock_s3_set_metadata,
-    mock_s3_get_metadata,
-    mock_s3_upload,
     mock_has_permissions,
     mock_get_users_by_service,
     mock_get_service_statistics,
@@ -1908,6 +1959,14 @@ def test_upload_csvfile_with_international_validates(
     should_allow_international,
     service_one,
 ):
+
+    mocker.patch("app.main.views.send.set_metadata_on_csv_upload")
+
+    mocker.patch(
+        "app.main.views.send.get_csv_metadata",
+        return_value={"original_file_name": "example.csv"},
+    )
+    mocker.patch("app.main.views.send.s3upload", return_value=sample_uuid())
     if international_sms_permission:
         service_one["permissions"] += ("sms", "international_sms")
     mocker.patch(
@@ -1942,15 +2001,20 @@ def test_test_message_can_only_be_sent_now(
     mocker,
     service_one,
     mock_get_service_template,
-    mock_s3_download,
     mock_get_users_by_service,
     mock_get_service_statistics,
     mock_get_job_doesnt_exist,
     mock_get_jobs,
-    mock_s3_get_metadata,
+    mock_s3_download,
-    mock_s3_set_metadata,
     fake_uuid,
 ):
+
+    mocker.patch("app.main.views.send.set_metadata_on_csv_upload")
+
+    mocker.patch(
+        "app.main.views.send.get_csv_metadata",
+        return_value={"original_file_name": "example.csv"},
+    )
     content = client_request.get(
         "main.check_messages",
         service_id=service_one["id"],
@@ -1972,8 +2036,12 @@ def test_preview_button_is_correctly_labelled(
     mock_get_job_doesnt_exist,
     mock_get_jobs,
     fake_uuid,
-    mock_s3_get_metadata,
 ):
+
+    mocker.patch(
+        "app.main.views.send.get_csv_metadata",
+        return_value={"original_file_name": "example.csv"},
+    )
     mocker.patch(
         "app.main.views.send.s3download",
         return_value="\n".join(["phone_number"] + (["2028670123"] * 1000)),
@@ -2058,7 +2126,6 @@ def test_route_permissions(
     mock_get_jobs,
     mock_get_notifications,
     mock_create_job,
-    mock_s3_upload,
     fake_uuid,
     route,
     response_code,
@@ -2092,8 +2159,8 @@ def test_route_permissions_send_check_notifications(
     response_code,
     method,
     mock_create_job,
-    mock_s3_upload,
 ):
+    mocker.patch("app.main.views.send.s3upload", return_value=sample_uuid())
     with client_request.session_transaction() as session:
         session["recipient"] = "2028675301"
         session["placeholders"] = {"name": "a"}
@@ -2173,8 +2240,6 @@ def test_check_messages_back_link(
     mock_get_job_doesnt_exist,
     mock_get_jobs,
     mock_s3_download,
-    mock_s3_get_metadata,
-    mock_s3_set_metadata,
     fake_uuid,
     mocker,
     template_type,
@@ -2182,6 +2247,13 @@ def test_check_messages_back_link(
     extra_args,
     expected_url,
 ):
+
+    mocker.patch("app.main.views.send.set_metadata_on_csv_upload")
+
+    mocker.patch(
+        "app.main.views.send.get_csv_metadata",
+        return_value={"original_file_name": "example.csv"},
+    )
     content = "Hi there ((name))" if has_placeholders else "Hi there"
     template_data = create_template(
         template_id=fake_uuid, template_type=template_type, content=content
@@ -2236,8 +2308,12 @@ def test_check_messages_shows_too_many_messages_errors(
     fake_uuid,
     num_requested,
     expected_msg,
-    mock_s3_get_metadata,
 ):
+
+    mocker.patch(
+        "app.main.views.send.get_csv_metadata",
+        return_value={"original_file_name": "example.csv"},
+    )
     # csv with 100 phone numbers
     mocker.patch(
         "app.main.views.send.s3download",
@@ -2281,7 +2357,6 @@ def test_check_messages_shows_too_many_messages_errors(
 
 def test_check_messages_shows_trial_mode_error(
     client_request,
-    mock_s3_get_metadata,
     mock_get_users_by_service,
     mock_get_service_template,
     mock_has_permissions,
@@ -2291,6 +2366,11 @@ def test_check_messages_shows_trial_mode_error(
     fake_uuid,
     mocker,
 ):
+
+    mocker.patch(
+        "app.main.views.send.get_csv_metadata",
+        return_value={"original_file_name": "example.csv"},
+    )
     mocker.patch(
         "app.main.views.send.s3download",
         return_value=("phone number,\n2028675209"),  # Not in team
@@ -2426,8 +2506,12 @@ def test_check_messages_column_error_doesnt_show_optional_columns(
     mock_get_service_statistics,
     mock_get_job_doesnt_exist,
     mock_get_jobs,
-    mock_s3_get_metadata,
 ):
+
+    mocker.patch(
+        "app.main.views.send.get_csv_metadata",
+        return_value={"original_file_name": "example.csv"},
+    )
     mocker.patch(
         "app.main.views.send.s3download",
         return_value="\n".join(
@@ -2468,10 +2552,17 @@ def test_check_messages_adds_sender_id_in_session_to_metadata(
     mock_get_service_statistics,
     mock_get_job_doesnt_exist,
     mock_get_jobs,
-    mock_s3_get_metadata,
-    mock_s3_set_metadata,
     fake_uuid,
 ):
+
+    mock_s3_set_metadata = mocker.patch(
+        "app.main.views.send.set_metadata_on_csv_upload"
+    )
+
+    mocker.patch(
+        "app.main.views.send.get_csv_metadata",
+        return_value={"original_file_name": "example.csv"},
+    )
     mocker.patch(
         "app.main.views.send.s3download", return_value=("phone number,\n2028675209")
     )
@@ -2508,11 +2599,15 @@ def test_check_messages_shows_over_max_row_error(
     mock_get_service_statistics,
     mock_get_job_doesnt_exist,
     mock_get_jobs,
-    mock_s3_get_metadata,
     mock_s3_download,
     fake_uuid,
     mocker,
 ):
+
+    mocker.patch(
+        "app.main.views.send.get_csv_metadata",
+        return_value={"original_file_name": "example.csv"},
+    )
     mock_recipients = mocker.patch("app.main.views.send.RecipientCSV").return_value
     mock_recipients.max_rows = 11111
     mock_recipients.__len__.return_value = 99999
@@ -2662,8 +2757,8 @@ def test_send_notification_submits_data(
     expected_personalisation,
     mocker,
     mock_create_job,
-    mock_s3_upload,
 ):
+    mocker.patch("app.main.views.send.s3upload", return_value=sample_uuid())
     with client_request.session_transaction() as session:
         session["recipient"] = recipient
         session["placeholders"] = placeholders
@@ -2690,8 +2785,8 @@ def test_send_notification_clears_session(
     mock_get_service_template,
     mocker,
     mock_create_job,
-    mock_s3_upload,
 ):
+    mocker.patch("app.main.views.send.s3upload", return_value=sample_uuid())
     with client_request.session_transaction() as session:
         session["recipient"] = "2028675301"
         session["placeholders"] = {"a": "b"}
@@ -2752,8 +2847,8 @@ def test_send_notification_redirects_to_view_page(
     extra_redirect_args,
     mocker,
     mock_create_job,
-    mock_s3_upload,
 ):
+    mocker.patch("app.main.views.send.s3upload", return_value=sample_uuid())
     with client_request.session_transaction() as session:
         session["recipient"] = "2028675301"
         session["placeholders"] = {"a": "b"}
@@ -2812,8 +2907,9 @@ def test_send_notification_shows_error_if_400(
     exception_msg,
     expected_h1,
     expected_err_details,
-    mock_s3_upload,
 ):
+    mocker.patch("app.main.views.send.s3upload", return_value=sample_uuid())
+
     class MockHTTPError(HTTPError):
         message = exception_msg
 
@@ -2851,8 +2947,9 @@ def test_send_notification_shows_email_error_in_trial_mode(
     mocker,
     mock_get_service_email_template,
     mock_create_job,
-    mock_s3_upload,
 ):
+    mocker.patch("app.main.views.send.s3upload", return_value=sample_uuid())
+
     class MockHTTPError(HTTPError):
         message = TRIAL_MODE_MSG
         status_code = 400
@@ -2897,9 +2994,6 @@ def test_reply_to_is_previewed_if_chosen(
     client_request,
     mocker,
     mock_get_service_email_template,
-    mock_s3_download,
-    mock_s3_get_metadata,
-    mock_s3_set_metadata,
     mock_get_users_by_service,
     mock_get_service_statistics,
     mock_get_job_doesnt_exist,
@@ -2910,6 +3004,8 @@ def test_reply_to_is_previewed_if_chosen(
     extra_args,
     reply_to_address,
 ):
+    mocker.patch("app.main.views.send.set_metadata_on_csv_upload")
+
     mocker.patch(
         "app.main.views.send.s3download",
         return_value="""
@@ -2952,9 +3048,6 @@ def test_sms_sender_is_previewed(
     client_request,
     mocker,
     mock_get_service_template,
-    mock_s3_download,
-    mock_s3_get_metadata,
-    mock_s3_set_metadata,
     mock_get_users_by_service,
     mock_get_service_statistics,
     mock_get_job_doesnt_exist,
@@ -2965,6 +3058,13 @@ def test_sms_sender_is_previewed(
     extra_args,
     sms_sender,
 ):
+
+    mocker.patch("app.main.views.send.set_metadata_on_csv_upload")
+
+    mocker.patch(
+        "app.main.views.send.get_csv_metadata",
+        return_value={"original_file_name": "example.csv"},
+    )
     mocker.patch(
         "app.main.views.send.s3download",
         return_value="""

tests/conftest.py

@@ -1843,14 +1843,6 @@ def mock_get_users_by_service(mocker):
     )
 
 
-@pytest.fixture()
-def mock_s3_upload(mocker):
-    def _upload(service_id, filedata):
-        return sample_uuid()
-
-    return mocker.patch("app.main.views.send.s3upload", side_effect=_upload)
-
-
 @pytest.fixture()
 def mock_s3_download(mocker):
     def _download(service_id, upload_id):
@@ -1863,21 +1855,6 @@ def mock_s3_download(mocker):
     return mocker.patch("app.main.views.send.s3download", side_effect=_download)
 
 
-@pytest.fixture()
-def mock_s3_get_metadata(mocker):
-    def _get_metadata(service_id, upload_id):
-        return {"original_file_name": "example.csv"}
-
-    return mocker.patch(
-        "app.main.views.send.get_csv_metadata", side_effect=_get_metadata
-    )
-
-
-@pytest.fixture()
-def mock_s3_set_metadata(mocker):
-    return mocker.patch("app.main.views.send.set_metadata_on_csv_upload")
-
-
 @pytest.fixture()
 def sample_invite(mocker, service_one):
     id_ = USER_ONE_ID

tests/notifications_utils/test_s3.py

@@ -2,6 +2,7 @@ from urllib.parse import parse_qs
 
 import botocore
 import pytest
+from moto import mock_aws
 
 from notifications_utils.s3 import S3ObjectNotFound, s3download, s3upload
 
@@ -12,6 +13,7 @@ location = "some_file_location"
 content_type = "binary/octet-stream"
 
 
+@mock_aws
 def test_s3upload_save_file_to_bucket(mocker):
     mocked = mocker.patch("notifications_utils.s3.Session.resource")
     s3upload(