Fixing the provider's confine subsystem so the logs are more useful.

I apparently lost some context in these logs when I switched to this
separate subsystem.

Note that this also fixes some of the informational issues in

Signed-off-by: Luke Kanies <luke@madstop.com>

3 files changed, 48 insertions, 24 deletions

diff --git a/spec/unit/provider/confine.rb b/spec/unit/provider/confine.rb
index 867b6e6be..626f79b27 100755
--- a/spec/unit/provider/confine.rb
+++ b/spec/unit/provider/confine.rb
@@ -34,7 +34,10 @@ describe Puppet::Provider::Confine do
     end
 
     describe "when testing all values" do
-        before { @confine = Puppet::Provider::Confine.new(%w{a b c}) }
+        before do
+            @confine = Puppet::Provider::Confine.new(%w{a b c})
+            @confine.label = "foo"
+        end
 
         it "should be invalid if any values fail" do
             @confine.stubs(:pass?).returns true
@@ -51,6 +54,14 @@ describe Puppet::Provider::Confine do
             @confine.expects(:pass?).once.returns false
             @confine.valid?
         end
+
+        it "should log failing confines with the label and message" do
+            @confine.stubs(:pass?).returns false
+            @confine.expects(:message).returns "My message"
+            @confine.expects(:label).returns "Mylabel"
+            Puppet.expects(:debug).with("Mylabel: My message")
+            @confine.valid?
+        end
     end
 
     describe "when testing the result of the values" do
diff --git a/spec/unit/provider/confine_collection.rb b/spec/unit/provider/confine_collection.rb
index 1598b5f99..444281c77 100755
--- a/spec/unit/provider/confine_collection.rb
+++ b/spec/unit/provider/confine_collection.rb
@@ -6,69 +6,81 @@ require 'puppet/provider/confine_collection'
 
 describe Puppet::Provider::ConfineCollection do
     it "should be able to add confines" do
-        Puppet::Provider::ConfineCollection.new.should respond_to(:confine)
+        Puppet::Provider::ConfineCollection.new("label").should respond_to(:confine)
+    end
+
+    it "should require a label at initialization" do
+        lambda { Puppet::Provider::ConfineCollection.new }.should raise_error(ArgumentError)
+    end
+
+    it "should make its label available" do
+        Puppet::Provider::ConfineCollection.new("mylabel").label.should == "mylabel"
     end
 
     describe "when creating confine instances" do
         it "should create an instance of the named test with the provided values" do
             test_class = mock 'test_class'
-            test_class.expects(:new).with(%w{my values})
+            test_class.expects(:new).with(%w{my values}).returns(stub('confine', :label= => nil))
             Puppet::Provider::Confine.expects(:test).with(:foo).returns test_class
 
-            Puppet::Provider::ConfineCollection.new.confine :foo => %w{my values}
+            Puppet::Provider::ConfineCollection.new("label").confine :foo => %w{my values}
         end
 
-        describe "and the test cannot be found" do
-            before do
-                @variable = mock 'variable_test'
+        it "should copy its label to the confine instance" do
+            confine = mock 'confine'
+            test_class = mock 'test_class'
+            test_class.expects(:new).returns confine
+            Puppet::Provider::Confine.expects(:test).returns test_class
 
-                Puppet::Provider::Confine.expects(:test).with(:foo).returns nil
-                Puppet::Provider::Confine.expects(:test).with(:variable).returns @variable
-            end
+            confine.expects(:label=).with("label")
 
+            Puppet::Provider::ConfineCollection.new("label").confine :foo => %w{my values}
+        end
+
+        describe "and the test cannot be found" do
             it "should create a Facter test with the provided values and set the name to the test name" do
-                confine = mock 'confine'
+                confine = Puppet::Provider::Confine.test(:variable).new(%w{my values})
                 confine.expects(:name=).with(:foo)
-                @variable.expects(:new).with(%w{my values}).returns confine
-                Puppet::Provider::ConfineCollection.new.confine :foo => %w{my values}
+                confine.class.expects(:new).with(%w{my values}).returns confine
+                Puppet::Provider::ConfineCollection.new("label").confine(:foo => %w{my values})
             end
         end
 
         describe "and the 'for_binary' option was provided" do
             it "should mark the test as a binary confine" do
-                confine = mock 'confine'
+                confine = Puppet::Provider::Confine.test(:exists).new(:bar)
                 confine.expects(:for_binary=).with true
                 Puppet::Provider::Confine.test(:exists).expects(:new).with(:bar).returns confine
-                Puppet::Provider::ConfineCollection.new.confine :exists => :bar, :for_binary => true
+                Puppet::Provider::ConfineCollection.new("label").confine :exists => :bar, :for_binary => true
             end
         end
     end
 
     it "should be valid if no confines are present" do
-        Puppet::Provider::ConfineCollection.new.should be_valid
+        Puppet::Provider::ConfineCollection.new("label").should be_valid
     end
 
     it "should be valid if all confines pass" do
-        c1 = mock 'c1', :valid? => true
-        c2 = mock 'c2', :valid? => true
+        c1 = stub 'c1', :valid? => true, :label= => nil
+        c2 = stub 'c2', :valid? => true, :label= => nil
 
         Puppet::Provider::Confine.test(:true).expects(:new).returns(c1)
         Puppet::Provider::Confine.test(:false).expects(:new).returns(c2)
 
-        confiner = Puppet::Provider::ConfineCollection.new
+        confiner = Puppet::Provider::ConfineCollection.new("label")
         confiner.confine :true => :bar, :false => :bee
 
         confiner.should be_valid
     end
 
     it "should not be valid if any confines fail" do
-        c1 = stub 'c1', :valid? => true
-        c2 = stub 'c2', :valid? => false
+        c1 = stub 'c1', :valid? => true, :label= => nil
+        c2 = stub 'c2', :valid? => false, :label= => nil
 
         Puppet::Provider::Confine.test(:true).expects(:new).returns(c1)
         Puppet::Provider::Confine.test(:false).expects(:new).returns(c2)
 
-        confiner = Puppet::Provider::ConfineCollection.new
+        confiner = Puppet::Provider::ConfineCollection.new("label")
         confiner.confine :true => :bar, :false => :bee
 
         confiner.should_not be_valid
@@ -76,7 +88,7 @@ describe Puppet::Provider::ConfineCollection do
 
     describe "when providing a summary" do
         before do
-            @confiner = Puppet::Provider::ConfineCollection.new
+            @confiner = Puppet::Provider::ConfineCollection.new("label")
         end
 
         it "should return a hash" do
diff --git a/spec/unit/provider/confiner.rb b/spec/unit/provider/confiner.rb
index 078fc4420..0a0d67fb5 100755
--- a/spec/unit/provider/confiner.rb
+++ b/spec/unit/provider/confiner.rb
@@ -30,7 +30,8 @@ describe Puppet::Provider::Confiner do
     end
 
     it "should create a new confine collection if one does not exist" do
-        Puppet::Provider::ConfineCollection.expects(:new).returns "mycoll"
+        Puppet::Provider::ConfineCollection.expects(:new).with("mylabel").returns "mycoll"
+        @object.expects(:to_s).returns "mylabel"
         @object.confine_collection.should == "mycoll"
     end