Skip to content
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

Convert go_md5 function to modern API #392

Merged
merged 2 commits into from
Dec 7, 2019

Conversation

alexjfisher
Copy link
Member

I'm not actually sure if anybody uses this or whether this works with
recent GoCD servers. It's pretty easy to port to the new puppet
function API though.

# @summary
# Retrieves and returns specific file's md5 from GoCD server md5 checksum file
# @api private
# @see http://www.thoughtworks.com/products/docs/go/12.4/help/Artifacts_API.html
Copy link
Member Author

Choose a reason for hiding this comment

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

I took this 'broken' link directly from the old function. I was going to update with eg. https://api.gocd.org/current/#get-artifact-file, but that says Available since v14.3.0. This is why I suspect this function (and archive::go) might be outdated and of limited value until a GoCD user comes along and fixes them up.

spec/defines/go_spec.rb Outdated Show resolved Hide resolved
I'm not actually sure if anybody uses this or whether this works with
recent GoCD servers.  It's pretty easy to port to the new puppet
function API though.
uri = URI(url)
PuppetX::Bodeco::Util.stubs(:content).with(uri, username: 'user', password: 'pass').returns(example_md5)
is_expected.to run.with_params('user', 'pass', 'filea', url).and_return('283158c7da8c0ada74502794fa8745eb')
end
Copy link
Member

Choose a reason for hiding this comment

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

Consider adding a test that demonstrates the error state and hits the raise in the function.

Copy link
Member Author

Choose a reason for hiding this comment

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

I was aiming to limit my changes to just converting the function, but actually, I think you're right that the error handling code is broken. Fixing that up should be simple enough.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixes in 9c5d632

@ghoneycutt
Copy link
Member

Suggest adding an acceptance test that pulls a static file, such as from VP's website, and shows that the md5 is as expected.

@alexjfisher
Copy link
Member Author

Suggest adding an acceptance test that pulls a static file, such as from VP's website, and shows that the md5 is as expected.

At the moment this module hasn't got any acceptance tests. This function probably isn't where I'd start. PRs welcome of course! :)

@alexjfisher alexjfisher merged commit 3877d12 into voxpupuli:master Dec 7, 2019
@alexjfisher alexjfisher deleted the convert_go_md5_function branch December 7, 2019 09:49
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.

3 participants