Improve test suite (Travis updates, librarian-puppet removal, better utilize puppet-lint)

.fixtures.yml

@@ -1,3 +1,7 @@
 fixtures:
   symlinks:
     nginx: "#{source_dir}"
+  forge_modules:
+    apt: "puppetlabs/apt"
+    concat: "puppetlabs/concat"
+    stdlib: "puppetlabs/stdlib"

.gitignore

@@ -1,8 +1,6 @@
 files/server_test.crt
 files/server_test.pem
 pkg/
-.librarian/
-.tmp/
 pkg/
 Gemfile.lock
 spec/fixtures/

.puppet-lint.rc

@@ -0,0 +1,4 @@
+--fail-on-warnings
+--relative
+--no-80chars-check
+--no-class_inherits_from_params_class-check

.travis.yml

@@ -4,22 +4,25 @@ branches:
   - refactor
   - gh-pages
 language: ruby
-script: "bundle exec rake --rakefile $PWD/.travis/Rakefile spec SPEC_OPTS='--format documentation'"
-rvm:
-  - 1.9.3
-  - 2.0.0
-gemfile: .travis/Gemfile
-env:
-  matrix:
-    - PUPPET_GEM_VERSION="~> 2.7.0"
-    - PUPPET_GEM_VERSION="~> 3.0"
+script: "bundle exec rake validate && bundle exec rake lint && bundle exec rake spec SPEC_OPTS='--format documentation'"
+bundler_args: --without rake
 matrix:
-  exclude:
-    - rvm: 1.9.3
+  fast_finish: true
+  include:
+    - rvm: 1.8.7
       env: PUPPET_GEM_VERSION="~> 2.7.0"
-      gemfile: .travis/Gemfile
+    - rvm: 1.8.7
+      env: PUPPET_GEM_VERSION="~> 3.0"
+    - rvm: 1.9.3
+      env: PUPPET_GEM_VERSION="~> 3.0"
     - rvm: 2.0.0
+      env: PUPPET_GEM_VERSION="~> 3.0"
+    - rvm: 2.1.0
+      env: PUPPET_GEM_VERSION="~> 3.0"
+  allow_failures:
+    - rvm: 1.8.7
       env: PUPPET_GEM_VERSION="~> 2.7.0"
-      gemfile: .travis/Gemfile
+    - rvm: 1.8.7
+      env: PUPPET_GEM_VERSION="~> 3.0"
 notifications:
   email: false  

Gemfile

@@ -1,13 +1,24 @@
 source 'https://rubygems.org'
 
+group :test do
+  gem 'rest-client', '~> 1.6.8', :require => false, :platforms => :ruby_18
+end
+
+group :rake, :test do
+  gem 'puppetlabs_spec_helper',  :require => false
+  gem 'puppet-blacksmith',       :require => false
+  gem 'rspec-system-puppet',     :require => false
+end
+
 group :rake do
-  gem 'puppet',       '>=3.0.1'
   gem 'rspec-puppet', '>=1.0.1'
   gem 'rake',         '>=0.9.2.2'
   gem 'puppet-lint',  '>=0.1.12'
-  gem 'puppetlabs_spec_helper'
-  gem 'puppet-blacksmith'
-  gem 'librarian-puppet', '<1.1.0'
-  gem 'rspec-system-puppet',     :require => false
   gem 'rspec-system-serverspec', :require => false
 end
+
+if puppetversion = ENV['PUPPET_GEM_VERSION']
+  gem 'puppet', puppetversion, :require => false
+else
+  gem 'puppet', :require => false
+end

Rakefile

@@ -9,11 +9,4 @@ require 'puppetlabs_spec_helper/rake_tasks'
 require 'puppet_blacksmith/rake_tasks'
 require 'rspec-system/rake_task'
 
-# use librarian-puppet to manage fixtures instead of .fixtures.yml
-# offers more possibilities like explicit version management, forge downloads,...
-task :librarian_spec_prep do
- sh "librarian-puppet install --path=spec/fixtures/modules/"
-end
-task :spec_prep => :librarian_spec_prep
-
 task :default => [:clean, :spec]

manifests/config.pp

@@ -131,10 +131,10 @@
   }
 
   file { "${conf_dir}/sites-available":
+    ensure => directory,
     owner  => $sites_available_owner,
     group  => $sites_available_group,
     mode   => $sites_available_mode,
-    ensure => directory,
   }
 
   if $vhost_purge == true {

manifests/package/debian.pp

@@ -47,7 +47,7 @@
 
         apt::source { 'nginx':
           location   => 'https://oss-binaries.phusionpassenger.com/apt/passenger',
-          repos      => "main",
+          repos      => 'main',
           key        => '561F9B9CAC40B2F7',
           key_source => 'https://oss-binaries.phusionpassenger.com/auto-software-signing-gpg-key.txt',
           notify     => Exec['apt_get_update_for_nginx'],

manifests/resource/geo.pp

@@ -76,17 +76,19 @@
   include nginx::params
   $root_group = $nginx::params::root_group
 
+  $ensure_real = $ensure ? {
+    'absent' => 'absent',
+    default  => 'file',
+  }
+
   File {
     owner => 'root',
     group => $root_group,
     mode  => '0644',
   }
 
   file { "${nginx::config::conf_dir}/conf.d/${name}-geo.conf":
-    ensure  => $ensure ? {
-      'absent' => absent,
-      default  => 'file',
-    },
+    ensure  => $ensure_real,
     content => template('nginx/conf.d/geo.erb'),
     notify  => Class['nginx::service'],
   }

manifests/resource/location.pp

@@ -291,7 +291,7 @@
   $config_file = "${nginx::config::conf_dir}/sites-available/${vhost_sanitized}.conf"
 
   $location_sanitized_tmp = regsubst($location, '\/', '_', 'G')
-  $location_sanitized = regsubst($location_sanitized_tmp, "\\\\", '_', 'G')
+  $location_sanitized = regsubst($location_sanitized_tmp, '\\\\', '_', 'G')
 
   ## Check for various error conditions
   if ($vhost == undef) {
@@ -336,15 +336,15 @@
   if ($ssl_only != true) {
     $tmpFile=md5("${vhost_sanitized}-${priority}-${location_sanitized}")
 
-    concat::fragment { "${tmpFile}":
+    concat::fragment { $tmpFile:
       ensure  => present,
       target  => $config_file,
       content => join([
         template('nginx/vhost/location_header.erb'),
         $content_real,
         template('nginx/vhost/location_footer.erb')
       ], ''),
-      order   => "${priority}",
+      order   => "${priority}", #lint:ignore:only_variable_string waiting on https://github.com/puppetlabs/puppetlabs-concat/commit/f70881fbfd01c404616e9e4139d98dad78d5a918
     }
   }
 
@@ -353,15 +353,15 @@
     $ssl_priority = $priority + 300
 
     $sslTmpFile=md5("${vhost_sanitized}-${ssl_priority}-${location_sanitized}-ssl")
-    concat::fragment {"${sslTmpFile}":
+    concat::fragment { $sslTmpFile:
       ensure  => present,
       target  => $config_file,
       content => join([
         template('nginx/vhost/location_header.erb'),
         $content_real,
         template('nginx/vhost/location_footer.erb')
       ], ''),
-      order   => "${ssl_priority}",
+      order   => "${ssl_priority}", #lint:ignore:only_variable_string waiting on https://github.com/puppetlabs/puppetlabs-concat/commit/f70881fbfd01c404616e9e4139d98dad78d5a918
     }
   }
 

manifests/resource/map.pp

@@ -60,17 +60,19 @@
   include nginx::params
   $root_group = $nginx::params::root_group
 
+  $ensure_real = $ensure ? {
+    'absent' => absent,
+    default  => 'file',
+  }
+
   File {
     owner => 'root',
     group => $root_group,
     mode  => '0644',
   }
 
   file { "${nginx::config::conf_dir}/conf.d/${name}-map.conf":
-    ensure  => $ensure ? {
-      'absent' => absent,
-      default  => 'file',
-    },
+    ensure  => $ensure_real,
     content => template('nginx/conf.d/map.erb'),
     notify  => Class['nginx::service'],
   }

manifests/resource/upstream.pp

@@ -57,19 +57,20 @@
   include nginx::params
   $root_group = $nginx::params::root_group
 
+  $ensure_real = $ensure ? {
+    'absent' => absent,
+    default  => present,
+  }
+
   Concat {
     owner => 'root',
     group => $root_group,
     mode  => '0644',
   }
 
   concat { "${nginx::config::conf_dir}/conf.d/${name}-upstream.conf":
-    ensure  => $ensure ? {
-      'absent' => absent,
-      'file'   => present,
-      default  => present,
-    },
-    notify  => Class['nginx::service'],
+    ensure => $ensure_real,
+    notify => Class['nginx::service'],
   }
 
   # Uses: $name, $upstream_cfg_prepend

manifests/resource/vhost.pp

@@ -436,7 +436,7 @@
 
   # Add IPv6 Logic Check - Nginx service will not start if ipv6 is enabled
   # and support does not exist for it in the kernel.
-  if ($ipv6_enable == true) and (!$ipaddress6) {
+  if ($ipv6_enable == true) and (!$::ipaddress6) {
     warning('nginx: IPv6 support is not enabled or configured properly')
   }
 

spec/classes/nginx_spec.rb

@@ -34,6 +34,7 @@
           :operatingsystem => 'Debian',
           :osfamily        => 'Debian',
           :lsbdistcodename => 'precise',
+          :lsbdistid       => 'Debian',
         }
       end
     end

spec/classes/package_spec.rb

@@ -48,8 +48,13 @@
     end
   end
 
-  shared_examples 'debian' do |operatingsystem, lsbdistcodename|
-    let(:facts) {{ :operatingsystem => operatingsystem, :osfamily => 'Debian', :lsbdistcodename => lsbdistcodename }}
+  shared_examples 'debian' do |operatingsystem, lsbdistcodename, lsbdistid|
+    let(:facts) {{
+      :operatingsystem => operatingsystem,
+      :osfamily        => 'Debian',
+      :lsbdistcodename => lsbdistcodename,
+      :lsbdistid       => lsbdistid
+    }}
 
     context "using defaults" do
       it { should contain_package('nginx') }
@@ -105,8 +110,8 @@
   end
 
   context 'debian' do
-    it_behaves_like 'debian', 'debian', 'wheezy'
-    it_behaves_like 'debian', 'ubuntu', 'precise'
+    it_behaves_like 'debian', 'debian', 'wheezy', 'debian'
+    it_behaves_like 'debian', 'ubuntu', 'precise', 'ubuntu'
   end
 
   context 'suse' do

tests/location_params.pp

@@ -1,12 +1,12 @@
 include nginx
 
 nginx::resource::location { 'www.test.com-params':
-    ensure         => present,
-    location       => '/some/url',
-    vhost          => 'www.test.com',
-    fastcgi_param  => {
-    	'APP_ENV'  		=> 'production',
-    	'APP_VERSION' 	=> '0.1.10',
-    	'APP_SECRET'	=> 'hisfaihicasagfkjsa',
+    ensure        => present,
+    location      => '/some/url',
+    vhost         => 'www.test.com',
+    fastcgi_param => {
+      'APP_ENV'     => 'production',
+      'APP_VERSION' => '0.1.10',
+      'APP_SECRET'  => 'hisfaihicasagfkjsa',
     },
 }

tests/vhost.pp

@@ -1,16 +1,16 @@
 include nginx
 
 nginx::resource::vhost { 'test.local test':
-  ensure       => present,
-  ipv6_enable  => true,
-  proxy        => 'http://proxypass',
+  ensure      => present,
+  ipv6_enable => true,
+  proxy       => 'http://proxypass',
 }
 
 nginx::resource::vhost { 'test.local:8080':
-  ensure       => present,
-  listen_port  => 8080,
-  server_name  => ['test.local test'],
-  ipv6_enable  => true,
-  proxy        => 'http://proxypass',
+  ensure      => present,
+  listen_port => 8080,
+  server_name => ['test.local test'],
+  ipv6_enable => true,
+  proxy       => 'http://proxypass',
 }