Add live test case for blob copy scenario#2788
Add live test case for blob copy scenario#2788troydai merged 2 commits intoAzure:masterfrom troydai:storage.test
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2788 +/- ##
=========================================
+ Coverage 62.89% 62.9% +<.01%
=========================================
Files 480 480
Lines 25994 26000 +6
Branches 3942 3943 +1
=========================================
+ Hits 16350 16354 +4
- Misses 8617 8622 +5
+ Partials 1027 1024 -3
Continue to review full report at Codecov.
|
|
/cc @tjprescott |
| container = self.create_container(account_info) | ||
|
|
||
| self.cmd('storage container create -n {}'.format(container)) | ||
| self.storage_cmd('storage blob exists -n {} -c {}', account_info, blob_name, container) |
There was a problem hiding this comment.
Is there a reason you removed the check from here? If not to check that the output is false, you could just remove this.
There was a problem hiding this comment.
Missed. I'll add it back.
| JMESPathCheck('properties.contentSettings.contentType', 'application/test-content'), | ||
| JMESPathCheck('properties.contentLength', file_size_kb * 1024)]) | ||
| self.storage_cmd('storage blob show -n {} -c {}', account_info, blob_name, container) \ | ||
| .assert_with_checks([JMESPathCheck('properties.contentSettings.contentType', 'application/test-content'), |
There was a problem hiding this comment.
What is .storage_cmd and why do we have to call .assert_with_checks instead of just using the checks kwarg like you do for regular .cmd?
There was a problem hiding this comment.
Unfortunately, following form of signature is supported in Python 3 only
def foo(para_1, para_2, *args, checks=None):
pass
The parameters after *args is what I envisioned to have. However it is rejected in Python 2. The current form will more fit into a fluent API design.
There was a problem hiding this comment.
I would request we either make self.cmd support .assert_with_checks as well or that we make self.storage_cmd work with the checks kwarg... or remove it altogether. I don't see what it does that isn't accomplished by simple text replacement of the connection parameters.
My concern is having fragmented methods of accomplishing the same thing making it difficult to recommend to third parties the right way to do things.
There was a problem hiding this comment.
assert_with_checks is supported by self.cmd. It is added to the ExecuteResult.
| self.cmd('storage blob show -n {} -c {}'.format(blob_name, container), | ||
| checks=JMESPathCheck('name', blob_name)) # TODO: more checks | ||
| self.storage_cmd('storage blob show -n {} -c {}', account_info, blob_name, container) \ | ||
| .assert_with_checks(JMESPathCheck('name', blob_name)) # TODO: more checks |
There was a problem hiding this comment.
Would this be a good time to do the "TODO: more checks"? Or if you don't think it's necessary, remove the comment.
There was a problem hiding this comment.
I will remove the TODO
|
Ping @tjprescott |
tjprescott
left a comment
There was a problem hiding this comment.
We should also update the test authoring help to reference .assert_with_checks.
There isn't product change included in this pull request.
This checklist is used to make sure that common guidelines for a pull request are followed.
General Guidelines
Command Guidelines
(see Authoring Command Modules)