Please review pull request #187: Ticket/master/4561 add structured data opened by (kbarber)

Description:

THIS COMMIT IS NOT READY TO MERGE

This pull request is for early code review so I can get some feedback on direction and implementation for adding structured facts to facter. Feel free to try this code out for yourself, and comment on the code if you think I'm doing something wrong.

I've supplied here a number of sample facts showing off how structured facts might be used that are primarily aimed at the Mac platform:

  • arp_table
  • active_ip
  • testfact
  • testconfinedfact
  • network_interfaces

These facts will probably not make it into this release, as we are targeting core facts to be structured at a later date.

  • Opened: Tue Mar 13 06:34:01 UTC 2012
  • Based on: puppetlabs:master (1e78602b38ae53e02ff9cfdba9cf5b33792ec0c7)
  • Requested merge: kbarber:ticket/master/4561-add_structured_data (19aca065f72d407c1ea11d5f789b12ca3d2aa9ae)

Diff follows:

diff --git a/.gitignore b/.gitignore
new file mode 100644
index 0000000..9b64ae8
--- /dev/null
+++ b/.gitignore
@@ -0,0 +1,2 @@
+doc/
+.*.swp
diff --git a/bin/facter b/bin/facter
index 6c5d4ce..a6874a1 100755
--- a/bin/facter
+++ b/bin/facter
@@ -6,7 +6,7 @@
 #
 # = Usage
 #
-#   facter [-d|--debug] [-h|--help] [-p|--puppet] [-v|--version] [-y|--yaml] [-j|--json] [fact] [fact] [...]
+#   facter [-d|--debug] [-h|--help] [-p|--puppet] [-v|--version] [-y|--yaml] [-j|--json] [-n|--nocolor] [fact] [fact] [...]
 #
 # = Description
 #
@@ -42,6 +42,9 @@
 # timing::
 #   Enable timing.
 #
+# nocolor::
+#   Disable colored output.
+#
 # = Example
 #
 #   facter kernel
diff --git a/lib/facter.rb b/lib/facter.rb
index c5b1167..8f09c1d 100644
--- a/lib/facter.rb
+++ b/lib/facter.rb
@@ -21,11 +21,12 @@ module Util; end
   require 'facter/util/fact'
   require 'facter/util/collection'
   require 'facter/util/monkey_patches'
+  require 'facter/util/text'
 
   include Comparable
   include Enumerable
 
-  FACTERVERSION = '1.6.6'
+  FACTERVERSION = '2.0.0'
   # = Facter
   # Functions as a hash of 'facts' you might care about about your
   # system, such as mac address, IP address, Video card, etc.
@@ -39,11 +40,10 @@ module Util; end
   # puts Facter['operatingsystem']
   #
 
-  GREEN = " [0;32m"
-  RESET = " [0m"
   @@debug = 0
   @@timing = 0
   @@messages = {}
+  @@color = 1
 
   # module methods
 
@@ -65,7 +65,8 @@ def self.debug(string)
       return
     end
     if self.debugging?
-      puts GREEN + string + RESET
+      t = Facter::Util::Text.new
+      puts t.green + string + t.reset
     end
   end
 
@@ -75,7 +76,8 @@ def self.debugging?
 
   # show the timing information
   def self.show_time(string)
-    puts "#{GREEN}#{string}#{RESET}" if string and Facter.timing?
+    t = Facter::Util::Text.new
+    puts "#{t.green}#{string}#{t.reset}" if string and Facter.timing?
   end
 
   def self.timing?
@@ -164,44 +166,52 @@ def self.clear_messages
 
   # Set debugging on or off.
   def self.debugging(bit)
+    @@debug = bitcheck(bit)
+  end
+
+  # Set timing on or off.
+  def self.timing(bit)
+    @@timing = bitcheck(bit)
+  end
+
+  # Turn console color support on or off.
+  def self.color(bit)
+    @@color = bitcheck(bit)
+  end
+
+  # Return true if color support is available and switched on.
+  def self.color?
+    if Facter::Util::Config.is_windows?
+      false
+    else
+      @@color != 0
+    end
+  end
+
+  # This convenience method allows us to support the bit setting methodology
+  # for global facter settings.
+  def self.bitcheck(bit)
     if bit
       case bit
-      when TrueClass; @@debug = 1
-      when FalseClass; @@debug = 0
+      when TrueClass; return 1
+      when FalseClass; return 0
       when Fixnum
         if bit > 0
-          @@debug = 1
+          return 1
         else
-          @@debug = 0
+          return 0
         end
       when String;
         if bit.downcase == 'off'
-          @@debug = 0
+          return 0
         else
-          @@debug = 1
+          return 1
         end
       else
-        @@debug = 0
-      end
-    else
-      @@debug = 0
-    end
-  end
-
-  # Set timing on or off.
-  def self.timing(bit)
-    if bit
-      case bit
-      when TrueClass; @@timing = 1
-      when Fixnum
-        if bit > 0
-          @@timing = 1
-        else
-          @@timing = 0
-        end
+        return 0
       end
     else
-      @@timing = 0
+      return 0
     end
   end
 
diff --git a/lib/facter/active_ip.rb b/lib/facter/active_ip.rb
new file mode 100644
index 0000000..5873524
--- /dev/null
+++ b/lib/facter/active_ip.rb
@@ -0,0 +1,43 @@
+Facter.add("active_ip") do
+  confine :kernel => :darwin
+
+  setcode do
+    table = []
+
+    netstat = Facter::Util::Resolution.exec("netstat -anW")
+
+    netstat.each_line do |line|
+      next if line =~ /^Active Internet connections/
+      next if line =~ /^Proto Recv-Q/
+      break if line =~ /^Active LOCAL/
+
+      part = line.split
+
+      next if part[3] == "*.*" and part[4] == "*.*"
+
+      proto = part[0]
+      part[3] =~ /(.+)\.(.+)/
+      local_address = $1
+      local_port = $2
+      part[4] =~ /(.+)\.(.+)/
+      foreign_address = $1
+      foreign_port = $2
+      state = part[5]
+
+      hash = {
+        "proto" => proto,
+        "local_address" => local_address,
+        "local_port" => local_port,
+        "foreign_address" => foreign_address,
+        "foreign_port" => foreign_port,
+      }
+      hash["state"] =state if state
+
+      table << hash
+
+      #break if line =~ /UDP/
+    end
+
+    table
+  end
+end
diff --git a/lib/facter/application.rb b/lib/facter/application.rb
index 3f0f375..eb58123 100644
--- a/lib/facter/application.rb
+++ b/lib/facter/application.rb
@@ -1,3 +1,5 @@
+require 'facter/util/text'
+
 module Facter
   module Application
     def self.run(argv)
@@ -49,12 +51,10 @@ def self.run(argv)
       # name and separated by "=>"
       if facts.length == 1
         if value = facts.values.first
-          puts value
+          pretty_output(value)
         end
       else
-        facts.sort_by{ |fact| fact.first }.each do |name,value|
-          puts "#{name} => #{value}"
-        end
+        facter_output(facts)
       end
 
     rescue => e
@@ -71,12 +71,13 @@ def self.run(argv)
     def self.parse(argv)
       options = {}
       OptionParser.new do |opts|
-        opts.on("-y", "--yaml")   { |v| options[:yaml]   = v }
-        opts.on("-j", "--json")   { |v| options[:json]   = v }
-        opts.on(      "--trace")  { |v| options[:trace]  = v }
-        opts.on("-d", "--debug")  { |v| Facter.debugging(1) }
-        opts.on("-t", "--timing") { |v| Facter.timing(1) }
-        opts.on("-p", "--puppet") { |v| load_puppet }
+        opts.on("-y", "--yaml")    { |v| options[:yaml]    = v }
+        opts.on("-j", "--json")    { |v| options[:json]    = v }
+        opts.on(      "--trace")   { |v| options[:trace]   = v }
+        opts.on("-d", "--debug")   { |v| Facter.debugging(1) }
+        opts.on("-t", "--timing")  { |v| Facter.timing(1) }
+        opts.on("-p", "--puppet")  { |v| load_puppet }
+        opts.on("-n", "--nocolor") { |v| Facter.color(0) }
 
         opts.on_tail("-v", "--version") do
           puts Facter.version
@@ -119,5 +120,84 @@ def self.load_puppet
       $stderr.puts "Could not load Puppet: #{detail}"
     end
 
+    # This provides a wrapper around pretty_output which outputs the top level
+    # keys as puppet-style variables. For example:
+    #
+    #     $key1 = value
+    #     $key2 = [
+    #       'value1',
+    #       'value2',
+    #     ]
+    #
+    def self.facter_output(data)
+      t = Facter::Util::Text.new
+
+      # Line up equals signs
+      size_ordered = data.keys.sort_by {|x| x.length}
+      max_var_length = size_ordered[-1].length
+
+      data.sort.each do |e|
+        k,v = e
+        $stdout.write("#{t.yellow}$#{k}#{t.reset}")
+        indent(max_var_length - k.length, " ")
+        $stdout.write(" = ")
+        pretty_output(v)
+      end
+    end
+
+    # This method returns formatted output (with color where applicable) from
+    # basic types (hashes, arrays, strings, booleans and numbers).
+    def self.pretty_output(data, indent = 0)
+      t = Facter::Util::Text.new
+
+      case data
+      when Hash
+        puts "#{t.magenta}{#{t.reset}"
+        indent = indent+1
+        data.sort.each do |e|
+          k,v = e
+          indent(indent)
+          $stdout.write "#{t.green}\"#{k}\"#{t.reset} => "
+          case v
+          when String,TrueClass,FalseClass
+            pretty_output(v, indent)
+          when Hash,Array
+            pretty_output(v, indent)
+          end
+        end
+        indent(indent-1)
+        puts "#{t.magenta}}#{t.reset}#{tc(indent-1)}"
+      when Array
+        puts "#{t.magenta}[#{t.reset}"
+        indent = indent+1
+        data.each do |e|
+          indent(indent)
+          case e
+          when String,TrueClass,FalseClass
+            pretty_output(e, indent)
+          when Hash,Array
+            pretty_output(e, indent)
+          end
+        end
+        indent(indent-1)
+        puts "#{t.magenta}]#{t.reset}#{tc(indent-1)}"
+      when TrueClass,FalseClass,Numeric
+        puts "#{t.cyan}#{data}#{t.reset}#{tc(indent)}"
+      when String
+        puts "#{t.green}\"#{data}\"#{t.reset}#{tc(indent)}"
+      end
+    end
+
+    # Provide a trailing comma if required
+    def self.tc(indent)
+      indent > 0 ? "," : ""
+    end
+
+    # Return a series of space characters based on a provided indent size and
+    # the number of indents required.
+    def self.indent(num, indent = "  ")
+      num.times { $stdout.write(indent) }
+    end
+
   end
 end
diff --git a/lib/facter/arp_table.rb b/lib/facter/arp_table.rb
new file mode 100644
index 0000000..d38c5cc
--- /dev/null
+++ b/lib/facter/arp_table.rb
@@ -0,0 +1,36 @@
+Facter.add("arp_table") do
+  confine :kernel => :darwin
+
+  setcode do
+    output = Facter::Util::Resolution.exec('arp -an')
+
+    # Prepare top level entries
+    arp_table = []
+
+    output.each_line do |line|
+      values = line.split(" ")
+
+      # Clean up some of the values first
+
+      # Protocol is always last, but the number of items is variable so our
+      # lookup is relative
+      protocol    = values[-1].gsub(/[\[\]]/, '')
+      hostname    = values[0].gsub(/^\?$/, '')
+      ip_address  = values[1].gsub(/[()]/, '')
+      mac_address = values[3].downcase
+
+      # Populate a hash of properties from each line
+      arp_entry = {
+        'mac_address' => mac_address,
+        'interface'   => values[5],
+        'protocol'    => protocol,
+        'hostname'    => hostname,
+        'ip_address'  => ip_address,
+      }
+
+      arp_table << arp_entry
+    end
+
+    arp_table
+  end
+end
diff --git a/lib/facter/network_interfaces.rb b/lib/facter/network_interfaces.rb
new file mode 100644
index 0000000..91ed3c4
--- /dev/null
+++ b/lib/facter/network_interfaces.rb
@@ -0,0 +1,58 @@
+Facter.add("network_interfaces") do
+  confine :kernel => :darwin
+
+  setcode do
+    ifconfig = Facter::Util::Resolution.exec("ifconfig -a")
+
+    interfaces = []
+
+    iface = {}
+    ifconfig.each_line do |line|
+      case line
+      when /^(.+):\s+flags=\d+<(.*)>\s+mtu\s+(\d+)\s*$/
+        # Store the old interface if there is one in the interfaces array
+        interfaces << iface if iface["name"]
+
+        # And now reset, as everything is assigned to the new interface
+        iface = {}
+
+        iface["name"] = $1
+        iface["mtu"] = $3 if $3
+        iface["flags"] = $2.split(",")
+      when /^\s+options=\d+<(.*)>\s*$/
+        raise Exception("Parse error - no interface") unless iface["name"]
+        iface["options"] = $1.split(",")
+      when /^\s+inet6\s+(.+)\s+prefixlen\s+(\d+)(\s+scopeid\s+(.+))\s*$/
+        raise Exception("Parse error - no interface") unless iface["name"]
+        inet6 = {
+          "address"   => $1,
+          "prefixlen" => $2,
+          "scopeid"   => $3,
+        }
+        iface["inet6"] ||= []
+        iface["inet6"] << inet6
+      when /^\s+inet\s+(.+)\s+netmask\s+(.+)\s*$/
+        raise Exception("Parse error - no interface") unless iface["name"]
+        inet4 = {
+          "address" => $1,
+          "netmask" => $2,
+        }
+        iface["inet4"] ||= []
+        iface["inet4"] << inet4
+      when /^\s+ether\s+(.+)\s*$/
+        raise Exception("Parse error - no interface") unless iface["name"]
+        iface["ether"] = $1
+      when /^\s+media:\s+(.+)\s+(.+)\s*$/
+        raise Exception("Parse error - no interface") unless iface["name"]
+        iface["media"] ||= {}
+        iface["media"]["setting"] = $1
+        iface["media"]["real"] = $2
+      when /^\s+status:\s+(.+)\s*$/
+        raise Exception("Parse error - no interface") unless iface["name"]
+        iface["status"] = $1
+      end
+    end
+
+    interfaces
+  end
+end
diff --git a/lib/facter/testconfinedfact.rb b/lib/facter/testconfinedfact.rb
new file mode 100644
index 0000000..3374653
--- /dev/null
+++ b/lib/facter/testconfinedfact.rb
@@ -0,0 +1,10 @@
+Facter.add("testconfinedfact") do
+  # A block confine for confining using structured facts
+  confine do
+    Facter["testfact"].value["test1"].include?("test1")
+  end
+
+  setcode do
+    ["I worked because my confine was true"]
+  end
+end
diff --git a/lib/facter/testfact.rb b/lib/facter/testfact.rb
new file mode 100644
index 0000000..53d522f
--- /dev/null
+++ b/lib/facter/testfact.rb
@@ -0,0 +1,7 @@
+Facter.add("testfact") do
+  setcode do
+    {
+      "test1" => ["test1", "test2"]
+    }
+  end
+end
diff --git a/lib/facter/util/confine_block.rb b/lib/facter/util/confine_block.rb
new file mode 100644
index 0000000..db910eb
--- /dev/null
+++ b/lib/facter/util/confine_block.rb
@@ -0,0 +1,15 @@
+# This provides a block based mechanism for provider resolution confines.
+#
+# This allows us to provide more complex logic then the normal confine provides
+# so we can support structured facts and things like that.
+class Facter::Util::ConfineBlock
+  # Store the block of code for later evaluation
+  def initialize(block)
+    @block = block
+  end
+
+  # Evaluate the fact, returning true or false.
+  def true?
+    @block.call
+  end
+end
diff --git a/lib/facter/util/fact.rb b/lib/facter/util/fact.rb
index d53a688..812a18a 100644
--- a/lib/facter/util/fact.rb
+++ b/lib/facter/util/fact.rb
@@ -59,40 +59,82 @@ def flush
   def value
     return @value if @value
 
-    if @resolves.length == 0
-      Facter.debug "No resolves for %s" % @name
+    suitable_resolves = @resolves.select {|r| r.suitable? }
+
+    if suitable_resolves.length == 0
+      Facter.debug "Found no suitable resolves of %s for %s" %
+        [@resolves.length, @name]
       return nil
+    else
+      searching do
+        suitable_resolves.each do |resolve|
+          val = resolve.value
+          if valid? val
+            @value = val
+            break
+          end
+        end
+      end
     end
 
-    searching do
-      @value = nil
-
-      foundsuits = false
-      @value = @resolves.inject(nil) { |result, resolve|
-        next unless resolve.suitable?
-        foundsuits = true
+    if @value.nil?
+      Facter.debug("Could not resolve fact #{@name}")
+    end
 
-        tmp = resolve.value
+    @value
+  end
 
-        break tmp unless tmp.nil? or tmp == ""
-      }
+  private
 
-      unless foundsuits
-        Facter.debug "Found no suitable resolves of %s for %s" % [@resolves.length, @name]
+  # Validate that an object is either a string, hash, array or
+  # combination thereof.
+  def valid?(value)
+    case value
+    when String,TrueClass,FalseClass,Numeric then
+      return true
+    when Array then
+      validity = true
+      # Validate each item by calling validate again
+      value.each do |item|
+        if (validity = valid?(item)) == false
+          break
+        end
+      end
+      return validity
+    when Hash then
+      validity = true
+      value.each do |k,v|
+        # Validate key first
+        if (validity = validate_value_lhs(k)) == false
+          break
+        end
+
+        # Validate value by calling validate again
+        if (validity = valid?(v)) == false
+          break
+        end
       end
+      return validity
+    else
+      return false
     end
+  end
 
-    if @value.nil?
-      # nothing
-      Facter.debug("value for %s is still nil" % @name)
-      return nil
+  # This method is used for validating the left hand side in
+  # a hash.
+  def validate_value_lhs(value)
+    case value
+    when String
+      if value.empty?
+        return false
+      else
+        return true
+      end
     else
-      return @value
+      return false
     end
   end
 
-  private
-
   # Are we in the midst of a search?
   def searching?
     @searching
diff --git a/lib/facter/util/resolution.rb b/lib/facter/util/resolution.rb
index 7e266c7..d67bee8 100644
--- a/lib/facter/util/resolution.rb
+++ b/lib/facter/util/resolution.rb
@@ -4,6 +4,7 @@
 # confinements specified must all be true for the resolution to be
 # suitable.
 require 'facter/util/confine'
+require 'facter/util/confine_block'
 require 'facter/util/config'
 
 require 'timeout'
@@ -119,10 +120,24 @@ def self.exec(code, interpreter = nil)
 
   end
 
-  # Add a new confine to the resolution mechanism.
-  def confine(confines)
-    confines.each do |fact, values|
-      @confines.push Facter::Util::Confine.new(fact, *values)
+  # Add a new confine to the resolution mechanism. This supports either passing
+  # a hash of key value pairs in the form of:
+  #
+  #   confine :fact => :value_constraint
+  #
+  # Or a block which allows you to provide your own confine logic:
+  #
+  #   confine do
+  #     Facter["operatingsystem"].value =~ /Red/
+  #   end
+  #
+  def confine(confines = [], &block)
+    if(block_given?)
+      @confines.push Facter::Util::ConfineBlock.new(block)
+    else
+      confines.each do |fact, values|
+        @confines.push Facter::Util::Confine.new(fact, *values)
+      end
     end
   end
 
diff --git a/lib/facter/util/text.rb b/lib/facter/util/text.rb
new file mode 100644
index 0000000..b50bb56
--- /dev/null
+++ b/lib/facter/util/text.rb
@@ -0,0 +1,39 @@
+require 'facter'
+
+class Facter::Util::Text
+  def initialize
+    @ansi_colors = {
+      :black   => 0,
+      :red     => 1,
+      :green   => 2,
+      :yellow  => 3,
+      :blue    => 4,
+      :magenta => 5,
+      :cyan    => 6,
+      :white   => 7,
+    }
+  end
+
+  def bright
+    color? ? "\e[1m" : ""
+  end
+
+  def reset
+    color? ? "\e[0m" : ""
+  end
+
+  def method_missing(meth, *args, &block)
+    if @ansi_colors.include?(meth)
+      color_num = @ansi_colors[meth]
+      return color? ? "\e[3#{color_num}m" : ""
+    else
+      super
+    end
+  end
+
+  private
+
+  def color?
+    Facter.color?
+  end
+end
diff --git a/spec/unit/facter_spec.rb b/spec/unit/facter_spec.rb
index c6cedd3..a137442 100755
--- a/spec/unit/facter_spec.rb
+++ b/spec/unit/facter_spec.rb
@@ -208,6 +208,27 @@
     end
   end
 
+  describe "when using bitcheck method" do
+    it "should return true if set to 1" do
+      Facter.bitcheck(1).should == 1
+    end
+    it "should return true if set to true" do
+      Facter.bitcheck(1).should == 1
+    end
+    it "should return true if any string except off" do
+      Facter.bitcheck('aaaaa').should == 1
+    end
+    it "should return false if set to 0" do
+      Facter.bitcheck(0).should be_zero
+    end
+    it "should return false if set to false" do
+      Facter.bitcheck(false).should be_zero
+    end
+    it "should return false if set to off" do
+      Facter.bitcheck('off').should be_zero
+    end
+  end
+
   describe "when setting debugging mode" do
     it "should have debugging enabled using 1" do
       Facter.debugging(1)
@@ -254,6 +275,25 @@
     end
   end
 
+  describe "when setting color mode" do
+    it "should have color enabled using 1" do
+      Facter.color(1)
+      Facter.should be_color
+    end
+    it "should have color enabled using true" do
+      Facter.color(true)
+      Facter.should be_color
+    end
+    it "should have color disabled using 0" do
+      Facter.color(0)
+      Facter.should_not be_color
+    end
+    it "should have color disabled using false" do
+      Facter.color(false)
+      Facter.should_not be_color
+    end
+  end
+
   describe "when registering directories to search" do
     after { Facter.instance_variable_set("@search_path", []) }
 
diff --git a/spec/unit/util/confine_block_spec.rb b/spec/unit/util/confine_block_spec.rb
new file mode 100755
index 0000000..5abd337
--- /dev/null
+++ b/spec/unit/util/confine_block_spec.rb
@@ -0,0 +1,23 @@
+#!/usr/bin/env rspec
+
+require 'spec_helper'
+require 'facter/util/confine_block'
+
+describe Facter::Util::ConfineBlock do
+  it "should accept a block" do
+    Facter::Util::ConfineBlock.new(lambda { true })
+  end
+
+  context "true? method" do
+    it "should return true when block returns true" do
+      confine = Facter::Util::ConfineBlock.new(lambda { true })
+      confine.true?.should be_true
+    end
+
+    it "should return false when block returns false" do
+      confine = Facter::Util::ConfineBlock.new(lambda { false })
+      confine.true?.should be_false
+    end
+  end
+
+end
diff --git a/spec/unit/util/fact_spec.rb b/spec/unit/util/fact_spec.rb
index 24c46e8..18dc74a 100755
--- a/spec/unit/util/fact_spec.rb
+++ b/spec/unit/util/fact_spec.rb
@@ -59,6 +59,65 @@
     Facter::Util::Fact.new("yay").should respond_to(:value)
   end
 
+  describe "when validating fact resolutions" do
+    before do
+      @fact = Facter::Util::Fact.new("yay")
+    end
+
+    [
+      true,
+      false,
+      "string",
+      1.0,
+      1000,
+      true,
+      false,
+      ["yay"],
+      {"test" => "fact"},
+      {"test" => ["fact","value","var"]},
+      {"test" => { "test2" => { "test3" => { "test4" => "value" }}}},
+      {"foo" => [], "bar" => [{}, {}, []]},
+      "",
+      [],
+      {},
+    ].each do |valid|
+      it "should return the valid value \"#{valid.inspect}\"" do
+        r1 = stub 'r1', :suitable? => true, :value => valid
+        Facter::Util::Resolution.expects(:new).returns r1
+        @fact.add { }
+
+        @fact.value.should == valid
+      end
+    end
+
+    [
+      nil,
+      :yay,
+      {:foo => nil},
+      {:foo => ""},
+      {:foo => [], :bar => [{}, {}, []]},
+      Object.new,
+      {"test" => Object.new},
+      [nil, nil, "", :yay],
+      {:foo => "bar"},
+      {:foo => [], :yay => "hi"},
+      {:foo => [false], :yay => "hi"},
+      {"test" => ["fact", { "deep" => Object.new }, "value"] },
+      {"" => "value"},
+      {"" => {"" => "value"}},
+      [Object.new],
+      [{Object.new => "test"}],
+    ].each do |invalid|
+      it "should return nil for the empty type \"#{invalid.inspect}\"" do
+        r1 = stub 'r1', :suitable? => true, :value => invalid
+        Facter::Util::Resolution.expects(:new).returns r1
+        @fact.add { }
+
+        @fact.value.should be_nil
+      end
+    end
+  end
+
   describe "when returning a value" do
     before do
       @fact = Facter::Util::Fact.new("yay")
@@ -99,14 +158,6 @@
 
       @fact.value.should == "yay"
     end
-
-    it "should return nil if the value is the empty string" do
-      r1 = stub 'r1', :suitable? => true, :value => ""
-      Facter::Util::Resolution.expects(:new).returns r1
-      @fact.add { }
-
-      @fact.value.should be_nil
-    end
   end
 
   it "should have a method for flushing the cached fact" do
diff --git a/spec/unit/util/resolution_spec.rb b/spec/unit/util/resolution_spec.rb
index e48e81a..bc8d955 100755
--- a/spec/unit/util/resolution_spec.rb
+++ b/spec/unit/util/resolution_spec.rb
@@ -283,6 +283,13 @@ def handy_method()
     @resolve.weight.should == 2
   end
 
+  it "should include block confines when measuring weight" do
+    @resolve = Facter::Util::Resolution.new("yay")
+    @resolve.confine "one" => "foo", "two" => "fee"
+    @resolve.confine { true }
+    @resolve.weight.should == 3
+  end
+
   it "should return 0 confines when no confines have been added" do
     Facter::Util::Resolution.new("yay").weight.should == 0
   end
@@ -314,6 +321,10 @@ def handy_method()
       lambda { @resolve.confine :_one_ => "two" }.should_not raise_error
     end
 
+    it "should accept a block" do
+      expect { @resolve.confine { true } }.to_not raise_error
+    end
+
     it "should create a Util::Confine instance for every argument in the provided hash" do
       Facter::Util::Confine.expects(:new).with("one", "foo")
       Facter::Util::Confine.expects(:new).with("two", "fee")
@@ -321,6 +332,10 @@ def handy_method()
       @resolve.confine "one" => "foo", "two" => "fee"
     end
 
+    it "should create a Util::ConfineBlock instance when adding a block confine" do
+      Facter::Util::ConfineBlock.expects(:new)
+      @resolve.confine { true }
+    end
   end
 
   describe "when determining suitability" do

    

--
You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to [email protected].
For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.

Reply via email to