[Pkg-puppet-devel] [facter] 242/352: (FACT-239) Move resolution interaction to facts

[Stig Sandbeck Mathisen]

This is an automated email from the git hooks/post-receive script.

ssm pushed a commit to branch master
in repository facter.

commit b53427303bba400bf27270eeefb818efb1897bba
Author: Adrien Thebo <git at somethingsinistral.net>
Date:   Fri Jan 31 12:42:15 2014 -0800

    (FACT-239) Move resolution interaction to facts
    
    Facter::Util::Collection never directly interacts with resolutions, so
    it doesn't make a lot of sense for the collection to be setting options
    on resolution instances. This commit pushes the last of that
    entanglement into Util::Fact, which does interact with resolutions.
---
 lib/facter/util/collection.rb     | 11 +----------
 lib/facter/util/fact.rb           | 26 +++++++++++---------------
 spec/unit/util/collection_spec.rb |  9 ++-------
 spec/unit/util/fact_spec.rb       |  4 ++--
 4 files changed, 16 insertions(+), 34 deletions(-)

diff --git a/lib/facter/util/collection.rb b/lib/facter/util/collection.rb
index abf83b1..aac876f 100644
--- a/lib/facter/util/collection.rb
+++ b/lib/facter/util/collection.rb
@@ -47,16 +47,7 @@ class Facter::Util::Collection
   def add(name, options = {}, &block)
     fact = create_or_return_fact(name, options)
 
-    if block_given?
-      resolve = fact.add(&block)
-    else
-      resolve = fact.add
-    end
-
-    # Set any resolve-appropriate options
-    if resolve
-      resolve.set_options(options)
-    end
+    fact.add(options, &block)
 
     return fact
   end
diff --git a/lib/facter/util/fact.rb b/lib/facter/util/fact.rb
index baec577..38d0418 100644
--- a/lib/facter/util/fact.rb
+++ b/lib/facter/util/fact.rb
@@ -40,38 +40,34 @@ class Facter::Util::Fact
   # block, which will then be evaluated in the context of the new
   # resolution.
   #
-  # @return [void]
+  # @param options [Hash] A hash of options to set on the resolution
+  #
+  # @return [Facter::Util::Resolution]
   #
   # @api private
-  def add(value = nil, &block)
-    begin
-      resolve = Facter::Util::Resolution.new(@name, self)
-
-      resolve.evaluate(&block) if block
-      @resolves << resolve
-
-      resolve
-    rescue => e
-      Facter.warn "Unable to add resolve for #{@name}: #{e}"
-      nil
-    end
+  def add(options = {}, &block)
+    define_resolution(nil, options, &block)
   end
 
   # Define a new named resolution or return an existing resolution with
   # the given name.
   #
   # @param resolve_name [String] The name of the resolve to define or look up
-  # @return [void]
+  # @param options [Hash] A hash of options to set on the resolution
+  # @return [Facter::Util::Resolution]
   #
   # @api public
-  def define_resolution(resolve_name, &block)
+  def define_resolution(resolve_name, options = {}, &block)
     resolve = self.resolution(resolve_name)
 
     if resolve.nil?
       resolve = Facter::Util::Resolution.new(resolve_name, self)
+      resolve.set_options(options) unless options.empty?
+
       resolve.evaluate(&block) if block
       @resolves << resolve
     else
+      resolve.set_options(options) unless options.empty?
       resolve.evaluate(&block) if block
     end
 
diff --git a/spec/unit/util/collection_spec.rb b/spec/unit/util/collection_spec.rb
index 3b975d1..78591f4 100755
--- a/spec/unit/util/collection_spec.rb
+++ b/spec/unit/util/collection_spec.rb
@@ -30,20 +30,15 @@ describe Facter::Util::Collection do
       collection.add(:myname, :timeout => 1) { }
     end
 
-    it "should set appropriate options on the resolution instance" do
+    it "passes resolution specific options to the fact" do
       fact = Facter::Util::Fact.new(:myname)
       Facter::Util::Fact.expects(:new).with(:myname, {:timeout => 'myval'}).returns fact
 
-      resolve = Facter::Util::Resolution.new(:myname, fact) {}
-      fact.expects(:add).returns resolve
+      fact.expects(:add).with({:timeout => 'myval'})
 
       collection.add(:myname, :timeout => "myval") {}
     end
 
-    it "should fail if invalid options are provided" do
-      lambda { collection.add(:myname, :foo => :bar) }.should raise_error(ArgumentError)
-    end
-
     describe "and a block is provided" do
       it "should use the block to add a resolution to the fact" do
         fact = mock 'fact'
diff --git a/spec/unit/util/fact_spec.rb b/spec/unit/util/fact_spec.rb
index 46624a3..8c50e5b 100755
--- a/spec/unit/util/fact_spec.rb
+++ b/spec/unit/util/fact_spec.rb
@@ -47,8 +47,9 @@ describe Facter::Util::Fact do
 
   describe "adding resolution mechanisms by name" do
 
+    let(:res) { stub 'resolution', :name => 'named', :set_options => nil }
+
     it "creates a new resolution if no such resolution exists" do
-      res = stub 'resolution', :name => 'named'
       Facter::Util::Resolution.expects(:new).once.with('named', fact).returns(res)
 
       fact.define_resolution('named')
@@ -57,7 +58,6 @@ describe Facter::Util::Fact do
     end
 
     it "returns existing resolutions by name" do
-      res = stub 'resolution', :name => 'named'
       Facter::Util::Resolution.expects(:new).once.with('named', fact).returns(res)
 
       fact.define_resolution('named')

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-puppet/facter.git