Fixing #920 -- I have replaced the existing mount test with an

rspec version.  It's not perfect, in that it only tests
the :ensure state, but that's where 90% of the behaviour is.

5 files changed, 202 insertions, 378 deletions

diff --git a/lib/puppet/property.rb b/lib/puppet/property.rb
index f3d879ee2..fcaa19d48 100644
--- a/lib/puppet/property.rb
+++ b/lib/puppet/property.rb
@@ -158,7 +158,7 @@ class Property < Puppet::Parameter
     def call_valuemethod(name, value)
         event = nil
         if method = self.class.value_option(name, :method) and self.respond_to?(method)
-            self.debug "setting %s (currently %s)" % [value, self.retrieve]
+            #self.debug "setting %s (currently %s)" % [value, self.retrieve]
 
             begin
                 event = self.send(method)
diff --git a/lib/puppet/type/mount.rb b/lib/puppet/type/mount.rb
index 0ad879e4f..7ba411452 100755
--- a/lib/puppet/type/mount.rb
+++ b/lib/puppet/type/mount.rb
@@ -44,15 +44,12 @@ module Puppet
             newvalue(:mounted, :event => :mount_mounted) do
                 # Create the mount point if it does not already exist.
                 current_value = self.retrieve
-                if current_value.nil?  or current_value == :absent 
-                    provider.create
-                end
+                provider.create if current_value.nil? or current_value == :absent 
 
                 syncothers()
+
                 # The fs can be already mounted if it was absent but mounted
-                unless provider.mounted?
-                    provider.mount
-                end
+                provider.mount unless provider.mounted?
             end
 
             def retrieve
@@ -71,11 +68,11 @@ module Puppet
             def syncothers
                 # We have to flush any changes to disk.
                 currentvalues = @resource.retrieve
+
+                # Determine if there are any out-of-sync properties.
                 oos = @resource.send(:properties).find_all do |prop|
                     unless currentvalues.include?(prop)
-                        raise Puppet::DevError, 
-                          "Parent has property %s but it doesn't appear in the current vallues",
-                          [prop.name]
+                        raise Puppet::DevError, "Parent has property %s but it doesn't appear in the current values", [prop.name]
                     end
                     if prop.name == :ensure
                         false
@@ -198,9 +195,7 @@ module Puppet
 
         def refresh
             # Only remount if we're supposed to be mounted.
-            if ens = @parameters[:ensure] and ens.should == :mounted
-                provider.remount
-            end
+            provider.remount if provider.mounted?
         end
 
         def value(name)
diff --git a/spec/lib/monkey_patches/add_confine_and_runnable_to_rspec_dsl.rb b/spec/lib/monkey_patches/add_confine_and_runnable_to_rspec_dsl.rb
index a2467eac8..bfa2a0c3c 100644
--- a/spec/lib/monkey_patches/add_confine_and_runnable_to_rspec_dsl.rb
+++ b/spec/lib/monkey_patches/add_confine_and_runnable_to_rspec_dsl.rb
@@ -1,7 +1,8 @@
 dir = File.expand_path(File.dirname(__FILE__))
-$LOAD_PATH.unshift("#{dir}/../../lib")
-$LOAD_PATH.unshift("#{dir}/../../../lib")
-$LOAD_PATH.unshift("#{dir}/../../../test/lib")  # Add the old test dir, so that we can still find our local mocha and spec
+[ "#{dir}/../../lib", "#{dir}/../../../lib", "#{dir}/../../../test/lib"].each do |dir|
+    fulldir = File.expand_path(dir)
+    $LOAD_PATH.unshift(fulldir) unless $LOAD_PATH.include?(fulldir)
+end
 
 require 'spec'
 require 'puppettest'
@@ -17,7 +18,7 @@ module Spec
           behaviour.run(@options.reporter, @options.dry_run, @options.reverse, @options.timeout)
         end
       end
-		end
+    end
   end
 end
 
diff --git a/spec/unit/ral/types/mount.rb b/spec/unit/ral/types/mount.rb
new file mode 100755
index 000000000..afe798041
--- /dev/null
+++ b/spec/unit/ral/types/mount.rb
@@ -0,0 +1,189 @@
+#!/usr/bin/env ruby
+
+require File.dirname(__FILE__) + '/../../../spec_helper'
+
+require 'puppet/type/mount'
+
+describe Puppet::Type::Mount do
+    it "should have a :refreshable feature that requires the :remount method" do
+        Puppet::Type::Mount.provider_feature(:refreshable).methods.should == [:remount]
+    end
+
+    it "should have no default value" do
+        mount = Puppet::Type::Mount.create(:name => "yay")
+        mount.should(:ensure).should be_nil
+    end
+
+    after { Puppet::Type::Mount.clear }
+end
+
+describe Puppet::Type::Mount, "when validating attributes" do
+    [:name, :remounts].each do |param|
+        it "should have a #{param} parameter" do
+            Puppet::Type::Mount.attrtype(param).should == :param
+        end
+    end
+
+    [:ensure, :device, :blockdevice, :fstype, :options, :pass, :dump, :atboot, :target].each do |param|
+        it "should have a #{param} property" do
+            Puppet::Type::Mount.attrtype(param).should == :property
+        end
+    end
+end
+
+describe Puppet::Type::Mount::Ensure, "when validating values" do
+    before do
+        @provider = stub 'provider', :class => Puppet::Type::Mount.defaultprovider, :clear => nil
+        Puppet::Type::Mount.defaultprovider.expects(:new).returns(@provider)
+    end
+
+    it "should support :present as a value to :ensure" do
+        Puppet::Type::Mount.create(:name => "yay", :ensure => :present)
+    end
+
+    it "should alias :unmounted to :present as a value to :ensure" do
+        mount = Puppet::Type::Mount.create(:name => "yay", :ensure => :unmounted)
+        mount.should(:ensure).should == :present
+    end
+
+    it "should support :absent as a value to :ensure" do
+        Puppet::Type::Mount.create(:name => "yay", :ensure => :absent)
+    end
+
+    it "should support :mounted as a value to :ensure" do
+        Puppet::Type::Mount.create(:name => "yay", :ensure => :mounted)
+    end
+
+    after { Puppet::Type::Mount.clear }
+end
+
+module MountEvaluationTesting
+    def setup
+        @provider = stub 'provider', :class => Puppet::Type::Mount.defaultprovider, :clear => nil, :satisfies? => true, :name => :mock
+        Puppet::Type::Mount.defaultprovider.stubs(:new).returns(@provider)
+        @mount = Puppet::Type::Mount.create(:name => "yay", :check => :ensure)
+
+        @ensure = @mount.property(:ensure)
+    end
+
+    def mount_stub(params)
+        Puppet::Type::Mount.validproperties.each do |prop|
+            unless params[prop]
+                params[prop] = :absent
+                @mount[prop] = :absent
+            end
+        end
+
+        params.each do |param, value|
+            @provider.stubs(param).returns(value)
+        end
+    end
+
+    def teardown
+        Puppet::Type::Mount.clear
+    end
+end
+
+describe Puppet::Type::Mount::Ensure, "when retrieving its current state" do
+    include MountEvaluationTesting
+
+    it "should return the provider's value if it is :absent" do
+        @provider.expects(:ensure).returns(:absent)
+        @ensure.retrieve.should == :absent
+    end
+
+    it "should return :mounted if the provider indicates it is mounted and the value is not :absent" do
+        @provider.expects(:ensure).returns(:present)
+        @provider.expects(:mounted?).returns(true)
+        @ensure.retrieve.should == :mounted
+    end
+
+    it "should return :present if the provider indicates it is not mounted and the value is not :absent" do
+        @provider.expects(:ensure).returns(:present)
+        @provider.expects(:mounted?).returns(false)
+        @ensure.retrieve.should == :present
+    end
+end
+
+describe Puppet::Type::Mount::Ensure, "when changing the host" do
+    include MountEvaluationTesting
+
+    it "should destroy itself if it should be absent" do
+        @provider.stubs(:mounted?).returns(false)
+        @provider.expects(:destroy)
+        @ensure.should = :absent
+        @ensure.sync
+    end
+
+    it "should unmount itself before destroying if it is mounted and should be absent" do
+        @provider.expects(:mounted?).returns(true)
+        @provider.expects(:unmount)
+        @provider.expects(:destroy)
+        @ensure.should = :absent
+        @ensure.sync
+    end
+
+    it "should create itself if it is absent and should be present" do
+        @provider.stubs(:mounted?).returns(false)
+        @provider.expects(:create)
+        @ensure.should = :present
+        @ensure.sync
+    end
+
+    it "should unmount itself if it is mounted and should be present" do
+        @provider.stubs(:mounted?).returns(true)
+
+        # The interface here is just too much work to test right now.
+        @ensure.stubs(:syncothers)
+        @provider.expects(:unmount)
+        @ensure.should = :present
+        @ensure.sync
+    end
+
+    it "should create and mount itself if it does not exist and should be mounted" do
+        @provider.stubs(:ensure).returns(:absent)
+        @provider.stubs(:mounted?).returns(false)
+        @provider.expects(:create)
+        @ensure.stubs(:syncothers)
+        @provider.expects(:mount)
+        @ensure.should = :mounted
+        @ensure.sync
+    end
+
+    it "should mount itself if it is present and should be mounted" do
+        @provider.stubs(:ensure).returns(:present)
+        @provider.stubs(:mounted?).returns(false)
+        @ensure.stubs(:syncothers)
+        @provider.expects(:mount)
+        @ensure.should = :mounted
+        @ensure.sync
+    end
+
+    it "should create but not mount itself if it is absent and mounted and should be mounted" do
+        @provider.stubs(:ensure).returns(:absent)
+        @provider.stubs(:mounted?).returns(true)
+        @ensure.stubs(:syncothers)
+        @provider.expects(:create)
+        @ensure.should = :mounted
+        @ensure.sync
+    end
+end
+
+describe Puppet::Type::Mount, "when responding to events" do
+    include MountEvaluationTesting
+
+    it "should remount if it is currently mounted" do
+        Puppet::Util::Log.newdestination(:console)
+        @provider.expects(:mounted?).returns(true)
+        @provider.expects(:remount)
+
+        @mount.refresh
+    end
+
+    it "should not remount if it is not currently mounted" do
+        @provider.expects(:mounted?).returns(false)
+        @provider.expects(:remount).never
+
+        @mount.refresh
+    end
+end
diff --git a/test/ral/types/mount.rb b/test/ral/types/mount.rb
deleted file mode 100755
index 1119894d0..000000000
--- a/test/ral/types/mount.rb
+++ /dev/null
@@ -1,361 +0,0 @@
-#!/usr/bin/env ruby
-
-require File.dirname(__FILE__) + '/../../lib/puppettest'
-
-require 'puppettest'
-require 'mocha'
-
-unless Facter.value(:operatingsystem) == "Darwin"
-class TestMounts < PuppetTest::TestCase
-	include PuppetTest
-
-    p = Puppet::Type.type(:mount).provide :fake, :parent => PuppetTest::FakeParsedProvider do
-        @name = :fake
-        apimethods :ensure
-
-        attr_accessor :mounted
-
-        def self.default_target
-            :yayness
-        end
-
-        def self.instances
-            []
-        end
-
-        def create
-            @ensure = :present
-            @resource.class.validproperties.each do |property|
-                if value = @resource.should(property)
-                    self.send(property.to_s + "=", value)
-                end
-            end
-        end
-
-        def destroy
-            @ensure = :absent
-        end
-
-        def exists?
-            if defined? @ensure and @ensure == :present
-                true
-            else
-                false
-            end
-        end
-
-        def mounted?
-            @ensure == :mounted
-        end
-
-        def mount
-            @ensure = :mounted
-        end
-
-        def remount
-        end
-
-        def unmount
-            @ensure = :present
-        end
-    end
-
-    FakeMountProvider = p
-
-    @@fakeproviders[:mount] = p
-
-    def setup
-        super
-        @mount = Puppet::Type.type(:mount)
-        @realprovider = @mount.defaultprovider
-        @mount.defaultprovider = FakeMountProvider
-    end
-
-    def teardown
-        Puppet.type(:mount).clear
-        if @realprovider.respond_to?(:clear)
-            @realprovider.clear
-        end
-        Puppet::Type.type(:mount).defaultprovider = nil
-        super
-    end
-
-    def mkmount
-        mount = nil
-
-        assert_nothing_raised {
-            mount = Puppet.type(:mount).create(mkmount_args)
-        }
-
-        return mount
-    end
-
-    def mkmount_args
-        if defined? @pcount
-            @pcount += 1
-        else
-            @pcount = 1
-        end
-        args = {
-            :name => "/fspuppet%s" % @pcount,
-            :device => "/dev/dsk%s" % @pcount,
-        }
-
-        [@mount.validproperties, @mount.parameters].flatten.each do |field|
-            next if [:path, :provider, :target, :ensure, :remounts].include?(field)
-            unless args.include? field
-                args[field] = "fake%s" % @pcount
-            end
-        end
-
-        return args
-    end
-
-    def test_simplemount
-        mount = mkmount
-        mount[:ensure] = :mounted
-
-        assert_apply(mount)
-        mount.send(:properties).each do |property|
-            assert_equal(property.should, mount.provider.send(property.name),
-                "%s was not set to %s" % [property.name, property.should])
-        end
-        assert_events([], mount)
-
-        assert_nothing_raised { mount.retrieve }
-
-        # Now modify a field
-        mount[:dump] = 2
-        mount[:options] = "defaults,ro"
-
-        assert_events([:mount_changed,:mount_changed, :triggered], mount)
-        assert_equal(2, mount.provider.dump, "Changes did not get flushed")
-        assert_equal("defaults,ro", mount.provider.options, "Changes did not get flushed")
-
-        # Now modify a field in addition to change :ensure.
-        mount[:ensure] = :present
-        mount[:options] = "defaults"
-
-        assert_apply(mount)
-        assert(! mount.provider.mounted?, "mount was still mounted")
-        assert_equal("defaults", mount.provider.options)
-
-        # Now remount it and make sure changes get flushed then, too.
-        mount[:ensure] = :mounted
-        mount[:options] = "aftermount"
-
-        assert_apply(mount)
-        assert(mount.provider.mounted?, "mount was not mounted")
-        assert_equal("aftermount", mount.provider.options)
-    end
-
-    # Make sure fs mounting behaves appropriately.  This is more a test of
-    # whether things get mounted and unmounted based on the value of 'ensure'.
-    def test_mountfs
-        obj = mkmount
-
-        assert_apply(obj)
-
-        # Verify we can remove the mount
-        assert_nothing_raised {
-            obj[:ensure] = :absent
-        }
-
-        assert_events([:mount_deleted], obj)
-        assert_events([], obj)
-
-        # And verify it's gone
-        assert(!obj.provider.mounted?, "Object is mounted after being removed")
-
-        assert_nothing_raised {
-            obj[:ensure] = :present
-        }
-
-        assert_events([:mount_created, :triggered], obj)
-        assert_events([], obj)
-
-        assert(! obj.provider.mounted?, "Object is mounted incorrectly")
-
-        assert_nothing_raised {
-            obj[:ensure] = :mounted
-        }
-
-        assert_events([:mount_mounted, :triggered], obj)
-        assert_events([], obj)
-
-        current_values = nil
-        assert_nothing_raised { current_values =  obj.retrieve }
-        assert_equal(:mounted, current_values[obj.property(:ensure)])
-
-        assert_nothing_raised { current_values =  obj.retrieve }
-        assert(obj.provider.mounted?, "Object is not mounted")
-    end
-
-    def test_defaults
-        obj = mkmount
-        args = mkmount_args
-        args.delete(:pass)
-        args.delete(:dump)
-        mount = nil
-
-        assert_nothing_raised {
-            mount = Puppet.type(:mount).create(args)
-        }
-
-        assert_equal(0, mount.should(:pass), "Did not set default for pass")
-        assert_equal(0, mount.should(:dump), "Did not set default for dump")
-    end
-    
-    # Darwin doesn't put its mount table into netinfo
-    unless Facter.value(:operatingsystem) == "Darwin"
-    def test_instances
-        instances = nil
-        assert(@mount.respond_to?(:instances),
-            "No instances method defined for mount")
-
-        assert_nothing_raised do
-            instances = Puppet::Type.type(:mount).instances
-        end
-        
-        assert(instances.length > 0, "Did not return any mounts")
-
-        root = instances.find { |o| o[:name] == "/" }
-        assert(root, "Could not find root root filesystem in instances results")
-  
-        current_values = nil
-        assert_nothing_raised do
-            current_values = root.retrieve
-        end
-
-        assert(current_values[root.property(:device)], "Device was not set")
-    end
-    end
-
-    # Make sure we actually remove the object from the file and such.
-    # Darwin will actually write to netinfo here.
-    if Facter.value(:operatingsystem) != "Darwin" or Process.uid == 0
-    def test_removal
-        # Reset the provider so that we're using the real thing
-        @mount.defaultprovider = nil
-
-        provider = @mount.defaultprovider
-        assert(provider, "Could not retrieve default provider")
-
-        if provider.respond_to?(:default_target)
-            file = provider.default_target
-            assert(FileTest.exists?(file),
-                "FSTab %s does not exist" % file)
-
-            # Now switch to ram, so we're just doing this there, not really on disk.
-            oldtype = provider.filetype
-            provider.filetype = :ram
-            cleanup { provider.filetype = oldtype }
-            #provider.target_object(file).write text
-        end
-
-        mount = mkmount
-
-        mount[:ensure] = :present
-
-        assert_events([:mount_created, :triggered], mount)
-        assert_events([], mount)
-
-        mount[:ensure] = :absent
-        assert_events([:mount_deleted], mount)
-        assert_events([], mount)
-
-        # Now try listing and making sure the object is actually gone.
-        instances = mount.provider.class.instances
-        assert(! instances.find { |r| r[:name] == mount[:name] },
-            "Mount was not actually removed")
-    end
-    end
-
-    # Make sure that the name gets correctly set if they set the path,
-    # which used to be the namevar.
-    def test_name_and_path
-        mount = nil
-        args = mkmount_args
-        args[:name] = "mount_name"
-        args[:path] = "mount_path"
-
-        assert_nothing_raised do
-            mount = @mount.create(args)
-        end
-
-        assert_equal("mount_path", mount[:name], "Name did not get copied over")
-    end
-    
-    def test_refresh
-        mount = mkmount
-        mount[:ensure] = :mounted
-        
-        remounted = false
-        mount.provider.meta_def(:remount) do
-            remounted = true
-        end
-        
-        # First make sure we correctly call the provider
-        assert_nothing_raised do
-            mount.refresh
-        end
-        assert(remounted, "did not call remount on provider")
-        
-        # then make sure it gets called during transactions
-        remounted = false
-        mount[:device] = "/dev/yayness"
-        
-        assert_apply(mount)
-        
-        assert(remounted, "did not remount when mount changed")
-
-        # Now make sure it doesn't remount if the mount is just 'present'
-        mount[:ensure] = :present
-        mount[:device] = "/dev/funtest"
-        remounted = false
-        assert_apply(mount)
-
-        assert(! remounted, "remounted even though not supposed to be mounted")
-    end
-
-    def test_no_default_for_ensure
-        mount = mkmount
-        mount.finish
-
-        assert_nil(mount.should(:ensure), "Found default for ensure")
-    end
-
-    def disabled_test_retrieving_a_single_mount
-        @mount.defaultprovider = nil
-
-        provider = @mount.defaultprovider
-        assert(provider, "Could not retrieve default provider")
-
-        mount = Puppet::Type.type(:mount).create(:name => "/", :check => :all)
-        values = nil
-        assert_nothing_raised("Could not retrieve values for /") do
-            values = mount.retrieve
-        end
-        values.each do |property, value|
-            assert(value != :absent, "Got :absent for %s" % property.name)
-        end
-    end
-
-    # #726 - when filesystems are mounted but absent, Puppet does not write them out.
-    def test_mounted_but_absent
-        mount = @mount.create(:name => "/testing", :ensure => :mounted, :provider => :fake, :device => "/dev/something")
-
-        class << mount.provider
-            def mounted?
-                true
-            end
-        end
-
-        mount.provider.destroy
-        mount.provider.expects(:create)
-        mount.provider.expects(:mount).never
-        assert_apply(mount)
-    end
-end
-end
-