search

saltstack-formulas / logrotate-formula

http://docs.saltstack.com/en/latest/topics/development/conventions/formulas.html
Other
23 stars 71 forks source link

configuring travis-ci tests #34

Closed daks closed 5 years ago

daks commented 6 years ago

Still WIP

daks commented 5 years ago

@javierbertoli thanks to your help (and more available time), I pushed new code to add travis tests. Those work now, but tests on CentOS fail :(

javierbertoli commented 5 years ago

Checking locally, it seems RedHat family does not use a systemd timer to run logrotate, but a plain old cron, so the service state fails.

It seems this PR is already finding bugs in the formula :smile:

I think we'd probably need a conditional on that state file, to run only on Debian family and eventually add tests for service running / cron existence depending on the family?

wdyt?

daks commented 5 years ago

It seems this PR is already finding bugs in the formula smile

and this is great!

I think we'd probably need a conditional on that state file, to run only on Debian family and eventually add tests for service running / cron existence depending on the family?

I'm not fan of conditionals in the state, but maybe there is not better option. Eventually it can be activated with an option defined (depending of OS) in the map.jinja.

In any case, I'm not sure I'll be able to look at it soon so don't you think it would be better to merge this PR (eventually without the faulty test) and create an issue for later?

javierbertoli commented 5 years ago

I'm not fan of conditionals in the state, but maybe there is not better option. Eventually it can be activated with an option defined (depending of OS) in the map.jinja.

Neither am I, but there are a few other formulas with states that are apply depending on the OS, as some OS differ this much (ie, systemd timer vs. cron), so I think in this case it would be the quickest solution.

In any case, I'm not sure I'll be able to look at it soon so don't you think it would be better to merge this PR (eventually without the faulty test) and create an issue for later?

Problem is, what's failing is a state file, not a test. The formula is not applying completely because that state fails. I'll try to get a few minutes later today, modify these things we're discussing and probably merge this PR + push another PR to fix that part of the formula. Are you OK with this?

daks commented 5 years ago

I'm ok.

javierbertoli commented 5 years ago

Where I said 'a few minutes later today' should have read 'these days' but well... merged. :smile: Thanks, @daks.