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-3211: fix broken strict_variable tests #1409

Merged
merged 1 commit into from Mar 29, 2016

Conversation

jlambert121
Copy link
Contributor

No description provided.

@witjoh
Copy link
Contributor

witjoh commented Mar 29, 2016

Hi,

I applied your PR locally, and it seems to compile ok. One thought:
I think tremoving the the rspec file mod/dev_spec.rb is a bad idea. Always better to make it work. Adding following line makes rspec working again:

--- dev_spec.rb_orig    2016-03-29 13:54:35.825263541 +0000
+++ dev_spec.rb 2016-03-29 13:51:18.271206754 +0000
@@ -2,7 +2,10 @@

 describe 'apache::mod::dev', :type => :class do
   it_behaves_like "a mod class, without including apache"
-
+  let(:pre_condition) {[
+    'include apache'
+  ]}
+  
   [
     ['RedHat',  '6', 'Santiago', 'Linux'],
     ['Debian',  '6', 'squeeze', 'Linux'],

Also in the param.pp, if google did gave me the right result for the default pidfile ... ,
$pidfile = '/var/run/apache2.pid'

(someone familiar with gentoo should confirm this.)

Grts and thx for this PR.,. Did solve my rspec problems
Johan

@jlambert121
Copy link
Contributor Author

I considered this solution, but I try to keep my unit tests testing the class itself, not the effects of a class called by a class (that secondary class should be tested by it's own unit tests). Tests just for the sake of quantity aren't always as effective. That's the reason I decided to remove it, but I can put it back if needed.

Thanks for the Gentoo PID file. I've updated with that (but can change it if needed).

@igalic
Copy link
Contributor

igalic commented Mar 29, 2016

i like @jlambert121 solution better than trying to fake it in the spec tests.

@igalic igalic merged commit 11806ca into puppetlabs:master Mar 29, 2016
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

4 participants