(#7184) Load the core of obsolete versions of Faces.

When we define an action on an older version of a Face, we must be sure to
directly load the core of that version, not just define it with the external
Action(s) that it had.

Otherwise we break our contract, which is that any core Actions for a specific
version will be available to your external Action for as long as we support
that core version.

Reviewed-By: Pieter van de Bruggen <pieter@puppetlabs.com>

4 files changed, 39 insertions, 8 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..00f20f096
--- /dev/null
+++ b/spec/lib/puppet/face/1.0.0/huzzah.rb
@@ -0,0 +1,9 @@
+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..47cc3f32c 100755
--- a/spec/lib/puppet/face/huzzah.rb
+++ b/spec/lib/puppet/face/huzzah.rb
@@ -4,4 +4,6 @@ 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/unit/interface/face_collection_spec.rb b/spec/unit/interface/face_collection_spec.rb
index 8f9c349b6..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
 
@@ -110,6 +112,27 @@ describe Puppet::Interface::FaceCollection do
       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
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)