TrueCloudLab/frostfs-testcases
25
0
Fork 21
Code Issues 2 Pull requests 2 Releases Activity Actions

Add check for Error while deleting objects #32

Merged
JuliaKovshova merged 1 commit from EliChin/frostfs-testcases:feature/delete_objects into master 2023-05-02 12:22:09 +00:00
Conversation 0 Commits 1 Files changed 1 +3
1 changed files with 3 additions and 0 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
pytest_tests/steps/s3_gate_object.py
View file

@ -168,6 +168,9 @@ def delete_objects_s3(s3_client, bucket: str, object_keys: list):
response = s3_client.delete_objects(Bucket=bucket, Delete=_make_objs_dict(object_keys)) response = s3_client.delete_objects(Bucket=bucket, Delete=_make_objs_dict(object_keys))
log_command_execution("S3 Delete objects result", response) log_command_execution("S3 Delete objects result", response)
abereziny marked this conversation as resolved Outdated
abereziny commented 2023-04-18 16:05:16 +00:00
Outdated
Review
Copy link

What about boto3 client?

Also, I think it's bad practice to check errors here since we may have negative cases like this one:
https://git.frostfs.info/TrueCloudLab/frostfs-testcases/src/branch/master/pytest_tests/testsuites/services/s3_gate/test_s3_object.py#L531

This test will be broken after such changes.

What about boto3 client? Also, I think it's bad practice to check errors here since we may have negative cases like this one: https://git.frostfs.info/TrueCloudLab/frostfs-testcases/src/branch/master/pytest_tests/testsuites/services/s3_gate/test_s3_object.py#L531 This test will be broken after such changes.
EliChin commented 2023-04-20 15:22:50 +00:00
Outdated
Review
Copy link

I think it's a bad practice here to add this check in aaaall separate testcases, so I tried to check it more precisely here so that no problem will be with negative tests.

I think it's a bad practice here to add this check in aaaall separate testcases, so I tried to check it more precisely here so that no problem will be with negative tests.
abereziny commented 2023-04-21 08:46:30 +00:00
Outdated
Review
Copy link

Is the OBJECT_NOT_FOUND the one and only error which we want to check?

This can be simplified to just:

assert "Errors" not in response, f"The following objects have not been <...>"

The must have here is to preserve original error messages in a some way in our exception.

Is the OBJECT_NOT_FOUND the one and only error which we want to check? This can be simplified to just: ``` assert "Errors" not in response, f"The following objects have not been <...>" ``` The must have here is to preserve original error messages in a some way in our exception.
sleep(S3_SYNC_WAIT_TIME) sleep(S3_SYNC_WAIT_TIME)
assert (
abereziny marked this conversation as resolved Outdated
abereziny commented 2023-04-18 16:09:49 +00:00
Outdated
Review
Copy link

It would be nice to include error message in logger output

It would be nice to include error message in logger output
"Errors" not in response
abereziny marked this conversation as resolved Outdated
JuliaKovshova commented 2023-04-18 15:03:35 +00:00
Outdated
Review
Copy link

please raise specific error

please raise specific error
), f'The following objects have not been deleted: {[err_info["Key"] for err_info in response["Errors"]]}.\nError Message: {response["Errors"]["Message"]}'
return response return response
except ClientError as err: except ClientError as err: