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

[Modules 5492] - Include treatment for absolute, relative and pipe paths for JkLogFile and JkShmFile for class mod::jk #1671

Merged
merged 44 commits into from
Aug 25, 2017

Conversation

EmersonPrado
Copy link
Contributor

  • Prepends logdir in relative paths
  • Uses unchanged absolute / pipe paths
  • Uses defaults for unspecified paths

Previous version prepended logdir in any filename, resulting in incorrect paths for absolute and pipe paths.

@eputnam
Copy link
Contributor

eputnam commented Aug 16, 2017

@EmersonPrado, good to see you working on apache again! Would you mind adding some unit tests for this and update the README to document/explain the new defaults and how those params now work?

@EmersonPrado
Copy link
Contributor Author

@eputnam Sure! We're using this module extensively at work, so we're really willing to improve as much as we can.
Here are the docs. I'll work on the spec tests now.
Thanks!

Emerson Prado added 10 commits August 17, 2017 15:59
Revert "Try using symbol to test conf file in mod::jk spec test"

This reverts commit b02c31d.

Revert "Set mod_dir variable in mod::jk spec test"

This reverts commit 9f7e941.

Revert "Create mod_dir variable in mod::jk class to ease spec test"

This reverts commit 39986b8.

Revert "Remove content check for jk.conf in mod::jk spec test"

This reverts commit d85e8a2.

Revert "Include pipe paths context in mod::jk spec test"

This reverts commit 95dd58b.

Revert "Include absolute paths context in mod::jk spec test"

This reverts commit 70e36e2.

Revert "Include relative paths context in mod::jk spec test"

This reverts commit 68c15ae.

Revert "Include conf file check for new contexts in mod::jk spec test"

This reverts commit 45d040c.

Revert "Include context loop with one item in mod::jk spec test"

This reverts commit f6bb97f.
@EmersonPrado
Copy link
Contributor Author

OK, I have to admit I know squat about spec tests. I keep hoping its documentation improves to the point I can really solve this kind of stuff. Gave up for today.

@@ -115,7 +115,7 @@

it_behaves_like 'minimal resources'
it do
is_expected.to contain_file("/etc/httpd/conf.d/jk.conf")
is_expected.to contain_file("#{:mod_dir}/jk.conf")
Copy link
Contributor

Choose a reason for hiding this comment

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

this is a bit of a gotcha. contain_file is referencing the file resource in the catalog by its title. If you look in jk.pp you'll see that the config file has a hard-coded title of 'jk.conf' which is why this test wasn't working. I think what you're looking for is something more like this:

is_expected.to contain_file('jk.conf').with({ :path => "#{mod_dir}/jk.conf" })

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, that should be it! I was using the path, when I should be using the resource title. I'll start over pushing very little at a time, so we can get somewhere. Thanks for the hint!

@EmersonPrado
Copy link
Contributor Author

@eputnam Now we've got updated docs and functional tests covering all situations envolved. Anything else comes to mind?
Regards,
Emerson

it {
verify_contents(catalogue, 'jk.conf', ['<IfModule jk_module>', '</IfModule>'])
}

end

shm_log_paths = {
Copy link
Contributor

Choose a reason for hiding this comment

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

this should/can be a let statement as well:

let (:shm_log_paths) do
  { ... }
end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

rspec-puppet didn't like the change:
https://travis-ci.org/puppetlabs/puppetlabs-apache/jobs/268018580#L799
https://travis-ci.org/puppetlabs/puppetlabs-apache/jobs/268018581#L680
'shm_log_paths is not available on an example group (e.g. a describe or context block). It is only available from within individual examples (e.g. it blocks) or from constructs that run in the scope of an example (e.g. before, let, etc).'

What is the correct way to reference local variables declared in let statements? Current form is this

@eputnam
Copy link
Contributor

eputnam commented Aug 23, 2017

I left one more tiny comment regarding some rspec-fu and then we'll get this merged

@eputnam
Copy link
Contributor

eputnam commented Aug 24, 2017

oh i see what happened. i'm sorry, i guess i was wrong. what you can do is call each right on the hash instead of assigning it to a variable if you're only using it the one time.

@eputnam eputnam merged commit 8ebc42a into puppetlabs:master Aug 25, 2017
cegeka-jenkins pushed a commit to cegeka/puppet-apache that referenced this pull request Jul 15, 2020
…ths for JkLogFile and JkShmFile for class mod::jk (puppetlabs#1671)

* Move logroot treatment away from files treatment in mod::jk

* Treat Log/Shm files with absolute paths in mod::jk

* Add default case for Log/Shm files in mod::jk

* Include in README subsection on ...mod::jk::logroot parameter

* Include in README subsection on ...mod::jk::(shm|log)_file parameters

* Include in README examples on ...mod::jk::(shm|log)_file parameters

* Include context loop with one item in mod::jk spec test

* Include conf file check for new contexts in mod::jk spec test

* Include relative paths context in mod::jk spec test

* Include absolute paths context in mod::jk spec test

* Include pipe paths context in mod::jk spec test

* Remove content check for jk.conf in mod::jk spec test

* Create mod_dir variable in mod::jk class to ease spec test

* Set mod_dir variable in mod::jk spec test

* Try using symbol to test conf file in mod::jk spec test

* Reverts all failed attempts to spec test jk.conf

Revert "Try using symbol to test conf file in mod::jk spec test"

This reverts commit b02c31d.

Revert "Set mod_dir variable in mod::jk spec test"

This reverts commit 9f7e941.

Revert "Create mod_dir variable in mod::jk class to ease spec test"

This reverts commit 39986b8.

Revert "Remove content check for jk.conf in mod::jk spec test"

This reverts commit d85e8a2.

Revert "Include pipe paths context in mod::jk spec test"

This reverts commit 95dd58b.

Revert "Include absolute paths context in mod::jk spec test"

This reverts commit 70e36e2.

Revert "Include relative paths context in mod::jk spec test"

This reverts commit 68c15ae.

Revert "Include conf file check for new contexts in mod::jk spec test"

This reverts commit 45d040c.

Revert "Include context loop with one item in mod::jk spec test"

This reverts commit f6bb97f.

* Add mod_dir variable in mod::apache class to allow spec tests

* Add mod_dir variable in mod::apache spec test

* Correct mod_dir variable reference in class mod::jk

* Add parameter in shared example of mod::jk spec test

* Include path check for jk.conf in mod::jk spec test

* Correct Debian path for jk.conf in mod::jk spec test

* Compound matchers for same resource in mod::jk spec test

* Include "and" in additional matcher for jk.conf in mod::jk spec test

* Try to correct compound matchers syntax in mod::jk spec test

* Remove compound matchers in mod::jk spec test

* Move simple matcher to single line in mod::jk spec test

* Include jk.conf check via "with_content" in mod::jk spec test

* Correct jk.conf contents in mod::jk spec test

* Correct line endings in jk.conf contents in mod::jk spec test

* Create hash for (shm|log)_file parameters in mod::jk spec test

* Add context iterated for (shm|log)_file in mod::jk spec test

* Include iterated file content check in mod::jk spec test

* Remove redundant jk.conf content check in mod::jk spec test

* Add relative paths for (shm|log)_file in mod::jk spec test

* Remove remaining redundant jk.conf content check in mod::jk spec test

* Add absolute paths for (shm|log)_file in mod::jk spec test

* Add pipe paths for (shm|log)_file in mod::jk spec test

* Correct log_file pipe parameter in mod::jk spec test

* Use "let" to set local variable in mod::jk spec test

* Change symbol to var name in assignment in mod::jk spec test

* Quote var name in assignment in mod::jk spec test

* Correct symbol reference in mod::jk spec test

* Change unecessary var for direct hash iteration in mod::jk spec test
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.

None yet

3 participants