Fix bug in ensure => absent
authorNate Butler <nathan.butler@gmail.com>
Thu, 1 Sep 2016 19:47:27 +0000 (15:47 -0400)
committerNate Butler <nathan.butler@gmail.com>
Thu, 1 Sep 2016 19:54:31 +0000 (15:54 -0400)
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
spec/unit/puppet/type/vcsrepo_spec.rb [new file with mode: 0755]

index e2ef0b7..f35bb18 100644 (file)
@@ -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 (executable)
index 0000000..0070f49
--- /dev/null
@@ -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