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

init.pp: get rid of the ugly hack to compute $check_notify #388

Closed

Conversation

amosshapira
Copy link

Proposed cleanup of the "ugly hack".

@amosshapira amosshapira changed the title init.pp: get rid of the ugly hack to compute $client_notify init.pp: get rid of the ugly hack to compute $check_notify Jul 23, 2015
@amosshapira
Copy link
Author

It appears that the failures in build 692 (https://travis-ci.org/sensu/sensu-puppet/builds/72221455) are due to Mocha interface change with Ruby 1.9, nothing to do with my code change.

@jlambert121
Copy link
Contributor

Can you rebase and see if that fixes the tests?

@amosshapira amosshapira force-pushed the Remove-notifications-ugly-hack branch from db2bbdd to 36c6057 Compare July 28, 2015 22:51
@amosshapira
Copy link
Author

I rebased on latest master and updated the tests to always expect an array in notify.
This gets all tests green except the Puppet 3.3 tests.
There is still a problem with notify being empty in all Puppet 3.3.0 tests.
I suspect that the issue is old version of puppet-stdlib including a broken version of delete_undef_values(), but I don't see the exact version used in the test in, for instance, https://travis-ci.org/sensu/sensu-puppet/jobs/73102376 and am not familiar enough with the tools to reproduce it locally on the Vagrant box.
Could you give me some more direction on how can I execute the tests inside the Vagrant box?
Thanks.

@jlambert121
Copy link
Contributor

I was just trying to duplicate this on my machine without any luck. I installed puppet 3.3.2 for my tests using bundler:

PUPPET_GEM_VERSION=3.3.2 bundle update

and all of the tests ran fine.

➜  sensu-puppet git:(master) bundle exec rake test
---> syntax:manifests
---> syntax:templates
---> syntax:hiera:yaml
Cloning into 'spec/fixtures/modules/apt'...
remote: Counting objects: 106, done.
remote: Compressing objects: 100% (86/86), done.
remote: Total 106 (delta 9), reused 58 (delta 7), pack-reused 0
Receiving objects: 100% (106/106), 66.03 KiB | 0 bytes/s, done.
Resolving deltas: 100% (9/9), done.
Checking connectivity... done.
Cloning into 'spec/fixtures/modules/stdlib'...
remote: Counting objects: 409, done.
remote: Compressing objects: 100% (273/273), done.
remote: Total 409 (delta 149), reused 265 (delta 122), pack-reused 0
Receiving objects: 100% (409/409), 168.76 KiB | 0 bytes/s, done.
Resolving deltas: 100% (149/149), done.
Checking connectivity... done.
Cloning into 'spec/fixtures/modules/wget'...
remote: Counting objects: 38, done.
remote: Compressing objects: 100% (31/31), done.
remote: Total 38 (delta 5), reused 22 (delta 4), pack-reused 0
Receiving objects: 100% (38/38), 18.31 KiB | 0 bytes/s, done.
Resolving deltas: 100% (5/5), done.
Checking connectivity... done.
/System/Library/Frameworks/Ruby.framework/Versions/2.0/usr/bin/ruby -I/Users/jlambert/Documents/evenup/git/puppet/sensu-puppet/.bundle/vendor/gems/rspec-core-3.3.2/lib:/Users/jlambert/Documents/evenup/git/puppet/sensu-puppet/.bundle/vendor/gems/rspec-support-3.3.0/lib /Users/jlambert/Documents/evenup/git/puppet/sensu-puppet/.bundle/vendor/gems/rspec-core-3.3.2/exe/rspec --pattern spec/\{classes,defines,unit,functions,hosts,integration\}/\*\*/\*_spec.rb --color
.......................................................................................................................................................................

Finished in 19.7 seconds (files took 1.77 seconds to load)
167 examples, 0 failures

➜  sensu-puppet git:(master) bundle exec puppet --version
3.3.2

I'm not quite sure what's different with travis right now.

@amosshapira
Copy link
Author

Thanks for testing. Travis mentions 3.3.0 specifically. Also I'd like to verify the stdlib version used in that 3.3.0 environment.

@jlambert121
Copy link
Contributor

The .travis file specifies ~> 3.3.0 so it should be the latest patch in the 3.3 series (which is 3.3.2). The version of stdlib it is using is master from git (in the .fixtures file it specifies the modules)

@amosshapira
Copy link
Author

Using the commands you quoted above, I managed to replicated the problem inside the sensu-server vagrant box:

/usr/bin/ruby1.9.1 -I/var/lib/gems/1.9.1/gems/rspec-core-3.3.2/lib:/var/lib/gems/1.9.1/gems/rspec-support-3.3.0/lib /var/lib/gems/1.9.1/gems/rspec-core-3.3.2/exe/rspec --pattern spec/\{classes,defines,unit,functions,hosts,integration\}/\*\*/\*_spec.rb --color
...........................................................................................FFF.........................................................................

Failures:

  1) sensu::check notifications only client should contain Sensu_check[mycheck] with notify => ["Class[Sensu::Client::Service]"]
     Failure/Error: it { should contain_sensu_check('mycheck').with(:notify => ['Class[Sensu::Client::Service]'] ) }
       expected that the catalogue would contain Sensu_check[mycheck] with notify set to ["Class[Sensu::Client::Service]"] but it is set to []
     # ./spec/defines/sensu_check_spec.rb:104:in `block (4 levels) in <top (required)>'

  2) sensu::check notifications only server should contain Sensu_check[mycheck] with notify => ["Class[Sensu::Server::Service]"]
     Failure/Error: it { should contain_sensu_check('mycheck').with(:notify => ['Class[Sensu::Server::Service]'] ) }
       expected that the catalogue would contain Sensu_check[mycheck] with notify set to ["Class[Sensu::Server::Service]"] but it is set to []
     # ./spec/defines/sensu_check_spec.rb:109:in `block (4 levels) in <top (required)>'

  3) sensu::check notifications only api should contain Sensu_check[mycheck] with notify => ["Class[Sensu::Api::Service]"]
     Failure/Error: it { should contain_sensu_check('mycheck').with(:notify => ['Class[Sensu::Api::Service]'] ) }
       expected that the catalogue would contain Sensu_check[mycheck] with notify set to ["Class[Sensu::Api::Service]"] but it is set to []
     # ./spec/defines/sensu_check_spec.rb:114:in `block (4 levels) in <top (required)>'

Finished in 1 minute 41.95 seconds (files took 1.74 seconds to load)
167 examples, 3 failures

Failed examples:

rspec ./spec/defines/sensu_check_spec.rb:104 # sensu::check notifications only client should contain Sensu_check[mycheck] with notify => ["Class[Sensu::Client::Service]"]
rspec ./spec/defines/sensu_check_spec.rb:109 # sensu::check notifications only server should contain Sensu_check[mycheck] with notify => ["Class[Sensu::Server::Service]"]
rspec ./spec/defines/sensu_check_spec.rb:114 # sensu::check notifications only api should contain Sensu_check[mycheck] with notify => ["Class[Sensu::Api::Service]"]

/usr/bin/ruby1.9.1 -I/var/lib/gems/1.9.1/gems/rspec-core-3.3.2/lib:/var/lib/gems/1.9.1/gems/rspec-support-3.3.0/lib /var/lib/gems/1.9.1/gems/rspec-core-3.3.2/exe/rspec --pattern spec/\{classes,defines,unit,functions,hosts,integration\}/\*\*/\*_spec.rb --color failed

I have to step away from keyboard for a few days now. I'll get back to this later. Thanks for the tip.

@ghoneycutt ghoneycutt self-assigned this Jul 9, 2017
ghoneycutt pushed a commit to ghoneycutt/sensu-puppet that referenced this pull request Jul 9, 2017
@ghoneycutt
Copy link
Collaborator

Thanks @amosshapira @jlambert121

I've fixed this up and rebased in PR #725

@ghoneycutt ghoneycutt closed this Jul 9, 2017
ghoneycutt added a commit that referenced this pull request Jul 9, 2017
@ghoneycutt
Copy link
Collaborator

Functionality released in v2.19.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants