php: Add tests for more S3 APIs #78
Conversation
krisis
commented
Jul 5, 2017
krisis
commented
- HeadObject
- ListObjects
- ListObjectsV2
- ListMultipartUploads
- ListParts
- UploadPartCopy
| } | ||
|
|
||
| /** | ||
| * testListMultipartUploads tests ListMultipartUploads, ListParts and |
There was a problem hiding this comment.
Avoided this test on purpose, This won't work on GCS and we also planning the same style for FS backend as well. We can avoid ListMultipartUploads for now.
There was a problem hiding this comment.
@harshavardhana ListMultipartUploads and ListParts will work on GCS gateway and FS mode (once the backend format changes land in master). They will return an empty response, which doesn't result in failure of this test. Also, erasure code backend will return the list of ongoing multipart uploads as expected and needs to be tested. Since this test is only invoking these list APIs without making any assumption about the response it is OK to have these.
There was a problem hiding this comment.
If you are only looking for 200 OK please add a comment and update why you don't validate the results. Also does the paginator return exception upon non 200 OK?
There was a problem hiding this comment.
Added a comment explaining why we are not verifying the contents of ListMultipartUploads response.
Also does the paginator return exception upon non 200 OK?
yes
There was a problem hiding this comment.
Also does the paginator return exception upon non 200 OK?
I can't find a document that explicitly says so about paginators but https://docs.aws.amazon.com/aws-sdk-php/v3/guide/getting-started/basic-usage.html outlines how the SDK treats successful API operation Vs unsuccessful ones.
My understanding is that paginators would throw an exception when they see one.
| } | ||
|
|
||
| $paginator = $s3Client->getPaginator('ListObjects', ['Bucket' => $bucket]); | ||
| foreach ($paginator->search('Contents[].Key') as $key) { |
There was a problem hiding this comment.
Can you add negative test cases for listObjects as well? or are they already added?
run/core/aws-sdk-php/quick-tests.php
Outdated
| } | ||
|
|
||
| // Run failure tests | ||
| $params = [ |
There was a problem hiding this comment.
Shouldn't you validate headObject() for the resultant metadata returned as well?
|
@krisis not exactly related to this PR but I wanted to add another point here. Could you pls add logging to Currently this is the only output in aws-sdk-php logs |
|
@nitisht added logging support. |
- HeadObject - ListObjects - ListObjectsV2 - ListMultipartUploads - ListParts - UploadPartCopy
|
@harshavardhana are your comments addressed here? |
