Add tests and Puppet-Strings based reference docs #41
Conversation
| gem 'puppet-lint', '>= 1.0.0' | ||
| gem 'puppetlabs_spec_helper', '>= 2.11.0' | ||
| gem 'rspec-puppet-facts', '>= 1.8.0' | ||
| gem 'metadata-json-lint' |
There was a problem hiding this comment.
Without this gem explicitly declared, the validate rake task won't check the metadata.json file.
| # Cerebro Puppet Module [](https://forge.puppet.com/yano3/cerebro) | ||
| # Cerebro Puppet Module | ||
| [](https://forge.puppet.com/yano3/cerebro) | ||
| [](https://github.com/yano3/puppet-cerebro/blob/master/LICENSE) |
There was a problem hiding this comment.
Because who doesn't like badges! :)
| # @api private | ||
| class cerebro::config | ||
| { | ||
| $secret = $cerebro::secret |
There was a problem hiding this comment.
Since the class is private and only ever expected to be declared from the base class, I've moved the variable declarations from being class parameters to being in the body of the class.
| Optional[Stdlib::Unixpath] $java_home = $::cerebro::params::java_home, | ||
| Optional[Hash] $basic_auth_settings = $::cerebro::params::basic_auth_settings, | ||
| Optional[Stdlib::Ip_address] $address = $::cerebro::params::address, | ||
| Stdlib::Ensure::Service $service_ensure = $cerebro::params::service_ensure, |
There was a problem hiding this comment.
Note, Stdlib::Ensure::Service doesn't accept Booleans. I could change it to Variant[Stdlib::Ensure::Service, Boolean] if you prefer.
| Array[Struct[{ | ||
| host => Stdlib::HTTPUrl, | ||
| name => String[1] | ||
| }]] $hosts = $cerebro::params::hosts, |
There was a problem hiding this comment.
This data type should be correct (and there's a spec test for this). The other advantage of using this data type is that puppet-strings will add it to the reference documentation.
| Array[String[1]] $java_opts = $cerebro::params::java_opts, | ||
| Optional[Stdlib::Unixpath] $java_home = $cerebro::params::java_home, | ||
| Optional[Hash] $basic_auth_settings = $cerebro::params::basic_auth_settings, | ||
| Optional[Stdlib::IP::Address] $address = $cerebro::params::address, |
There was a problem hiding this comment.
Stdlib::Ip_address was removed in stdlib 5. Stdlib::IP::Address is available since 4.25.0.
| ) inherits cerebro::params { | ||
|
|
||
| if $manage_user { |
There was a problem hiding this comment.
I moved the if to the cerebro::user class.
| ) inherits cerebro::params { | ||
|
|
||
| if $manage_user { | ||
| contain '::cerebro::user' | ||
| contain cerebro::user |
There was a problem hiding this comment.
Leading :: is no longer, (since Puppet 4), needed or recommended. See https://github.com/voxpupuli/puppet-lint-absolute_classname-check#relative-class-name-inclusion
| content => template('cerebro/etc/tmpfiles.d/cerebro.conf.erb'), | ||
| } | ||
|
|
||
|
|
||
| if ($::operatingsystem == 'Amazon') { | ||
| if $facts['os']['name'] == 'Amazon' { |
There was a problem hiding this comment.
I've replaced legacy facts with modern equivalents (available since Facter 3 which was shipped with puppet 4.2.0).
| @@ -14,19 +14,19 @@ | |||
| "dependencies": [ | |||
| { | |||
| "name": "puppetlabs/stdlib", | |||
| "version_requirement": ">= 4.0.0 < 5.0.0" | |||
| "version_requirement": ">= 4.25.0 < 6.0.0" | |||
There was a problem hiding this comment.
4.25.0 is needed for Stdlib::IP::Address
There was a problem hiding this comment.
Either that or you can use Stdlib::Compat::IP_address, which was available on earlier versions.
Not sure how it happened, but this issue leaked into my environment. I had to manually patch this module to get it to work. I'll evaluate updating stdlib to 4.25.0 but it will take me some time, since several of our own modules depend on it.
There was a problem hiding this comment.
It's not super clear, but I don't think you're supposed to use the compat types generally.
https://github.com/puppetlabs/puppetlabs-stdlib#for-module-developers
After releasing this version, you can release another breaking change release where you remove all compat types and all calls to validate_legacy.
They were just there as a mechanism for moving away from the old validate_x functions.
There was a problem hiding this comment.
I see, thanks for the clarification!
| context 'by default' do | ||
| let(:pre_condition) { 'include cerebro' } | ||
|
|
||
| # TODO: Verify template contents |
There was a problem hiding this comment.
There's actually one example of testing template contents below.
| end | ||
|
|
||
| context 'with hosts set' do | ||
| let(:pre_condition) { 'class {\'cerebro\': hosts => [{ "name" => "my_cluster", "host" => "http://127.0.0.1:9200"}] }' } |
There was a problem hiding this comment.
This is where I test
Array[Struct[{
host => Stdlib::HTTPUrl,
name => String[1]
}]]
|
@yano3 Do you have a travis account? If so, I could add a |
A couple of the data-types are tightened slightly, but otherwise, no functional changes anyone should notice.