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

Validate global_options and defaults_options. #207

Merged
merged 1 commit into from
Dec 7, 2015

Conversation

tlimoncelli
Copy link

  • validate_hash() global_options and defaults_options
  • Add spec tests to verify that non-hash values produce errors
  • Add spec test to verify functionality of non-merge behavior

FYI: This validation would have prevented a minor outage we had after upgrading to the latest commit. Hopefully by adding this validate_*() other users will not suffer like we did.

The documentation clearly states that defaults_options must be a hash but we had been sending it an array. This worked in the old code but not the new code. Oops!

@tlimoncelli tlimoncelli force-pushed the validate_hash_options branch 3 times, most recently from f89ce6f to d035857 Compare November 20, 2015 20:42
  * validate_hash() global_options and defaults_options
  * Add spec tests to verify that non-hash values produce errors
  * Add spec test to verify functionality of non-merge behavior
@tlimoncelli tlimoncelli force-pushed the validate_hash_options branch from d035857 to 478b029 Compare November 20, 2015 20:46
@tlimoncelli
Copy link
Author

This is ready to be considered for merge.

This fixes https://tickets.puppetlabs.com/browse/MODULES-2857

# For testing the merging functionality we restrict ourselves to
# Debian OS family so that we don't have to juggle different sets of
# global_options and defaults_options (like for FreeBSD).
['Debian' ].each do |osfamily|
Copy link

Choose a reason for hiding this comment

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

i'm confused why this is a loop

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for raising that. I'm copying the style of this kind of test above on line 373 (https://github.com/puppetlabs/puppetlabs-haproxy/pull/207/files#diff-7be58596f7d58684b9342fe66e3aeee3L373)

DavidS added a commit that referenced this pull request Dec 7, 2015
Validate global_options and defaults_options.
@DavidS DavidS merged commit 6a003d4 into puppetlabs:master Dec 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants