From 5e46486d3ea2d15984edfb28875471436f466a65 Mon Sep 17 00:00:00 2001 From: Nate Butler Date: Thu, 1 Sep 2016 15:47:27 -0400 Subject: Fix bug in ensure => absent The insync? method wasn't accounting for a condition where the state was absent and the desired state was absent. This manifest itself in puppet runs that would constantly output the following after setting ensure => absent. 2016-09-01T16:39:27.314283+00:00 vcstest puppet-agent[1161]: (/Stage[main]/Main/Node[vcstest]/Vcsrepo[/home/vagrant/test]/ensure) created 2016-09-01T16:40:22.583125+00:00 vcstest puppet-agent[1727]: (/Stage[main]/Main/Node[vcstest]/Vcsrepo[/home/vagrant/test]/ensure) created 2016-09-01T16:41:04.031750+00:00 vcstest puppet-agent[2267]: (/Stage[main]/Main/Node[vcstest]/Vcsrepo[/home/vagrant/test]/ensure) created 2016-09-01T16:42:51.779816+00:00 vcstest puppet-agent[2911]: (/Stage[main]/Main/Node[vcstest]/Vcsrepo[/home/vagrant/test]/ensure) created 2016-09-01T16:43:42.189035+00:00 vcstest puppet-agent[3466]: (/Stage[main]/Main/Node[vcstest]/Vcsrepo[/home/vagrant/test]/ensure) created I added unit tests for the vcsrepo type and then fixed the code. --- lib/puppet/type/vcsrepo.rb | 2 + spec/unit/puppet/type/vcsrepo_spec.rb | 116 ++++++++++++++++++++++++++++++++++ 2 files changed, 118 insertions(+) create mode 100755 spec/unit/puppet/type/vcsrepo_spec.rb diff --git a/lib/puppet/type/vcsrepo.rb b/lib/puppet/type/vcsrepo.rb index e2ef0b7..f35bb18 100644 --- a/lib/puppet/type/vcsrepo.rb +++ b/lib/puppet/type/vcsrepo.rb @@ -71,6 +71,8 @@ Puppet::Type.newtype(:vcsrepo) do return is == :bare when :mirror return is == :mirror + when :absent + return is == :absent end end diff --git a/spec/unit/puppet/type/vcsrepo_spec.rb b/spec/unit/puppet/type/vcsrepo_spec.rb new file mode 100755 index 0000000..0070f49 --- /dev/null +++ b/spec/unit/puppet/type/vcsrepo_spec.rb @@ -0,0 +1,116 @@ +#! /usr/bin/env ruby +require 'spec_helper' + +describe Puppet::Type.type(:vcsrepo) do + + before :each do + Puppet::Type.type(:vcsrepo).stubs(:defaultprovider).returns(providerclass) + end + + let(:providerclass) do + described_class.provide(:fake_vcsrepo_provider) do + attr_accessor :property_hash + def create; end + def destroy; end + def exists? + get(:ensure) != :absent + end + mk_resource_methods + end + end + + let(:provider) do + providerclass.new(:name => 'fake-vcs') + end + + let(:resource) do + described_class.new(:name => '/repo', + :ensure => :present, + :provider => provider) + end + + let(:ensureprop) do + resource.property(:ensure) + end + + properties = [ :ensure ] + + properties.each do |property| + it "should have a #{property} property" do + expect(described_class.attrclass(property).ancestors).to be_include(Puppet::Property) + end + end + + parameters = [ :ensure ] + + parameters.each do |parameter| + it "should have a #{parameter} parameter" do + expect(described_class.attrclass(parameter).ancestors).to be_include(Puppet::Parameter) + end + end + + describe 'default resource with required params' do + it 'should have a valid name parameter' do + expect(resource[:name]).to eq('/repo') + end + + it 'should have ensure set to present' do + expect(resource[:ensure]).to eq(:present) + end + + it 'should have path set to /repo' do + expect(resource[:path]).to eq('/repo') + end + + defaults = { + :owner => nil, + :group => nil, + :user => nil, + :revision => nil, + } + + defaults.each_pair do |param, value| + it "should have #{param} parameter set to #{value}" do + expect(resource[param]).to eq(value) + end + end + end + + describe 'when changing the ensure' do + it 'should be in sync if it is :absent and should be :absent' do + ensureprop.should = :absent + expect(ensureprop.safe_insync?(:absent)).to eq(true) + end + + it 'should be in sync if it is :present and should be :present' do + ensureprop.should = :present + expect(ensureprop.safe_insync?(:present)).to eq(true) + end + + it 'should be out of sync if it is :absent and should be :present' do + ensureprop.should = :present + expect(ensureprop.safe_insync?(:absent)).not_to eq(true) + end + + it 'should be out of sync if it is :present and should be :absent' do + ensureprop.should = :absent + expect(ensureprop.safe_insync?(:present)).not_to eq(true) + end + end + + describe 'when running the type it should autorequire packages' do + before :each do + @catalog = Puppet::Resource::Catalog.new + ['git', 'git-core', 'mercurial'].each do |pkg| + @catalog.add_resource(Puppet::Type.type(:package).new(:name => pkg)) + end + end + + it 'should require package packages' do + @resource = described_class.new(:name => '/foo', :provider => provider) + @catalog.add_resource(@resource) + req = @resource.autorequire + expect(req.size).to eq(3) + end + end +end -- cgit v1.2.3