-
Notifications
You must be signed in to change notification settings - Fork 189
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
Add support for box_download_ca_cert #274
Conversation
typo and gnarled unless logic
…r234 Signed-off-by: Seth Thomas <[email protected]>
it "sets :box_download_ca_cert to a custom value" do | ||
config[:box_download_ca_cert] = "cacert.pem" | ||
|
||
expect(driver[:box_download_ca_cert]).to eq("cacert.pem") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confused about this test, I would have expected '/kroot/cacert.pem'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably because the path does not get resolved with kitchen_root
until the render_template
method.
it "sets :box_download_ca_cert to a custom value" do | ||
config[:box_download_ca_cert] = "cacert.pem" | ||
|
||
expect(driver[:box_download_ca_cert]).to eq("cacert.pem") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably because the path does not get resolved with kitchen_root
until the render_template
method.
@@ -326,6 +328,9 @@ def load_needed_dependencies! | |||
# @raise [ActionFailed] if the Vagrantfile template was not found | |||
# @api private | |||
def render_template | |||
config[:box_download_ca_cert] = File.expand_path( | |||
config[:box_download_ca_cert], config[:kitchen_root]) unless | |||
config[:box_download_ca_cert].nil? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd move this to finalize_config
which I believe will mean you can more easily test this (see comment below)
Signed-off-by: Seth Thomas <[email protected]>
Moved the code around as suggested and the tests now seem like they are working right, lemme know if it needs any more work! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Closes #234