fix broken unit tests #368
Conversation
|
merge this, rebase and merge #367 afterwards |
|
okay I'm not sure why the tests fail on Ubuntu 16 :( |
templates/redis-sentinel.conf.erb
Outdated
| @@ -5,7 +5,9 @@ port <%= @sentinel_port %> | |||
| dir <%= @working_dir %> | |||
| daemonize <%= @daemonize ? 'yes' : 'no' %> | |||
| pidfile <%= @pid_file %> | |||
| <% if @facts['os']['release']['major'] != '16.04' -%> | |||
There was a problem hiding this comment.
This is not what I had in mind. This is much more like it:
puppet-redis/manifests/instance.pp
Line 402 in d443861
bastelfreak
force-pushed
the
tests
branch
4 times, most recently
from
9005b1b
to
22763bd
Compare
|
I'm going to knock up an alternative PR |
bastelfreak
force-pushed
the
tests
branch
5 times, most recently
from
1cfcb00
to
094f04c
Compare
chronological order: * voxpupuli#272 was opened * voxpupuli#365 was opened * voxpupuli#365 was merged * voxpupuli#272 was merged without a rebase * tests are broken
| ) inherits redis::params { | ||
| require 'redis' | ||
|
|
||
| if $package_ensure =~ /^([0-9]+:)?[0-9]+\.[0-9]/ { | ||
| if ':' in $package_ensure { |
There was a problem hiding this comment.
I had to duplicate the whole block because $package_ensure for sentinel can be something differen than in init.pp/instance.pp
| @@ -139,9 +139,23 @@ | |||
| Stdlib::Absolutepath $working_dir = $redis::params::sentinel_working_dir, | |||
| Optional[Stdlib::Absolutepath] $notification_script = undef, | |||
| Optional[Stdlib::Absolutepath] $client_reconfig_script = undef, | |||
| String[5] $minimum_version = $redis::params::minimum_version, | |||
There was a problem hiding this comment.
Like in instance.pp, I think this is considered private and should just be in the class body instead.
|
I've get the tests to work with following workaround in the template: i know, it's doubled if statement but i want to have it for later use, when the final solution exists. the default value for protected mode is "no" if not set, so it's also fine for newer versions starting from 3.2.0. let's give the option to the user, he should know how to and when to use this. My workaround is implemented in #356 (sorry for the massive commits :( ) Edit: |
|
I think about it a little bit. When protected mode is enabled by default, then you can write the config only if we want to disable it. Or like @alexjfisher mentioned: |
|
In #374 I took an alternative approach. |
chronological order:
Pull Request (PR) description
This Pull Request (PR) fixes the following issues