Merge branch '2.7.x'

Conflicts:
	lib/puppet/type/file/source.rb
	spec/unit/resource/catalog_spec.rb

12 files changed, 182 insertions, 38 deletions

diff --git a/spec/lib/puppet/face/1.0.0/huzzah.rb b/spec/lib/puppet/face/1.0.0/huzzah.rb
new file mode 100755
index 000000000..8a1311704
--- /dev/null
+++ b/spec/lib/puppet/face/1.0.0/huzzah.rb
@@ -0,0 +1,8 @@
+require 'puppet/face'
+Puppet::Face.define(:huzzah, '1.0.0') do
+  copyright "Puppet Labs", 2011
+  license   "Apache 2 license; see COPYING"
+  summary "life is a thing for celebration"
+  script :obsolete_in_core do |_| "you are in obsolete core now!" end
+  script :call_newer do |_| method_on_newer end
+end
diff --git a/spec/lib/puppet/face/huzzah.rb b/spec/lib/puppet/face/huzzah.rb
index ab465d9e0..f3d18a797 100755
--- a/spec/lib/puppet/face/huzzah.rb
+++ b/spec/lib/puppet/face/huzzah.rb
@@ -4,4 +4,5 @@ Puppet::Face.define(:huzzah, '2.0.1') do
   license   "Apache 2 license; see COPYING"
   summary "life is a thing for celebration"
   script :bar do |options| "is where beer comes from" end
+  script :call_older do |_| method_on_older end
 end
diff --git a/spec/lib/puppet/face/huzzah/obsolete.rb b/spec/lib/puppet/face/huzzah/obsolete.rb
new file mode 100644
index 000000000..1f717ea2f
--- /dev/null
+++ b/spec/lib/puppet/face/huzzah/obsolete.rb
@@ -0,0 +1,6 @@
+Puppet::Face.define(:huzzah, '1.0.0') do
+  action :obsolete do
+    summary "This is an action on version 1.0.0 of the face"
+    when_invoked do |options| options end
+  end
+end
diff --git a/spec/unit/application/face_base_spec.rb b/spec/unit/application/face_base_spec.rb
index 0a4a86be6..bebc26210 100755
--- a/spec/unit/application/face_base_spec.rb
+++ b/spec/unit/application/face_base_spec.rb
@@ -55,6 +55,7 @@ describe Puppet::Application::FaceBase do
     it "should stop if the first thing found is not an action" do
       app.command_line.stubs(:args).returns %w{banana count_args}
       expect { app.run }.to exit_with 1
+      @logs.first.should_not be_nil
       @logs.first.message.should =~ /has no 'banana' action/
     end
 
diff --git a/spec/unit/application/inspect_spec.rb b/spec/unit/application/inspect_spec.rb
index 9f12c83ad..be5887f01 100755
--- a/spec/unit/application/inspect_spec.rb
+++ b/spec/unit/application/inspect_spec.rb
@@ -12,6 +12,11 @@ describe Puppet::Application::Inspect do
 
   before :each do
     @inspect = Puppet::Application[:inspect]
+    @inspect.preinit
+  end
+
+  it "should operate in agent run_mode" do
+    @inspect.class.run_mode.name.should == :agent
   end
 
   describe "during setup" do
diff --git a/spec/unit/face/certificate_spec.rb b/spec/unit/face/certificate_spec.rb
index 0cb905b75..9291d7523 100755
--- a/spec/unit/face/certificate_spec.rb
+++ b/spec/unit/face/certificate_spec.rb
@@ -10,14 +10,26 @@ describe Puppet::Face[:certificate, '0.0.1'] do
   end
 
   it "should set the ca location when invoked" do
-    Puppet::SSL::Host.expects(:ca_location=).with(:foo)
+    Puppet::SSL::Host.expects(:ca_location=).with(:local)
     Puppet::SSL::Host.indirection.expects(:save)
-    subject.sign "hello, friend", :ca_location => :foo
+    subject.sign "hello, friend", :ca_location => :local
   end
 
   it "(#7059) should set the ca location when an inherited action is invoked" do
-    Puppet::SSL::Host.expects(:ca_location=).with(:foo)
+    Puppet::SSL::Host.expects(:ca_location=).with(:local)
     subject.indirection.expects(:find)
-    subject.find "hello, friend", :ca_location => :foo
+    subject.find "hello, friend", :ca_location => :local
+  end
+
+  it "should validate the option as required" do
+    expect do
+      subject.find 'hello, friend'
+    end.to raise_exception ArgumentError, /required/i
+  end
+
+  it "should validate the option as a supported value" do
+    expect do
+      subject.find 'hello, friend', :ca_location => :foo
+    end.to raise_exception ArgumentError, /valid values/i
   end
 end
diff --git a/spec/unit/indirector/face_spec.rb b/spec/unit/indirector/face_spec.rb
index 943ff7991..8e324f019 100755
--- a/spec/unit/indirector/face_spec.rb
+++ b/spec/unit/indirector/face_spec.rb
@@ -12,6 +12,8 @@ describe Puppet::Indirector::Face do
     instance
   end
 
+  it { should be_option :extra }
+
   it "should be able to return a list of indirections" do
     Puppet::Indirector::Face.indirections.should be_include("catalog")
   end
@@ -48,7 +50,7 @@ describe Puppet::Indirector::Face do
     end
     it "should forward passed options" do
       subject.indirection.expects(method).with(:test, {'one'=>'1'})
-      subject.send(method, :test, {'one'=>'1'})
+      subject.send(method, :test, :extra => {'one'=>'1'})
     end
   end
 
diff --git a/spec/unit/indirector/report/processor_spec.rb b/spec/unit/indirector/report/processor_spec.rb
index bafbe6ee7..fbc70a104 100755
--- a/spec/unit/indirector/report/processor_spec.rb
+++ b/spec/unit/indirector/report/processor_spec.rb
@@ -25,9 +25,11 @@ describe Puppet::Transaction::Report::Processor, " when saving a report" do
 
   it "should not process the report if reports are set to 'none'" do
     Puppet::Reports.expects(:report).never
-    Puppet.settings.expects(:value).with(:reports).returns("none")
+    Puppet[:reports] = 'none'
 
-    request = stub 'request', :instance => mock("report")
+    request = Puppet::Indirector::Request.new(:indirection_name, :head, "key")
+    report = Puppet::Transaction::Report.new('apply')
+    request.instance = report
 
     @reporter.save(request)
   end
@@ -40,14 +42,14 @@ end
 
 describe Puppet::Transaction::Report::Processor, " when processing a report" do
   before do
-    Puppet.settings.stubs(:value).with(:reports).returns("one")
+    Puppet[:reports] = "one"
     Puppet.settings.stubs(:use)
     @reporter = Puppet::Transaction::Report::Processor.new
 
     @report_type = mock 'one'
     @dup_report = mock 'dupe report'
     @dup_report.stubs(:process)
-    @report = mock 'report'
+    @report = Puppet::Transaction::Report.new('apply')
     @report.expects(:dup).returns(@dup_report)
 
     @request = stub 'request', :instance => @report
@@ -74,7 +76,7 @@ describe Puppet::Transaction::Report::Processor, " when processing a report" do
   end
 
   it "should not raise exceptions" do
-    Puppet.settings.stubs(:value).with(:trace).returns(false)
+    Puppet[:trace] = false
     @dup_report.expects(:process).raises(ArgumentError)
     proc { @reporter.save(@request) }.should_not raise_error
   end
diff --git a/spec/unit/interface/action_spec.rb b/spec/unit/interface/action_spec.rb
index d71a7d000..c3f08e817 100755
--- a/spec/unit/interface/action_spec.rb
+++ b/spec/unit/interface/action_spec.rb
@@ -121,6 +121,7 @@ describe Puppet::Interface::Action do
       let :face do
         Puppet::Interface.new(:ruby_api, '1.0.0') do
           action :bar do
+            option "--bar"
             when_invoked do |*args|
               args.last
             end
@@ -138,8 +139,8 @@ describe Puppet::Interface::Action do
         options.should == { :bar => "beer" }
       end
 
-      it "should call #validate_args on the action when invoked" do
-        face.get_action(:bar).expects(:validate_args).with([1, :two, 'three', {}])
+      it "should call #validate_and_clean on the action when invoked" do
+        face.get_action(:bar).expects(:validate_and_clean).with({}).returns({})
         face.bar 1, :two, 'three'
       end
     end
@@ -171,14 +172,28 @@ describe Puppet::Interface::Action do
       face.get_action(:foo).options.should =~ [:bar]
     end
 
-    it "should only list options and not aliases" do
-      face = Puppet::Interface.new(:action_level_options, '0.0.1') do
-        action :foo do
-          when_invoked do |options| true end
-          option "--bar", "-b", "--foo-bar"
+    describe "option aliases" do
+      let :option do action.get_option :bar end
+      let :action do face.get_action :foo end
+      let :face do
+        Puppet::Interface.new(:action_level_options, '0.0.1') do
+          action :foo do
+            when_invoked do |options| options end
+            option "--bar", "--foo", "-b"
+          end
+        end
+      end
+
+      it "should only list options and not aliases" do
+        action.options.should =~ [:bar]
+      end
+
+      it "should use the canonical option name when passed aliases" do
+        name = option.name
+        option.aliases.each do |input|
+          face.foo(input => 1).should == { name => 1 }
         end
       end
-      face.get_action(:foo).options.should =~ [:bar]
     end
 
     describe "with both face and action options" do
@@ -436,12 +451,12 @@ describe Puppet::Interface::Action do
         end
 
         it "should be invoked when calling a child action" do
-          subject.on_child(:foo => true, :bar => true).should == :on_child
+          subject.on_child(:foo => true).should == :on_child
           subject.reported.should == [ :child_before ]
         end
 
         it "should be invoked when calling a parent action" do
-          subject.on_parent(:foo => true, :bar => true).should == :on_parent
+          subject.on_parent(:foo => true).should == :on_parent
           subject.reported.should == [ :child_before ]
         end
       end
@@ -453,12 +468,12 @@ describe Puppet::Interface::Action do
         end
 
         it "should be invoked when calling a child action" do
-          subject.on_child(:foo => true, :bar => true).should == :on_child
+          subject.on_child(:foo => true).should == :on_child
           subject.reported.should == [ :parent_before ]
         end
 
         it "should be invoked when calling a parent action" do
-          subject.on_parent(:foo => true, :bar => true).should == :on_parent
+          subject.on_parent(:foo => true).should == :on_parent
           subject.reported.should == [ :parent_before ]
         end
       end
@@ -534,7 +549,7 @@ describe Puppet::Interface::Action do
     it "should return the block if asked"
   end
 
-  context "#validate_args" do
+  context "#validate_and_clean" do
     subject do
       Puppet::Interface.new(:validate_args, '1.0.0') do
         script :test do |options| true end
@@ -551,6 +566,16 @@ describe Puppet::Interface::Action do
       expect { subject.test :foo => true, :f => true }.
         to raise_error ArgumentError, /Multiple aliases for the same option/
     end
+
+    it "should fail if an unknown option is passed" do
+      expect { subject.test :unknown => true }.
+        to raise_error ArgumentError, /Unknown options passed: unknown/
+    end
+
+    it "should report all the unknown options passed" do
+      expect { subject.test :unknown => true, :unseen => false }.
+        to raise_error ArgumentError, /Unknown options passed: unknown, unseen/
+    end
   end
 
   context "default option values" do
diff --git a/spec/unit/interface/face_collection_spec.rb b/spec/unit/interface/face_collection_spec.rb
index 98887a778..514a624b1 100755
--- a/spec/unit/interface/face_collection_spec.rb
+++ b/spec/unit/interface/face_collection_spec.rb
@@ -35,7 +35,8 @@ describe Puppet::Interface::FaceCollection do
     end
 
     it "should attempt to load the face if it isn't found" do
-      subject.expects(:require).with('puppet/face/bar')
+      subject.expects(:require).once.with('puppet/face/bar')
+      subject.expects(:require).once.with('puppet/face/0.0.1/bar')
       subject["bar", '0.0.1']
     end
 
@@ -64,7 +65,8 @@ describe Puppet::Interface::FaceCollection do
 
     it "should return false if the face file itself is missing" do
       subject.stubs(:require).
-        raises(LoadError, 'no such file to load -- puppet/face/bar')
+        raises(LoadError, 'no such file to load -- puppet/face/bar').then.
+        raises(LoadError, 'no such file to load -- puppet/face/0.0.1/bar')
       subject["bar", '0.0.1'].should be_false
     end
 
@@ -97,6 +99,42 @@ describe Puppet::Interface::FaceCollection do
     end
   end
 
+  describe "::get_action_for_face" do
+    it "should return an action on the current face" do
+      Puppet::Face::FaceCollection.get_action_for_face(:huzzah, :bar, :current).
+        should be_an_instance_of Puppet::Interface::Action
+    end
+
+    it "should return an action on an older version of a face" do
+      action = Puppet::Face::FaceCollection.
+        get_action_for_face(:huzzah, :obsolete, :current)
+
+      action.should be_an_instance_of Puppet::Interface::Action
+      action.face.version.should == SemVer.new('1.0.0')
+    end
+
+    it "should load the full older version of a face" do
+      action = Puppet::Face::FaceCollection.
+        get_action_for_face(:huzzah, :obsolete, :current)
+
+      action.face.version.should == SemVer.new('1.0.0')
+      action.face.should be_action :obsolete_in_core
+    end
+
+    it "should not add obsolete actions to the current version" do
+      action = Puppet::Face::FaceCollection.
+        get_action_for_face(:huzzah, :obsolete, :current)
+
+      action.face.version.should == SemVer.new('1.0.0')
+      action.face.should be_action :obsolete_in_core
+
+      current = Puppet::Face[:huzzah, :current]
+      current.version.should == SemVer.new('2.0.1')
+      current.should_not be_action :obsolete_in_core
+      current.should_not be_action :obsolete
+    end
+  end
+
   describe "::register" do
     it "should store the face by name" do
       face = Puppet::Face.new(:my_face, '0.0.1')
diff --git a/spec/unit/interface_spec.rb b/spec/unit/interface_spec.rb
index 4cb1f8743..4ff71ac3d 100755
--- a/spec/unit/interface_spec.rb
+++ b/spec/unit/interface_spec.rb
@@ -126,14 +126,11 @@ describe Puppet::Interface do
   end
 
   it "should try to require faces that are not known" do
-    pending "mocking require causes random stack overflow"
-    subject::FaceCollection.expects(:require).with "puppet/face/foo"
-    subject[:foo, '0.0.1']
+    subject::FaceCollection.expects(:load_face).with(:foo, :current)
+    subject::FaceCollection.expects(:load_face).with(:foo, '0.0.1')
+    expect { subject[:foo, '0.0.1'] }.to raise_error Puppet::Error
   end
 
-  it "should be able to load all actions in all search paths"
-
-
   it_should_behave_like "things that declare options" do
     def add_options_to(&block)
       subject.new(:with_options, '0.0.1', &block)
diff --git a/spec/unit/resource/catalog_spec.rb b/spec/unit/resource/catalog_spec.rb
index c03f05dee..ce783e393 100755
--- a/spec/unit/resource/catalog_spec.rb
+++ b/spec/unit/resource/catalog_spec.rb
@@ -484,7 +484,7 @@ describe Puppet::Resource::Catalog, "when compiling" do
       lambda { @catalog.alias(@one, "other") }.should_not raise_error
     end
 
-    it "should create aliases for resources isomorphic resources whose names do not match their titles" do
+    it "should create aliases for isomorphic resources whose names do not match their titles" do
       resource = Puppet::Type::File.new(:title => "testing", :path => @basepath+"/something")
 
       @catalog.add_resource(resource)
@@ -492,7 +492,7 @@ describe Puppet::Resource::Catalog, "when compiling" do
       @catalog.resource(:file, @basepath+"/something").should equal(resource)
     end
 
-    it "should not create aliases for resources non-isomorphic resources whose names do not match their titles", :fails_on_windows => true do
+    it "should not create aliases for non-isomorphic resources whose names do not match their titles", :fails_on_windows => true do
       resource = Puppet::Type.type(:exec).new(:title => "testing", :command => "echo", :path => %w{/bin /usr/bin /usr/local/bin})
 
       @catalog.add_resource(resource)
@@ -508,11 +508,6 @@ describe Puppet::Resource::Catalog, "when compiling" do
       @catalog.resource("notify", "other").should equal(@one)
     end
 
-    it "should ignore conflicting aliases that point to the aliased resource" do
-      @catalog.alias(@one, "other")
-      lambda { @catalog.alias(@one, "other") }.should_not raise_error
-    end
-
     it "should fail to add an alias if the aliased name already exists" do
       @catalog.add_resource @one
       proc { @catalog.alias @two, "one" }.should raise_error(ArgumentError)
@@ -566,6 +561,58 @@ describe Puppet::Resource::Catalog, "when compiling" do
       @catalog.create_resource :file, args
       @catalog.resource("File[/yay]").should equal(resource)
     end
+
+    describe "when adding resources with multiple namevars" do
+      before :each do
+        Puppet::Type.newtype(:multiple) do
+          newparam(:color, :namevar => true)
+          newparam(:designation, :namevar => true)
+
+          def self.title_patterns
+            [ [
+                /^(\w+) (\w+)$/,
+                [
+                  [:color,  lambda{|x| x}],
+                  [:designation, lambda{|x| x}]
+                ]
+            ] ]
+          end
+        end
+      end
+
+      it "should add an alias using the uniqueness key" do
+        @resource = Puppet::Type.type(:multiple).new(:title => "some resource", :color => "red", :designation => "5")
+
+        @catalog.add_resource(@resource)
+        @catalog.resource(:multiple, "some resource").must == @resource
+        @catalog.resource("Multiple[some resource]").must == @resource
+        @catalog.resource("Multiple[red 5]").must == @resource
+      end
+
+      it "should conflict with a resource with the same uniqueness key" do
+        @resource = Puppet::Type.type(:multiple).new(:title => "some resource", :color => "red", :designation => "5")
+        @other    = Puppet::Type.type(:multiple).new(:title => "another resource", :color => "red", :designation => "5")
+
+        @catalog.add_resource(@resource)
+        expect { @catalog.add_resource(@other) }.to raise_error(ArgumentError, /Cannot alias Multiple\[another resource\] to \["red", "5"\].*resource \["Multiple", "red", "5"\] already defined/)
+      end
+
+      it "should conflict when its uniqueness key matches another resource's title" do
+        @resource = Puppet::Type.type(:file).new(:title => "/tmp/foo")
+        @other    = Puppet::Type.type(:file).new(:title => "another file", :path => "/tmp/foo")
+
+        @catalog.add_resource(@resource)
+        expect { @catalog.add_resource(@other) }.to raise_error(ArgumentError, /Cannot alias File\[another file\] to \["\/tmp\/foo"\].*resource \["File", "\/tmp\/foo"\] already defined/)
+      end
+
+      it "should conflict when its uniqueness key matches the uniqueness key derived from another resource's title" do
+        @resource = Puppet::Type.type(:multiple).new(:title => "red leader")
+        @other    = Puppet::Type.type(:multiple).new(:title => "another resource", :color => "red", :designation => "leader")
+
+        @catalog.add_resource(@resource)
+        expect { @catalog.add_resource(@other) }.to raise_error(ArgumentError, /Cannot alias Multiple\[another resource\] to \["red", "leader"\].*resource \["Multiple", "red", "leader"\] already defined/)
+      end
+    end
   end
 
   describe "when applying" do