code review feedback

tests/app/aws/test_s3.py

@@ -633,28 +633,6 @@ def test_set_and_get_job_cache_and_expiry(monkeypatch):
     assert "k" not in job_cache
 
 
-# def test_list_s3_objects_pagination_and_filtering(monkeypatch):
-#     fake_now = datetime.datetime(2025, 6, 29, tzinfo=timezone.utc)
-#     recent = fake_now - datetime.timedelta(days=1)
-#     old = fake_now - datetime.timedelta(days=10)
-#     client = MagicMock()
-#     client.list_objectes_v2.side_effect = [
-#         {
-#             "Contents": [{"Key": "new", "LastModified": recent}],
-#             "NextContinuationToken": "token",
-#         },
-#         {"Contents": [{"Key": "old", "LastModified": old}]},
-#     ]
-#     monkeypatch.setattr(s3, "get_s3_client", lambda: client)
-
-#     # set bucket name config
-#     s3.current_app = MagicMock()
-#     s3.current_app.config = {"CSV_UPLOAD_BUCKET": {"bucket": "b"}}
-#     s3.current_app.logger = MagicMock()
-#     keys = list(s3.list_s3_objects())
-#     assert "new" in keys and "old" not in keys
-
-
 def test_read_s3_file_populates_cache(monkeypatch):
     fake_csv = "Phone number,Name\r\n+1-555-1234,Alice"
     obj = MagicMock()

tests/app/clients/test_aws_cloudwatch.py

@@ -128,34 +128,6 @@ def fake_event():
     }
 
 
-# @patch("app.clients.cloudwatch.aws_cloudwatch.client")
-# @patch("app.clients.cloudwatch.aws_cloudwatch.cloud_config")
-# def test_init_app_with_localstack(mock_cloud_config, mock_client):
-#     os.environ["LOCALSTACK_ENDPOINT_URL"] = "http://localhost:4566"
-#     mock_cloud_config.sns_region = "us-north-1"
-#     mock_cloud_config.sns_access_key = "key"
-#     mock_cloud_config.sns_secret_key = "sekret"  # pragma: allowlist secret
-#     mock_app = MagicMock()
-#     client_instance = AwsCloudwatchClient()
-#     client_instance.init_app(mock_app)
-#     assert client_instance.is_localstack() is True
-#     del os.environ["LOCALSTACK_ENDPOINT_URL"]
-
-
-# @patch("app.clients.cloudwatch.aws_cloudwatch.client")
-# @patch("app.clients.cloudwatch.aws_cloudwatch.cloud_config")
-# def test_init_app_without_localstack(mock_cloud_config, mock_client):
-#     if "LOCALSTACK_ENDPOINT_URL" in os.environ:
-#         del os.environ["LOCALSTACK_ENDPOINT_URL"]
-#     mock_cloud_config.sns_region = "us-north-1"
-#     mock_cloud_config.sns_access_key = "key"
-#     mock_cloud_config.sns_secret_key = "sekret"  # pragma: allowlist secret
-#     mock_app = MagicMock()
-#     client_instance = AwsCloudwatchClient()
-#     client_instance.init_app(mock_app)
-#     assert client_instance.is_localstack() is False
-
-
 @patch("app.clients.cloudwatch.aws_cloudwatch.current_app")
 def test_warn_if_dev_is_opted_out(current_app_mock):
     # os.environ["NOTIFIY_ENVIRONMENT"] = "development"

tests/app/service/test_rest.py

@@ -2914,25 +2914,6 @@ def test_get_all_notifications_for_service_includes_template_redacted(
     assert resp["notifications"][1]["template"]["redact_personalisation"] is True
 
 
-# TODO: check whether all hidden templates are also precompiled letters
-# def test_get_all_notifications_for_service_includes_template_hidden(admin_request, sample_service):
-#     letter_template = create_template(sample_service, template_type=TemplateType.LETTER)
-
-#     with freeze_time('2000-01-01'):
-#         letter_noti = create_notification(letter_template)
-
-#     resp = admin_request.get(
-#         'service.get_all_notifications_for_service',
-#         service_id=sample_service.id
-#     )
-
-#     assert resp['notifications'][0]['id'] == str(precompiled_noti.id)
-#     assert resp['notifications'][0]['template']['is_precompiled_letter'] is True
-
-#     assert resp['notifications'][1]['id'] == str(letter_noti.id)
-#     assert resp['notifications'][1]['template']['is_precompiled_letter'] is False
-
-
 @pytest.mark.skip(
     reason="We can't search on recipient if recipient is not kept in the db"
 )