Splitting the different classes in Facter up, and adding some tests.

The Confine and Resolution classes are now in separate files, and I've
got tests for Confine.

5 files changed, 297 insertions, 226 deletions

diff --git a/lib/facter.rb b/lib/facter.rb
index 65ea57b..675b95d 100644
--- a/lib/facter.rb
+++ b/lib/facter.rb
@@ -18,6 +18,8 @@
 #--
 
 class Facter
+    require 'facter/resolution'
+
     include Comparable
     include Enumerable
 
@@ -376,209 +378,6 @@ class Facter
         end
     end
 
-    # An actual fact resolution mechanism.  These are largely just chunks of
-    # code, with optional confinements restricting the mechanisms to only working on
-    # specific systems.  Note that the confinements are always ANDed, so any
-    # confinements specified must all be true for the resolution to be
-    # suitable.
-    class Resolution
-        attr_accessor :interpreter, :code, :name, :fact
-
-        def Resolution.have_which
-            if @have_which.nil?
-                %x{which which 2>/dev/null}
-                @have_which = ($? == 0)
-            end
-            @have_which
-        end
-
-        # Execute a chunk of code.
-        def Resolution.exec(code, interpreter = "/bin/sh")
-            if interpreter == "/bin/sh"
-                binary = code.split(/\s+/).shift
-
-                if have_which
-                    path = nil
-                    if binary !~ /^\//
-                        path = %x{which #{binary} 2>/dev/null}.chomp
-                        if path == ""
-                            # we don't have the binary necessary
-                            return nil
-                        end
-                    else
-                        path = binary
-                    end
-
-                    unless FileTest.exists?(path)
-                        # our binary does not exist
-                        return nil
-                    end
-                end
-
-                out = nil
-                begin
-                    out = %x{#{code}}.chomp
-                rescue => detail
-                    $stderr.puts detail
-                    return nil
-                end
-                if out == ""
-                    return nil
-                else
-                    return out
-                end
-            else
-                raise ArgumentError,
-                    "non-sh interpreters are not currently supported"
-            end
-        end
-
-        # Add a new confine to the resolution mechanism.
-        def confine(*args)
-            if args[0].is_a? Hash
-                args[0].each do |fact, values|
-                    @confines.push Confine.new(fact,*values)
-                end
-            else
-                fact = args.shift
-                @confines.push Confine.new(fact,*args)
-            end
-        end
-
-        # Create a new resolution mechanism.
-        def initialize(name)
-            @name = name
-            @confines = []
-            @value = nil
-        end
-
-        # Return the number of confines.
-        def length
-            @confines.length
-        end
-
-        # Set our code for returning a value.
-        def setcode(string = nil, interp = nil, &block)
-            if string
-                @code = string
-                @interpreter = interp || "/bin/sh"
-            else
-                unless block_given?
-                    raise ArgumentError, "You must pass either code or a block"
-                end
-                @code = block
-            end
-        end
-
-        # Set the name by which this parameter is known in LDAP.  The default
-        # is just the fact name.
-        def setldapname(name)
-            @fact.ldapname = name.to_s
-        end
-
-        # Is this resolution mechanism suitable on the system in question?
-        def suitable?
-            unless defined? @suitable
-                @suitable = true
-                if @confines.length == 0
-                    return true
-                end
-                @confines.each { |confine|
-                    unless confine.true?
-                        @suitable = false
-                    end
-                }
-            end
-
-            return @suitable
-        end
-
-        # Set tags on our parent fact.
-        def tag(*values)
-            @fact.tag(*values)
-        end
-
-        def to_s
-            return self.value()
-        end
-
-        # How we get a value for our resolution mechanism.
-        def value
-            value = nil
-
-            if @code.is_a?(Proc)
-                value = @code.call()
-            else
-                unless defined? @interpreter
-                    @interpreter = "/bin/sh"
-                end
-                if @code.nil?
-                    $stderr.puts "Code for %s is nil" % @name
-                else
-                    value = Resolution.exec(@code,@interpreter)
-                end
-            end
-
-            if value == ""
-                value = nil
-            end
-
-            return value
-        end
-
-    end
-
-    # A restricting tag for fact resolution mechanisms.  The tag must be true
-    # for the resolution mechanism to be suitable.
-    class Confine
-        attr_accessor :fact, :op, :value
-
-        # Add the tag.  Requires the fact name, an operator, and the value
-        # we're comparing to.
-        def initialize(fact, *values)
-            fact = fact.to_s if fact.is_a? Symbol
-            @fact = fact
-            @values = values.collect do |value|
-                if value.is_a? String
-                    value
-                else
-                    value.to_s
-                end
-            end
-        end
-
-        def to_s
-            return "'%s' '%s'" % [@fact, @values.join(",")]
-        end
-
-        # Evaluate the fact, returning true or false.
-        def true?
-            fact = nil
-            unless fact = Facter[@fact]
-                Facter.debug "No fact for %s" % @fact
-                return false
-            end
-            value = fact.value
-
-            if value.nil?
-                return false
-            end
-
-            retval = @values.find { |v|
-                if value.downcase == v.downcase
-                    break true
-                end
-            }
-
-            if retval
-                retval = true
-            else
-                retval = false
-            end
-
-            return retval || false
-        end
-    end
 
     # Load all of the default facts
     def self.loadfacts
diff --git a/lib/facter/confine.rb b/lib/facter/confine.rb
new file mode 100644
index 0000000..4d30630
--- /dev/null
+++ b/lib/facter/confine.rb
@@ -0,0 +1,53 @@
+# A restricting tag for fact resolution mechanisms.  The tag must be true
+# for the resolution mechanism to be suitable.
+class Facter::Confine
+    attr_accessor :fact, :values
+
+    # Add the restriction.  Requires the fact name, an operator, and the value
+    # we're comparing to.
+    def initialize(fact, *values)
+        raise ArgumentError, "The fact name must be provided" unless fact
+        raise ArgumentError, "One or more values must be provided" if values.empty?
+        fact = fact.to_s if fact.is_a? Symbol
+        @fact = fact
+        @values = values.collect do |value|
+            if value.is_a? String
+                value
+            else
+                value.to_s
+            end
+        end
+    end
+
+    def to_s
+        return "'%s' '%s'" % [@fact, @values.join(",")]
+    end
+
+    # Evaluate the fact, returning true or false.
+    def true?
+        fact = nil
+        unless fact = Facter[@fact]
+            Facter.debug "No fact for %s" % @fact
+            return false
+        end
+        value = fact.value
+
+        if value.nil?
+            return false
+        end
+
+        retval = @values.find { |v|
+            if value.downcase == v.downcase
+                break true
+            end
+        }
+
+        if retval
+            retval = true
+        else
+            retval = false
+        end
+
+        return retval || false
+    end
+end
diff --git a/lib/facter/resolution.rb b/lib/facter/resolution.rb
new file mode 100644
index 0000000..77d9155
--- /dev/null
+++ b/lib/facter/resolution.rb
@@ -0,0 +1,154 @@
+# An actual fact resolution mechanism.  These are largely just chunks of
+# code, with optional confinements restricting the mechanisms to only working on
+# specific systems.  Note that the confinements are always ANDed, so any
+# confinements specified must all be true for the resolution to be
+# suitable.
+require 'facter/confine'
+
+class Facter::Resolution
+    attr_accessor :interpreter, :code, :name, :fact
+
+    def self.have_which
+        if @have_which.nil?
+            %x{which which 2>/dev/null}
+            @have_which = ($? == 0)
+        end
+        @have_which
+    end
+
+    # Execute a chunk of code.
+    def self.exec(code, interpreter = "/bin/sh")
+        if interpreter == "/bin/sh"
+            binary = code.split(/\s+/).shift
+
+            if have_which
+                path = nil
+                if binary !~ /^\//
+                    path = %x{which #{binary} 2>/dev/null}.chomp
+                    if path == ""
+                        # we don't have the binary necessary
+                        return nil
+                    end
+                else
+                    path = binary
+                end
+
+                unless FileTest.exists?(path)
+                    # our binary does not exist
+                    return nil
+                end
+            end
+
+            out = nil
+            begin
+                out = %x{#{code}}.chomp
+            rescue => detail
+                $stderr.puts detail
+                return nil
+            end
+            if out == ""
+                return nil
+            else
+                return out
+            end
+        else
+            raise ArgumentError,
+                "non-sh interpreters are not currently supported"
+        end
+    end
+
+    # Add a new confine to the resolution mechanism.
+    def confine(*args)
+        if args[0].is_a? Hash
+            args[0].each do |fact, values|
+                @confines.push Facter::Confine.new(fact,*values)
+            end
+        else
+            fact = args.shift
+            @confines.push Facter::Confine.new(fact,*args)
+        end
+    end
+
+    # Create a new resolution mechanism.
+    def initialize(name)
+        @name = name
+        @confines = []
+        @value = nil
+    end
+
+    # Return the number of confines.
+    def length
+        @confines.length
+    end
+
+    # Set our code for returning a value.
+    def setcode(string = nil, interp = nil, &block)
+        if string
+            @code = string
+            @interpreter = interp || "/bin/sh"
+        else
+            unless block_given?
+                raise ArgumentError, "You must pass either code or a block"
+            end
+            @code = block
+        end
+    end
+
+    # Set the name by which this parameter is known in LDAP.  The default
+    # is just the fact name.
+    def setldapname(name)
+        @fact.ldapname = name.to_s
+    end
+
+    # Is this resolution mechanism suitable on the system in question?
+    def suitable?
+        unless defined? @suitable
+            @suitable = true
+            if @confines.length == 0
+                return true
+            end
+            @confines.each { |confine|
+                unless confine.true?
+                    @suitable = false
+                end
+            }
+        end
+
+        return @suitable
+    end
+
+    # Set tags on our parent fact.
+    def tag(*values)
+        @fact.tag(*values)
+    end
+
+    def to_s
+        return self.value()
+    end
+
+    # How we get a value for our resolution mechanism.
+    def value
+        value = nil
+
+        if @code.is_a?(Proc)
+            value = @code.call()
+        else
+            unless defined? @interpreter
+                @interpreter = "/bin/sh"
+            end
+            if @code.nil?
+                $stderr.puts "Code for %s is nil" % @name
+            else
+                value = Facter::Resolution.exec(@code,@interpreter)
+            end
+        end
+
+        if value == ""
+            value = nil
+        end
+
+        return value
+    end
+
+end
+
diff --git a/spec/unit/facter.rb b/spec/unit/facter.rb
index 76b6ab2..8ca160c 100755
--- a/spec/unit/facter.rb
+++ b/spec/unit/facter.rb
@@ -2,10 +2,6 @@
 
 require File.dirname(__FILE__) + '/../spec_helper'
 
-#if __FILE__ == $0
-#    Facter.debugging(true)
-#end
-
 describe Facter do
     def tearhook(&block)
         @tearhooks << block
@@ -28,33 +24,27 @@ describe Facter do
             end
         end
     end
-
-    def test_version 
-        #Could match /[0-9.]+/
-        #Strict match: /^[0-9]+(\.[0-9]+)*$/
-        #ok: 1.0.0 1.0 1
-        #notok: 1..0 1. .1 1a
-        assert(Facter.version =~ /^[0-9]+(\.[0-9]+)*$/ )
+    
+    it "should have a version" do
+        Facter.version.should =~ /^[0-9]+(\.[0-9]+)*$/
     end
 
-    def test_noconfines_sh
-        assert_nothing_raised {
-            Facter.add("testing") do
+    describe "when provided code as a string" do
+        it "should execute the code in the shell" do
+            Facter.add("shell_testing") do
                 setcode "echo yup"
             end
-        }
 
-        assert_equal("yup", Facter["testing"].value)
+            Facter["shell_testing"].value.should == "yup"
+        end
     end
 
-    def test_noconfines
-        assert_nothing_raised {
-            Facter.add("testing") do
-                setcode { "foo" }
-            end
-        }
+    describe "when passed code as a block" do
+        it "should execute the provided block" do
+            Facter.add("block_testing") { setcode { "foo" } }
 
-        assert_equal("foo", Facter["testing"].value)
+            Facter["block_testing"].value.should == "foo"
+        end
     end
 
     def test_onetrueconfine
diff --git a/spec/unit/facter/confine.rb b/spec/unit/facter/confine.rb
new file mode 100755
index 0000000..385f7c6
--- /dev/null
+++ b/spec/unit/facter/confine.rb
@@ -0,0 +1,75 @@
+#!/usr/bin/env ruby
+
+require File.dirname(__FILE__) + '/../../spec_helper'
+
+require 'facter/confine'
+
+describe Facter::Confine do
+    it "should require a fact name" do
+        Facter::Confine.new("yay", true).fact.should == "yay"
+    end
+
+    it "should accept a value specified individually" do
+        Facter::Confine.new("yay", "test").values.should == ["test"]
+    end
+
+    it "should accept multiple values specified at once" do
+        Facter::Confine.new("yay", "test", "other").values.should == ["test", "other"]
+    end
+
+    it "should convert all values to strings" do
+        Facter::Confine.new("yay", :test).values.should == %w{test}
+    end
+
+    it "should fail if no fact name is provided" do
+        lambda { Facter::Confine.new(nil, :test) }.should raise_error(ArgumentError)
+    end
+
+    it "should fail if no values were provided" do
+        lambda { Facter::Confine.new("yay") }.should raise_error(ArgumentError)
+    end
+
+    it "should have a method for testing whether it matches" do
+        Facter::Confine.new("yay", :test).should respond_to(:true?)
+    end
+
+    describe "when evaluating" do
+        before do
+            @confine = Facter::Confine.new("yay", "one", "two")
+            @fact = mock 'fact'
+            Facter.stubs(:[]).returns @fact
+        end
+
+        it "should return false if the fact does not exist" do
+            Facter.expects(:[]).with("yay").returns nil
+
+            @confine.true?.should be_false
+        end
+
+        it "should use the returned fact to get the value" do
+            Facter.expects(:[]).with("yay").returns @fact
+
+            @fact.expects(:value).returns nil
+
+            @confine.true?
+        end
+
+        it "should return false if the fact has no value" do
+            @fact.stubs(:value).returns nil
+
+            @confine.true?.should be_false
+        end
+
+        it "should return true if any of the provided values matches the fact's value" do
+            @fact.stubs(:value).returns "two"
+
+            @confine.true?.should be_true
+        end
+
+        it "should return false if none of the provided values matches the fact's value" do
+            @fact.stubs(:value).returns "three"
+
+            @confine.true?.should be_false
+        end
+    end
+end