Skip to content

Mocks for put_object and public_url #375

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 1, 2018

Conversation

timuralp
Copy link
Member

Fixing up a prior PR (#341) that added these mocks.

break if chunk.empty?
dgst.update chunk
end
elsif data.is_a?(String)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer Object#kind_of? over Object#is_a?.

dgst = Digest::MD5.new
if block_given?
while true do
chunk = block.call

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use yield instead of block.call.

def put_object(container, object, data, options = {}, &block)
dgst = Digest::MD5.new
if block_given?
while true do

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use Kernel#loop for infinite loops.
Literal true appeared as a condition.
Do not use do with multi-line while.

class Mock
require 'digest'

def put_object(container, object, data, options = {}, &block)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused method argument - container. If it's necessary, use _ or _container as an argument name to indicate that it won't be used.
Unused method argument - object. If it's necessary, use _ or _object as an argument name to indicate that it won't be used.
Unused method argument - options. If it's necessary, use _ or _options as an argument name to indicate that it won't be used.

@timuralp timuralp force-pushed the improvement/mock-put-public_url branch from fff44e3 to e1e0306 Compare February 28, 2018 23:25
class Mock
require 'digest'

def put_object(_container, _object, data, _options = {}, &block)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused method argument - block. If it's necessary, use _ or _block as an argument name to indicate that it won't be used.

Now that put_object mock is implemented, we can enable some of the tests
that rely on it.
@timuralp timuralp force-pushed the improvement/mock-put-public_url branch from e1e0306 to 40e150a Compare February 28, 2018 23:27
@timuralp
Copy link
Member Author

I squashed two related commits and added a small fix-up for the public_url mock. Also, enabled tests for put object after making sure it will return a valid etag for each request.

@gildub
Copy link
Collaborator

gildub commented Mar 1, 2018

@timuralp, thanks for fixing it.

@giglemad, sorry the discussion about moving built-in Mock was putting a hold on #341, which shouldn't be blocker.

Anyway, this looks good to me.

@gildub gildub merged commit 55abb2c into fog:master Mar 1, 2018
@hugolepetit
Copy link
Contributor

I could follow the discussion from a distance and was glad to see the discussion and communication expanded to achieve the best level of open source possible. Thanks for this and keep up the good work!

@timuralp timuralp deleted the improvement/mock-put-public_url branch March 2, 2018 00:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants