(MODULES-2683) fix version compare to properly suppress show_diff for…

Gemfile

@@ -14,9 +14,9 @@ group :development, :unit_tests do
   gem 'rspec-core', '3.1.7',     :require => false
   gem 'puppetlabs_spec_helper',  :require => false
   gem 'simplecov',               :require => false
-  gem 'puppet_facts',            :require => false
   gem 'json',                    :require => false
   gem 'metadata-json-lint',      :require => false
+  gem 'rspec-puppet-facts',      :require => false
 end
 
 group :system_tests do

manifests/server/root_password.pp

@@ -35,7 +35,7 @@
     }
 
     # show_diff was added with puppet 3.0
-    if versioncmp($::puppetversion, '3.0') <= 0 {
+    if versioncmp($::puppetversion, '3.0') >= 0 {
       File["${::root_home}/.my.cnf"] { show_diff => false }
     }
     if $mysql::server::create_root_user == true {

spec/acceptance/mysql_server_spec.rb

@@ -64,5 +64,21 @@ class { 'mysql::server':
       apply_manifest(pp, :catch_changes => true)
     end
   end
-end
 
+  describe 'when changing the password' do
+    let(:password) { 'THE NEW SECRET' }
+    let(:manifest) { "class { 'mysql::server': root_password => '#{password}' }" }
+
+    it 'should not display the password' do
+      result = apply_manifest(manifest, :expect_changes => true)
+      # this does not actually prove anything, as show_diff in the puppet config defaults to false.
+      expect(result.stdout).not_to match /#{password}/
+    end
+
+    it 'should be idempotent' do
+      result = apply_manifest(manifest, :catch_changes => true)
+    end
+
+  end
+
+end

spec/classes/graceful_failures_spec.rb

@@ -1,15 +1,19 @@
 require 'spec_helper'
 
 describe 'mysql::server' do
-  on_pe_unsupported_platforms.each do |pe_version,pe_platforms|
-    pe_platforms.each do |pe_platform,facts|
-      describe "on #{pe_version} #{pe_platform}" do
-        let(:facts) { facts }
+  context "on an unsupported OS" do
+    # fetch any sets of facts to modify them
+    os, facts = on_supported_os.first
 
-        context 'should gracefully fail' do
-          it { expect { is_expected.to compile}.to raise_error(Puppet::Error, /Unsupported platform:/) }
-        end
-      end
+    let(:facts) {
+      facts.merge({
+        :osfamily => 'UNSUPPORTED',
+        :operatingsystem => 'UNSUPPORTED',
+      })
+    }
+
+    it 'should gracefully fail' do
+      is_expected.to compile.and_raise_error(/Unsupported platform:/)
     end
   end
 end

spec/classes/mycnf_template_spec.rb

@@ -2,76 +2,78 @@
 
 describe 'mysql::server' do
   context 'my.cnf template' do
-    on_pe_supported_platforms(PLATFORMS).each do |pe_version,pe_platforms|
-      pe_platforms.each do |pe_platform,facts|
-        describe "on #{pe_version} #{pe_platform}" do
-          let(:facts) { facts }
+    on_supported_os.each do |os, facts|
+      context "on #{os}" do
+        let(:facts) {
+          facts.merge({
+            :root_home => '/root',
+          })
+        }
 
-          context 'normal entry' do
-            let(:params) {{ :override_options => { 'mysqld' => { 'socket' => '/var/lib/mysql/mysql.sock' } } }}
-            it do
-              is_expected.to contain_file('mysql-config-file').with({
-                :mode                    => '0644',
-                :selinux_ignore_defaults => true,
-              }).with_content(/socket = \/var\/lib\/mysql\/mysql.sock/)
-            end
-          end
-
-          describe 'array entry' do
-            let(:params) {{ :override_options => { 'mysqld' => { 'replicate-do-db' => ['base1', 'base2'], } }}}
-            it do
-              is_expected.to contain_file('mysql-config-file').with_content(
-                /.*replicate-do-db = base1\nreplicate-do-db = base2.*/
-              )
-            end
+        context 'normal entry' do
+          let(:params) {{ :override_options => { 'mysqld' => { 'socket' => '/var/lib/mysql/mysql.sock' } } }}
+          it do
+            is_expected.to contain_file('mysql-config-file').with({
+              :mode                    => '0644',
+              :selinux_ignore_defaults => true,
+            }).with_content(/socket = \/var\/lib\/mysql\/mysql.sock/)
           end
+        end
 
-          describe 'ssl set to true' do
-            let(:params) {{ :override_options => { 'mysqld' => { 'ssl' => true }}}}
-            it { is_expected.to contain_file('mysql-config-file').with_content(/ssl/) }
-            it { is_expected.to contain_file('mysql-config-file').without_content(/ssl = true/) }
+        describe 'array entry' do
+          let(:params) {{ :override_options => { 'mysqld' => { 'replicate-do-db' => ['base1', 'base2'], } }}}
+          it do
+            is_expected.to contain_file('mysql-config-file').with_content(
+              /.*replicate-do-db = base1\nreplicate-do-db = base2.*/
+            )
           end
+        end
 
-          describe 'ssl set to false' do
-            let(:params) {{ :override_options => { 'mysqld' => { 'ssl' => false }}}}
-            it { is_expected.to contain_file('mysql-config-file').with_content(/ssl = false/) }
-          end
+        describe 'ssl set to true' do
+          let(:params) {{ :override_options => { 'mysqld' => { 'ssl' => true }}}}
+          it { is_expected.to contain_file('mysql-config-file').with_content(/ssl/) }
+          it { is_expected.to contain_file('mysql-config-file').without_content(/ssl = true/) }
+        end
 
-          # ssl-disable (and ssl) are special cased within mysql.
-          describe 'possibility of disabling ssl completely' do
-            let(:params) {{ :override_options => { 'mysqld' => { 'ssl' => true, 'ssl-disable' => true }}}}
-            it { is_expected.to contain_file('mysql-config-file').without_content(/ssl = true/) }
-          end
+        describe 'ssl set to false' do
+          let(:params) {{ :override_options => { 'mysqld' => { 'ssl' => false }}}}
+          it { is_expected.to contain_file('mysql-config-file').with_content(/ssl = false/) }
+        end
 
-          describe 'a non ssl option set to true' do
-            let(:params) {{ :override_options => { 'mysqld' => { 'test' => true }}}}
-            it { is_expected.to contain_file('mysql-config-file').with_content(/^test$/) }
-            it { is_expected.to contain_file('mysql-config-file').without_content(/test = true/) }
-          end
+        # ssl-disable (and ssl) are special cased within mysql.
+        describe 'possibility of disabling ssl completely' do
+          let(:params) {{ :override_options => { 'mysqld' => { 'ssl' => true, 'ssl-disable' => true }}}}
+          it { is_expected.to contain_file('mysql-config-file').without_content(/ssl = true/) }
+        end
 
-          context 'with includedir' do
-            let(:params) {{ :includedir => '/etc/my.cnf.d' }}
-            it 'makes the directory' do
-              is_expected.to contain_file('/etc/my.cnf.d').with({
-                :ensure => :directory,
-                :mode   => '0755',
-              })
-            end
+        describe 'a non ssl option set to true' do
+          let(:params) {{ :override_options => { 'mysqld' => { 'test' => true }}}}
+          it { is_expected.to contain_file('mysql-config-file').with_content(/^test$/) }
+          it { is_expected.to contain_file('mysql-config-file').without_content(/test = true/) }
+        end
 
-            it { is_expected.to contain_file('mysql-config-file').with_content(/!includedir/) }
+        context 'with includedir' do
+          let(:params) {{ :includedir => '/etc/my.cnf.d' }}
+          it 'makes the directory' do
+            is_expected.to contain_file('/etc/my.cnf.d').with({
+              :ensure => :directory,
+              :mode   => '0755',
+            })
           end
 
-          context 'without includedir' do
-            let(:params) {{ :includedir => '' }}
-            it 'shouldnt contain the directory' do
-              is_expected.not_to contain_file('mysql-config-file').with({
-                :ensure => :directory,
-                :mode   => '0755',
-              })
-            end
+          it { is_expected.to contain_file('mysql-config-file').with_content(/!includedir/) }
+        end
 
-            it { is_expected.to contain_file('mysql-config-file').without_content(/!includedir/) }
+        context 'without includedir' do
+          let(:params) {{ :includedir => '' }}
+          it 'shouldnt contain the directory' do
+            is_expected.not_to contain_file('mysql-config-file').with({
+              :ensure => :directory,
+              :mode   => '0755',
+            })
           end
+
+          it { is_expected.to contain_file('mysql-config-file').without_content(/!includedir/) }
         end
       end
     end

spec/classes/mysql_bindings_spec.rb

@@ -1,30 +1,32 @@
 require 'spec_helper'
 
 describe 'mysql::bindings' do
-  on_pe_supported_platforms(PLATFORMS).each do |pe_version,pe_platforms|
-    pe_platforms.each do |pe_platform,facts|
-      describe "on #{pe_version} #{pe_platform}" do
-        let(:facts) { facts }
+  on_supported_os.each do |os, facts|
+    context "on #{os}" do
+      let(:facts) {
+        facts.merge({
+          :root_home => '/root',
+        })
+      }
 
-        let(:params) {{
-          'java_enable'             => true,
-          'perl_enable'             => true,
-          'php_enable'              => true,
-          'python_enable'           => true,
-          'ruby_enable'             => true,
-          'client_dev'              => true,
-          'daemon_dev'              => true,
-          'client_dev_package_name' => 'libmysqlclient-devel',
-          'daemon_dev_package_name' => 'mysql-devel',
-        }}
+      let(:params) {{
+        'java_enable'             => true,
+        'perl_enable'             => true,
+        'php_enable'              => true,
+        'python_enable'           => true,
+        'ruby_enable'             => true,
+        'client_dev'              => true,
+        'daemon_dev'              => true,
+        'client_dev_package_name' => 'libmysqlclient-devel',
+        'daemon_dev_package_name' => 'mysql-devel',
+      }}
 
-        it { is_expected.to contain_package('mysql-connector-java') }
-        it { is_expected.to contain_package('perl_mysql') }
-        it { is_expected.to contain_package('python-mysqldb') }
-        it { is_expected.to contain_package('ruby_mysql') }
-        it { is_expected.to contain_package('mysql-client_dev') }
-        it { is_expected.to contain_package('mysql-daemon_dev') }
-      end
+      it { is_expected.to contain_package('mysql-connector-java') }
+      it { is_expected.to contain_package('perl_mysql') }
+      it { is_expected.to contain_package('python-mysqldb') }
+      it { is_expected.to contain_package('ruby_mysql') }
+      it { is_expected.to contain_package('mysql-client_dev') }
+      it { is_expected.to contain_package('mysql-daemon_dev') }
     end
   end
 end

spec/classes/mysql_client_spec.rb

@@ -1,36 +1,38 @@
 require 'spec_helper'
 
 describe 'mysql::client' do
-  on_pe_supported_platforms(PLATFORMS).each do |pe_version,pe_platforms|
-    pe_platforms.each do |pe_platform,facts|
-      describe "on #{pe_version} #{pe_platform}" do
-        let(:facts) { facts }
-
-        context 'with defaults' do
-          it { is_expected.not_to contain_class('mysql::bindings') }
-          it { is_expected.to contain_package('mysql_client') }
-        end
-
-        context 'with bindings enabled' do
-          let(:params) {{ :bindings_enable => true }}
+  on_supported_os.each do |os, facts|
+    context "on #{os}" do
+      let(:facts) {
+        facts.merge({
+          :root_home => '/root',
+        })
+      }
+
+      context 'with defaults' do
+        it { is_expected.not_to contain_class('mysql::bindings') }
+        it { is_expected.to contain_package('mysql_client') }
+      end
 
-          it { is_expected.to contain_class('mysql::bindings') }
-          it { is_expected.to contain_package('mysql_client') }
-        end
+      context 'with bindings enabled' do
+        let(:params) {{ :bindings_enable => true }}
 
-        context 'with package_manage set to true' do
-          let(:params) {{ :package_manage => true }}
+        it { is_expected.to contain_class('mysql::bindings') }
+        it { is_expected.to contain_package('mysql_client') }
+      end
 
-          it { is_expected.to contain_package('mysql_client') }
-        end
+      context 'with package_manage set to true' do
+        let(:params) {{ :package_manage => true }}
 
-        context 'with package_manage set to false' do
-          let(:params) {{ :package_manage => false }}
+        it { is_expected.to contain_package('mysql_client') }
+      end
 
-          it { is_expected.not_to contain_package('mysql_client') }
-        end
+      context 'with package_manage set to false' do
+        let(:params) {{ :package_manage => false }}
 
+        it { is_expected.not_to contain_package('mysql_client') }
       end
+
     end
   end
 end

spec/classes/mysql_server_account_security_spec.rb

@@ -1,10 +1,16 @@
 require 'spec_helper'
 
 describe 'mysql::server::account_security' do
-  on_pe_supported_platforms(PLATFORMS).each do |pe_version,pe_platforms|
-    pe_platforms.each do |pe_platform,facts|
-      describe "on #{pe_version} #{pe_platform}" do
-        let(:facts) { facts.merge({:fqdn => 'myhost.mydomain', :hostname => 'myhost'}) }
+  on_supported_os.each do |os, facts|
+    context "on #{os}" do
+      context "with fqdn==myhost.mydomain" do
+        let(:facts) {
+          facts.merge({
+            :root_home => '/root',
+            :fqdn => 'myhost.mydomain',
+            :hostname => 'myhost',
+          })
+        }
 
         [ '[email protected]',
           '[email protected]',
@@ -32,8 +38,14 @@
         end
       end
 
-      describe "on #{pe_version} #{pe_platform} with fqdn==localhost" do
-        let(:facts) { facts.merge({:fqdn => 'localhost', :hostname => 'localhost'}) }
+      context "with fqdn==localhost" do
+        let(:facts) {
+          facts.merge({
+            :root_home => '/root',
+            :fqdn => 'localhost',
+            :hostname => 'localhost',
+          })
+        }
 
         [ '[email protected]',
           'root@::1',
@@ -48,8 +60,14 @@
         end
       end
 
-      describe "on #{pe_version} #{pe_platform} with fqdn==localhost.localdomain" do
-        let(:facts) { facts.merge({:fqdn => 'localhost.localdomain', :hostname => 'localhost'}) }
+      context "with fqdn==localhost.localdomain" do
+        let(:facts) {
+          facts.merge({
+            :root_home => '/root',
+            :fqdn => 'localhost.localdomain',
+            :hostname => 'localhost',
+          })
+        }
 
         [ '[email protected]',
           'root@::1',