From 90e8bb131342150f8a8f98c28c0ca8061fff642a Mon Sep 17 00:00:00 2001 From: Bert Blommers Date: Thu, 9 Nov 2023 22:17:46 -0100 Subject: [PATCH] Techdebt: Improve/fix exception handling in tests (#7011) --- moto/ecr/models.py | 4 ++-- moto/ecs/models.py | 5 +++- moto/glacier/models.py | 3 +++ moto/sqs/responses.py | 1 + tests/test_dynamodb/test_dynamodb.py | 4 ++-- tests/test_ec2/test_flow_logs.py | 1 + tests/test_ecr/test_ecr_boto3.py | 7 +++++- tests/test_ecs/test_ecs_boto3.py | 8 +++---- tests/test_glacier/test_glacier_archives.py | 7 ++++-- tests/test_glacier/test_glacier_vaults.py | 5 +++- tests/test_opsworks/test_apps.py | 14 ++++++----- tests/test_opsworks/test_instances.py | 17 +++++++------- tests/test_opsworks/test_layers.py | 16 +++++++------ tests/test_s3/test_s3.py | 4 ++-- tests/test_s3/test_s3_multipart.py | 4 ++-- tests/test_sqs/test_sqs.py | 26 +++++++++------------ 16 files changed, 73 insertions(+), 53 deletions(-) diff --git a/moto/ecr/models.py b/moto/ecr/models.py index 90ed7b28d..3de2ceb59 100644 --- a/moto/ecr/models.py +++ b/moto/ecr/models.py @@ -578,8 +578,8 @@ class ECRBackend(BaseBackend): parsed_image_manifest["imageManifest"] = image_manifest_mediatype else: if "mediaType" not in parsed_image_manifest: - raise Exception( - "image manifest mediatype not provided in manifest or parameter" + raise InvalidParameterException( + message="image manifest mediatype not provided in manifest or parameter" ) else: image_manifest_mediatype = parsed_image_manifest["mediaType"] diff --git a/moto/ecs/models.py b/moto/ecs/models.py index 86da09461..f632f7b4b 100644 --- a/moto/ecs/models.py +++ b/moto/ecs/models.py @@ -1836,7 +1836,10 @@ class EC2ContainerServiceBackend(BaseBackend): if container_instance is None: raise Exception("{0} is not a container id in the cluster") if not force and container_instance.running_tasks_count > 0: - raise Exception("Found running tasks on the instance.") + raise JsonRESTError( + error_type="InvalidParameter", + message="Found running tasks on the instance.", + ) # Currently assume that people might want to do something based around deregistered instances # with tasks left running on them - but nothing if no tasks were running already elif force and container_instance.running_tasks_count > 0: diff --git a/moto/glacier/models.py b/moto/glacier/models.py index b9dfe696e..bbf5578dc 100644 --- a/moto/glacier/models.py +++ b/moto/glacier/models.py @@ -3,6 +3,7 @@ import datetime from typing import Any, Dict, List, Optional, Union from moto.core import BaseBackend, BackendDict, BaseModel +from moto.core.exceptions import JsonRESTError from moto.utilities.utils import md5_hash from .utils import get_job_id @@ -189,6 +190,8 @@ class GlacierBackend(BaseBackend): self.vaults: Dict[str, Vault] = {} def get_vault(self, vault_name: str) -> Vault: + if vault_name not in self.vaults: + raise JsonRESTError(error_type="VaultNotFound", message="Vault not found") return self.vaults[vault_name] def create_vault(self, vault_name: str) -> None: diff --git a/moto/sqs/responses.py b/moto/sqs/responses.py index 44168094e..9c5207a12 100644 --- a/moto/sqs/responses.py +++ b/moto/sqs/responses.py @@ -730,6 +730,7 @@ class SQSResponse(BaseResponse): template = self.response_template(UNTAG_QUEUE_RESPONSE) return template.render() + @jsonify_error def list_queue_tags(self) -> str: queue_name = self._get_queue_name() diff --git a/tests/test_dynamodb/test_dynamodb.py b/tests/test_dynamodb/test_dynamodb.py index 679ae20db..8c8b186d2 100644 --- a/tests/test_dynamodb/test_dynamodb.py +++ b/tests/test_dynamodb/test_dynamodb.py @@ -1846,7 +1846,7 @@ def test_describe_continuous_backups_errors(): client = boto3.client("dynamodb", region_name="us-east-1") # when - with pytest.raises(Exception) as e: + with pytest.raises(ClientError) as e: client.describe_continuous_backups(TableName="not-existing-table") # then @@ -1930,7 +1930,7 @@ def test_update_continuous_backups_errors(): client = boto3.client("dynamodb", region_name="us-east-1") # when - with pytest.raises(Exception) as e: + with pytest.raises(ClientError) as e: client.update_continuous_backups( TableName="not-existing-table", PointInTimeRecoverySpecification={"PointInTimeRecoveryEnabled": True}, diff --git a/tests/test_ec2/test_flow_logs.py b/tests/test_ec2/test_flow_logs.py index be4514923..df7377371 100644 --- a/tests/test_ec2/test_flow_logs.py +++ b/tests/test_ec2/test_flow_logs.py @@ -663,6 +663,7 @@ def test_describe_flow_logs_filtering(): )["FlowLogs"] assert len(fl_by_tag_key) == 0 + # NotYetImplemented with pytest.raises(Exception): client.describe_flow_logs(Filters=[{"Name": "unknown", "Values": ["foobar"]}]) diff --git a/tests/test_ecr/test_ecr_boto3.py b/tests/test_ecr/test_ecr_boto3.py index ac5464568..6499ca824 100644 --- a/tests/test_ecr/test_ecr_boto3.py +++ b/tests/test_ecr/test_ecr_boto3.py @@ -362,12 +362,17 @@ def test_put_image_without_mediatype(): image_manifest = _create_image_manifest() _ = image_manifest.pop("mediaType") - with pytest.raises(Exception): + with pytest.raises(ClientError) as exc: client.put_image( repositoryName="test_repository", imageManifest=json.dumps(image_manifest), imageTag="latest", ) + err = exc.value.response["Error"] + assert ( + err["Message"] + == "image manifest mediatype not provided in manifest or parameter" + ) @mock_ecr diff --git a/tests/test_ecs/test_ecs_boto3.py b/tests/test_ecs/test_ecs_boto3.py index 250826183..55e65f7af 100644 --- a/tests/test_ecs/test_ecs_boto3.py +++ b/tests/test_ecs/test_ecs_boto3.py @@ -1484,7 +1484,7 @@ def test_deregister_container_instance(): cluster=test_cluster_name, instanceIdentityDocument=instance_id_document ) container_instance_id = response["containerInstance"]["containerInstanceArn"] - response = ecs_client.deregister_container_instance( + ecs_client.deregister_container_instance( cluster=test_cluster_name, containerInstance=container_instance_id ) container_instances_response = ecs_client.list_container_instances( @@ -1520,12 +1520,12 @@ def test_deregister_container_instance(): containerInstances=[container_instance_id], startedBy="moto", ) - with pytest.raises(Exception): + with pytest.raises(ClientError) as exc: ecs_client.deregister_container_instance( cluster=test_cluster_name, containerInstance=container_instance_id ) - # TODO: Return correct error format - # should.contain("Found running tasks on the instance") + err = exc.value.response["Error"] + assert err["Message"] == "Found running tasks on the instance." container_instances_response = ecs_client.list_container_instances( cluster=test_cluster_name diff --git a/tests/test_glacier/test_glacier_archives.py b/tests/test_glacier/test_glacier_archives.py index b31bcb656..e3180cddb 100644 --- a/tests/test_glacier/test_glacier_archives.py +++ b/tests/test_glacier/test_glacier_archives.py @@ -3,6 +3,7 @@ import os import pytest from moto import mock_glacier +from botocore.exceptions import ClientError @mock_glacier @@ -48,8 +49,8 @@ def test_delete_archive(): delete = client.delete_archive(vaultName="asdf", archiveId=archive["archiveId"]) assert delete["ResponseMetadata"]["HTTPStatusCode"] == 204 - with pytest.raises(Exception): - # Not ideal - but this will throw an error if the archvie does not exist + with pytest.raises(ClientError) as exc: + # Not ideal - but this will throw an error if the archive does not exist # Which is a good indication that the deletion went through client.initiate_job( vaultName="myname", @@ -58,3 +59,5 @@ def test_delete_archive(): "Type": "archive-retrieval", }, ) + err = exc.value.response["Error"] + assert err["Code"] == "VaultNotFound" diff --git a/tests/test_glacier/test_glacier_vaults.py b/tests/test_glacier/test_glacier_vaults.py index 03847d297..dbee055f3 100644 --- a/tests/test_glacier/test_glacier_vaults.py +++ b/tests/test_glacier/test_glacier_vaults.py @@ -1,6 +1,7 @@ import boto3 import pytest +from botocore.exceptions import ClientError from moto import mock_glacier from moto.core import DEFAULT_ACCOUNT_ID as ACCOUNT_ID from uuid import uuid4 @@ -31,8 +32,10 @@ def test_delete_vault_boto3(): client.delete_vault(vaultName="myvault") - with pytest.raises(Exception): + with pytest.raises(ClientError) as exc: client.describe_vault(vaultName="myvault") + err = exc.value.response["Error"] + assert err["Code"] == "VaultNotFound" @mock_glacier diff --git a/tests/test_opsworks/test_apps.py b/tests/test_opsworks/test_apps.py index 454f88c36..6bbe53f6e 100644 --- a/tests/test_opsworks/test_apps.py +++ b/tests/test_opsworks/test_apps.py @@ -1,7 +1,9 @@ import boto3 -from freezegun import freeze_time import pytest +from botocore.exceptions import ClientError +from freezegun import freeze_time + from moto import mock_opsworks @@ -32,12 +34,12 @@ def test_create_app_response(): assert "AppId" in response # ClientError - with pytest.raises(Exception) as exc: + with pytest.raises(ClientError) as exc: client.create_app(StackId=stack_id, Type="other", Name="TestApp") assert r'already an app named "TestApp"' in exc.value.response["Error"]["Message"] # ClientError - with pytest.raises(Exception) as exc: + with pytest.raises(ClientError) as exc: client.create_app(StackId="nothere", Type="other", Name="TestApp") assert exc.value.response["Error"]["Message"] == "nothere" @@ -61,20 +63,20 @@ def test_describe_apps(): assert rv1["Apps"][0]["Name"] == "TestApp" # ClientError - with pytest.raises(Exception) as exc: + with pytest.raises(ClientError) as exc: client.describe_apps(StackId=stack_id, AppIds=[app_id]) assert exc.value.response["Error"]["Message"] == ( "Please provide one or more app IDs or a stack ID" ) # ClientError - with pytest.raises(Exception) as exc: + with pytest.raises(ClientError) as exc: client.describe_apps(StackId="nothere") assert exc.value.response["Error"]["Message"] == ( "Unable to find stack with ID nothere" ) # ClientError - with pytest.raises(Exception) as exc: + with pytest.raises(ClientError) as exc: client.describe_apps(AppIds=["nothere"]) assert exc.value.response["Error"]["Message"] == "nothere" diff --git a/tests/test_opsworks/test_instances.py b/tests/test_opsworks/test_instances.py index 8b9fca89d..a0e17543c 100644 --- a/tests/test_opsworks/test_instances.py +++ b/tests/test_opsworks/test_instances.py @@ -1,6 +1,7 @@ import boto3 import pytest +from botocore.exceptions import ClientError from moto import mock_opsworks from moto import mock_ec2 from tests import EXAMPLE_AMI_ID @@ -43,7 +44,7 @@ def test_create_instance(): assert "InstanceId" in response - with pytest.raises(Exception) as exc: + with pytest.raises(ClientError) as exc: client.create_instance( StackId="nothere", LayerIds=[layer_id], InstanceType="t2.micro" ) @@ -51,14 +52,14 @@ def test_create_instance(): "Unable to find stack with ID nothere" ) - with pytest.raises(Exception) as exc: + with pytest.raises(ClientError) as exc: client.create_instance( StackId=stack_id, LayerIds=["nothere"], InstanceType="t2.micro" ) assert exc.value.response["Error"]["Message"] == "nothere" # ClientError - with pytest.raises(Exception) as exc: + with pytest.raises(ClientError) as exc: client.create_instance( StackId=stack_id, LayerIds=[second_layer_id], InstanceType="t2.micro" ) @@ -67,7 +68,7 @@ def test_create_instance(): ) # ClientError - with pytest.raises(Exception) as exc: + with pytest.raises(ClientError) as exc: client.start_instance(InstanceId="nothere") assert exc.value.response["Error"]["Message"] == ( "Unable to find instance with ID nothere" @@ -160,22 +161,22 @@ def test_describe_instances(): assert S2L1_i1 not in [i["InstanceId"] for i in response] # ClientError - with pytest.raises(Exception) as exc: + with pytest.raises(ClientError) as exc: client.describe_instances(StackId=S1, LayerId=S1L1) assert "Please provide either one or more" in exc.value.response["Error"]["Message"] # ClientError - with pytest.raises(Exception) as exc: + with pytest.raises(ClientError) as exc: client.describe_instances(StackId="nothere") assert "nothere" in exc.value.response["Error"]["Message"] # ClientError - with pytest.raises(Exception) as exc: + with pytest.raises(ClientError) as exc: client.describe_instances(LayerId="nothere") assert "nothere" in exc.value.response["Error"]["Message"] # ClientError - with pytest.raises(Exception) as exc: + with pytest.raises(ClientError) as exc: client.describe_instances(InstanceIds=["nothere"]) assert exc.value.response["Error"]["Message"] == "nothere" diff --git a/tests/test_opsworks/test_layers.py b/tests/test_opsworks/test_layers.py index dfe2c90e7..198779102 100644 --- a/tests/test_opsworks/test_layers.py +++ b/tests/test_opsworks/test_layers.py @@ -1,7 +1,9 @@ import boto3 -from freezegun import freeze_time import pytest +from botocore.exceptions import ClientError +from freezegun import freeze_time + from moto import mock_opsworks @@ -42,7 +44,7 @@ def test_create_layer_response(): assert "LayerId" in response # ClientError - with pytest.raises(Exception) as exc: + with pytest.raises(ClientError) as exc: client.create_layer( StackId=stack_id, Type="custom", Name="TestLayer", Shortname="_" ) @@ -51,7 +53,7 @@ def test_create_layer_response(): ) # ClientError - with pytest.raises(Exception) as exc: + with pytest.raises(ClientError) as exc: client.create_layer( StackId=stack_id, Type="custom", Name="_", Shortname="TestLayerShortName" ) @@ -60,7 +62,7 @@ def test_create_layer_response(): ) in exc.value.response["Error"]["Message"] # ClientError - with pytest.raises(Exception) as exc: + with pytest.raises(ClientError) as exc: client.create_layer( StackId="nothere", Type="custom", Name="TestLayer", Shortname="_" ) @@ -91,20 +93,20 @@ def test_describe_layers(): assert rv1["Layers"][0]["Name"] == "TestLayer" # ClientError - with pytest.raises(Exception) as exc: + with pytest.raises(ClientError) as exc: client.describe_layers(StackId=stack_id, LayerIds=[layer_id]) assert exc.value.response["Error"]["Message"] == ( "Please provide one or more layer IDs or a stack ID" ) # ClientError - with pytest.raises(Exception) as exc: + with pytest.raises(ClientError) as exc: client.describe_layers(StackId="nothere") assert exc.value.response["Error"]["Message"] == ( "Unable to find stack with ID nothere" ) # ClientError - with pytest.raises(Exception) as exc: + with pytest.raises(ClientError) as exc: client.describe_layers(LayerIds=["nothere"]) assert exc.value.response["Error"]["Message"] == "nothere" diff --git a/tests/test_s3/test_s3.py b/tests/test_s3/test_s3.py index 30ed9760a..748673309 100644 --- a/tests/test_s3/test_s3.py +++ b/tests/test_s3/test_s3.py @@ -599,10 +599,10 @@ def test_cannot_restore_standard_class_object(): bucket.create() key = bucket.put_object(Key="the-key", Body=b"somedata") - with pytest.raises(Exception) as err: + with pytest.raises(ClientError) as exc: key.restore_object(RestoreRequest={"Days": 1}) - err = err.value.response["Error"] + err = exc.value.response["Error"] assert err["Code"] == "InvalidObjectState" assert err["StorageClass"] == "STANDARD" assert err["Message"] == ( diff --git a/tests/test_s3/test_s3_multipart.py b/tests/test_s3/test_s3_multipart.py index b3925dff8..2e1c2af47 100644 --- a/tests/test_s3/test_s3_multipart.py +++ b/tests/test_s3/test_s3_multipart.py @@ -582,11 +582,11 @@ def test_s3_abort_multipart_data_with_invalid_upload_and_key(): client.create_bucket(Bucket="blah") - with pytest.raises(Exception) as err: + with pytest.raises(ClientError) as exc: client.abort_multipart_upload( Bucket="blah", Key="foobar", UploadId="dummy_upload_id" ) - err = err.value.response["Error"] + err = exc.value.response["Error"] assert err["Code"] == "NoSuchUpload" assert err["Message"] == ( "The specified upload does not exist. The upload ID may be invalid, " diff --git a/tests/test_sqs/test_sqs.py b/tests/test_sqs/test_sqs.py index dc461b960..dd6eb3805 100644 --- a/tests/test_sqs/test_sqs.py +++ b/tests/test_sqs/test_sqs.py @@ -2831,13 +2831,11 @@ def test_send_message_to_fifo_without_message_group_id(): Attributes={"FifoQueue": "true", "ContentBasedDeduplication": "true"}, ) - with pytest.raises(Exception) as e: + with pytest.raises(ClientError) as exc: queue.send_message(MessageBody="message-1") - ex = e.value - assert ex.response["Error"]["Code"] == "MissingParameter" - assert ex.response["Error"]["Message"] == ( - "The request must contain the parameter MessageGroupId." - ) + err = exc.value.response["Error"] + assert err["Code"] == "MissingParameter" + assert err["Message"] == "The request must contain the parameter MessageGroupId." @mock_sqs @@ -2848,13 +2846,11 @@ def test_send_messages_to_fifo_without_message_group_id(): Attributes={"FifoQueue": "true", "ContentBasedDeduplication": "true"}, ) - with pytest.raises(Exception) as e: + with pytest.raises(ClientError) as exc: queue.send_messages(Entries=[{"Id": "id_1", "MessageBody": "body_1"}]) - ex = e.value - assert ex.response["Error"]["Code"] == "MissingParameter" - assert ex.response["Error"]["Message"] == ( - "The request must contain the parameter MessageGroupId." - ) + err = exc.value.response["Error"] + assert err["Code"] == "MissingParameter" + assert err["Message"] == "The request must contain the parameter MessageGroupId." @mock_sqs @@ -2862,10 +2858,10 @@ def test_maximum_message_size_attribute_default(): sqs = boto3.resource("sqs", region_name="eu-west-3") queue = sqs.create_queue(QueueName=str(uuid4())) assert int(queue.attributes["MaximumMessageSize"]) == MAXIMUM_MESSAGE_LENGTH - with pytest.raises(Exception) as e: + with pytest.raises(ClientError) as exc: queue.send_message(MessageBody="a" * (MAXIMUM_MESSAGE_LENGTH + 1)) - ex = e.value - assert ex.response["Error"]["Code"] == "InvalidParameterValue" + err = exc.value.response["Error"] + assert err["Code"] == "InvalidParameterValue" @mock_sqs