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

Fixing bad default setting - if ENV is not set we are accidently setting log_level to nil for whole run #633

Merged
merged 2 commits into from
Mar 12, 2015

Conversation

tyler-ball
Copy link
Contributor

This is a follow up to https://github.com/test-kitchen/test-kitchen/pull/600/files, which introduced a bug. Not setting ENV[:log_level] causes the converge step to fail because we create the provisioner string chef-client --log-level with no supplied log level.

\cc @fnichol @schisamo

@schisamo
Copy link
Contributor

@tyler-ball This fixes the issue for me. It would be nice to add some test coverage to this PR so we ensure the issue doesn't bite us again in the future!

@schisamo
Copy link
Contributor

/cc @scotthain @curiositycasualty

@scotthain
Copy link
Contributor

+1 to testing these defaults but yay! Thanks y'all!

@fnichol fnichol self-assigned this Mar 11, 2015
@tyler-ball
Copy link
Contributor Author

Well I am really glad I added coverage - because I found a bug with my original code! I'll merge this if the test looks good to yall

@schisamo
Copy link
Contributor

@tyler-ball once Travis is happy you should :shipit:

@scotthain
Copy link
Contributor

👍 rock on

@fnichol
Copy link
Contributor

fnichol commented Mar 12, 2015

Yippie, agreed!

fnichol added a commit that referenced this pull request Mar 12, 2015
Fixing bad default setting - if ENV is not set we are accidently setting log_level to nil for whole run
@fnichol fnichol merged commit 2fba692 into master Mar 12, 2015
@fnichol fnichol deleted the tball/bad-default branch March 12, 2015 16:59
@test-kitchen test-kitchen locked and limited conversation to collaborators Nov 16, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants