Re-enable and fix acceptance tests. (Don't manage /var/run/redis on Debian systems)
#299
Conversation
alexjfisher
force-pushed
the
acceptance_tests
branch
3 times, most recently
from
bd07354 to
92d774c
Compare
alexjfisher
changed the title
/var/run/redis on Debian systems)
| - set: ubuntu1604-64m | ||
| - set: centos6-64m | ||
| - set: centos7-64m | ||
| - set: debian8-64m |
There was a problem hiding this comment.
why is there an m at the end of each line?
There was a problem hiding this comment.
Role master was needed for one of the beaker helpers. run_task IIRC.
| file { '/var/run/redis': | ||
| ensure => 'directory', | ||
| owner => $::redis::config_owner, | ||
| group => $var_run_redis_group, | ||
| mode => $var_run_redis_mode, | ||
| group => $::redis::config_group, |
There was a problem hiding this comment.
latest modulesync doesn't allow :: after $, so you might want to directly remove it.
There was a problem hiding this comment.
IMHO not. The module is filled with it and this change keeps it consistent.
There was a problem hiding this comment.
That's only actually for classes. I think there'll be a separate PR where the variables are sorted out.
| file { '/var/run/redis': | ||
| ensure => 'directory', | ||
| owner => $::redis::config_owner, | ||
| group => $var_run_redis_group, | ||
| mode => $var_run_redis_mode, | ||
| group => $::redis::config_group, |
There was a problem hiding this comment.
IMHO not. The module is filled with it and this change keeps it consistent.
| @@ -0,0 +1,13 @@ | |||
| --- | |||
There was a problem hiding this comment.
Would it be an idea to add these nodesets as unmanaged? Longer term I'd like to remove them and if this module isn't using them, let's not pollute its history.
There was a problem hiding this comment.
Sure. Can you remind me of the correct syntax for .sync.yml?
There was a problem hiding this comment.
| @@ -9,6 +9,13 @@ | |||
| 'redis' | |||
| end | |||
|
|
|||
| # We're testing with `manage_repo` true. In redis-sentinel 5, the daemon has its own rundir | |||
| pidfile = if fact('operatingsystem') == 'Ubuntu' && fact('operatingsystemmajrelease') == '16.04' | |||
There was a problem hiding this comment.
Could you implement this in pure puppet below?
There was a problem hiding this comment.
I suppose I may as well. Ideally, the module itself would set the default correctly, but that's for another day.
There was a problem hiding this comment.
Done (assuming I've not messed it up).
alexjfisher
force-pushed
the
acceptance_tests
branch
from
daf49d9 to
91cd6ed
Compare
alexjfisher
force-pushed
the
acceptance_tests
branch
from
91cd6ed to
000d519
Compare
alexjfisher
force-pushed
the
acceptance_tests
branch
from
000d519 to
6c21e7d
Compare
No description provided.